Re: [PATCH v3 5/5] tpm: Return QMP error when TPM is disabled in build

[Markus Armbruster]

Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> When the management layer queries a binary built using --disable-tpm
> for TPM devices, it gets confused by getting empty responses:
>
>   { "execute": "query-tpm" }
>   {
>       "return": [
>       ]
>   }
>   { "execute": "query-tpm-types" }
>   {
>       "return": [
>       ]
>   }
>   { "execute": "query-tpm-models" }
>   {
>       "return": [
>       ]
>   }
>
> To make it clearer by returning an error:
> - Make the TPM QAPI schema conditional

All of tpm.json is now 'if': 'defined(CONFIG_TPM)'.  Good.

> - Adapt the HMP command
> - Remove stubs which became unnecessary
>
> The management layer now gets a 'CommandNotFound' error:
>
>   { "execute": "query-tpm" }
>   {
>       "error": {
>           "class": "CommandNotFound",
>           "desc": "The command query-tpm has not been found"
>       }
>   }
>
> Suggested-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  qapi/tpm.json      | 28 +++++++++++++++++++---------
>  monitor/hmp-cmds.c |  4 ++++
>  stubs/tpm.c        | 25 -------------------------
>  stubs/meson.build  |  1 -
>  4 files changed, 23 insertions(+), 35 deletions(-)
>  delete mode 100644 stubs/tpm.c
>
> diff --git a/qapi/tpm.json b/qapi/tpm.json
> index 6a10c9ed8d2..75590979fde 100644
> --- a/qapi/tpm.json
> +++ b/qapi/tpm.json
> @@ -17,7 +17,9 @@
>  #
>  # Since: 1.5
>  ##
> -{ 'enum': 'TpmModel', 'data': [ 'tpm-tis', 'tpm-crb', 'tpm-spapr' ] }
> +{ 'enum': 'TpmModel', 'data': [ 'tpm-tis', 'tpm-crb', 'tpm-spapr' ],
> +  'if': 'defined(CONFIG_TPM)' }
> +
>  ##
>  # @query-tpm-models:
>  #
> @@ -33,7 +35,8 @@
>  # <- { "return": [ "tpm-tis", "tpm-crb", "tpm-spapr" ] }
>  #
>  ##
> -{ 'command': 'query-tpm-models', 'returns': ['TpmModel'] }
> +{ 'command': 'query-tpm-models', 'returns': ['TpmModel'],
> +  'if': 'defined(CONFIG_TPM)' }
>  
>  ##
>  # @TpmType:
> @@ -46,7 +49,8 @@
>  #
>  # Since: 1.5
>  ##
> -{ 'enum': 'TpmType', 'data': [ 'passthrough', 'emulator' ] }
> +{ 'enum': 'TpmType', 'data': [ 'passthrough', 'emulator' ],
> +  'if': 'defined(CONFIG_TPM)' }
>  
>  ##
>  # @query-tpm-types:
> @@ -63,7 +67,8 @@
>  # <- { "return": [ "passthrough", "emulator" ] }
>  #
>  ##
> -{ 'command': 'query-tpm-types', 'returns': ['TpmType'] }
> +{ 'command': 'query-tpm-types', 'returns': ['TpmType'],
> +  'if': 'defined(CONFIG_TPM)' }
>  
>  ##
>  # @TPMPassthroughOptions:
> @@ -79,7 +84,8 @@
>  ##
>  { 'struct': 'TPMPassthroughOptions',
>    'data': { '*path': 'str',
> -            '*cancel-path': 'str' } }
> +            '*cancel-path': 'str' },
> +  'if': 'defined(CONFIG_TPM)' }
>  
>  ##
>  # @TPMEmulatorOptions:
> @@ -90,7 +96,8 @@
>  #
>  # Since: 2.11
>  ##
> -{ 'struct': 'TPMEmulatorOptions', 'data': { 'chardev' : 'str' } }
> +{ 'struct': 'TPMEmulatorOptions', 'data': { 'chardev' : 'str' },
> +  'if': 'defined(CONFIG_TPM)' }
>  
>  ##
>  # @TpmTypeOptions:
> @@ -104,7 +111,8 @@
>  ##
>  { 'union': 'TpmTypeOptions',
>     'data': { 'passthrough' : 'TPMPassthroughOptions',
> -             'emulator': 'TPMEmulatorOptions' } }
> +             'emulator': 'TPMEmulatorOptions' },
> +  'if': 'defined(CONFIG_TPM)' }
>  
>  ##
>  # @TPMInfo:
> @@ -122,7 +130,8 @@
>  { 'struct': 'TPMInfo',
>    'data': {'id': 'str',
>             'model': 'TpmModel',
> -           'options': 'TpmTypeOptions' } }
> +           'options': 'TpmTypeOptions' },
> +  'if': 'defined(CONFIG_TPM)' }
>  
>  ##
>  # @query-tpm:
> @@ -152,4 +161,5 @@
>  #    }
>  #
>  ##
> -{ 'command': 'query-tpm', 'returns': ['TPMInfo'] }
> +{ 'command': 'query-tpm', 'returns': ['TPMInfo'],
> +  'if': 'defined(CONFIG_TPM)' }
> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> index d10ee141109..f6cadede40f 100644
> --- a/monitor/hmp-cmds.c
> +++ b/monitor/hmp-cmds.c
> @@ -901,6 +901,9 @@ void hmp_info_pci(Monitor *mon, const QDict *qdict)
>  
>  void hmp_info_tpm(Monitor *mon, const QDict *qdict)
>  {
> +#ifndef CONFIG_TPM
> +    monitor_printf(mon, "TPM device not supported\n");
> +#else

I dislike #ifndef ... #else.  Matter of taste.

>      TPMInfoList *info_list, *info;
>      Error *err = NULL;
>      unsigned int c = 0;
> @@ -946,6 +949,7 @@ void hmp_info_tpm(Monitor *mon, const QDict *qdict)
>          c++;
>      }
>      qapi_free_TPMInfoList(info_list);
> +#endif /* CONFIG_TPM */
>  }
>  
>  void hmp_quit(Monitor *mon, const QDict *qdict)
> diff --git a/stubs/tpm.c b/stubs/tpm.c
> deleted file mode 100644
> index e79bd2a6c2d..00000000000
> --- a/stubs/tpm.c
> +++ /dev/null
> @@ -1,25 +0,0 @@
> -/*
> - * TPM stubs
> - *
> - * This work is licensed under the terms of the GNU GPL, version 2 or later.
> - * See the COPYING file in the top-level directory.
> - */
> -
> -#include "qemu/osdep.h"
> -#include "qapi/qapi-commands-tpm.h"
> -#include "hw/acpi/tpm.h"
> -
> -TPMInfoList *qmp_query_tpm(Error **errp)
> -{
> -    return NULL;
> -}
> -
> -TpmTypeList *qmp_query_tpm_types(Error **errp)
> -{
> -    return NULL;
> -}
> -
> -TpmModelList *qmp_query_tpm_models(Error **errp)
> -{
> -    return NULL;
> -}
> diff --git a/stubs/meson.build b/stubs/meson.build
> index 65c22c0568c..d4e9549dc99 100644
> --- a/stubs/meson.build
> +++ b/stubs/meson.build
> @@ -38,7 +38,6 @@
>  stub_ss.add(files('sysbus.c'))
>  stub_ss.add(files('target-get-monitor-def.c'))
>  stub_ss.add(files('target-monitor-defs.c'))
> -stub_ss.add(files('tpm.c'))
>  stub_ss.add(files('trace-control.c'))
>  stub_ss.add(files('uuid.c'))
>  stub_ss.add(files('vmgenid.c'))

Reviewed-by: Markus Armbruster <armbru@redhat.com>