1. Patchew
  2. QEMU
  3. [Qemu-devel] [PATCH v2 0/9] Provide support for the software TPM
  4. View patch

[Qemu-devel] [PATCH v2 6/9] tpm-backend: Remove unneeded destroy() method from TpmDriverOps interface

Amarnath Valluri posted 9 patches 8 years, 10 months ago
There is a newer version of this series
[Qemu-devel] [PATCH v2 6/9] tpm-backend: Remove unneeded destroy() method from TpmDriverOps interface
Posted by Amarnath Valluri 8 years, 10 months ago 
As TPMBackend is a Qemu Object, we can use object_unref() inplace of
tpm_backend_destroy() to free the backend object, hence removed destroy() from
TPMDriverOps interface.

Signed-off-by: Amarnath Valluri <amarnath.valluri@intel.com>
---
 backends/tpm.c               | 11 -----------
 hw/tpm/tpm_passthrough.c     | 14 --------------
 include/sysemu/tpm_backend.h |  7 -------
 tpm.c                        |  2 +-
 4 files changed, 1 insertion(+), 33 deletions(-)

diff --git a/backends/tpm.c b/backends/tpm.c
index c96f462..3493df6 100644
--- a/backends/tpm.c
+++ b/backends/tpm.c
@@ -51,17 +51,6 @@ const char *tpm_backend_get_desc(TPMBackend *s)
     return k->ops->desc ? k->ops->desc() : "";
 }
 
-void tpm_backend_destroy(TPMBackend *s)
-{
-    TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
-
-    if (k->ops->destroy) {
-        k->ops->destroy(s);
-    }
-
-    object_unref(OBJECT(s));
-}
-
 int tpm_backend_init(TPMBackend *s, TPMState *state,
                      TPMRecvDataCB *datacb)
 {
diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
index 71bdf25..8e11ed3 100644
--- a/hw/tpm/tpm_passthrough.c
+++ b/hw/tpm/tpm_passthrough.c
@@ -428,19 +428,6 @@ err_exit:
     return NULL;
 }
 
-static void tpm_passthrough_destroy(TPMBackend *tb)
-{
-    TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
-
-    tpm_passthrough_cancel_cmd(tb);
-
-    qemu_close(tpm_pt->tpm_fd);
-    qemu_close(tpm_pt->cancel_fd);
-    g_free(tpm_pt->tpm_dev);
-
-    qapi_free_TPMPassthroughOptions(tpm_pt->ops);
-}
-
 static TPMOptions *tpm_passthrough_get_tpm_options(TPMBackend *tb)
 {
     TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
@@ -483,7 +470,6 @@ static const TPMDriverOps tpm_passthrough_driver = {
     .opts                     = tpm_passthrough_cmdline_opts,
     .desc                     = tpm_passthrough_create_desc,
     .create                   = tpm_passthrough_create,
-    .destroy                  = tpm_passthrough_destroy,
     .realloc_buffer           = tpm_passthrough_realloc_buffer,
     .reset                    = tpm_passthrough_reset,
     .had_startup_error        = tpm_passthrough_get_startup_error,
diff --git a/include/sysemu/tpm_backend.h b/include/sysemu/tpm_backend.h
index 7f4d621..8f8f133 100644
--- a/include/sysemu/tpm_backend.h
+++ b/include/sysemu/tpm_backend.h
@@ -77,7 +77,6 @@ struct TPMDriverOps {
     const char *(*desc)(void);
 
     TPMBackend *(*create)(QemuOpts *opts, const char *id);
-    void (*destroy)(TPMBackend *t);
 
     /* initialize the backend */
     int (*init)(TPMBackend *t);
@@ -119,12 +118,6 @@ enum TpmType tpm_backend_get_type(TPMBackend *s);
 const char *tpm_backend_get_desc(TPMBackend *s);
 
 /**
- * tpm_backend_destroy:
- * @s: the backend to destroy
- */
-void tpm_backend_destroy(TPMBackend *s);
-
-/**
  * tpm_backend_init:
  * @s: the backend to initialized
  * @state: TPMState
diff --git a/tpm.c b/tpm.c
index 1b6b550..43d980e 100644
--- a/tpm.c
+++ b/tpm.c
@@ -197,7 +197,7 @@ void tpm_cleanup(void)
 
     QLIST_FOREACH_SAFE(drv, &tpm_backends, list, next) {
         QLIST_REMOVE(drv, list);
-        tpm_backend_destroy(drv);
+        object_unref(OBJECT(drv));
     }
 }
 
-- 
2.7.4
Re: [Qemu-devel] [PATCH v2 6/9] tpm-backend: Remove unneeded destroy() method from TpmDriverOps interface
Posted by Stefan Berger 8 years, 9 months ago 
On 04/07/2017 10:30 AM, Amarnath Valluri wrote:
> As TPMBackend is a Qemu Object, we can use object_unref() inplace of
> tpm_backend_destroy() to free the backend object, hence removed destroy() from
> TPMDriverOps interface.
>
> Signed-off-by: Amarnath Valluri <amarnath.valluri@intel.com>
> ---
>   backends/tpm.c               | 11 -----------
>   hw/tpm/tpm_passthrough.c     | 14 --------------
>   include/sysemu/tpm_backend.h |  7 -------
>   tpm.c                        |  2 +-
>   4 files changed, 1 insertion(+), 33 deletions(-)
>
> diff --git a/backends/tpm.c b/backends/tpm.c
> index c96f462..3493df6 100644
> --- a/backends/tpm.c
> +++ b/backends/tpm.c
> @@ -51,17 +51,6 @@ const char *tpm_backend_get_desc(TPMBackend *s)
>       return k->ops->desc ? k->ops->desc() : "";
>   }
>
> -void tpm_backend_destroy(TPMBackend *s)
> -{
> -    TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
> -
> -    if (k->ops->destroy) {
> -        k->ops->destroy(s);
> -    }
> -
> -    object_unref(OBJECT(s));
> -}
> -
>   int tpm_backend_init(TPMBackend *s, TPMState *state,
>                        TPMRecvDataCB *datacb)
>   {
> diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
> index 71bdf25..8e11ed3 100644
> --- a/hw/tpm/tpm_passthrough.c
> +++ b/hw/tpm/tpm_passthrough.c
> @@ -428,19 +428,6 @@ err_exit:
>       return NULL;
>   }
>
> -static void tpm_passthrough_destroy(TPMBackend *tb)
> -{
> -    TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
> -
> -    tpm_passthrough_cancel_cmd(tb);
> -
> -    qemu_close(tpm_pt->tpm_fd);
> -    qemu_close(tpm_pt->cancel_fd);
> -    g_free(tpm_pt->tpm_dev);

This g_free should be in the finalizer as well, right? It isn't. So we 
will leak some memory.


> -
> -    qapi_free_TPMPassthroughOptions(tpm_pt->ops);
> -}
> -
>   static TPMOptions *tpm_passthrough_get_tpm_options(TPMBackend *tb)
>   {
>       TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
> @@ -483,7 +470,6 @@ static const TPMDriverOps tpm_passthrough_driver = {
>       .opts                     = tpm_passthrough_cmdline_opts,
>       .desc                     = tpm_passthrough_create_desc,
>       .create                   = tpm_passthrough_create,
> -    .destroy                  = tpm_passthrough_destroy,
>       .realloc_buffer           = tpm_passthrough_realloc_buffer,
>       .reset                    = tpm_passthrough_reset,
>       .had_startup_error        = tpm_passthrough_get_startup_error,
> diff --git a/include/sysemu/tpm_backend.h b/include/sysemu/tpm_backend.h
> index 7f4d621..8f8f133 100644
> --- a/include/sysemu/tpm_backend.h
> +++ b/include/sysemu/tpm_backend.h
> @@ -77,7 +77,6 @@ struct TPMDriverOps {
>       const char *(*desc)(void);
>
>       TPMBackend *(*create)(QemuOpts *opts, const char *id);
> -    void (*destroy)(TPMBackend *t);
>
>       /* initialize the backend */
>       int (*init)(TPMBackend *t);
> @@ -119,12 +118,6 @@ enum TpmType tpm_backend_get_type(TPMBackend *s);
>   const char *tpm_backend_get_desc(TPMBackend *s);
>
>   /**
> - * tpm_backend_destroy:
> - * @s: the backend to destroy
> - */
> -void tpm_backend_destroy(TPMBackend *s);
> -
> -/**
>    * tpm_backend_init:
>    * @s: the backend to initialized
>    * @state: TPMState
> diff --git a/tpm.c b/tpm.c
> index 1b6b550..43d980e 100644
> --- a/tpm.c
> +++ b/tpm.c
> @@ -197,7 +197,7 @@ void tpm_cleanup(void)
>
>       QLIST_FOREACH_SAFE(drv, &tpm_backends, list, next) {
>           QLIST_REMOVE(drv, list);
> -        tpm_backend_destroy(drv);
> +        object_unref(OBJECT(drv));
>       }
>   }
>

I *think* you should apply this patch after introducing the 
tpm_passthrough_inst_finalize or merge it into that one.
Re: [Qemu-devel] [PATCH v2 6/9] tpm-backend: Remove unneeded destroy() method from TpmDriverOps interface
Posted by Amarnath Valluri 8 years, 9 months ago 

On 04/25/2017 09:59 PM, Stefan Berger wrote:
> On 04/07/2017 10:30 AM, Amarnath Valluri wrote:
>> As TPMBackend is a Qemu Object, we can use object_unref() inplace of
>> tpm_backend_destroy() to free the backend object, hence removed 
>> destroy() from
>> TPMDriverOps interface.
>>
>> Signed-off-by: Amarnath Valluri <amarnath.valluri@intel.com>
>> ---
>>   backends/tpm.c               | 11 -----------
>>   hw/tpm/tpm_passthrough.c     | 14 --------------
>>   include/sysemu/tpm_backend.h |  7 -------
>>   tpm.c                        |  2 +-
>>   4 files changed, 1 insertion(+), 33 deletions(-)
>>
>> diff --git a/backends/tpm.c b/backends/tpm.c
>> index c96f462..3493df6 100644
>> --- a/backends/tpm.c
>> +++ b/backends/tpm.c
>> @@ -51,17 +51,6 @@ const char *tpm_backend_get_desc(TPMBackend *s)
>>       return k->ops->desc ? k->ops->desc() : "";
>>   }
>>
>> -void tpm_backend_destroy(TPMBackend *s)
>> -{
>> -    TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
>> -
>> -    if (k->ops->destroy) {
>> -        k->ops->destroy(s);
>> -    }
>> -
>> -    object_unref(OBJECT(s));
>> -}
>> -
>>   int tpm_backend_init(TPMBackend *s, TPMState *state,
>>                        TPMRecvDataCB *datacb)
>>   {
>> diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
>> index 71bdf25..8e11ed3 100644
>> --- a/hw/tpm/tpm_passthrough.c
>> +++ b/hw/tpm/tpm_passthrough.c
>> @@ -428,19 +428,6 @@ err_exit:
>>       return NULL;
>>   }
>>
>> -static void tpm_passthrough_destroy(TPMBackend *tb)
>> -{
>> -    TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
>> -
>> -    tpm_passthrough_cancel_cmd(tb);
>> -
>> -    qemu_close(tpm_pt->tpm_fd);
>> -    qemu_close(tpm_pt->cancel_fd);
>> -    g_free(tpm_pt->tpm_dev);
>
> This g_free should be in the finalizer as well, right? It isn't. So we 
> will leak some memory.
Yes, I will fix this.
>
>
>> -
>> -    qapi_free_TPMPassthroughOptions(tpm_pt->ops);
>> -}
>> -
>>   static TPMOptions *tpm_passthrough_get_tpm_options(TPMBackend *tb)
>>   {
>>       TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb);
>> @@ -483,7 +470,6 @@ static const TPMDriverOps tpm_passthrough_driver = {
>>       .opts                     = tpm_passthrough_cmdline_opts,
>>       .desc                     = tpm_passthrough_create_desc,
>>       .create                   = tpm_passthrough_create,
>> -    .destroy                  = tpm_passthrough_destroy,
>>       .realloc_buffer           = tpm_passthrough_realloc_buffer,
>>       .reset                    = tpm_passthrough_reset,
>>       .had_startup_error        = tpm_passthrough_get_startup_error,
>> diff --git a/include/sysemu/tpm_backend.h b/include/sysemu/tpm_backend.h
>> index 7f4d621..8f8f133 100644
>> --- a/include/sysemu/tpm_backend.h
>> +++ b/include/sysemu/tpm_backend.h
>> @@ -77,7 +77,6 @@ struct TPMDriverOps {
>>       const char *(*desc)(void);
>>
>>       TPMBackend *(*create)(QemuOpts *opts, const char *id);
>> -    void (*destroy)(TPMBackend *t);
>>
>>       /* initialize the backend */
>>       int (*init)(TPMBackend *t);
>> @@ -119,12 +118,6 @@ enum TpmType tpm_backend_get_type(TPMBackend *s);
>>   const char *tpm_backend_get_desc(TPMBackend *s);
>>
>>   /**
>> - * tpm_backend_destroy:
>> - * @s: the backend to destroy
>> - */
>> -void tpm_backend_destroy(TPMBackend *s);
>> -
>> -/**
>>    * tpm_backend_init:
>>    * @s: the backend to initialized
>>    * @state: TPMState
>> diff --git a/tpm.c b/tpm.c
>> index 1b6b550..43d980e 100644
>> --- a/tpm.c
>> +++ b/tpm.c
>> @@ -197,7 +197,7 @@ void tpm_cleanup(void)
>>
>>       QLIST_FOREACH_SAFE(drv, &tpm_backends, list, next) {
>>           QLIST_REMOVE(drv, list);
>> -        tpm_backend_destroy(drv);
>> +        object_unref(OBJECT(drv));
>>       }
>>   }
>>
>
> I *think* you should apply this patch after introducing the 
> tpm_passthrough_inst_finalize or merge it into that one.
>
Ok, i even think it makes sense to meld this commit with 
baeb9df(tpm-backend: Initialize and free data members in it's own methods).

- Amarnath

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.