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

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

On 1/5/23 08:00, 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>
> 
> ---
> v4: add TpmConfiOptions
> ---
>   backends/tpm/tpm_emulator.c    | 24 ++++-----
>   backends/tpm/tpm_passthrough.c | 25 +++-------
>   include/sysemu/tpm.h           |  4 +-
>   include/sysemu/tpm_backend.h   |  2 +-
>   qapi/tpm.json                  | 19 +++++++
>   softmmu/tpm.c                  | 90 ++++++++++++++--------------------
>   softmmu/vl.c                   | 19 +------
>   7 files changed, 76 insertions(+), 107 deletions(-)
> 
> diff --git a/backends/tpm/tpm_emulator.c b/backends/tpm/tpm_emulator.c
> index 49cc3d749d..cb6bf9d7c2 100644
> --- a/backends/tpm/tpm_emulator.c
> +++ b/backends/tpm/tpm_emulator.c
> @@ -584,33 +584,28 @@ err_exit:
>       return -1;
>   }
>   
> -static int tpm_emulator_handle_device_opts(TPMEmulator *tpm_emu, QemuOpts *opts)
> +static int tpm_emulator_handle_device_opts(TPMEmulator *tpm_emu, TpmCreateOptions *opts)
>   {
> -    const char *value;
>       Error *err = NULL;
>       Chardev *dev;
>   
> -    value = qemu_opt_get(opts, "chardev");
> -    if (!value) {
> -        error_report("tpm-emulator: parameter 'chardev' is missing");
> -        goto err;
> -    }
> +    tpm_emu->options = QAPI_CLONE(TPMEmulatorOptions, &opts->u.emulator);
> +    tpm_emu->data_ioc = NULL;
>   
> -    dev = qemu_chr_find(value);
> +    dev = qemu_chr_find(opts->u.emulator.chardev);
>       if (!dev) {
> -        error_report("tpm-emulator: tpm chardev '%s' not found", value);
> +        error_report("tpm-emulator: tpm chardev '%s' not found",
> +                opts->u.emulator.chardev);
>           goto err;
>       }
>   
>       if (!qemu_chr_fe_init(&tpm_emu->ctrl_chr, dev, &err)) {
>           error_prepend(&err, "tpm-emulator: No valid chardev found at '%s':",
> -                      value);
> +                      opts->u.emulator.chardev);
>           error_report_err(err);
>           goto err;
>       }
>   
> -    tpm_emu->options->chardev = g_strdup(value);
> -
>       if (tpm_emulator_prepare_data_fd(tpm_emu) < 0) {
>           goto err;
>       }
> @@ -649,7 +644,7 @@ err:
>       return -1;
>   }
>   
> -static TPMBackend *tpm_emulator_create(QemuOpts *opts)
> +static TPMBackend *tpm_emulator_create(TpmCreateOptions *opts)
>   {
>       TPMBackend *tb = TPM_BACKEND(object_new(TYPE_TPM_EMULATOR));
>   
> @@ -972,7 +967,6 @@ static void tpm_emulator_inst_init(Object *obj)
>   
>       trace_tpm_emulator_inst_init();
>   
> -    tpm_emu->options = g_new0(TPMEmulatorOptions, 1);
>       tpm_emu->cur_locty_number = ~0;
>       qemu_mutex_init(&tpm_emu->mutex);
>       tpm_emu->vmstate =
> @@ -990,7 +984,7 @@ static void tpm_emulator_shutdown(TPMEmulator *tpm_emu)
>   {
>       ptm_res res;
>   
> -    if (!tpm_emu->options->chardev) {
> +    if (!tpm_emu->data_ioc) {
>           /* was never properly initialized */
>           return;
>       }
> diff --git a/backends/tpm/tpm_passthrough.c b/backends/tpm/tpm_passthrough.c
> index 179697a3a9..4a30143257 100644
> --- a/backends/tpm/tpm_passthrough.c
> +++ b/backends/tpm/tpm_passthrough.c
> @@ -252,21 +252,11 @@ static int tpm_passthrough_open_sysfs_cancel(TPMPassthruState *tpm_pt)
>   }
>   
>   static int
> -tpm_passthrough_handle_device_opts(TPMPassthruState *tpm_pt, QemuOpts *opts)
> +tpm_passthrough_handle_device_opts(TPMPassthruState *tpm_pt, TpmCreateOptions *opts)
>   {
> -    const char *value;
> +    tpm_pt->options = QAPI_CLONE(TPMPassthroughOptions, &opts->u.passthrough);
>   
> -    value = qemu_opt_get(opts, "cancel-path");
> -    if (value) {
> -        tpm_pt->options->cancel_path = g_strdup(value);
> -    }
> -
> -    value = qemu_opt_get(opts, "path");
> -    if (value) {
> -        tpm_pt->options->path = g_strdup(value);
> -    }
> -
> -    tpm_pt->tpm_dev = value ? value : TPM_PASSTHROUGH_DEFAULT_DEVICE;
> +    tpm_pt->tpm_dev = opts->u.passthrough.path ? opts->u.passthrough.path : TPM_PASSTHROUGH_DEFAULT_DEVICE;

WARNING: line over 80 characters
#31: FILE: backends/tpm/tpm_emulator.c:587:
+static int tpm_emulator_handle_device_opts(TPMEmulator *tpm_emu, TpmCreateOptions *opts)

WARNING: line over 80 characters
#102: FILE: backends/tpm/tpm_passthrough.c:255:
+tpm_passthrough_handle_device_opts(TPMPassthruState *tpm_pt, TpmCreateOptions *opts)

ERROR: line over 90 characters
#118: FILE: backends/tpm/tpm_passthrough.c:259:
+    tpm_pt->tpm_dev = opts->u.passthrough.path ? opts->u.passthrough.path : TPM_PASSTHROUGH_DEFAULT_DEVICE;

ERROR: line over 90 characters
#143: FILE: backends/tpm/tpm_passthrough.c:313:
+    options->u.passthrough.data = QAPI_CLONE(TPMPassthroughOptions, TPM_PASSTHROUGH(tb)->options);

total: 2 errors, 2 warnings, 349 lines checked

mssim.v5/0001-tpm-convert-tpmdev-options-processing-to-new-visitor.patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.



> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index 798e1dc933..8c2655dbc2 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -328,16 +328,6 @@ static QemuOptsList qemu_object_opts = {
>       },
>   };
>   
> -static QemuOptsList qemu_tpmdev_opts = {
> -    .name = "tpmdev",
> -    .implied_opt_name = "type",
> -    .head = QTAILQ_HEAD_INITIALIZER(qemu_tpmdev_opts.head),
> -    .desc = {
> -        /* options are defined in the TPM backends */
> -        { /* end of list */ }
> -    },
> -};
> -
>   static QemuOptsList qemu_overcommit_opts = {
>       .name = "overcommit",
>       .head = QTAILQ_HEAD_INITIALIZER(qemu_overcommit_opts.head),
> @@ -1934,9 +1924,7 @@ static void qemu_create_late_backends(void)
>   
>       object_option_foreach_add(object_create_late);
>   
> -    if (tpm_init() < 0) {
> -        exit(1);
> -    }
> +    tpm_init();
>   
>       qemu_opts_foreach(qemu_find_opts("mon"),
>                         mon_init_func, NULL, &error_fatal);
> @@ -2658,7 +2646,6 @@ void qemu_init(int argc, char **argv)
>       qemu_add_opts(&qemu_boot_opts);
>       qemu_add_opts(&qemu_add_fd_opts);
>       qemu_add_opts(&qemu_object_opts);
> -    qemu_add_opts(&qemu_tpmdev_opts);
>       qemu_add_opts(&qemu_overcommit_opts);
>       qemu_add_opts(&qemu_msg_opts);
>       qemu_add_opts(&qemu_name_opts);
> @@ -2906,9 +2893,7 @@ void qemu_init(int argc, char **argv)
>                   break;
>   #ifdef CONFIG_TPM
>               case QEMU_OPTION_tpmdev:
> -                if (tpm_config_parse(qemu_find_opts("tpmdev"), optarg) < 0) {
> -                    exit(1);
> -                }
> +                tpm_config_parse(optarg);

The comment to v4 still applies:

$ qemu-system-x86_64 --tpmdev help
Supported TPM types (choose only one):
  passthrough   Passthrough TPM backend driver
     emulator   TPM emulator backend driver
        mssim   TPM mssim emulator backend driver
VNC server running on ::1:5900


>                   break;
>   #endif
>               case QEMU_OPTION_mempath: