From: Li Zhang <zhlcindy(a)linux.vnet.ibm.com&gt; To avoid the collision for creating USB controllers in machine->init() and -device xx command line, it needs to set usb=off to avoid one USB controller created in machine->init(). So that libvirt can use -device or -usb to create USB controller sucessfully. So QEMU_CAPS_MACHINE_USB_OPT capability is added, and it is for QEMU v1.3.0 onwards which supports USB option. Signed-off-by: Li Zhang <zhlcindy(a)linux.vnet.ibm.com&gt; --- v2 -> v1: * Rename QEMU_CAPS_USB_OPT to QEMU_CAPS_MACHINE_USB_OPT suggested by Daniel * Corret QEMU version with v1.3.0 suggested by Daniel src/qemu/qemu_capabilities.c | 7 ++++++- src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 6 ++++++ 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 778e825..6ea09cc 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -211,7 +211,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "rng-random", /* 130 */ "rng-egd", "virtio-ccw", - "machine-opt" + "machine-opt", + "machine-usb-opt" ); struct _virQEMUCaps { @@ -2446,6 +2447,10 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps, /* machine option is supported for newer version */ virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_OPT); + /* USB option is supported v1.3.0 onwards */ + if (qemuCaps->version >= 1003000) + virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_USB_OPT); + if (!(archstr = qemuMonitorGetTargetArch(mon))) goto cleanup; diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 66df556..a23df1f 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -173,6 +173,7 @@ enum virQEMUCapsFlags { QEMU_CAPS_OBJECT_RNG_EGD = 131, /* EGD protocol daemon for rng */ QEMU_CAPS_VIRTIO_CCW = 132, /* -device virtio-*-ccw */ QEMU_CAPS_MACHINE_OPT = 133, /* -machine xxxx*/ + QEMU_CAPS_MACHINE_USB_OPT = 134, /* -machine xxxx*/

QEMU_CAPS_LAST, /* this must always be the last item */ }; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c39faf0..5472c8b 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4964,6 +4964,12 @@ qemuBuildMachineArgStr(virCommandPtr cmd, virCommandAddArg(cmd, "-machine"); virBufferAsprintf(&buf, "%s", def->os.machine); + /* To avoid the collision of creating USB controllers when calling + * machine->init in QEMU, it needs to set usb=off + */ + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_USB_OPT)) + virBufferAsprintf(&buf, ",usb=off"); + if (def->mem.dump_core) { if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DUMP_GUEST_CORE)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED,

