* [Qemu-devel] [PATCH] Move TPM passthrough specific command line options to backend structure
@ 2013-04-02 17:30 Stefan Berger
2013-04-10 18:51 ` Corey Bryant
0 siblings, 1 reply; 3+ messages in thread
From: Stefan Berger @ 2013-04-02 17:30 UTC (permalink / raw)
To: qemu-devel@nongnu.org; +Cc: coreyb
Move the TPM passthrough specific command line options to the passthrough
backend implementation and attach them to the backend's interface structure.
Add code to tpm.c for validating the TPM command line options.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
---
tpm/tpm.c | 10 +++++++++-
tpm/tpm_int.h | 8 ++++++++
tpm/tpm_passthrough.c | 16 ++++++++++++++++
vl.c | 15 ---------------
4 files changed, 33 insertions(+), 16 deletions(-)
Index: qemu-git.pt/vl.c
===================================================================
--- qemu-git.pt.orig/vl.c
+++ qemu-git.pt/vl.c
@@ -502,21 +502,6 @@ static QemuOptsList qemu_tpmdev_opts = {
.implied_opt_name = "type",
.head = QTAILQ_HEAD_INITIALIZER(qemu_tpmdev_opts.head),
.desc = {
- {
- .name = "type",
- .type = QEMU_OPT_STRING,
- .help = "Type of TPM backend",
- },
- {
- .name = "cancel-path",
- .type = QEMU_OPT_STRING,
- .help = "Sysfs file entry for canceling TPM commands",
- },
- {
- .name = "path",
- .type = QEMU_OPT_STRING,
- .help = "Path to TPM device on the host",
- },
{ /* end of list */ }
},
};
Index: qemu-git.pt/tpm/tpm.c
===================================================================
--- qemu-git.pt.orig/tpm/tpm.c
+++ qemu-git.pt/tpm/tpm.c
@@ -146,7 +146,7 @@ static int configure_tpm(QemuOpts *opts)
const char *id;
const TPMDriverOps *be;
TPMBackend *drv;
- Error *local_err = NULL;
+ Error *local_err = NULL, *errp = NULL;
if (!QLIST_EMPTY(&tpm_backends)) {
error_report("Only one TPM is allowed.\n");
@@ -174,6 +174,14 @@ static int configure_tpm(QemuOpts *opts)
return 1;
}
+ /* validate backend specific opts */
+ qemu_opts_validate(opts, be->opts, &errp);
+ if (error_is_set(&errp)) {
+ qerror_report_err(errp);
+ error_free(errp);
+ return 1;
+ }
+
drv = be->create(opts, id);
if (!drv) {
return 1;
Index: qemu-git.pt/tpm/tpm_passthrough.c
===================================================================
--- qemu-git.pt.orig/tpm/tpm_passthrough.c
+++ qemu-git.pt/tpm/tpm_passthrough.c
@@ -512,8 +512,24 @@ static void tpm_passthrough_destroy(TPMB
g_free(tpm_pt->tpm_dev);
}
+static const QemuOptDesc tpm_passthrough_cmdline_opts[] = {
+ TPM_STANDARD_CMDLINE_OPTS,
+ {
+ .name = "cancel-path",
+ .type = QEMU_OPT_STRING,
+ .help = "Sysfs file entry for canceling TPM commands",
+ },
+ {
+ .name = "path",
+ .type = QEMU_OPT_STRING,
+ .help = "Path to TPM device on the host",
+ },
+ { /* end of list */ },
+};
+
const TPMDriverOps tpm_passthrough_driver = {
.type = TPM_TYPE_PASSTHROUGH,
+ .opts = tpm_passthrough_cmdline_opts,
.desc = tpm_passthrough_create_desc,
.create = tpm_passthrough_create,
.destroy = tpm_passthrough_destroy,
Index: qemu-git.pt/tpm/tpm_int.h
===================================================================
--- qemu-git.pt.orig/tpm/tpm_int.h
+++ qemu-git.pt/tpm/tpm_int.h
@@ -40,6 +40,7 @@ typedef void (TPMRecvDataCB)(TPMState *,
struct TPMDriverOps {
enum TpmType type;
+ const QemuOptDesc *opts;
/* get a descriptive text of the backend to display to the user */
const char *(*desc)(void);
@@ -64,6 +65,13 @@ struct TPMDriverOps {
bool (*get_tpm_established_flag)(TPMBackend *t);
};
+#define TPM_STANDARD_CMDLINE_OPTS \
+ { \
+ .name = "type", \
+ .type = QEMU_OPT_STRING, \
+ .help = "Type of TPM backend", \
+ }
+
struct tpm_req_hdr {
uint16_t tag;
uint32_t len;
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [Qemu-devel] [PATCH] Move TPM passthrough specific command line options to backend structure
2013-04-02 17:30 [Qemu-devel] [PATCH] Move TPM passthrough specific command line options to backend structure Stefan Berger
@ 2013-04-10 18:51 ` Corey Bryant
2013-04-10 21:06 ` Stefan Berger
0 siblings, 1 reply; 3+ messages in thread
From: Corey Bryant @ 2013-04-10 18:51 UTC (permalink / raw)
To: Stefan Berger; +Cc: qemu-devel@nongnu.org
On 04/02/2013 01:30 PM, Stefan Berger wrote:
> Move the TPM passthrough specific command line options to the passthrough
> backend implementation and attach them to the backend's interface structure.
>
> Add code to tpm.c for validating the TPM command line options.
>
> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
>
> ---
> tpm/tpm.c | 10 +++++++++-
> tpm/tpm_int.h | 8 ++++++++
> tpm/tpm_passthrough.c | 16 ++++++++++++++++
> vl.c | 15 ---------------
> 4 files changed, 33 insertions(+), 16 deletions(-)
>
> Index: qemu-git.pt/vl.c
> ===================================================================
> --- qemu-git.pt.orig/vl.c
> +++ qemu-git.pt/vl.c
> @@ -502,21 +502,6 @@ static QemuOptsList qemu_tpmdev_opts = {
> .implied_opt_name = "type",
> .head = QTAILQ_HEAD_INITIALIZER(qemu_tpmdev_opts.head),
> .desc = {
> - {
> - .name = "type",
> - .type = QEMU_OPT_STRING,
> - .help = "Type of TPM backend",
> - },
> - {
> - .name = "cancel-path",
> - .type = QEMU_OPT_STRING,
> - .help = "Sysfs file entry for canceling TPM commands",
> - },
> - {
> - .name = "path",
> - .type = QEMU_OPT_STRING,
> - .help = "Path to TPM device on the host",
> - },
> { /* end of list */ }
Does this need a comment to say where these options are now defined?
> },
> };
> Index: qemu-git.pt/tpm/tpm.c
> ===================================================================
> --- qemu-git.pt.orig/tpm/tpm.c
> +++ qemu-git.pt/tpm/tpm.c
> @@ -146,7 +146,7 @@ static int configure_tpm(QemuOpts *opts)
> const char *id;
> const TPMDriverOps *be;
> TPMBackend *drv;
> - Error *local_err = NULL;
> + Error *local_err = NULL, *errp = NULL;
Can't you use local_err in the new code below instead of creating a new
variable?
>
> if (!QLIST_EMPTY(&tpm_backends)) {
> error_report("Only one TPM is allowed.\n");
> @@ -174,6 +174,14 @@ static int configure_tpm(QemuOpts *opts)
> return 1;
> }
>
> + /* validate backend specific opts */
> + qemu_opts_validate(opts, be->opts, &errp);
> + if (error_is_set(&errp)) {
> + qerror_report_err(errp);
> + error_free(errp);
> + return 1;
> + }
> +
This looks fine to me but I see this is the first call to
qemu_opts_validate() in QEMU. I wonder why.
> drv = be->create(opts, id);
> if (!drv) {
> return 1;
> Index: qemu-git.pt/tpm/tpm_passthrough.c
> ===================================================================
> --- qemu-git.pt.orig/tpm/tpm_passthrough.c
> +++ qemu-git.pt/tpm/tpm_passthrough.c
> @@ -512,8 +512,24 @@ static void tpm_passthrough_destroy(TPMB
> g_free(tpm_pt->tpm_dev);
> }
>
> +static const QemuOptDesc tpm_passthrough_cmdline_opts[] = {
> + TPM_STANDARD_CMDLINE_OPTS,
> + {
> + .name = "cancel-path",
> + .type = QEMU_OPT_STRING,
> + .help = "Sysfs file entry for canceling TPM commands",
> + },
> + {
> + .name = "path",
> + .type = QEMU_OPT_STRING,
> + .help = "Path to TPM device on the host",
> + },
> + { /* end of list */ },
> +};
> +
> const TPMDriverOps tpm_passthrough_driver = {
> .type = TPM_TYPE_PASSTHROUGH,
> + .opts = tpm_passthrough_cmdline_opts,
> .desc = tpm_passthrough_create_desc,
> .create = tpm_passthrough_create,
> .destroy = tpm_passthrough_destroy,
> Index: qemu-git.pt/tpm/tpm_int.h
> ===================================================================
> --- qemu-git.pt.orig/tpm/tpm_int.h
> +++ qemu-git.pt/tpm/tpm_int.h
> @@ -40,6 +40,7 @@ typedef void (TPMRecvDataCB)(TPMState *,
>
> struct TPMDriverOps {
> enum TpmType type;
> + const QemuOptDesc *opts;
> /* get a descriptive text of the backend to display to the user */
> const char *(*desc)(void);
>
> @@ -64,6 +65,13 @@ struct TPMDriverOps {
> bool (*get_tpm_established_flag)(TPMBackend *t);
> };
>
> +#define TPM_STANDARD_CMDLINE_OPTS \
> + { \
> + .name = "type", \
> + .type = QEMU_OPT_STRING, \
> + .help = "Type of TPM backend", \
> + }
> +
> struct tpm_req_hdr {
> uint16_t tag;
> uint32_t len;
>
>
--
Regards,
Corey Bryant
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [Qemu-devel] [PATCH] Move TPM passthrough specific command line options to backend structure
2013-04-10 18:51 ` Corey Bryant
@ 2013-04-10 21:06 ` Stefan Berger
0 siblings, 0 replies; 3+ messages in thread
From: Stefan Berger @ 2013-04-10 21:06 UTC (permalink / raw)
To: Corey Bryant; +Cc: qemu-devel@nongnu.org
On 04/10/2013 02:51 PM, Corey Bryant wrote:
>
>
> On 04/02/2013 01:30 PM, Stefan Berger wrote:
>> return 1;
>> }
>>
>> + /* validate backend specific opts */
>> + qemu_opts_validate(opts, be->opts, &errp);
>> + if (error_is_set(&errp)) {
>> + qerror_report_err(errp);
>> + error_free(errp);
>> + return 1;
>> + }
>> +
>
> This looks fine to me but I see this is the first call to
> qemu_opts_validate() in QEMU. I wonder why.
Well, the function does what is needed here, sorting out command line
parameters that are unknown. It also fits the available parameters.
I fixed the code related to the other comments. Thanks for the review.
Stefan
I
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-04-10 21:06 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-02 17:30 [Qemu-devel] [PATCH] Move TPM passthrough specific command line options to backend structure Stefan Berger
2013-04-10 18:51 ` Corey Bryant
2013-04-10 21:06 ` Stefan Berger
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).