Re: [Qemu-devel] [PATCH 27/42] tpm: remove unused opened code

[Marc-André Lureau]

Hi

----- Original Message -----
> On 10/09/2017 06:56 PM, Marc-André Lureau wrote:
> > Signed-off-by: Marc-André Lureau <address@hidden>
> > ---
> >   include/sysemu/tpm_backend.h | 12 ------------
> >   backends/tpm.c               | 42
> >   ------------------------------------------
> >   tpm.c                        |  6 ------
> >   3 files changed, 60 deletions(-)
> >
> > diff --git a/include/sysemu/tpm_backend.h b/include/sysemu/tpm_backend.h
> > index b12ae5b625..a893e586ae 100644
> > --- a/include/sysemu/tpm_backend.h
> > +++ b/include/sysemu/tpm_backend.h
> > @@ -81,8 +81,6 @@ struct TPMBackendClass {
> >   
> >       TpmTypeOptions *(*get_tpm_options)(TPMBackend *t);
> >   
> > -    void (*opened)(TPMBackend *s, Error **errp);
> > -
> >       void (*handle_request)(TPMBackend *s, TPMBackendCmd *cmd);
> >   };
> >   
> > @@ -172,16 +170,6 @@ bool tpm_backend_get_tpm_established_flag(TPMBackend
> > *s);
> >    */
> >   int tpm_backend_reset_tpm_established_flag(TPMBackend *s, uint8_t locty);
> >   
> > -/**
> > - * tpm_backend_open:
> > - * @s: the backend to open
> > - * @errp: a pointer to return the #Error object if an error occurs.
> > - *
> > - * This function will open the backend if it is not already open.  Calling
> > this
> > - * function on an already opened backend will not result in an error.
> > - */
> > -void tpm_backend_open(TPMBackend *s, Error **errp);
> > -
> >   /**
> >    * tpm_backend_get_tpm_version:
> >    * @s: the backend to call into
> > diff --git a/backends/tpm.c b/backends/tpm.c
> > index 0c48d18775..7e636fbc7a 100644
> > --- a/backends/tpm.c
> > +++ b/backends/tpm.c
> > @@ -159,52 +159,10 @@ TPMInfo *tpm_backend_query_tpm(TPMBackend *s)
> >       return info;
> >   }
> >   
> > -static bool tpm_backend_prop_get_opened(Object *obj, Error **errp)
> > -{
> > -    TPMBackend *s = TPM_BACKEND(obj);
> > -
> > -    return s->opened;
> > -}
> > -
> > -void tpm_backend_open(TPMBackend *s, Error **errp)
> > -{
> > -    object_property_set_bool(OBJECT(s), true, "opened", errp);
> > -}
> > -
> > -static void tpm_backend_prop_set_opened(Object *obj, bool value, Error
> > **errp)
> > -{
> > -    TPMBackend *s = TPM_BACKEND(obj);
> > -    TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
> > -    Error *local_err = NULL;
> > -
> > -    if (value == s->opened) {
> > -        return;
> > -    }
> > -
> > -    if (!value && s->opened) {
> > -        error_setg(errp, QERR_PERMISSION_DENIED);
> > -        return;
> > -    }
> > -
> > -    if (k->opened) {
> > -        k->opened(s, &local_err);
> > -        if (local_err) {
> > -            error_propagate(errp, local_err);
> > -            return;
> > -        }
> > -    }
> > -
> > -    s->opened = true;
> > -}
> > -
> >   static void tpm_backend_instance_init(Object *obj)
> >   {
> >       TPMBackend *s = TPM_BACKEND(obj);
> >   
> > -    object_property_add_bool(obj, "opened",
> > -                             tpm_backend_prop_get_opened,
> > -                             tpm_backend_prop_set_opened,
> > -                             NULL);
> >       s->bh = qemu_bh_new(tpm_backend_request_completed_bh, s);
> >   }
> >   
> > diff --git a/tpm.c b/tpm.c
> > index ce1543fcb4..a46ee5f144 100644
> > --- a/tpm.c
> > +++ b/tpm.c
> > @@ -134,12 +134,6 @@ static int tpm_init_tpmdev(void *dummy, QemuOpts
> > *opts, Error **errp)
> >           return 1;
> >       }
> >   
> > -    tpm_backend_open(drv, &local_err);
> > -    if (local_err) {
> > -        error_report_err(local_err);
> > -        return 1;
> > -    }
> > -
> >       QLIST_INSERT_HEAD(&tpm_backends, drv, list);
> >   
> >       return 0;
> 
> Since nothing is setting the "opened' anymore, would anyone notice
> because this has changed or is something else doing that now? Otherwise
> I don't mind removing it...

Was it ever used? I see the property was added in commit 
8f0605cc9caacbcc647a6df9ae541ed2da4b9bb0.

Code adapted from rng backend. But rng backend set "opened" on 
UserCreatableClass.complete() (TPM backend doesn't, but probably could - I have 
an experimental patch for that)

I think we can remove it that code safely.