| Message ID | 20210706100141.303960-37-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [PULL,01/40] Makefile: Remove /usr/bin/env wrapper from the SHELL variable | expand |
On Tue, Jul 6, 2021 at 6:43 AM Paolo Bonzini <pbonzini@redhat.com> wrote: > > Switch from QemuOpts to keyval. This enables the introduction > of non-scalar machine properties, and JSON syntax in the future. > > For JSON syntax to be supported right now, we would have to > consider what would happen if string-based dictionaries (produced by > -M key=val) were to be merged with strongly-typed dictionaries > (produced by -M {'key': 123}). > > The simplest way out is to never enter the situation, and only allow one > -M option when JSON syntax is in use. However, we want options such as > -smp to become syntactic sugar for -M, and this is a problem; as soon > as -smp becomes a shortcut for -M, QEMU would forbid using -M '{....}' > together with -smp. Therefore, allowing JSON syntax right now for -M > would be a forward-compatibility nightmare and it would be impossible > anyway to introduce -M incrementally in tools. > > Instead, support for JSON syntax is delayed until after the main > options are converted to QOM compound properties. These include -boot, > -acpitable, -smbios, -m, -semihosting-config, -rtc and -fw_cfg. Once JSON > syntax is introduced, these options will _also_ be forbidden together > with -M '{...}'. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > softmmu/vl.c | 315 ++++++++++++++++++++++++--------------------------- > 1 file changed, 146 insertions(+), 169 deletions(-) Xen's osstest bisector found this commit breaks starting a Xen vm: https://lore.kernel.org/xen-devel/E1m1HlC-0005Ra-Ff@osstest.test-lab.xenproject.org/ qemu fails to start with: Unexpected error in object_property_find_err() at ../qemu-xen-dir-remote/qom/object.c:1299: qemu-system-i386: Property 'xenfv-3.1-machine.accel' not found The Xen machines have `default_machine_opts = "accel=xen,suppress-vmdesc=on"` which may be the problem? Regards, Jason > diff --git a/softmmu/vl.c b/softmmu/vl.c > index 7dd2d72d0b..f848abd31a 100644 > --- a/softmmu/vl.c > +++ b/softmmu/vl.c > @@ -145,6 +145,8 @@ static const char *cpu_option; > static const char *mem_path; > static const char *incoming; > static const char *loadvm; > +static const char *accelerators; > +static QDict *machine_opts_dict; > static QTAILQ_HEAD(, ObjectOption) object_opts = QTAILQ_HEAD_INITIALIZER(object_opts); > static ram_addr_t maxram_size; > static uint64_t ram_slots; > @@ -235,21 +237,6 @@ static QemuOptsList qemu_option_rom_opts = { > }, > }; > > -static QemuOptsList qemu_machine_opts = { > - .name = "machine", > - .implied_opt_name = "type", > - .merge_lists = true, > - .head = QTAILQ_HEAD_INITIALIZER(qemu_machine_opts.head), > - .desc = { > - /* > - * no elements => accept any > - * sanity checking will happen later > - * when setting machine properties > - */ > - { } > - }, > -}; > - > static QemuOptsList qemu_accel_opts = { > .name = "accel", > .implied_opt_name = "accel", > @@ -498,16 +485,6 @@ static QemuOptsList qemu_action_opts = { > }, > }; > > -/** > - * Get machine options > - * > - * Returns: machine options (never null). > - */ > -static QemuOpts *qemu_get_machine_opts(void) > -{ > - return qemu_find_opts_singleton("machine"); > -} > - > const char *qemu_get_vm_name(void) > { > return qemu_name; > @@ -815,33 +792,6 @@ static MachineClass *find_default_machine(GSList *machines) > return default_machineclass; > } > > -static int machine_help_func(QemuOpts *opts, MachineState *machine) > -{ > - ObjectProperty *prop; > - ObjectPropertyIterator iter; > - > - if (!qemu_opt_has_help_opt(opts)) { > - return 0; > - } > - > - object_property_iter_init(&iter, OBJECT(machine)); > - while ((prop = object_property_iter_next(&iter))) { > - if (!prop->set) { > - continue; > - } > - > - printf("%s.%s=%s", MACHINE_GET_CLASS(machine)->name, > - prop->name, prop->type); > - if (prop->description) { > - printf(" (%s)\n", prop->description); > - } else { > - printf("\n"); > - } > - } > - > - return 1; > -} > - > static void version(void) > { > printf("QEMU emulator version " QEMU_FULL_VERSION "\n" > @@ -1554,33 +1504,31 @@ static gint machine_class_cmp(gconstpointer a, gconstpointer b) > object_class_get_name(OBJECT_CLASS(mc1))); > } > > -static MachineClass *machine_parse(const char *name, GSList *machines) > +static void machine_help_func(const QDict *qdict) > { > - MachineClass *mc; > - GSList *el; > + GSList *machines, *el; > + const char *type = qdict_get_try_str(qdict, "type"); > > - if (is_help_option(name)) { > - printf("Supported machines are:\n"); > - machines = g_slist_sort(machines, machine_class_cmp); > - for (el = machines; el; el = el->next) { > - MachineClass *mc = el->data; > - if (mc->alias) { > - printf("%-20s %s (alias of %s)\n", mc->alias, mc->desc, mc->name); > - } > - printf("%-20s %s%s%s\n", mc->name, mc->desc, > - mc->is_default ? " (default)" : "", > - mc->deprecation_reason ? " (deprecated)" : ""); > + machines = object_class_get_list(TYPE_MACHINE, false); > + if (type) { > + ObjectClass *machine_class = OBJECT_CLASS(find_machine(type, machines)); > + if (machine_class) { > + type_print_class_properties(object_class_get_name(machine_class)); > + return; > } > - exit(0); > } > > - mc = find_machine(name, machines); > - if (!mc) { > - error_report("unsupported machine type"); > - error_printf("Use -machine help to list supported machines\n"); > - exit(1); > + printf("Supported machines are:\n"); > + machines = g_slist_sort(machines, machine_class_cmp); > + for (el = machines; el; el = el->next) { > + MachineClass *mc = el->data; > + if (mc->alias) { > + printf("%-20s %s (alias of %s)\n", mc->alias, mc->desc, mc->name); > + } > + printf("%-20s %s%s%s\n", mc->name, mc->desc, > + mc->is_default ? " (default)" : "", > + mc->deprecation_reason ? " (deprecated)" : ""); > } > - return mc; > } > > static const char *pid_file; > @@ -1633,32 +1581,31 @@ static const QEMUOption *lookup_opt(int argc, char **argv, > return popt; > } > > -static MachineClass *select_machine(void) > +static MachineClass *select_machine(QDict *qdict, Error **errp) > { > + const char *optarg = qdict_get_try_str(qdict, "type"); > GSList *machines = object_class_get_list(TYPE_MACHINE, false); > - MachineClass *machine_class = find_default_machine(machines); > - const char *optarg; > - QemuOpts *opts; > - Location loc; > + MachineClass *machine_class; > + Error *local_err = NULL; > > - loc_push_none(&loc); > - > - opts = qemu_get_machine_opts(); > - qemu_opts_loc_restore(opts); > - > - optarg = qemu_opt_get(opts, "type"); > if (optarg) { > - machine_class = machine_parse(optarg, machines); > + machine_class = find_machine(optarg, machines); > + qdict_del(qdict, "type"); > + if (!machine_class) { > + error_setg(&local_err, "unsupported machine type"); > + } > + } else { > + machine_class = find_default_machine(machines); > + if (!machine_class) { > + error_setg(&local_err, "No machine specified, and there is no default"); > + } > } > > - if (!machine_class) { > - error_report("No machine specified, and there is no default"); > - error_printf("Use -machine help to list supported machines\n"); > - exit(1); > - } > - > - loc_pop(&loc); > g_slist_free(machines); > + if (local_err) { > + error_append_hint(&local_err, "Use -machine help to list supported machines\n"); > + error_propagate(errp, local_err); > + } > return machine_class; > } > > @@ -1677,42 +1624,70 @@ static int object_parse_property_opt(Object *obj, > return 0; > } > > -static int machine_set_property(void *opaque, > - const char *name, const char *value, > - Error **errp) > +/* *Non*recursively replace underscores with dashes in QDict keys. */ > +static void keyval_dashify(QDict *qdict, Error **errp) > { > - g_autofree char *qom_name = g_strdup(name); > + const QDictEntry *ent, *next; > char *p; > > - for (p = qom_name; *p; p++) { > - if (*p == '_') { > - *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); > } > +} > + > +static void qemu_apply_legacy_machine_options(QDict *qdict) > +{ > + const char *value; > + > + keyval_dashify(qdict, &error_fatal); > > /* Legacy options do not correspond to MachineState properties. */ > - if (g_str_equal(qom_name, "accel")) { > - return 0; > - } > - if (g_str_equal(qom_name, "igd-passthru")) { > - object_register_sugar_prop(ACCEL_CLASS_NAME("xen"), qom_name, value, > - false); > - return 0; > - } > - if (g_str_equal(qom_name, "kvm-shadow-mem")) { > - object_register_sugar_prop(ACCEL_CLASS_NAME("kvm"), qom_name, value, > - false); > - return 0; > - } > - if (g_str_equal(qom_name, "kernel-irqchip")) { > - object_register_sugar_prop(ACCEL_CLASS_NAME("kvm"), qom_name, value, > - false); > - object_register_sugar_prop(ACCEL_CLASS_NAME("whpx"), qom_name, value, > - false); > - return 0; > + value = qdict_get_try_str(qdict, "accel"); > + if (value) { > + accelerators = g_strdup(value); > + qdict_del(qdict, "accel"); > } > > - return object_parse_property_opt(opaque, name, value, "type", errp); > + value = qdict_get_try_str(qdict, "igd-passthru"); > + if (value) { > + object_register_sugar_prop(ACCEL_CLASS_NAME("xen"), "igd-passthru", value, > + false); > + qdict_del(qdict, "igd-passthru"); > + } > + > + value = qdict_get_try_str(qdict, "kvm-shadow-mem"); > + if (value) { > + object_register_sugar_prop(ACCEL_CLASS_NAME("kvm"), "kvm-shadow-mem", value, > + false); > + qdict_del(qdict, "kvm-shadow-mem"); > + } > + > + value = qdict_get_try_str(qdict, "kernel-irqchip"); > + if (value) { > + 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"); > + } > } > > static void object_option_foreach_add(bool (*type_opt_predicate)(const char *)) > @@ -1827,16 +1802,14 @@ static bool object_create_early(const char *type) > return true; > } > > -static void qemu_apply_machine_options(void) > +static void qemu_apply_machine_options(QDict *qdict) > { > MachineClass *machine_class = MACHINE_GET_CLASS(current_machine); > - QemuOpts *machine_opts = qemu_get_machine_opts(); > const char *boot_order = NULL; > const char *boot_once = NULL; > QemuOpts *opts; > > - qemu_opt_foreach(machine_opts, machine_set_property, current_machine, > - &error_fatal); > + object_set_properties_from_keyval(OBJECT(current_machine), qdict, false, &error_fatal); > current_machine->ram_size = ram_size; > current_machine->maxram_size = maxram_size; > current_machine->ram_slots = ram_slots; > @@ -1865,10 +1838,8 @@ static void qemu_apply_machine_options(void) > current_machine->boot_once = boot_once; > > if (semihosting_enabled() && !semihosting_get_argc()) { > - const char *kernel_filename = qemu_opt_get(machine_opts, "kernel"); > - const char *kernel_cmdline = qemu_opt_get(machine_opts, "append") ?: ""; > /* fall back to the -kernel/-append */ > - semihosting_arg_fallback(kernel_filename, kernel_cmdline); > + semihosting_arg_fallback(current_machine->kernel_filename, current_machine->kernel_cmdline); > } > } > > @@ -1924,8 +1895,7 @@ static void qemu_create_early_backends(void) > > /* > * Note: we need to create audio and block backends before > - * machine_set_property(), so machine properties can refer to > - * them. > + * setting machine properties, so they can be referred to. > */ > configure_blockdev(&bdo_queue, machine_class, snapshot); > audio_init_audiodevs(); > @@ -2091,16 +2061,14 @@ static void set_memory_options(MachineClass *mc) > loc_pop(&loc); > } > > -static void qemu_create_machine(MachineClass *machine_class) > +static void qemu_create_machine(QDict *qdict) > { > + MachineClass *machine_class = select_machine(qdict, &error_fatal); > object_set_machine_compat_props(machine_class->compat_props); > > set_memory_options(machine_class); > > current_machine = MACHINE(object_new_with_class(OBJECT_CLASS(machine_class))); > - if (machine_help_func(qemu_get_machine_opts(), current_machine)) { > - exit(0); > - } > object_property_add_child(object_get_root(), "machine", > OBJECT(current_machine)); > object_property_add_child(container_get(OBJECT(current_machine), > @@ -2131,8 +2099,12 @@ static void qemu_create_machine(MachineClass *machine_class) > * specified either by the configuration file or by the command line. > */ > if (machine_class->default_machine_opts) { > - qemu_opts_set_defaults(qemu_find_opts("machine"), > - machine_class->default_machine_opts, 0); > + QDict *default_opts = > + keyval_parse(machine_class->default_machine_opts, NULL, NULL, > + &error_abort); > + object_set_properties_from_keyval(OBJECT(current_machine), default_opts, > + false, &error_abort); > + qobject_unref(default_opts); > } > } > > @@ -2154,7 +2126,8 @@ static int global_init_func(void *opaque, QemuOpts *opts, Error **errp) > */ > static bool is_qemuopts_group(const char *group) > { > - if (g_str_equal(group, "object")) { > + if (g_str_equal(group, "object") || > + g_str_equal(group, "machine")) { > return false; > } > return true; > @@ -2167,6 +2140,13 @@ static void qemu_record_config_group(const char *group, QDict *dict, > Visitor *v = qobject_input_visitor_new_keyval(QOBJECT(dict)); > object_option_add_visitor(v); > visit_free(v); > + } else if (g_str_equal(group, "machine")) { > + /* > + * Cannot merge string-valued and type-safe dictionaries, so JSON > + * is not accepted yet for -M. > + */ > + assert(!from_json); > + keyval_merge(machine_opts_dict, dict, errp); > } else { > abort(); > } > @@ -2297,13 +2277,11 @@ static int do_configure_accelerator(void *opaque, QemuOpts *opts, Error **errp) > > static void configure_accelerators(const char *progname) > { > - const char *accelerators; > bool init_failed = false; > > qemu_opts_foreach(qemu_find_opts("icount"), > do_configure_icount, NULL, &error_fatal); > > - accelerators = qemu_opt_get(qemu_get_machine_opts(), "accel"); > if (QTAILQ_EMPTY(&qemu_accel_opts.head)) { > char **accel_list, **tmp; > > @@ -2391,12 +2369,11 @@ static void create_default_memdev(MachineState *ms, const char *path) > &error_fatal); > } > > -static void qemu_validate_options(void) > +static void qemu_validate_options(const QDict *machine_opts) > { > - QemuOpts *machine_opts = qemu_get_machine_opts(); > - const char *kernel_filename = qemu_opt_get(machine_opts, "kernel"); > - const char *initrd_filename = qemu_opt_get(machine_opts, "initrd"); > - const char *kernel_cmdline = qemu_opt_get(machine_opts, "append"); > + const char *kernel_filename = qdict_get_try_str(machine_opts, "kernel"); > + const char *initrd_filename = qdict_get_try_str(machine_opts, "initrd"); > + const char *kernel_cmdline = qdict_get_try_str(machine_opts, "append"); > > if (kernel_filename == NULL) { > if (kernel_cmdline != NULL) { > @@ -2736,7 +2713,6 @@ void qemu_init(int argc, char **argv, char **envp) > qemu_add_opts(&qemu_trace_opts); > qemu_plugin_add_opts(); > qemu_add_opts(&qemu_option_rom_opts); > - qemu_add_opts(&qemu_machine_opts); > qemu_add_opts(&qemu_accel_opts); > qemu_add_opts(&qemu_mem_opts); > qemu_add_opts(&qemu_smp_opts); > @@ -2777,6 +2753,7 @@ void qemu_init(int argc, char **argv, char **envp) > } > } > > + machine_opts_dict = qdict_new(); > if (userconfig) { > qemu_read_default_config_file(&error_fatal); > } > @@ -2866,8 +2843,7 @@ void qemu_init(int argc, char **argv, char **envp) > parse_display(optarg); > break; > case QEMU_OPTION_nographic: > - olist = qemu_find_opts("machine"); > - qemu_opts_parse_noisily(olist, "graphics=off", false); > + qdict_put_str(machine_opts_dict, "graphics", "off"); > nographic = true; > dpy.type = DISPLAY_TYPE_NONE; > break; > @@ -2891,16 +2867,16 @@ void qemu_init(int argc, char **argv, char **envp) > } > break; > case QEMU_OPTION_kernel: > - qemu_opts_set(qemu_find_opts("machine"), "kernel", optarg, &error_abort); > + qdict_put_str(machine_opts_dict, "kernel", optarg); > break; > case QEMU_OPTION_initrd: > - qemu_opts_set(qemu_find_opts("machine"), "initrd", optarg, &error_abort); > + qdict_put_str(machine_opts_dict, "initrd", optarg); > break; > case QEMU_OPTION_append: > - qemu_opts_set(qemu_find_opts("machine"), "append", optarg, &error_abort); > + qdict_put_str(machine_opts_dict, "append", optarg); > break; > case QEMU_OPTION_dtb: > - qemu_opts_set(qemu_find_opts("machine"), "dtb", optarg, &error_abort); > + qdict_put_str(machine_opts_dict, "dtb", optarg); > break; > case QEMU_OPTION_cdrom: > drive_add(IF_DEFAULT, 2, optarg, CDROM_OPTS); > @@ -3010,7 +2986,7 @@ void qemu_init(int argc, char **argv, char **envp) > } > break; > case QEMU_OPTION_bios: > - qemu_opts_set(qemu_find_opts("machine"), "firmware", optarg, &error_abort); > + qdict_put_str(machine_opts_dict, "firmware", optarg); > break; > case QEMU_OPTION_singlestep: > singlestep = 1; > @@ -3281,17 +3257,20 @@ void qemu_init(int argc, char **argv, char **envp) > preconfig_requested = true; > break; > case QEMU_OPTION_enable_kvm: > - olist = qemu_find_opts("machine"); > - qemu_opts_parse_noisily(olist, "accel=kvm", false); > + qdict_put_str(machine_opts_dict, "accel", "kvm"); > break; > case QEMU_OPTION_M: > case QEMU_OPTION_machine: > - olist = qemu_find_opts("machine"); > - opts = qemu_opts_parse_noisily(olist, optarg, true); > - if (!opts) { > - exit(1); > + { > + bool help; > + > + keyval_parse_into(machine_opts_dict, optarg, "type", &help, &error_fatal); > + if (help) { > + machine_help_func(machine_opts_dict); > + exit(EXIT_SUCCESS); > + } > + break; > } > - break; > case QEMU_OPTION_accel: > accel_opts = qemu_opts_parse_noisily(qemu_find_opts("accel"), > optarg, true); > @@ -3318,12 +3297,10 @@ void qemu_init(int argc, char **argv, char **envp) > } > break; > case QEMU_OPTION_usb: > - olist = qemu_find_opts("machine"); > - qemu_opts_parse_noisily(olist, "usb=on", false); > + qdict_put_str(machine_opts_dict, "usb", "on"); > break; > case QEMU_OPTION_usbdevice: > - olist = qemu_find_opts("machine"); > - qemu_opts_parse_noisily(olist, "usb=on", false); > + qdict_put_str(machine_opts_dict, "usb", "on"); > add_device_config(DEV_USB, optarg); > break; > case QEMU_OPTION_device: > @@ -3342,12 +3319,10 @@ void qemu_init(int argc, char **argv, char **envp) > vnc_parse(optarg); > break; > case QEMU_OPTION_no_acpi: > - olist = qemu_find_opts("machine"); > - qemu_opts_parse_noisily(olist, "acpi=off", false); > + qdict_put_str(machine_opts_dict, "acpi", "off"); > break; > case QEMU_OPTION_no_hpet: > - olist = qemu_find_opts("machine"); > - qemu_opts_parse_noisily(olist, "hpet=off", false); > + qdict_put_str(machine_opts_dict, "hpet", "off"); > break; > case QEMU_OPTION_no_reboot: > olist = qemu_find_opts("action"); > @@ -3600,7 +3575,7 @@ void qemu_init(int argc, char **argv, char **envp) > */ > loc_set_none(); > > - qemu_validate_options(); > + qemu_validate_options(machine_opts_dict); > qemu_process_sugar_options(); > > /* > @@ -3633,7 +3608,7 @@ void qemu_init(int argc, char **argv, char **envp) > > configure_rtc(qemu_find_opts_singleton("rtc")); > > - qemu_create_machine(select_machine()); > + qemu_create_machine(machine_opts_dict); > > suspend_mux_open(); > > @@ -3641,12 +3616,14 @@ void qemu_init(int argc, char **argv, char **envp) > qemu_create_default_devices(); > qemu_create_early_backends(); > > - qemu_apply_machine_options(); > + qemu_apply_legacy_machine_options(machine_opts_dict); > + qemu_apply_machine_options(machine_opts_dict); > + qobject_unref(machine_opts_dict); > phase_advance(PHASE_MACHINE_CREATED); > > /* > * Note: uses machine properties such as kernel-irqchip, must run > - * after machine_set_property(). > + * after qemu_apply_machine_options. > */ > configure_accelerators(argv[0]); > phase_advance(PHASE_ACCEL_CREATED); > -- > 2.31.1 > > >
> Switch from QemuOpts to keyval. This enables the introduction > of non-scalar machine properties, and JSON syntax in the future. > > For JSON syntax to be supported right now, we would have to > consider what would happen if string-based dictionaries (produced by > -M key=val) were to be merged with strongly-typed dictionaries > (produced by -M {'key': 123}). > > The simplest way out is to never enter the situation, and only allow one > -M option when JSON syntax is in use. However, we want options such as > -smp to become syntactic sugar for -M, and this is a problem; as soon > as -smp becomes a shortcut for -M, QEMU would forbid using -M '{....}' > together with -smp. Therefore, allowing JSON syntax right now for -M > would be a forward-compatibility nightmare and it would be impossible > anyway to introduce -M incrementally in tools. > > Instead, support for JSON syntax is delayed until after the main > options are converted to QOM compound properties. These include -boot, > -acpitable, -smbios, -m, -semihosting-config, -rtc and -fw_cfg. Once JSON > syntax is introduced, these options will _also_ be forbidden together > with -M '{...}'. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > softmmu/vl.c | 315 ++++++++++++++++++++++++--------------------------- > 1 file changed, 146 insertions(+), 169 deletions(-) > > Since this commit QEMU fails to start on my system. ./qemu-system-i386 -machine pc,pcspk-audiodev=audio0 -audiodev pa,id=audio0 qemu-system-i386: Parameter 'audiodev' is missing With best regards, Volker
On Tue, 6 Jul 2021 12:01:37 +0200 Paolo Bonzini <pbonzini@redhat.com> wrote: > Switch from QemuOpts to keyval. This enables the introduction > of non-scalar machine properties, and JSON syntax in the future. > > For JSON syntax to be supported right now, we would have to > consider what would happen if string-based dictionaries (produced by > -M key=val) were to be merged with strongly-typed dictionaries > (produced by -M {'key': 123}). > > The simplest way out is to never enter the situation, and only allow one > -M option when JSON syntax is in use. However, we want options such as > -smp to become syntactic sugar for -M, and this is a problem; as soon > as -smp becomes a shortcut for -M, QEMU would forbid using -M '{....}' > together with -smp. Therefore, allowing JSON syntax right now for -M > would be a forward-compatibility nightmare and it would be impossible > anyway to introduce -M incrementally in tools. > > Instead, support for JSON syntax is delayed until after the main > options are converted to QOM compound properties. These include -boot, > -acpitable, -smbios, -m, -semihosting-config, -rtc and -fw_cfg. Once JSON > syntax is introduced, these options will _also_ be forbidden together > with -M '{...}'. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > softmmu/vl.c | 315 ++++++++++++++++++++++++--------------------------- > 1 file changed, 146 insertions(+), 169 deletions(-) This breaks the below long standing test VM. libvirt log and xml provided below. I'm using libvirt version 7.3.0. /usr/local/bin/qemu-system-x86_64 \ -name guest=Steam-GeForce,debug-threads=on \ -S \ -object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/var/lib/libvirt/qemu/domain-2-Steam-GeForce/master-key.aes"}' \ -blockdev '{"driver":"file","filename":"/usr/share/edk2/ovmf/OVMF_CODE.fd","node-name":"libvirt-pflash0-storage","auto-read-only":true,"discard":"unmap"}' \ -blockdev '{"node-name":"libvirt-pflash0-format","read-only":true,"driver":"raw","file":"libvirt-pflash0-storage"}' \ -blockdev '{"driver":"file","filename":"/var/lib/libvirt/qemu/nvram/Steam_VARS.fd","node-name":"libvirt-pflash1-storage","auto-read-only":true,"discard":"unmap"}' \ -blockdev '{"node-name":"libvirt-pflash1-format","read-only":false,"driver":"raw","file":"libvirt-pflash1-storage"}' \ -machine pc-i440fx-6.0,accel=kvm,usb=off,vmport=off,dump-guest-core=off,pflash0=libvirt-pflash0-format,pflash1=libvirt-pflash1-format,memory-backend=pc.ram \ -cpu host,migratable=on,hv-time,hv-relaxed,hv-vapic,hv-spinlocks=0x1fff,hv-vendor-id=KeenlyKVM,kvm=off \ -m 4096 \ -object '{"qom-type":"memory-backend-file","id":"pc.ram","mem-path":"/dev/hugepages/libvirt/qemu/2-Steam-GeForce","x-use-canonical-path-for-ramblock-id":false,"prealloc":true,"size":4294967296}' \ -overcommit mem-lock=off \ -smp 4,sockets=1,dies=1,cores=2,threads=2 \ -uuid 2b417d4b-f25b-4522-a5be-e105f032f99c \ -display none \ -no-user-config \ -nodefaults \ -chardev socket,id=charmonitor,fd=38,server=on,wait=off \ -mon chardev=charmonitor,id=monitor,mode=control \ -rtc base=localtime,driftfix=slew \ -global kvm-pit.lost_tick_policy=delay \ -no-hpet \ -no-shutdown \ -boot menu=on,strict=on \ -device nec-usb-xhci,id=usb,bus=pci.0,addr=0x8 \ -device virtio-scsi-pci,id=scsi0,num_queues=4,bus=pci.0,addr=0x5 \ -blockdev '{"driver":"file","filename":"/mnt/ssd/Steam-2019-08-02.img","node-name":"libvirt-2-storage","cache":{"direct":true,"no-flush":false},"auto-read-only":true,"discard":"unmap"}' \ -blockdev '{"node-name":"libvirt-2-format","read-only":true,"cache":{"direct":true,"no-flush":false},"driver":"raw","file":"libvirt-2-storage"}' \ -blockdev '{"driver":"file","filename":"/mnt/ssd/Steam.qcow2","node-name":"libvirt-1-storage","cache":{"direct":true,"no-flush":false},"auto-read-only":true,"discard":"unmap"}' \ -blockdev '{"node-name":"libvirt-1-format","read-only":false,"cache":{"direct":true,"no-flush":false},"driver":"qcow2","file":"libvirt-1-storage","backing":"libvirt-2-format"}' \ -device scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=0,device_id=drive-scsi0-0-0-0,drive=libvirt-1-format,id=scsi0-0-0-0,bootindex=2,write-cache=on \ -netdev tap,fd=41,id=hostnet0,vhost=on,vhostfd=42 \ -device virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:60:ef:ac,bus=pci.0,addr=0x3 \ -audiodev id=audio1,driver=none \ -device vfio-pci,host=0000:01:00.0,id=hostdev0,bus=pci.0,addr=0x4,rombar=1 \ -device vfio-pci,host=0000:01:00.1,id=hostdev1,bus=pci.0,addr=0x6,rombar=0 \ -device vfio-pci,host=0000:02:00.0,id=hostdev2,bus=pci.0,addr=0x2 \ -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ -msg timestamp=on 2021-07-13T19:37:27.680710Z qemu-system-x86_64: Parameter 'drive' is missing 2021-07-13 19:37:27.703+0000: shutting down, reason=failed <domain type='kvm'> <name>Steam-GeForce</name> <uuid>2b417d4b-f25b-4522-a5be-e105f032f99c</uuid> <memory unit='KiB'>4194304</memory> <currentMemory unit='KiB'>4194304</currentMemory> <memoryBacking> <hugepages/> </memoryBacking> <vcpu placement='static'>4</vcpu> <cputune> <vcpupin vcpu='0' cpuset='3'/> <vcpupin vcpu='1' cpuset='7'/> <vcpupin vcpu='2' cpuset='2'/> <vcpupin vcpu='3' cpuset='6'/> <emulatorpin cpuset='0,4'/> </cputune> <os> <type arch='x86_64' machine='pc-i440fx-6.0'>hvm</type> <loader readonly='yes' type='pflash'>/usr/share/edk2/ovmf/OVMF_CODE.fd</loader> <nvram template='/usr/share/edk2/ovmf/OVMF_VARS.fd'>/var/lib/libvirt/qemu/nvram/Steam_VARS.fd</nvram> <bootmenu enable='yes'/> </os> <features> <acpi/> <apic/> <pae/> <hyperv> <relaxed state='on'/> <vapic state='on'/> <spinlocks state='on' retries='8191'/> <vendor_id state='on' value='KeenlyKVM'/> </hyperv> <kvm> <hidden state='on'/> </kvm> <vmport state='off'/> </features> <cpu mode='host-passthrough' check='none' migratable='on'> <topology sockets='1' dies='1' cores='2' threads='2'/> </cpu> <clock offset='localtime'> <timer name='rtc' tickpolicy='catchup'/> <timer name='pit' tickpolicy='delay'/> <timer name='hpet' present='no'/> <timer name='hypervclock' present='yes'/> </clock> <on_poweroff>destroy</on_poweroff> <on_reboot>restart</on_reboot> <on_crash>restart</on_crash> <devices> <emulator>/usr/local/bin/qemu-system-x86_64</emulator> <disk type='file' device='disk'> <driver name='qemu' type='qcow2' cache='none'/> <source file='/mnt/ssd/Steam.qcow2'/> <target dev='sda' bus='scsi'/> <boot order='2'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <controller type='scsi' index='0' model='virtio-scsi'> <driver queues='4'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> </controller> <controller type='pci' index='0' model='pci-root'/> <controller type='usb' index='0' model='nec-xhci'> <address type='pci' domain='0x0000' bus='0x00' slot='0x08' function='0x0'/> </controller> <interface type='direct'> <mac address='52:54:00:60:ef:ac'/> <source dev='enp4s0' mode='bridge'/> <model type='virtio'/> <link state='up'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </interface> <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> <audio id='1' type='none'/> <hostdev mode='subsystem' type='pci' managed='yes'> <source> <address domain='0x0000' bus='0x01' slot='0x00' function='0x0'/> </source> <rom bar='on'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> </hostdev> <hostdev mode='subsystem' type='pci' managed='yes'> <source> <address domain='0x0000' bus='0x01' slot='0x00' function='0x1'/> </source> <rom bar='off'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> </hostdev> <hostdev mode='subsystem' type='pci' managed='yes'> <source> <address domain='0x0000' bus='0x02' slot='0x00' function='0x0'/> </source> <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> </hostdev> <memballoon model='none'/> </devices> </domain>
On Tue, 13 Jul 2021 13:43:47 -0600 Alex Williamson <alex.williamson@redhat.com> wrote: > On Tue, 6 Jul 2021 12:01:37 +0200 > Paolo Bonzini <pbonzini@redhat.com> wrote: > > > Switch from QemuOpts to keyval. This enables the introduction > > of non-scalar machine properties, and JSON syntax in the future. > > > > For JSON syntax to be supported right now, we would have to > > consider what would happen if string-based dictionaries (produced by > > -M key=val) were to be merged with strongly-typed dictionaries > > (produced by -M {'key': 123}). > > > > The simplest way out is to never enter the situation, and only allow one > > -M option when JSON syntax is in use. However, we want options such as > > -smp to become syntactic sugar for -M, and this is a problem; as soon > > as -smp becomes a shortcut for -M, QEMU would forbid using -M '{....}' > > together with -smp. Therefore, allowing JSON syntax right now for -M > > would be a forward-compatibility nightmare and it would be impossible > > anyway to introduce -M incrementally in tools. > > > > Instead, support for JSON syntax is delayed until after the main > > options are converted to QOM compound properties. These include -boot, > > -acpitable, -smbios, -m, -semihosting-config, -rtc and -fw_cfg. Once JSON > > syntax is introduced, these options will _also_ be forbidden together > > with -M '{...}'. > > > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > > --- > > softmmu/vl.c | 315 ++++++++++++++++++++++++--------------------------- > > 1 file changed, 146 insertions(+), 169 deletions(-) > > This breaks the below long standing test VM. libvirt log and xml > provided below. I'm using libvirt version 7.3.0. A trivial reproducer: qemu-system-x86_64 -blockdev '{"driver":"file","filename":"/usr/share/edk2/ovmf/OVMF_CODE.fd","node-name":"pflash0-storage","auto-read-only":true,"discard":"unmap"}' -blockdev '{"node-name":"pflash0-format","read-only":true,"driver":"raw","file":"pflash0-storage"}' -machine pc,pflash0=pflash0-format backtrace: #0 error_setg_internal (errp=0x7fffffffd6c8, src=0x55555618bc10 "../../home/alwillia/Work/qemu.git/qapi/qobject-input-visitor.c", line=172, func=0x55555618bf20 <__func__.27> "qobject_input_get_object", fmt=0x55555618bc98 "Parameter '%s' is missing") at ../../home/alwillia/Work/qemu.git/util/error.c:93 #1 0x0000555555f8d255 in qobject_input_get_object (qiv=0x5555569c59b0, name=0x555556920ef0 "drive", consume=true, errp=0x7fffffffd6c8) at ../../home/alwillia/Work/qemu.git/qapi/qobject-input-visitor.c:172 #2 0x0000555555f8d28b in qobject_input_get_keyval (qiv=0x5555569c59b0, name=0x555556920ef0 "drive", errp=0x7fffffffd6c8) at ../../home/alwillia/Work/qemu.git/qapi/qobject-input-visitor.c:184 #3 0x0000555555f8e3a9 in qobject_input_type_str_keyval (v=0x5555569c59b0, name=0x555556920ef0 "drive", obj=0x7fffffffd5a8, errp=0x7fffffffd6c8) at ../../home/alwillia/Work/qemu.git/qapi/qobject-input-visitor.c:551 #4 0x0000555555f59e51 in visit_type_str (v=0x5555569c59b0, name=0x555556920ef0 "drive", obj=0x7fffffffd5a8, errp=0x7fffffffd6c8) at ../../home/alwillia/Work/qemu.git/qapi/qapi-visit-core.c:337 #5 0x00005555558c3b70 in set_drive_helper (obj=0x5555569208a0, v=0x5555569c59b0, name=0x555556920ef0 "drive", opaque=0x5555564c8300 <pflash_cfi01_properties>, iothread=false, errp=0x7fffffffd6c8) at ../../home/alwillia/Work/qemu.git/hw/core/qdev-properties-system.c:97 #6 0x00005555558c3e0b in set_drive (obj=0x5555569208a0, v=0x5555569c59b0, name=0x555556920ef0 "drive", opaque=0x5555564c8300 <pflash_cfi01_properties>, errp=0x7fffffffd6c8) at ../../home/alwillia/Work/qemu.git/hw/core/qdev-properties-system.c:171 #7 0x0000555555edb821 in field_prop_set (obj=0x5555569208a0, v=0x5555569c59b0, name=0x555556920ef0 "drive", opaque=0x5555564c8300 <pflash_cfi01_properties>, errp=0x7fffffffd6c8) at ../../home/alwillia/Work/qemu.git/hw/core/qdev-properties.c:86 #8 0x0000555555eb7f15 in object_property_set (obj=0x5555569208a0, name=0x555556920ef0 "drive", v=0x5555569c59b0, errp=0x7fffffffd768) at ../../home/alwillia/Work/qemu.git/qom/object.c:1402 #9 0x0000555555ebad9e in property_set_alias (obj=0x55555683dde0, v=0x5555569c59b0, name=0x5555566fc8a0 "pflash0", opaque=0x555556920ed0, errp=0x7fffffffd768) at ../../home/alwillia/Work/qemu.git/qom/object.c:2695 #10 0x0000555555eb7f15 in object_property_set (obj=0x55555683dde0, name=0x5555566fc8a0 "pflash0", v=0x5555569c59b0, errp=0x7fffffffd7b8) at ../../home/alwillia/Work/qemu.git/qom/object.c:1402 #11 0x0000555555eb3aa3 in object_set_properties_from_qdict (obj=0x55555683dde0, qdict=0x5555566fa5b0, v=0x5555569c59b0, errp=0x5555565d9308 <error_fatal>) at ../../home/alwillia/Work/qemu.git/qom/object_interfaces.c:55 #12 0x0000555555eb3bc3 in object_set_properties_from_keyval (obj=0x55555683dde0, qdict=0x5555566fa5b0, from_json=false, errp=0x5555565d9308 <error_fatal>) at ../../home/alwillia/Work/qemu.git/qom/object_interfaces.c:79 #13 0x0000555555cb3f5f in qemu_apply_machine_options (qdict=0x5555566fa5b0) at ../../home/alwillia/Work/qemu.git/softmmu/vl.c:1833 #14 0x0000555555cb8606 in qemu_init (argc=7, argv=0x7fffffffdae8, envp=0x7fffffffdb28) at ../../home/alwillia/Work/qemu.git/softmmu/vl.c:3634 #15 0x00005555558326d1 in main (argc=7, argv=0x7fffffffdae8, envp=0x7fffffffdb28) at ../../home/alwillia/Work/qemu.git/softmmu/main.c:49
On 14/07/2021 00:10, Alex Williamson wrote: > On Tue, 13 Jul 2021 13:43:47 -0600 > Alex Williamson <alex.williamson@redhat.com> wrote: > >> On Tue, 6 Jul 2021 12:01:37 +0200 >> Paolo Bonzini <pbonzini@redhat.com> wrote: >> >>> Switch from QemuOpts to keyval. This enables the introduction >>> of non-scalar machine properties, and JSON syntax in the future. >>> >>> For JSON syntax to be supported right now, we would have to >>> consider what would happen if string-based dictionaries (produced by >>> -M key=val) were to be merged with strongly-typed dictionaries >>> (produced by -M {'key': 123}). >>> >>> The simplest way out is to never enter the situation, and only allow one >>> -M option when JSON syntax is in use. However, we want options such as >>> -smp to become syntactic sugar for -M, and this is a problem; as soon >>> as -smp becomes a shortcut for -M, QEMU would forbid using -M '{....}' >>> together with -smp. Therefore, allowing JSON syntax right now for -M >>> would be a forward-compatibility nightmare and it would be impossible >>> anyway to introduce -M incrementally in tools. >>> >>> Instead, support for JSON syntax is delayed until after the main >>> options are converted to QOM compound properties. These include -boot, >>> -acpitable, -smbios, -m, -semihosting-config, -rtc and -fw_cfg. Once JSON >>> syntax is introduced, these options will _also_ be forbidden together >>> with -M '{...}'. >>> >>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> >>> --- >>> softmmu/vl.c | 315 ++++++++++++++++++++++++--------------------------- >>> 1 file changed, 146 insertions(+), 169 deletions(-) >> >> This breaks the below long standing test VM. libvirt log and xml >> provided below. I'm using libvirt version 7.3.0. > > A trivial reproducer: > > qemu-system-x86_64 -blockdev '{"driver":"file","filename":"/usr/share/edk2/ovmf/OVMF_CODE.fd","node-name":"pflash0-storage","auto-read-only":true,"discard":"unmap"}' -blockdev '{"node-name":"pflash0-format","read-only":true,"driver":"raw","file":"pflash0-storage"}' -machine pc,pflash0=pflash0-format > > backtrace: > > #0 error_setg_internal > (errp=0x7fffffffd6c8, src=0x55555618bc10 "../../home/alwillia/Work/qemu.git/qapi/qobject-input-visitor.c", line=172, func=0x55555618bf20 <__func__.27> "qobject_input_get_object", fmt=0x55555618bc98 "Parameter '%s' is missing") at ../../home/alwillia/Work/qemu.git/util/error.c:93 > #1 0x0000555555f8d255 in qobject_input_get_object (qiv=0x5555569c59b0, name=0x555556920ef0 "drive", consume=true, errp=0x7fffffffd6c8) at ../../home/alwillia/Work/qemu.git/qapi/qobject-input-visitor.c:172 > #2 0x0000555555f8d28b in qobject_input_get_keyval (qiv=0x5555569c59b0, name=0x555556920ef0 "drive", errp=0x7fffffffd6c8) at ../../home/alwillia/Work/qemu.git/qapi/qobject-input-visitor.c:184 > #3 0x0000555555f8e3a9 in qobject_input_type_str_keyval (v=0x5555569c59b0, name=0x555556920ef0 "drive", obj=0x7fffffffd5a8, errp=0x7fffffffd6c8) > at ../../home/alwillia/Work/qemu.git/qapi/qobject-input-visitor.c:551 > #4 0x0000555555f59e51 in visit_type_str (v=0x5555569c59b0, name=0x555556920ef0 "drive", obj=0x7fffffffd5a8, errp=0x7fffffffd6c8) at ../../home/alwillia/Work/qemu.git/qapi/qapi-visit-core.c:337 > #5 0x00005555558c3b70 in set_drive_helper (obj=0x5555569208a0, v=0x5555569c59b0, name=0x555556920ef0 "drive", opaque=0x5555564c8300 <pflash_cfi01_properties>, iothread=false, errp=0x7fffffffd6c8) > at ../../home/alwillia/Work/qemu.git/hw/core/qdev-properties-system.c:97 > #6 0x00005555558c3e0b in set_drive (obj=0x5555569208a0, v=0x5555569c59b0, name=0x555556920ef0 "drive", opaque=0x5555564c8300 <pflash_cfi01_properties>, errp=0x7fffffffd6c8) > at ../../home/alwillia/Work/qemu.git/hw/core/qdev-properties-system.c:171 > #7 0x0000555555edb821 in field_prop_set (obj=0x5555569208a0, v=0x5555569c59b0, name=0x555556920ef0 "drive", opaque=0x5555564c8300 <pflash_cfi01_properties>, errp=0x7fffffffd6c8) > at ../../home/alwillia/Work/qemu.git/hw/core/qdev-properties.c:86 > #8 0x0000555555eb7f15 in object_property_set (obj=0x5555569208a0, name=0x555556920ef0 "drive", v=0x5555569c59b0, errp=0x7fffffffd768) at ../../home/alwillia/Work/qemu.git/qom/object.c:1402 > #9 0x0000555555ebad9e in property_set_alias (obj=0x55555683dde0, v=0x5555569c59b0, name=0x5555566fc8a0 "pflash0", opaque=0x555556920ed0, errp=0x7fffffffd768) > at ../../home/alwillia/Work/qemu.git/qom/object.c:2695 > #10 0x0000555555eb7f15 in object_property_set (obj=0x55555683dde0, name=0x5555566fc8a0 "pflash0", v=0x5555569c59b0, errp=0x7fffffffd7b8) at ../../home/alwillia/Work/qemu.git/qom/object.c:1402 > #11 0x0000555555eb3aa3 in object_set_properties_from_qdict (obj=0x55555683dde0, qdict=0x5555566fa5b0, v=0x5555569c59b0, errp=0x5555565d9308 <error_fatal>) > at ../../home/alwillia/Work/qemu.git/qom/object_interfaces.c:55 > #12 0x0000555555eb3bc3 in object_set_properties_from_keyval (obj=0x55555683dde0, qdict=0x5555566fa5b0, from_json=false, errp=0x5555565d9308 <error_fatal>) > at ../../home/alwillia/Work/qemu.git/qom/object_interfaces.c:79 > #13 0x0000555555cb3f5f in qemu_apply_machine_options (qdict=0x5555566fa5b0) at ../../home/alwillia/Work/qemu.git/softmmu/vl.c:1833 > #14 0x0000555555cb8606 in qemu_init (argc=7, argv=0x7fffffffdae8, envp=0x7fffffffdb28) at ../../home/alwillia/Work/qemu.git/softmmu/vl.c:3634 > #15 0x00005555558326d1 in main (argc=7, argv=0x7fffffffdae8, envp=0x7fffffffdb28) at ../../home/alwillia/Work/qemu.git/softmmu/main.c:49 Is this the same bug fixed by the proposed patch at https://lists.gnu.org/archive/html/qemu-devel/2021-07/msg03324.html? ATB, Mark.
On Wed, 14 Jul 2021 07:16:43 +0100 Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> wrote: > On 14/07/2021 00:10, Alex Williamson wrote: > > > On Tue, 13 Jul 2021 13:43:47 -0600 > > Alex Williamson <alex.williamson@redhat.com> wrote: > > > >> On Tue, 6 Jul 2021 12:01:37 +0200 > >> Paolo Bonzini <pbonzini@redhat.com> wrote: > >> > >>> Switch from QemuOpts to keyval. This enables the introduction > >>> of non-scalar machine properties, and JSON syntax in the future. > >>> > >>> For JSON syntax to be supported right now, we would have to > >>> consider what would happen if string-based dictionaries (produced by > >>> -M key=val) were to be merged with strongly-typed dictionaries > >>> (produced by -M {'key': 123}). > >>> > >>> The simplest way out is to never enter the situation, and only allow one > >>> -M option when JSON syntax is in use. However, we want options such as > >>> -smp to become syntactic sugar for -M, and this is a problem; as soon > >>> as -smp becomes a shortcut for -M, QEMU would forbid using -M '{....}' > >>> together with -smp. Therefore, allowing JSON syntax right now for -M > >>> would be a forward-compatibility nightmare and it would be impossible > >>> anyway to introduce -M incrementally in tools. > >>> > >>> Instead, support for JSON syntax is delayed until after the main > >>> options are converted to QOM compound properties. These include -boot, > >>> -acpitable, -smbios, -m, -semihosting-config, -rtc and -fw_cfg. Once JSON > >>> syntax is introduced, these options will _also_ be forbidden together > >>> with -M '{...}'. > >>> > >>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > >>> --- > >>> softmmu/vl.c | 315 ++++++++++++++++++++++++--------------------------- > >>> 1 file changed, 146 insertions(+), 169 deletions(-) > >> > >> This breaks the below long standing test VM. libvirt log and xml > >> provided below. I'm using libvirt version 7.3.0. > > > > A trivial reproducer: > > > > qemu-system-x86_64 -blockdev '{"driver":"file","filename":"/usr/share/edk2/ovmf/OVMF_CODE.fd","node-name":"pflash0-storage","auto-read-only":true,"discard":"unmap"}' -blockdev '{"node-name":"pflash0-format","read-only":true,"driver":"raw","file":"pflash0-storage"}' -machine pc,pflash0=pflash0-format > > > > backtrace: > > > > #0 error_setg_internal > > (errp=0x7fffffffd6c8, src=0x55555618bc10 "../../home/alwillia/Work/qemu.git/qapi/qobject-input-visitor.c", line=172, func=0x55555618bf20 <__func__.27> "qobject_input_get_object", fmt=0x55555618bc98 "Parameter '%s' is missing") at ../../home/alwillia/Work/qemu.git/util/error.c:93 > > #1 0x0000555555f8d255 in qobject_input_get_object (qiv=0x5555569c59b0, name=0x555556920ef0 "drive", consume=true, errp=0x7fffffffd6c8) at ../../home/alwillia/Work/qemu.git/qapi/qobject-input-visitor.c:172 > > #2 0x0000555555f8d28b in qobject_input_get_keyval (qiv=0x5555569c59b0, name=0x555556920ef0 "drive", errp=0x7fffffffd6c8) at ../../home/alwillia/Work/qemu.git/qapi/qobject-input-visitor.c:184 > > #3 0x0000555555f8e3a9 in qobject_input_type_str_keyval (v=0x5555569c59b0, name=0x555556920ef0 "drive", obj=0x7fffffffd5a8, errp=0x7fffffffd6c8) > > at ../../home/alwillia/Work/qemu.git/qapi/qobject-input-visitor.c:551 > > #4 0x0000555555f59e51 in visit_type_str (v=0x5555569c59b0, name=0x555556920ef0 "drive", obj=0x7fffffffd5a8, errp=0x7fffffffd6c8) at ../../home/alwillia/Work/qemu.git/qapi/qapi-visit-core.c:337 > > #5 0x00005555558c3b70 in set_drive_helper (obj=0x5555569208a0, v=0x5555569c59b0, name=0x555556920ef0 "drive", opaque=0x5555564c8300 <pflash_cfi01_properties>, iothread=false, errp=0x7fffffffd6c8) > > at ../../home/alwillia/Work/qemu.git/hw/core/qdev-properties-system.c:97 > > #6 0x00005555558c3e0b in set_drive (obj=0x5555569208a0, v=0x5555569c59b0, name=0x555556920ef0 "drive", opaque=0x5555564c8300 <pflash_cfi01_properties>, errp=0x7fffffffd6c8) > > at ../../home/alwillia/Work/qemu.git/hw/core/qdev-properties-system.c:171 > > #7 0x0000555555edb821 in field_prop_set (obj=0x5555569208a0, v=0x5555569c59b0, name=0x555556920ef0 "drive", opaque=0x5555564c8300 <pflash_cfi01_properties>, errp=0x7fffffffd6c8) > > at ../../home/alwillia/Work/qemu.git/hw/core/qdev-properties.c:86 > > #8 0x0000555555eb7f15 in object_property_set (obj=0x5555569208a0, name=0x555556920ef0 "drive", v=0x5555569c59b0, errp=0x7fffffffd768) at ../../home/alwillia/Work/qemu.git/qom/object.c:1402 > > #9 0x0000555555ebad9e in property_set_alias (obj=0x55555683dde0, v=0x5555569c59b0, name=0x5555566fc8a0 "pflash0", opaque=0x555556920ed0, errp=0x7fffffffd768) > > at ../../home/alwillia/Work/qemu.git/qom/object.c:2695 > > #10 0x0000555555eb7f15 in object_property_set (obj=0x55555683dde0, name=0x5555566fc8a0 "pflash0", v=0x5555569c59b0, errp=0x7fffffffd7b8) at ../../home/alwillia/Work/qemu.git/qom/object.c:1402 > > #11 0x0000555555eb3aa3 in object_set_properties_from_qdict (obj=0x55555683dde0, qdict=0x5555566fa5b0, v=0x5555569c59b0, errp=0x5555565d9308 <error_fatal>) > > at ../../home/alwillia/Work/qemu.git/qom/object_interfaces.c:55 > > #12 0x0000555555eb3bc3 in object_set_properties_from_keyval (obj=0x55555683dde0, qdict=0x5555566fa5b0, from_json=false, errp=0x5555565d9308 <error_fatal>) > > at ../../home/alwillia/Work/qemu.git/qom/object_interfaces.c:79 > > #13 0x0000555555cb3f5f in qemu_apply_machine_options (qdict=0x5555566fa5b0) at ../../home/alwillia/Work/qemu.git/softmmu/vl.c:1833 > > #14 0x0000555555cb8606 in qemu_init (argc=7, argv=0x7fffffffdae8, envp=0x7fffffffdb28) at ../../home/alwillia/Work/qemu.git/softmmu/vl.c:3634 > > #15 0x00005555558326d1 in main (argc=7, argv=0x7fffffffdae8, envp=0x7fffffffdb28) at ../../home/alwillia/Work/qemu.git/softmmu/main.c:49 > > Is this the same bug fixed by the proposed patch at > https://lists.gnu.org/archive/html/qemu-devel/2021-07/msg03324.html? Nope, doesn't fix it. Thanks, Alex
CC libvir-list On Tue, Jul 06, 2021 at 12:01:37 +0200, Paolo Bonzini wrote: > Switch from QemuOpts to keyval. This enables the introduction > of non-scalar machine properties, and JSON syntax in the future. > > For JSON syntax to be supported right now, we would have to > consider what would happen if string-based dictionaries (produced by > -M key=val) were to be merged with strongly-typed dictionaries > (produced by -M {'key': 123}). > > The simplest way out is to never enter the situation, and only allow one > -M option when JSON syntax is in use. However, we want options such as > -smp to become syntactic sugar for -M, and this is a problem; as soon > as -smp becomes a shortcut for -M, QEMU would forbid using -M '{....}' > together with -smp. Therefore, allowing JSON syntax right now for -M > would be a forward-compatibility nightmare and it would be impossible > anyway to introduce -M incrementally in tools. > > Instead, support for JSON syntax is delayed until after the main > options are converted to QOM compound properties. These include -boot, > -acpitable, -smbios, -m, -semihosting-config, -rtc and -fw_cfg. Once JSON > syntax is introduced, these options will _also_ be forbidden together > with -M '{...}'. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > softmmu/vl.c | 315 ++++++++++++++++++++++++--------------------------- > 1 file changed, 146 insertions(+), 169 deletions(-) This patch breaks detection of certain machine options features in libvirt which were being detected from 'query-command-line-options'. I presume the change simply removed this from the output of query-command-line-options due to the historical cruft how the aforementioned command works. Unfortunately I didn't find any suitable replacement from what we are querying. The entries which we now lack detection are: { "machine", "mem-merge", QEMU_CAPS_MEM_MERGE }, { "machine", "aes-key-wrap", QEMU_CAPS_AES_KEY_WRAP }, { "machine", "dea-key-wrap", QEMU_CAPS_DEA_KEY_WRAP }, { "machine", "kernel_irqchip", QEMU_CAPS_MACHINE_KERNEL_IRQCHIP }, { "machine", "loadparm", QEMU_CAPS_LOADPARM }, Note that the oldest supported qemu version in libvirt is 2.11 at this point and all the above flags are included in 2.11 and later. This means we can theoretically remove the detection and always assume the flags in case when they are unlikely to be removed in the future. Alternatively if you can suggest an option how to detect this it will be welcome. Thanks Peter
On 22/07/21 10:19, Peter Krempa wrote: > This patch breaks detection of certain machine options features in > libvirt which were being detected from 'query-command-line-options'. > > I presume the change simply removed this from the output of > query-command-line-options due to the historical cruft how the > aforementioned command works. > > Unfortunately I didn't find any suitable replacement from what we are > querying. Yep, there is already a patch queued for this. Paolo
On Thu, Jul 22, 2021 at 16:39:26 +0200, Paolo Bonzini wrote: > On 22/07/21 10:19, Peter Krempa wrote: > > This patch breaks detection of certain machine options features in > > libvirt which were being detected from 'query-command-line-options'. > > > > I presume the change simply removed this from the output of > > query-command-line-options due to the historical cruft how the > > aforementioned command works. > > > > Unfortunately I didn't find any suitable replacement from what we are > > querying. > > Yep, there is already a patch queued for this. So the desired state is that the properties stay in 'query-command-line-options'?
diff --git a/softmmu/vl.c b/softmmu/vl.c index 7dd2d72d0b..f848abd31a 100644 --- a/softmmu/vl.c +++ b/softmmu/vl.c @@ -145,6 +145,8 @@ static const char *cpu_option; static const char *mem_path; static const char *incoming; static const char *loadvm; +static const char *accelerators; +static QDict *machine_opts_dict; static QTAILQ_HEAD(, ObjectOption) object_opts = QTAILQ_HEAD_INITIALIZER(object_opts); static ram_addr_t maxram_size; static uint64_t ram_slots; @@ -235,21 +237,6 @@ static QemuOptsList qemu_option_rom_opts = { }, }; -static QemuOptsList qemu_machine_opts = { - .name = "machine", - .implied_opt_name = "type", - .merge_lists = true, - .head = QTAILQ_HEAD_INITIALIZER(qemu_machine_opts.head), - .desc = { - /* - * no elements => accept any - * sanity checking will happen later - * when setting machine properties - */ - { } - }, -}; - static QemuOptsList qemu_accel_opts = { .name = "accel", .implied_opt_name = "accel", @@ -498,16 +485,6 @@ static QemuOptsList qemu_action_opts = { }, }; -/** - * Get machine options - * - * Returns: machine options (never null). - */ -static QemuOpts *qemu_get_machine_opts(void) -{ - return qemu_find_opts_singleton("machine"); -} - const char *qemu_get_vm_name(void) { return qemu_name; @@ -815,33 +792,6 @@ static MachineClass *find_default_machine(GSList *machines) return default_machineclass; } -static int machine_help_func(QemuOpts *opts, MachineState *machine) -{ - ObjectProperty *prop; - ObjectPropertyIterator iter; - - if (!qemu_opt_has_help_opt(opts)) { - return 0; - } - - object_property_iter_init(&iter, OBJECT(machine)); - while ((prop = object_property_iter_next(&iter))) { - if (!prop->set) { - continue; - } - - printf("%s.%s=%s", MACHINE_GET_CLASS(machine)->name, - prop->name, prop->type); - if (prop->description) { - printf(" (%s)\n", prop->description); - } else { - printf("\n"); - } - } - - return 1; -} - static void version(void) { printf("QEMU emulator version " QEMU_FULL_VERSION "\n" @@ -1554,33 +1504,31 @@ static gint machine_class_cmp(gconstpointer a, gconstpointer b) object_class_get_name(OBJECT_CLASS(mc1))); } -static MachineClass *machine_parse(const char *name, GSList *machines) +static void machine_help_func(const QDict *qdict) { - MachineClass *mc; - GSList *el; + GSList *machines, *el; + const char *type = qdict_get_try_str(qdict, "type"); - if (is_help_option(name)) { - printf("Supported machines are:\n"); - machines = g_slist_sort(machines, machine_class_cmp); - for (el = machines; el; el = el->next) { - MachineClass *mc = el->data; - if (mc->alias) { - printf("%-20s %s (alias of %s)\n", mc->alias, mc->desc, mc->name); - } - printf("%-20s %s%s%s\n", mc->name, mc->desc, - mc->is_default ? " (default)" : "", - mc->deprecation_reason ? " (deprecated)" : ""); + machines = object_class_get_list(TYPE_MACHINE, false); + if (type) { + ObjectClass *machine_class = OBJECT_CLASS(find_machine(type, machines)); + if (machine_class) { + type_print_class_properties(object_class_get_name(machine_class)); + return; } - exit(0); } - mc = find_machine(name, machines); - if (!mc) { - error_report("unsupported machine type"); - error_printf("Use -machine help to list supported machines\n"); - exit(1); + printf("Supported machines are:\n"); + machines = g_slist_sort(machines, machine_class_cmp); + for (el = machines; el; el = el->next) { + MachineClass *mc = el->data; + if (mc->alias) { + printf("%-20s %s (alias of %s)\n", mc->alias, mc->desc, mc->name); + } + printf("%-20s %s%s%s\n", mc->name, mc->desc, + mc->is_default ? " (default)" : "", + mc->deprecation_reason ? " (deprecated)" : ""); } - return mc; } static const char *pid_file; @@ -1633,32 +1581,31 @@ static const QEMUOption *lookup_opt(int argc, char **argv, return popt; } -static MachineClass *select_machine(void) +static MachineClass *select_machine(QDict *qdict, Error **errp) { + const char *optarg = qdict_get_try_str(qdict, "type"); GSList *machines = object_class_get_list(TYPE_MACHINE, false); - MachineClass *machine_class = find_default_machine(machines); - const char *optarg; - QemuOpts *opts; - Location loc; + MachineClass *machine_class; + Error *local_err = NULL; - loc_push_none(&loc); - - opts = qemu_get_machine_opts(); - qemu_opts_loc_restore(opts); - - optarg = qemu_opt_get(opts, "type"); if (optarg) { - machine_class = machine_parse(optarg, machines); + machine_class = find_machine(optarg, machines); + qdict_del(qdict, "type"); + if (!machine_class) { + error_setg(&local_err, "unsupported machine type"); + } + } else { + machine_class = find_default_machine(machines); + if (!machine_class) { + error_setg(&local_err, "No machine specified, and there is no default"); + } } - if (!machine_class) { - error_report("No machine specified, and there is no default"); - error_printf("Use -machine help to list supported machines\n"); - exit(1); - } - - loc_pop(&loc); g_slist_free(machines); + if (local_err) { + error_append_hint(&local_err, "Use -machine help to list supported machines\n"); + error_propagate(errp, local_err); + } return machine_class; } @@ -1677,42 +1624,70 @@ static int object_parse_property_opt(Object *obj, return 0; } -static int machine_set_property(void *opaque, - const char *name, const char *value, - Error **errp) +/* *Non*recursively replace underscores with dashes in QDict keys. */ +static void keyval_dashify(QDict *qdict, Error **errp) { - g_autofree char *qom_name = g_strdup(name); + const QDictEntry *ent, *next; char *p; - for (p = qom_name; *p; p++) { - if (*p == '_') { - *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); } +} + +static void qemu_apply_legacy_machine_options(QDict *qdict) +{ + const char *value; + + keyval_dashify(qdict, &error_fatal); /* Legacy options do not correspond to MachineState properties. */ - if (g_str_equal(qom_name, "accel")) { - return 0; - } - if (g_str_equal(qom_name, "igd-passthru")) { - object_register_sugar_prop(ACCEL_CLASS_NAME("xen"), qom_name, value, - false); - return 0; - } - if (g_str_equal(qom_name, "kvm-shadow-mem")) { - object_register_sugar_prop(ACCEL_CLASS_NAME("kvm"), qom_name, value, - false); - return 0; - } - if (g_str_equal(qom_name, "kernel-irqchip")) { - object_register_sugar_prop(ACCEL_CLASS_NAME("kvm"), qom_name, value, - false); - object_register_sugar_prop(ACCEL_CLASS_NAME("whpx"), qom_name, value, - false); - return 0; + value = qdict_get_try_str(qdict, "accel"); + if (value) { + accelerators = g_strdup(value); + qdict_del(qdict, "accel"); } - return object_parse_property_opt(opaque, name, value, "type", errp); + value = qdict_get_try_str(qdict, "igd-passthru"); + if (value) { + object_register_sugar_prop(ACCEL_CLASS_NAME("xen"), "igd-passthru", value, + false); + qdict_del(qdict, "igd-passthru"); + } + + value = qdict_get_try_str(qdict, "kvm-shadow-mem"); + if (value) { + object_register_sugar_prop(ACCEL_CLASS_NAME("kvm"), "kvm-shadow-mem", value, + false); + qdict_del(qdict, "kvm-shadow-mem"); + } + + value = qdict_get_try_str(qdict, "kernel-irqchip"); + if (value) { + 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"); + } } static void object_option_foreach_add(bool (*type_opt_predicate)(const char *)) @@ -1827,16 +1802,14 @@ static bool object_create_early(const char *type) return true; } -static void qemu_apply_machine_options(void) +static void qemu_apply_machine_options(QDict *qdict) { MachineClass *machine_class = MACHINE_GET_CLASS(current_machine); - QemuOpts *machine_opts = qemu_get_machine_opts(); const char *boot_order = NULL; const char *boot_once = NULL; QemuOpts *opts; - qemu_opt_foreach(machine_opts, machine_set_property, current_machine, - &error_fatal); + object_set_properties_from_keyval(OBJECT(current_machine), qdict, false, &error_fatal); current_machine->ram_size = ram_size; current_machine->maxram_size = maxram_size; current_machine->ram_slots = ram_slots; @@ -1865,10 +1838,8 @@ static void qemu_apply_machine_options(void) current_machine->boot_once = boot_once; if (semihosting_enabled() && !semihosting_get_argc()) { - const char *kernel_filename = qemu_opt_get(machine_opts, "kernel"); - const char *kernel_cmdline = qemu_opt_get(machine_opts, "append") ?: ""; /* fall back to the -kernel/-append */ - semihosting_arg_fallback(kernel_filename, kernel_cmdline); + semihosting_arg_fallback(current_machine->kernel_filename, current_machine->kernel_cmdline); } } @@ -1924,8 +1895,7 @@ static void qemu_create_early_backends(void) /* * Note: we need to create audio and block backends before - * machine_set_property(), so machine properties can refer to - * them. + * setting machine properties, so they can be referred to. */ configure_blockdev(&bdo_queue, machine_class, snapshot); audio_init_audiodevs(); @@ -2091,16 +2061,14 @@ static void set_memory_options(MachineClass *mc) loc_pop(&loc); } -static void qemu_create_machine(MachineClass *machine_class) +static void qemu_create_machine(QDict *qdict) { + MachineClass *machine_class = select_machine(qdict, &error_fatal); object_set_machine_compat_props(machine_class->compat_props); set_memory_options(machine_class); current_machine = MACHINE(object_new_with_class(OBJECT_CLASS(machine_class))); - if (machine_help_func(qemu_get_machine_opts(), current_machine)) { - exit(0); - } object_property_add_child(object_get_root(), "machine", OBJECT(current_machine)); object_property_add_child(container_get(OBJECT(current_machine), @@ -2131,8 +2099,12 @@ static void qemu_create_machine(MachineClass *machine_class) * specified either by the configuration file or by the command line. */ if (machine_class->default_machine_opts) { - qemu_opts_set_defaults(qemu_find_opts("machine"), - machine_class->default_machine_opts, 0); + QDict *default_opts = + keyval_parse(machine_class->default_machine_opts, NULL, NULL, + &error_abort); + object_set_properties_from_keyval(OBJECT(current_machine), default_opts, + false, &error_abort); + qobject_unref(default_opts); } } @@ -2154,7 +2126,8 @@ static int global_init_func(void *opaque, QemuOpts *opts, Error **errp) */ static bool is_qemuopts_group(const char *group) { - if (g_str_equal(group, "object")) { + if (g_str_equal(group, "object") || + g_str_equal(group, "machine")) { return false; } return true; @@ -2167,6 +2140,13 @@ static void qemu_record_config_group(const char *group, QDict *dict, Visitor *v = qobject_input_visitor_new_keyval(QOBJECT(dict)); object_option_add_visitor(v); visit_free(v); + } else if (g_str_equal(group, "machine")) { + /* + * Cannot merge string-valued and type-safe dictionaries, so JSON + * is not accepted yet for -M. + */ + assert(!from_json); + keyval_merge(machine_opts_dict, dict, errp); } else { abort(); } @@ -2297,13 +2277,11 @@ static int do_configure_accelerator(void *opaque, QemuOpts *opts, Error **errp) static void configure_accelerators(const char *progname) { - const char *accelerators; bool init_failed = false; qemu_opts_foreach(qemu_find_opts("icount"), do_configure_icount, NULL, &error_fatal); - accelerators = qemu_opt_get(qemu_get_machine_opts(), "accel"); if (QTAILQ_EMPTY(&qemu_accel_opts.head)) { char **accel_list, **tmp; @@ -2391,12 +2369,11 @@ static void create_default_memdev(MachineState *ms, const char *path) &error_fatal); } -static void qemu_validate_options(void) +static void qemu_validate_options(const QDict *machine_opts) { - QemuOpts *machine_opts = qemu_get_machine_opts(); - const char *kernel_filename = qemu_opt_get(machine_opts, "kernel"); - const char *initrd_filename = qemu_opt_get(machine_opts, "initrd"); - const char *kernel_cmdline = qemu_opt_get(machine_opts, "append"); + const char *kernel_filename = qdict_get_try_str(machine_opts, "kernel"); + const char *initrd_filename = qdict_get_try_str(machine_opts, "initrd"); + const char *kernel_cmdline = qdict_get_try_str(machine_opts, "append"); if (kernel_filename == NULL) { if (kernel_cmdline != NULL) { @@ -2736,7 +2713,6 @@ void qemu_init(int argc, char **argv, char **envp) qemu_add_opts(&qemu_trace_opts); qemu_plugin_add_opts(); qemu_add_opts(&qemu_option_rom_opts); - qemu_add_opts(&qemu_machine_opts); qemu_add_opts(&qemu_accel_opts); qemu_add_opts(&qemu_mem_opts); qemu_add_opts(&qemu_smp_opts); @@ -2777,6 +2753,7 @@ void qemu_init(int argc, char **argv, char **envp) } } + machine_opts_dict = qdict_new(); if (userconfig) { qemu_read_default_config_file(&error_fatal); } @@ -2866,8 +2843,7 @@ void qemu_init(int argc, char **argv, char **envp) parse_display(optarg); break; case QEMU_OPTION_nographic: - olist = qemu_find_opts("machine"); - qemu_opts_parse_noisily(olist, "graphics=off", false); + qdict_put_str(machine_opts_dict, "graphics", "off"); nographic = true; dpy.type = DISPLAY_TYPE_NONE; break; @@ -2891,16 +2867,16 @@ void qemu_init(int argc, char **argv, char **envp) } break; case QEMU_OPTION_kernel: - qemu_opts_set(qemu_find_opts("machine"), "kernel", optarg, &error_abort); + qdict_put_str(machine_opts_dict, "kernel", optarg); break; case QEMU_OPTION_initrd: - qemu_opts_set(qemu_find_opts("machine"), "initrd", optarg, &error_abort); + qdict_put_str(machine_opts_dict, "initrd", optarg); break; case QEMU_OPTION_append: - qemu_opts_set(qemu_find_opts("machine"), "append", optarg, &error_abort); + qdict_put_str(machine_opts_dict, "append", optarg); break; case QEMU_OPTION_dtb: - qemu_opts_set(qemu_find_opts("machine"), "dtb", optarg, &error_abort); + qdict_put_str(machine_opts_dict, "dtb", optarg); break; case QEMU_OPTION_cdrom: drive_add(IF_DEFAULT, 2, optarg, CDROM_OPTS); @@ -3010,7 +2986,7 @@ void qemu_init(int argc, char **argv, char **envp) } break; case QEMU_OPTION_bios: - qemu_opts_set(qemu_find_opts("machine"), "firmware", optarg, &error_abort); + qdict_put_str(machine_opts_dict, "firmware", optarg); break; case QEMU_OPTION_singlestep: singlestep = 1; @@ -3281,17 +3257,20 @@ void qemu_init(int argc, char **argv, char **envp) preconfig_requested = true; break; case QEMU_OPTION_enable_kvm: - olist = qemu_find_opts("machine"); - qemu_opts_parse_noisily(olist, "accel=kvm", false); + qdict_put_str(machine_opts_dict, "accel", "kvm"); break; case QEMU_OPTION_M: case QEMU_OPTION_machine: - olist = qemu_find_opts("machine"); - opts = qemu_opts_parse_noisily(olist, optarg, true); - if (!opts) { - exit(1); + { + bool help; + + keyval_parse_into(machine_opts_dict, optarg, "type", &help, &error_fatal); + if (help) { + machine_help_func(machine_opts_dict); + exit(EXIT_SUCCESS); + } + break; } - break; case QEMU_OPTION_accel: accel_opts = qemu_opts_parse_noisily(qemu_find_opts("accel"), optarg, true); @@ -3318,12 +3297,10 @@ void qemu_init(int argc, char **argv, char **envp) } break; case QEMU_OPTION_usb: - olist = qemu_find_opts("machine"); - qemu_opts_parse_noisily(olist, "usb=on", false); + qdict_put_str(machine_opts_dict, "usb", "on"); break; case QEMU_OPTION_usbdevice: - olist = qemu_find_opts("machine"); - qemu_opts_parse_noisily(olist, "usb=on", false); + qdict_put_str(machine_opts_dict, "usb", "on"); add_device_config(DEV_USB, optarg); break; case QEMU_OPTION_device: @@ -3342,12 +3319,10 @@ void qemu_init(int argc, char **argv, char **envp) vnc_parse(optarg); break; case QEMU_OPTION_no_acpi: - olist = qemu_find_opts("machine"); - qemu_opts_parse_noisily(olist, "acpi=off", false); + qdict_put_str(machine_opts_dict, "acpi", "off"); break; case QEMU_OPTION_no_hpet: - olist = qemu_find_opts("machine"); - qemu_opts_parse_noisily(olist, "hpet=off", false); + qdict_put_str(machine_opts_dict, "hpet", "off"); break; case QEMU_OPTION_no_reboot: olist = qemu_find_opts("action"); @@ -3600,7 +3575,7 @@ void qemu_init(int argc, char **argv, char **envp) */ loc_set_none(); - qemu_validate_options(); + qemu_validate_options(machine_opts_dict); qemu_process_sugar_options(); /* @@ -3633,7 +3608,7 @@ void qemu_init(int argc, char **argv, char **envp) configure_rtc(qemu_find_opts_singleton("rtc")); - qemu_create_machine(select_machine()); + qemu_create_machine(machine_opts_dict); suspend_mux_open(); @@ -3641,12 +3616,14 @@ void qemu_init(int argc, char **argv, char **envp) qemu_create_default_devices(); qemu_create_early_backends(); - qemu_apply_machine_options(); + qemu_apply_legacy_machine_options(machine_opts_dict); + qemu_apply_machine_options(machine_opts_dict); + qobject_unref(machine_opts_dict); phase_advance(PHASE_MACHINE_CREATED); /* * Note: uses machine properties such as kernel-irqchip, must run - * after machine_set_property(). + * after qemu_apply_machine_options. */ configure_accelerators(argv[0]); phase_advance(PHASE_ACCEL_CREATED);
Switch from QemuOpts to keyval. This enables the introduction of non-scalar machine properties, and JSON syntax in the future. For JSON syntax to be supported right now, we would have to consider what would happen if string-based dictionaries (produced by -M key=val) were to be merged with strongly-typed dictionaries (produced by -M {'key': 123}). The simplest way out is to never enter the situation, and only allow one -M option when JSON syntax is in use. However, we want options such as -smp to become syntactic sugar for -M, and this is a problem; as soon as -smp becomes a shortcut for -M, QEMU would forbid using -M '{....}' together with -smp. Therefore, allowing JSON syntax right now for -M would be a forward-compatibility nightmare and it would be impossible anyway to introduce -M incrementally in tools. Instead, support for JSON syntax is delayed until after the main options are converted to QOM compound properties. These include -boot, -acpitable, -smbios, -m, -semihosting-config, -rtc and -fw_cfg. Once JSON syntax is introduced, these options will _also_ be forbidden together with -M '{...}'. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- softmmu/vl.c | 315 ++++++++++++++++++++++++--------------------------- 1 file changed, 146 insertions(+), 169 deletions(-)