On 2013年03月27日 23:08, Martin Kletzander wrote:
On 03/15/2013 10:19 AM, Li Zhang wrote:
From: Li Zhang <zhlcindy@linux.vnet.ibm.com>
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@linux.vnet.ibm.com> --- 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.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.
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.
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)) {
Yes, you are right. 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