On Thu, Sep 27, 2012 at 03:00:14PM +0200, Jiri Denemark wrote:
On Tue, Sep 25, 2012 at 19:00:13 +0100, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Start a QEMU process using
$QEMU -S -no-user-config -nodefconfig -nodefaults \ -nographic -M none -qmp stdio
-nodefconfig should not ever be used if QEMU supports -no-user-config. The reason is that -nodefconfig disables loading of all files even those that reside somewhere in /usr/share and may contain required data, such as CPU definitions (although they were moved back to the code recently) or machine type definitions if they ever implement the ideas to separate them from the code.
Hmm, yes, I forgot about that.
@@ -1282,6 +1285,7 @@ static struct qemuCapsStringFlags qemuCapsObjectPropsVirtioNet[] = { };
static struct qemuCapsStringFlags qemuCapsObjectPropsPciAssign[] = { + { "rombar", QEMU_CAPS_PCI_ROMBAR }, { "configfd", QEMU_CAPS_PCI_CONFIGFD }, { "bootindex", QEMU_CAPS_PCI_BOOTINDEX }, }; @@ -1857,6 +1861,10 @@ qemuCapsProbeQMPCommands(qemuCapsPtr caps, qemuCapsSet(caps, QEMU_CAPS_BLOCKJOB_ASYNC); else if (STREQ(name, "dump-guest-memory")) qemuCapsSet(caps, QEMU_CAPS_DUMP_GUEST_MEMORY); + else if (STREQ(name, "query-spice")) + qemuCapsSet(caps, QEMU_CAPS_SPICE); + else if (STREQ(name, "query-kvm")) + qemuCapsSet(caps, QEMU_CAPS_KVM); VIR_FREE(name); } VIR_FREE(commands);
Hmm, looks like these two hunks should rather go into the previous patch, shouldn't they?
No, the previous patch was just refactoring existing code. The existing approach to detecting spice/kvm was to use -help parsing. Only with the new QMP code in this patch, do we now detect it via the QMP command list
@@ -1979,18 +2078,252 @@ qemuCapsPtr qemuCapsNewForBinary(const char *binary) * understands the 0.13.0+ notion of "-device driver,". */ if (qemuCapsGet(caps, QEMU_CAPS_DEVICE) && strstr(help, "-device driver,?") && - qemuCapsExtractDeviceStr(binary, caps) < 0) - goto error; + qemuCapsExtractDeviceStr(caps->binary, caps) < 0) + goto cleanup;
if (qemuCapsProbeCPUModels(caps) < 0) - goto error; + goto cleanup;
if (qemuCapsProbeMachineTypes(caps) < 0) - goto error; + goto cleanup; + + ret = 0; +cleanup: + virCommandFree(cmd); + return ret; +}
I think you actually didn't want to remove VIR_FREE(help); from the cleanup section.
Yeah, bug.
+static int +qemuCapsInitQMP(qemuCapsPtr caps) +{ + int ret = -1; + virCommandPtr cmd = NULL; + virDomainObjPtr vm = NULL; + int socks[2] = { -1, -1 }; + qemuMonitorPtr mon = NULL; + int major, minor, micro; + char *package; + + VIR_DEBUG("caps=%p", caps); + + if (socketpair(PF_UNIX, SOCK_STREAM, 0, socks) < 0) { + virReportSystemError(errno, "%s", + _("unable to create socket pair")); + goto cleanup; + } + + if (!(vm = virDomainObjNew(NULL))) + goto cleanup;
Heh, AFAICT, vm in qemuMonitorOpenFD is only used to fill in mon->vm so that the VM pointer can be passed to various callbacks when something asynchronous happens in the monitor. Looks like we can just safely pass NULL to qemuMonitorOpenFD() instead.
There is one place in the monitor which uses it, but we can avoid it.
+ + cmd = virCommandNewArgList(caps->binary, + "-S", + "-no-user-config", + "-nodefconfig",
Remove the -nodefconfig parameter, since this code requires at least qemu-1.2 and thus it will either support -no-user-config or be unusable anyway.
+ "-nodefaults", + "-nographic", + "-M", "none", + "-qmp", "stdio", + NULL); + virCommandAddEnvPassCommon(cmd); + virCommandSetInputFD(cmd, socks[1]); + virCommandSetOutputFD(cmd, &socks[1]); + virCommandClearCaps(cmd); + + if (virCommandRunAsync(cmd, NULL) < 0) + goto cleanup;
However, if qemu is not new enough to support -no-user-config, virCommand will fail and this would consider it a fatal error instead of falling back to help parsing.
Actually not, because we're running async here, so we don't detect the failed QEMU at this point, only later in this method. There is a problem here though - my current code will cause log messages to be generated when hitting old QEMU which is undesirable. So I'm changing this to use virCommandRun and -daemonize QEMU, so we can detect exit status for old QEMU without polluting the log. Enough changes that I'll re-post this. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|