Re: [PATCH] vl: fix machine option containing underscores

[Philippe Mathieu-Daudé]

+Paolo/Markus

On 8/10/21 3:12 PM, Jean-Philippe Brucker wrote:
> Since commit d8fb7d0969d5 ("vl: switch -M parsing to keyval"),
> keyval_dashify() replaces all underscores with dashes in machine
> options. As a result the machine option "default_bus_bypass_iommu",
> which was introduced in the same release (c9e96b04fc19 6d7a85483a06), is
> not recognized:
> 
> $ qemu-system-aarch64 -M virt,default_bus_bypass_iommu=on
> qemu-system-aarch64: Property 'virt-6.1-machine.default-bus-bypass-iommu' not 
> found
> 
> Before that commit, dashification was only applied temporarily within
> machine_set_property() to check the legacy options. Restore that
> behavior by explicitly checking for aliases of these options instead of
> transforming all machine options.
> 
> Fixes: d8fb7d0969d5 ("vl: switch -M parsing to keyval")
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
> My first take was renaming default_bus_bypass_iommu, since it's the only
> machine option with underscores, but then we'd want to rename
> bypass_iommu as well for consistency and update all the docs. I prefer
> this but don't mind either way.
> ---
>  softmmu/vl.c | 56 +++++++++++++++++++---------------------------------
>  1 file changed, 20 insertions(+), 36 deletions(-)
> 
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index 5ca11e7469..3d3aa35279 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -1660,41 +1660,25 @@ static int object_parse_property_opt(Object *obj,
>      return 0;
>  }
>  
> -/* *Non*recursively replace underscores with dashes in QDict keys.  */
> -static void keyval_dashify(QDict *qdict, Error **errp)
> +static const char *find_option_alias(QDict *qdict, const char *key,
> +                                     const char *alias, const char **value)
>  {
> -    const QDictEntry *ent, *next;
> -    char *p;
> -
> -    for (ent = qdict_first(qdict); ent; ent = next) {
> -        g_autofree char *new_key = NULL;
> -
> -        next = qdict_next(qdict, ent);
> -        if (!strchr(ent->key, '_')) {
> -            continue;
> -        }
> -        new_key = g_strdup(ent->key);
> -        for (p = new_key; *p; p++) {
> -            if (*p == '_') {
> -                *p = '-';
> -            }
> -        }
> -        if (qdict_haskey(qdict, new_key)) {
> -            error_setg(errp, "Conflict between '%s' and '%s'", ent->key, 
> new_key);
> -            return;
> -        }
> -        qobject_ref(ent->value);
> -        qdict_put_obj(qdict, new_key, ent->value);
> -        qdict_del(qdict, ent->key);
> +    *value = qdict_get_try_str(qdict, key);
> +    if (*value) {
> +        return key;
> +    }
> +    *value = qdict_get_try_str(qdict, alias);
> +    if (*value) {
> +        return alias;
>      }
> +    return NULL;
>  }
>  
>  static void qemu_apply_legacy_machine_options(QDict *qdict)
>  {
> +    const char *key;
>      const char *value;
>  
> -    keyval_dashify(qdict, &error_fatal);
> -
>      /* Legacy options do not correspond to MachineState properties.  */
>      value = qdict_get_try_str(qdict, "accel");
>      if (value) {
> @@ -1702,27 +1686,27 @@ static void qemu_apply_legacy_machine_options(QDict 
> *qdict)
>          qdict_del(qdict, "accel");
>      }
>  
> -    value = qdict_get_try_str(qdict, "igd-passthru");
> -    if (value) {
> +    key = find_option_alias(qdict, "igd-passthru", "igd_passthru", &value);
> +    if (key) {
>          object_register_sugar_prop(ACCEL_CLASS_NAME("xen"), "igd-passthru", 
> value,
>                                     false);
> -        qdict_del(qdict, "igd-passthru");
> +        qdict_del(qdict, key);
>      }
>  
> -    value = qdict_get_try_str(qdict, "kvm-shadow-mem");
> -    if (value) {
> +    key = find_option_alias(qdict, "kvm-shadow-mem", "kvm_shadow_mem", 
> &value);
> +    if (key) {
>          object_register_sugar_prop(ACCEL_CLASS_NAME("kvm"), 
> "kvm-shadow-mem", value,
>                                     false);
> -        qdict_del(qdict, "kvm-shadow-mem");
> +        qdict_del(qdict, key);
>      }
>  
> -    value = qdict_get_try_str(qdict, "kernel-irqchip");
> -    if (value) {
> +    key = find_option_alias(qdict, "kernel-irqchip", "kernel_irqchip", 
> &value);
> +    if (key) {
>          object_register_sugar_prop(ACCEL_CLASS_NAME("kvm"), 
> "kernel-irqchip", value,
>                                     false);
>          object_register_sugar_prop(ACCEL_CLASS_NAME("whpx"), 
> "kernel-irqchip", value,
>                                     false);
> -        qdict_del(qdict, "kernel-irqchip");
> +        qdict_del(qdict, key);
>      }
>  }
>  
>