On Mon, Jan 5, 2015 at 11:37 AM, Jan Kiszka <address@hidden> wrote:
On 2015-01-05 12:22, Stefan Hajnoczi wrote:
Commit 49d2e648e8087d154d8bf8b91f27c8e05e79d5a6 ("machine: remove
qemu_machine_opts global list") removed option descriptions from the
-machine QemuOptsList to avoid repeating MachineState's QOM properties.
This change broke vl.c:usb_enabled() because qemu_opt_get_bool() cannot
be used on QemuOptsList without option descriptions since QemuOpts
doesn't know the type and therefore left an unparsed string value.
This patch avoids calling qemu_opt_get_bool() to fix the assertion
failure:
$ qemu-system-x86_64 -usb
qemu_opt_get_bool_helper: Assertion `opt->desc && opt->desc->type ==
QEMU_OPT_BOOL' failed.
Test the presence of -usb using qemu_opt_find() but use the
MachineState->usb field instead of qemu_opt_get_bool().
Cc: Marcel Apfelbaum <address@hidden>
Cc: Tiejun Chen <address@hidden>
Signed-off-by: Stefan Hajnoczi <address@hidden>
---
vl.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/vl.c b/vl.c
index bea9656..6e8889c 100644
--- a/vl.c
+++ b/vl.c
@@ -999,8 +999,11 @@ static int parse_name(QemuOpts *opts, void *opaque)
bool usb_enabled(bool default_usb)
{
- return qemu_opt_get_bool(qemu_get_machine_opts(), "usb",
- has_defaults && default_usb);
+ if (qemu_opt_find(qemu_get_machine_opts(), "usb")) {
+ return current_machine->usb;
+ } else {
+ return has_defaults && default_usb;
+ }
}
#ifndef _WIN32
That still leaves the other boolean machine options broken. A generic
fix would be good. Or revert the original commit until we have one.
I think we should revert the original commit.
qemu_option_get_*() callers need to be converted to query MachineState
fields instead of using QemuOpts functions.
This means we need to modify the way default values are processed.
MachineState fields should start with the default value and be
overridden by command-line options. Right now it works differently:
we query the command-line option and fall back to the default if the
option wasn't set. That approach doesn't work for MachineState fields
since C types (bool, int, etc) aren't tristate in the general case, so
they cannot indicate whether or not they were set.
The benefit of doing this work is that we eliminate the QemuOpts layer
and work directly with QOM properties instead.
Marcel: Do you want to do this or do you have another fix in mind?