From: Li Zhang <zhlcindy(a)linux.vnet.ibm.com&gt; Currently, -machine option is used only when dump-guest-core is set. To use options defined in machine option for newer version of QEMU, it needs to use -machine xxx, and to be compatible with older version -M, this patch addes QEMU_CAPS_MACHINE_OPT capability for newer version, say 1.2.0. Signed-off-by: Li Zhang <zhlcindy(a)linux.vnet.ibm.com&gt; --- v2 -> v1: * Split the patch to 2 parts suggested by Daniel P.Berrange * Rename QEMU_CAPS_MACH_OPT to QEMU_CAPS_MACHINE_OPT * Remove version 1.1 assertion for QEMU_CAPS_MACHINE_OPT src/qemu/qemu_capabilities.c | 6 +++++- src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 30 +++++++++++++++++++----------- tests/qemuxml2argvtest.c | 6 +++--- 4 files changed, 28 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 519d2c5..778e825 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -210,7 +210,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "rng-random", /* 130 */ "rng-egd", - "virtio-ccw" + "virtio-ccw", + "machine-opt" ); struct _virQEMUCaps { @@ -2442,6 +2443,9 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps, virQEMUCapsInitQMPBasic(qemuCaps); + /* machine option is supported for newer version */ + virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_OPT); + if (!(archstr = qemuMonitorGetTargetArch(mon))) goto cleanup; diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index da06e27..66df556 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -172,6 +172,7 @@ enum virQEMUCapsFlags { virtio rng */ QEMU_CAPS_OBJECT_RNG_EGD = 131, /* EGD protocol daemon for rng */ QEMU_CAPS_VIRTIO_CCW = 132, /* -device virtio-*-ccw */ + QEMU_CAPS_MACHINE_OPT = 133, /* -machine xxxx*/ QEMU_CAPS_LAST, /* this must always be the last item */ }; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index dc49d44..c39faf0 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4941,6 +4941,8 @@ qemuBuildMachineArgStr(virCommandPtr cmd, const virDomainDefPtr def, virQEMUCapsPtr qemuCaps) { + virBuffer buf = VIR_BUFFER_INITIALIZER; + /* This should *never* be NULL, since we always provide * a machine in the capabilities data for QEMU. So this * check is just here as a safety in case the unexpected @@ -4948,27 +4950,33 @@ qemuBuildMachineArgStr(virCommandPtr cmd, if (!def->os.machine) return 0; - if (!def->mem.dump_core) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_OPT)) { /* if no parameter to the machine type is needed, we still use * '-M' to keep the most of the compatibility with older versions. */ virCommandAddArgList(cmd, "-M", def->os.machine, NULL); } else { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DUMP_GUEST_CORE)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("dump-guest-core is not available " - " with this QEMU binary")); - return -1; - } /* However, in case there is a parameter to be added, we need to * use the "-machine" parameter because qemu is not parsing the * "-M" correctly */ + virCommandAddArg(cmd, "-machine"); - virCommandAddArgFormat(cmd, - "%s,dump-guest-core=%s", - def->os.machine, - virDomainMemDumpTypeToString(def->mem.dump_core)); + virBufferAsprintf(&buf, "%s", def->os.machine); + + if (def->mem.dump_core) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DUMP_GUEST_CORE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("dump-guest-core is not available " + " with this QEMU binary")); + return -1; + } + + virBufferAsprintf(&buf, ",dump-guest-core=%s", + virDomainMemDumpTypeToString(def->mem.dump_core)); + } + + virCommandAddArg(cmd, virBufferContentAndReset(&buf)); } return 0; diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index c77b73f..352b41f 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -363,9 +363,9 @@ mymain(void) DO_TEST("minimal-s390", QEMU_CAPS_NAME); DO_TEST("machine-aliases1", NONE); DO_TEST("machine-aliases2", QEMU_CAPS_KVM); - DO_TEST("machine-core-on", QEMU_CAPS_DUMP_GUEST_CORE); - DO_TEST("machine-core-off", QEMU_CAPS_DUMP_GUEST_CORE); - DO_TEST_FAILURE("machine-core-on", NONE); + DO_TEST("machine-core-on", QEMU_CAPS_MACHINE_OPT, QEMU_CAPS_DUMP_GUEST_CORE); + DO_TEST("machine-core-off", QEMU_CAPS_MACHINE_OPT, QEMU_CAPS_DUMP_GUEST_CORE); + DO_TEST_FAILURE("machine-core-on", QEMU_CAPS_MACHINE_OPT, NONE); DO_TEST("boot-cdrom", NONE); DO_TEST("boot-network", NONE); DO_TEST("boot-floppy", NONE);

On 03/15/2013 10:19 AM, Li Zhang wrote: > From: Li Zhang <zhlcindy(a)linux.vnet.ibm.com&gt; > > Currently, -machine option is used only when dump-guest-core is set. > > To use options defined in machine option for newer version of QEMU, > it needs to use -machine xxx, and to be compatible with older version > -M, this patch addes QEMU_CAPS_MACHINE_OPT capability for newer version, > say 1.2.0. > > Signed-off-by: Li Zhang <zhlcindy(a)linux.vnet.ibm.com&gt; > --- > v2 -> v1: > * Split the patch to 2 parts suggested by Daniel P.Berrange > * Rename QEMU_CAPS_MACH_OPT to QEMU_CAPS_MACHINE_OPT > * Remove version 1.1 assertion for QEMU_CAPS_MACHINE_OPT > > src/qemu/qemu_capabilities.c | 6 +++++- > src/qemu/qemu_capabilities.h | 1 + > src/qemu/qemu_command.c | 30 +++++++++++++++++++----------- > tests/qemuxml2argvtest.c | 6 +++--- > 4 files changed, 28 insertions(+), 15 deletions(-) > > diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c > index 519d2c5..778e825 100644 > --- a/src/qemu/qemu_capabilities.c > +++ b/src/qemu/qemu_capabilities.c > @@ -210,7 +210,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, > > "rng-random", /* 130 */ > "rng-egd", > - "virtio-ccw" > + "virtio-ccw", > + "machine-opt" > ); > > struct _virQEMUCaps { > @@ -2442,6 +2443,9 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps, > > virQEMUCapsInitQMPBasic(qemuCaps); > > + /* machine option is supported for newer version */ > + virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_OPT); > + But this is also supported when we don't probe by QMP.

