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

[Stefan Berger <stefanb@linux.vnet.ibm.com>]

On 10/10/2017 06:27 PM, Marc-André Lureau wrote:
> Hi
>
> ----- Original Message -----
>> On 10/09/2017 06:56 PM, Marc-André Lureau wrote:
>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>> ---
>>>    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.
>

Reviewed-by: Stefan Berger <stefanb@linux.vnet.ibm.com>