On Tue, 2017-10-10 at 06:47 -0400, Marc-André Lureau wrote:
> Hi
>
> ----- Original Message -----
> >
> > On Tue, 2017-10-10 at 00:56 +0200, Marc-André Lureau wrote:
> > >
> > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > ---
> > > include/sysemu/tpm_backend.h | 2 +-
> > > hw/tpm/tpm_emulator.c | 12 +++---------
> > > hw/tpm/tpm_passthrough.c | 9 +++------
> > > tpm.c | 3 ++-
> > > 4 files changed, 9 insertions(+), 17 deletions(-)
> > >
> > > diff --git a/include/sysemu/tpm_backend.h
> > > b/include/sysemu/tpm_backend.h
> > > index 594bb50782..881be97ee3 100644
> > > --- a/include/sysemu/tpm_backend.h
> > > +++ b/include/sysemu/tpm_backend.h
> > > @@ -64,7 +64,7 @@ struct TPMBackendClass {
> > > /* get a descriptive text of the backend to display to the
> > > user
> > > */
> > > const char *desc;
> > >
> > > - TPMBackend *(*create)(QemuOpts *opts, const char *id);
> > > + TPMBackend *(*create)(QemuOpts *opts);
> > >
> > > /* start up the TPM on the backend - optional */
> > > int (*startup_tpm)(TPMBackend *t);
> > > diff --git a/hw/tpm/tpm_emulator.c b/hw/tpm/tpm_emulator.c
> > > index 36454837b3..315819329b 100644
> > > --- a/hw/tpm/tpm_emulator.c
> > > +++ b/hw/tpm/tpm_emulator.c
> > > @@ -453,22 +453,16 @@ err:
> > > return -1;
> > > }
> > >
> > > -static TPMBackend *tpm_emulator_create(QemuOpts *opts, const
> > > char
> > > *id)
> > > +static TPMBackend *tpm_emulator_create(QemuOpts *opts)
> > > {
> > > TPMBackend *tb = TPM_BACKEND(object_new(TYPE_TPM_EMULATOR));
> > >
> > > - tb->id = g_strdup(id);
> > > -
> > > if (tpm_emulator_handle_device_opts(TPM_EMULATOR(tb), opts))
> > > {
> > > - goto err_exit;
> > > + object_unref(OBJECT(tb));
> > > + return NULL;
> > > }
> > >
> > > return tb;
> > > -
> > > -err_exit:
> > > - object_unref(OBJECT(tb));
> > > -
> > > - return NULL;
> > > }
> > >
> > > static TpmTypeOptions *tpm_emulator_get_tpm_options(TPMBackend
> > > *tb)
> > > diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
> > > index 9326cbfdc9..7371d50739 100644
> > > --- a/hw/tpm/tpm_passthrough.c
> > > +++ b/hw/tpm/tpm_passthrough.c
> > > @@ -284,13 +284,10 @@
> > > tpm_passthrough_handle_device_opts(TPMPassthruState *tpm_pt,
> > > QemuOpts
> > > *opts)
> > > return 1;
> > > }
> > >
> > > -static TPMBackend *tpm_passthrough_create(QemuOpts *opts, const
> > > char
> > > *id)
> > > +static TPMBackend *tpm_passthrough_create(QemuOpts *opts)
> > > {
> > > Object *obj = object_new(TYPE_TPM_PASSTHROUGH);
> > > - TPMBackend *tb = TPM_BACKEND(obj);
> > > - TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
> > > -
> > > - tb->id = g_strdup(id);
> > > + TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(obj);
> > >
> > > if (tpm_passthrough_handle_device_opts(tpm_pt, opts)) {
> > > goto err_exit;
> > > @@ -301,7 +298,7 @@ static TPMBackend
> > > *tpm_passthrough_create(QemuOpts *opts, const char *id)
> > > goto err_exit;
> > > }
> > >
> > > - return tb;
> > > + return TPM_BACKEND(obj);
> > >
> > > err_exit:
> > > object_unref(obj);
> > > diff --git a/tpm.c b/tpm.c
> > > index a46ee5f144..37298f3f03 100644
> > > --- a/tpm.c
> > > +++ b/tpm.c
> > > @@ -129,11 +129,12 @@ static int tpm_init_tpmdev(void *dummy,
> > > QemuOpts *opts, Error **errp)
> > > return 1;
> > > }
> > >
> > > - drv = be->create(opts, id);
> > > + drv = be->create(opts);
> > > if (!drv) {
> > > return 1;
> > > }
> > >
> > > + drv->id = g_strdup(id);
> > I kind of oppose this change, instead what about adding new
> > TPMBackend
> > api say - TPMBackend* tpm_backend_create(const char *type, const
> > QemuOpts *opts), that should handle this common code, and returns
> > the
> > newly instantiated backend object.
> That would be a more complicated refactoring than what I propose
> here, which is basic common code refactoring. To clarify your
> proposal, make a follow-up patch?
Yes, ok with a follow-up patch.
>
> Another interesting approach would be to implement USER_CREATABLE (I
> have an experimental patch for that). This allows to use -object tpm-
> passthrough,id=..,path=.. and will change the way backends are
> created & initialized.
This approach is really interesting.
- Amarnath