> if (!(archstr = qemuMonitorGetTargetArch(mon))) > goto cleanup; > > diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h > index da06e27..66df556 100644 > --- a/src/qemu/qemu_capabilities.h > +++ b/src/qemu/qemu_capabilities.h > @@ -172,6 +172,7 @@ enum virQEMUCapsFlags { > virtio rng */ > QEMU_CAPS_OBJECT_RNG_EGD = 131, /* EGD protocol daemon for rng */ > QEMU_CAPS_VIRTIO_CCW = 132, /* -device virtio-*-ccw */ > + QEMU_CAPS_MACHINE_OPT = 133, /* -machine xxxx*/ > > QEMU_CAPS_LAST, /* this must always be the last item */ > }; > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c > index dc49d44..c39faf0 100644 > --- a/src/qemu/qemu_command.c > +++ b/src/qemu/qemu_command.c > @@ -4941,6 +4941,8 @@ qemuBuildMachineArgStr(virCommandPtr cmd, > const virDomainDefPtr def, > virQEMUCapsPtr qemuCaps) > { > + virBuffer buf = VIR_BUFFER_INITIALIZER; > + > /* This should *never* be NULL, since we always provide > * a machine in the capabilities data for QEMU. So this > * check is just here as a safety in case the unexpected > @@ -4948,27 +4950,33 @@ qemuBuildMachineArgStr(virCommandPtr cmd, > if (!def->os.machine) > return 0; > > - if (!def->mem.dump_core) { > + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_OPT)) { This was intentionally done the way you see. I tried to avoid using '-machine' for various other qemu implementations (I couldn't find anywhere that '-machine' is guaranteed on all sorts of implementations). This is why the following comment is in place, which you obsoleted, but haven't removed, so it doesn't make much sense now.

Also this won't work with implementations where we don't probe by QMP, but '-machine dump-core=xx' is supported.

> /* if no parameter to the machine type is needed, we still use > * '-M' to keep the most of the compatibility with older versions. > */ > virCommandAddArgList(cmd, "-M", def->os.machine, NULL); > } else { > - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DUMP_GUEST_CORE)) { > - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > - "%s", _("dump-guest-core is not available " > - " with this QEMU binary")); > - return -1; > - } > > /* However, in case there is a parameter to be added, we need to > * use the "-machine" parameter because qemu is not parsing the > * "-M" correctly */ > + > virCommandAddArg(cmd, "-machine"); > - virCommandAddArgFormat(cmd, > - "%s,dump-guest-core=%s", > - def->os.machine, > - virDomainMemDumpTypeToString(def->mem.dump_core)); > + virBufferAsprintf(&buf, "%s", def->os.machine); > + > + if (def->mem.dump_core) { > + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DUMP_GUEST_CORE)) { There is also on pre-existing bug in my code which I see now. When probing by QMP, we don't set QEMU_CAPS_DUMP_GUEST_CORE flag and hence report this as unsupported even when the qemu knows this option :(

However, I believe we have to do some more investigation and rely on version-specific capabilities only as a last resort. But because there is no way to get information about ',usb=off' and more of these are already in the code and coming, we will probably need to switch to '-machine' anyway.

Replying to your [2/2] here as well, with what kind of configuration do you experience such problems? I see no problems with upstream git QEMU (reporting itself as version 1.4.50), nor 1.4.0.

Thanks, Martin