| Message ID | 20210810131228.2332728-1-jean-philippe@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | vl: fix machine option containing underscores | expand |
+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); > } > } > >
On 10/08/21 15:12, Jean-Philippe Brucker wrote: > My first take was renaming default_bus_bypass_iommu, since it's the only > machine option with underscores, We should do that, since the underscore variant still works and the result is a simple one-line patch. Paolo > 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.
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); } }
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(-)