Re: [PATCH v3 1/2] tpm: convert tpmdev options processing to new visitor format

[Daniel P . Berrangé]

On Mon, Dec 19, 2022 at 08:13:43AM -0500, James Bottomley wrote:
> From: James Bottomley <James.Bottomley@HansenPartnership.com>
> 
> Instead of processing the tpmdev options using the old qemu options,
> convert to the new visitor format which also allows the passing of
> json on the command line.
> 
> Signed-off-by: James Bottomley <jejb@linux.ibm.com>
> ---
>  backends/tpm/tpm_emulator.c    | 35 ++++++-------
>  backends/tpm/tpm_passthrough.c | 37 +++++---------
>  include/sysemu/tpm.h           |  4 +-
>  include/sysemu/tpm_backend.h   |  2 +-
>  monitor/hmp-cmds.c             |  4 +-
>  qapi/tpm.json                  | 26 ++--------
>  softmmu/tpm.c                  | 90 ++++++++++++++--------------------
>  softmmu/vl.c                   | 19 +------
>  8 files changed, 73 insertions(+), 144 deletions(-)
> 

> diff --git a/qapi/tpm.json b/qapi/tpm.json
> index 4e2ea9756a..d8cbd5ea0e 100644
> --- a/qapi/tpm.json
> +++ b/qapi/tpm.json
> @@ -99,39 +99,23 @@
>  { 'struct': 'TPMEmulatorOptions', 'data': { 'chardev' : 'str' },
>    'if': 'CONFIG_TPM' }
>  
> -##
> -# @TPMPassthroughOptionsWrapper:
> -#
> -# Since: 1.5
> -##
> -{ 'struct': 'TPMPassthroughOptionsWrapper',
> -  'data': { 'data': 'TPMPassthroughOptions' },
> -  'if': 'CONFIG_TPM' }
> -
> -##
> -# @TPMEmulatorOptionsWrapper:
> -#
> -# Since: 2.11
> -##
> -{ 'struct': 'TPMEmulatorOptionsWrapper',
> -  'data': { 'data': 'TPMEmulatorOptions' },
> -  'if': 'CONFIG_TPM' }
> -
>  ##
>  # @TpmTypeOptions:
>  #
>  # A union referencing different TPM backend types' configuration options
>  #
> +# @id: identifier of the backend
>  # @type: - 'passthrough' The configuration options for the TPM passthrough 
> type
>  #        - 'emulator' The configuration options for TPM emulator backend type
>  #
>  # Since: 1.5
>  ##
>  { 'union': 'TpmTypeOptions',
> -  'base': { 'type': 'TpmType' },
> +  'base': { 'type': 'TpmType',
> +            'id': 'str' },
>    'discriminator': 'type',
> -  'data': { 'passthrough' : 'TPMPassthroughOptionsWrapper',
> -            'emulator': 'TPMEmulatorOptionsWrapper' },
> +  'data': { 'passthrough' : 'TPMPassthroughOptions',
> +            'emulator': 'TPMEmulatorOptions' },
>    'if': 'CONFIG_TPM' }

This isn't a valid change todo, as it affects the public facing
data structure for the  query-tpm command.

I understand why you're doing it though, to get rid fo the
extra nesting, which is a hangover from earlier QAPI days
where we couldn't cope with flat unions.

Instead of changing TpmTypeOptions, you'll need to introduce
a new TpmTypeCreateOptions that eliminates the wrapping, and
use that in the CLI creation path, leaving the query-tpm
command unchanged.


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|