Re: [PATCH 05/22] qapi: Convert simple union TpmTypeOptions to flat one

[Stefan Berger]

On 9/13/21 8:39 AM, Markus Armbruster wrote:
> Simple unions predate flat unions.  Having both complicates the QAPI
> schema language and the QAPI generator.  We haven't been using simple
> unions in new code for a long time, because they are less flexible and
> somewhat awkward on the wire.
>
> To prepare for their removal, convert simple union TpmTypeOptions to
> an equivalent flat one, with existing enum TpmType replacing implicit
> enum TpmTypeOptionsKind.  Adds some boilerplate to the schema, which
> is a bit ugly, but a lot easier to maintain than the simple union
> feature.
>
> Cc: Stefan Berger <stefanb@linux.vnet.ibm.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

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


> ---
>   qapi/tpm.json                  | 24 ++++++++++++++++++++++--
>   backends/tpm/tpm_emulator.c    |  2 +-
>   backends/tpm/tpm_passthrough.c |  2 +-
>   monitor/hmp-cmds.c             |  8 ++++----
>   4 files changed, 28 insertions(+), 8 deletions(-)
>
> diff --git a/qapi/tpm.json b/qapi/tpm.json
> index f4dde2f646..b3ade008bf 100644
> --- a/qapi/tpm.json
> +++ b/qapi/tpm.json
> @@ -99,6 +99,24 @@
>   { 'struct': 'TPMEmulatorOptions', 'data': { 'chardev' : 'str' },
>     'if': 'CONFIG_TPM' }
>
> +##
> +# @TPMPassthroughOptionsWrapper:
> +#
> +# Since: 6.1
> +##
> +{ 'struct': 'TPMPassthroughOptionsWrapper',
> +  'data': { 'data': 'TPMPassthroughOptions' },
> +  'if': 'CONFIG_TPM' }
> +
> +##
> +# @TPMEmulatorOptionsWrapper:
> +#
> +# Since: 6.1
> +##
> +{ 'struct': 'TPMEmulatorOptionsWrapper',
> +  'data': { 'data': 'TPMEmulatorOptions' },
> +  'if': 'CONFIG_TPM' }
> +
>   ##
>   # @TpmTypeOptions:
>   #
> @@ -110,8 +128,10 @@
>   # Since: 1.5
>   ##
>   { 'union': 'TpmTypeOptions',
> -   'data': { 'passthrough' : 'TPMPassthroughOptions',
> -             'emulator': 'TPMEmulatorOptions' },
> +  'base': { 'type': 'TpmType' },
> +  'discriminator': 'type',
> +   'data': { 'passthrough' : 'TPMPassthroughOptionsWrapper',
> +             'emulator': 'TPMEmulatorOptionsWrapper' },
>     'if': 'CONFIG_TPM' }
>
>   ##
> diff --git a/backends/tpm/tpm_emulator.c b/backends/tpm/tpm_emulator.c
> index f8095d23d5..87d061e9bb 100644
> --- a/backends/tpm/tpm_emulator.c
> +++ b/backends/tpm/tpm_emulator.c
> @@ -623,7 +623,7 @@ static TpmTypeOptions 
> *tpm_emulator_get_tpm_options(TPMBackend *tb)
>       TPMEmulator *tpm_emu = TPM_EMULATOR(tb);
>       TpmTypeOptions *options = g_new0(TpmTypeOptions, 1);
>
> -    options->type = TPM_TYPE_OPTIONS_KIND_EMULATOR;
> +    options->type = TPM_TYPE_EMULATOR;
>       options->u.emulator.data = QAPI_CLONE(TPMEmulatorOptions, 
> tpm_emu->options);
>
>       return options;
> diff --git a/backends/tpm/tpm_passthrough.c b/backends/tpm/tpm_passthrough.c
> index 21b7459183..d5558fae6c 100644
> --- a/backends/tpm/tpm_passthrough.c
> +++ b/backends/tpm/tpm_passthrough.c
> @@ -321,7 +321,7 @@ static TpmTypeOptions 
> *tpm_passthrough_get_tpm_options(TPMBackend *tb)
>   {
>       TpmTypeOptions *options = g_new0(TpmTypeOptions, 1);
>
> -    options->type = TPM_TYPE_OPTIONS_KIND_PASSTHROUGH;
> +    options->type = TPM_TYPE_PASSTHROUGH;
>       options->u.passthrough.data = QAPI_CLONE(TPMPassthroughOptions,
>                                                TPM_PASSTHROUGH(tb)->options);
>
> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> index e00255f7ee..d6858407ad 100644
> --- a/monitor/hmp-cmds.c
> +++ b/monitor/hmp-cmds.c
> @@ -925,10 +925,10 @@ void hmp_info_tpm(Monitor *mon, const QDict *qdict)
>                          c, TpmModel_str(ti->model));
>
>           monitor_printf(mon, "  \\ %s: type=%s",
> -                       ti->id, TpmTypeOptionsKind_str(ti->options->type));
> +                       ti->id, TpmType_str(ti->options->type));
>
>           switch (ti->options->type) {
> -        case TPM_TYPE_OPTIONS_KIND_PASSTHROUGH:
> +        case TPM_TYPE_PASSTHROUGH:
>               tpo = ti->options->u.passthrough.data;
>               monitor_printf(mon, "%s%s%s%s",
>                              tpo->has_path ? ",path=" : "",
> @@ -936,11 +936,11 @@ void hmp_info_tpm(Monitor *mon, const QDict *qdict)
>                              tpo->has_cancel_path ? ",cancel-path=" : "",
>                              tpo->has_cancel_path ? tpo->cancel_path : "");
>               break;
> -        case TPM_TYPE_OPTIONS_KIND_EMULATOR:
> +        case TPM_TYPE_EMULATOR:
>               teo = ti->options->u.emulator.data;
>               monitor_printf(mon, ",chardev=%s", teo->chardev);
>               break;
> -        case TPM_TYPE_OPTIONS_KIND__MAX:
> +        case TPM_TYPE__MAX:
>               break;
>           }
>           monitor_printf(mon, "\n");