On 2013年03月27日 23:08, Martin Kletzander wrote:
On 03/15/2013 10:19 AM, Li Zhang wrote:From: Li Zhang <zhlcindy@xxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxx> --- 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.
Ah, okay, it should be set when using help string.
if (!(archstr = qemuMonitorGetTargetArch(mon))) goto cleanup;diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.hindex 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.
I see. You are right, the comment should be removed.
Also this won't work with implementations where we don't probe by QMP, but '-machine dump-core=xx' is supported.
Currently, it assumes that 1.2.0 supports QMP and -machine. As you mentioned, I will add this capability when using help string. Is it okay to support -machine with 1.1.0 onwards ?
/* 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 :(
So it needs to add this capability when using QMP.
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.
Yes, you are right.
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.
Ah, this is a problem on PowerPC.
To compare with PowerPC, X86 doesn't create USB controller in
machine->init().
X86 machine init code in QEMU is as the following:
if (pci_enabled && usb_enabled(false)) {
pci_create_simple(pci_bus, piix3_devfn + 2, "piix3-usb-uhci");
}
But PowerPC init code in QEMU is as the following:
if (usb_enabled(spapr->has_graphics)) {
pci_create_simple(phb->bus, -1, "pci-ohci");
if (spapr->has_graphics) {
usbdevice_create("keyboard");
usbdevice_create("mouse");
}
}
When spapr->has_graphics is true, it will create one USB controller
implicitly.
But in libvirt, it also creates one USB controller implicitly if no USB
controller specified.
So this will cause errors.I think you can also get errors by hacking X86 QEMU code with usb_enabled(true)
If with ,usb=off, it will get false with (usb_enabled(spapr->has_graphics)). Then libvirt can use its own controller created implicitly. Thanks.
Thanks, Martin
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list