Hi On Mon, Oct 8, 2018 at 9:33 PM Markus Armbruster <arm...@redhat.com> wrote: > > Calling error_report() in a function that takes an Error ** argument > is suspicious. tpm_init_tpmdev() does that, and then fails without > setting an error. Its caller main(), via tpm_init() and > qemu_opts_foreach(), is fine with it, but clean it up anyway. > > Cc: Stefan Berger <stef...@linux.vnet.ibm.com> > Signed-off-by: Markus Armbruster <arm...@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lur...@redhat.com> > --- > include/sysemu/tpm.h | 2 +- > stubs/tpm.c | 3 +-- > tpm.c | 22 +++++++++------------- > vl.c | 4 +--- > 4 files changed, 12 insertions(+), 19 deletions(-) > > diff --git a/include/sysemu/tpm.h b/include/sysemu/tpm.h > index 9ae1ab6da3..17a97ed77a 100644 > --- a/include/sysemu/tpm.h > +++ b/include/sysemu/tpm.h > @@ -16,7 +16,7 @@ > #include "qom/object.h" > > int tpm_config_parse(QemuOptsList *opts_list, const char *optarg); > -int tpm_init(void); > +void tpm_init(void); > void tpm_cleanup(void); > > typedef enum TPMVersion { > diff --git a/stubs/tpm.c b/stubs/tpm.c > index 6729bc8517..80939cd3db 100644 > --- a/stubs/tpm.c > +++ b/stubs/tpm.c > @@ -9,9 +9,8 @@ > #include "qapi/qapi-commands-tpm.h" > #include "sysemu/tpm.h" > > -int tpm_init(void) > +void tpm_init(void) > { > - return 0; > } > > void tpm_cleanup(void) > diff --git a/tpm.c b/tpm.c > index 93031723ad..9c9e20bbb7 100644 > --- a/tpm.c > +++ b/tpm.c > @@ -89,19 +89,19 @@ static int tpm_init_tpmdev(void *dummy, QemuOpts *opts, > Error **errp) > int i; > > if (!QLIST_EMPTY(&tpm_backends)) { > - error_report("Only one TPM is allowed."); > + error_setg(errp, "Only one TPM is allowed."); > return 1; > } > > id = qemu_opts_id(opts); > if (id == NULL) { > - error_report(QERR_MISSING_PARAMETER, "id"); > + error_setg(errp, QERR_MISSING_PARAMETER, "id"); > return 1; > } > > value = qemu_opt_get(opts, "type"); > if (!value) { > - error_report(QERR_MISSING_PARAMETER, "type"); > + error_setg(errp, QERR_MISSING_PARAMETER, "type"); > tpm_display_backend_drivers(); > return 1; > } > @@ -109,8 +109,8 @@ static int tpm_init_tpmdev(void *dummy, QemuOpts *opts, > Error **errp) > i = qapi_enum_parse(&TpmType_lookup, value, -1, NULL); > be = i >= 0 ? tpm_be_find_by_type(i) : NULL; > if (be == NULL) { > - error_report(QERR_INVALID_PARAMETER_VALUE, > - "type", "a TPM backend type"); > + error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "type", > + "a TPM backend type"); > tpm_display_backend_drivers(); > return 1; > } > @@ -118,7 +118,7 @@ static int tpm_init_tpmdev(void *dummy, QemuOpts *opts, > Error **errp) > /* validate backend specific opts */ > qemu_opts_validate(opts, be->opts, &local_err); > if (local_err) { > - error_report_err(local_err); > + error_propagate(errp, local_err); > return 1; > } > > @@ -151,14 +151,10 @@ void tpm_cleanup(void) > * Initialize the TPM. Process the tpmdev command line options describing the > * TPM backend. > */ > -int tpm_init(void) > +void tpm_init(void) > { > - if (qemu_opts_foreach(qemu_find_opts("tpmdev"), > - tpm_init_tpmdev, NULL, NULL)) { > - return -1; > - } > - > - return 0; > + qemu_opts_foreach(qemu_find_opts("tpmdev"), > + tpm_init_tpmdev, NULL, &error_fatal); > } > > /* > diff --git a/vl.c b/vl.c > index 5c159ca45c..b6f9212f09 100644 > --- a/vl.c > +++ b/vl.c > @@ -4312,9 +4312,7 @@ int main(int argc, char **argv, char **envp) > user_creatable_add_opts_foreach, > object_create_delayed, &error_fatal); > > - if (tpm_init() < 0) { > - exit(1); > - } > + tpm_init(); > > /* init the bluetooth world */ > if (foreach_device_config(DEV_BT, bt_parse)) > -- > 2.17.1 > > -- Marc-André Lureau