[libvirt] [PATCH 0/4] Switch to using QMP for capabilities detection
This is a fixup of the last patch in my previous series based on the feedback obtained, and some flaws I identified myself
From: "Daniel P. Berrange" <berrange@redhat.com> The qemuMonitorOpen method only needs a virDomainObjPtr in order to access the QEMU pid. This is not critical when detecting the QEMU capabilties, so can easily be skipped Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_monitor.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index cd41dd7..cb121e8 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -283,7 +283,7 @@ qemuMonitorOpenUnix(const char *monitor, pid_t cpid) break; if ((errno == ENOENT || errno == ECONNREFUSED) && - virProcessKill(cpid, 0) == 0) { + cpid && virProcessKill(cpid, 0) == 0) { /* ENOENT : Socket may not have shown up yet * ECONNREFUSED : Leftover socket hasn't been removed yet */ continue; @@ -788,7 +788,7 @@ qemuMonitorOpen(virDomainObjPtr vm, switch (config->type) { case VIR_DOMAIN_CHR_TYPE_UNIX: hasSendFD = true; - if ((fd = qemuMonitorOpenUnix(config->data.nix.path, vm->pid)) < 0) + if ((fd = qemuMonitorOpenUnix(config->data.nix.path, vm ? vm->pid : 0)) < 0) return NULL; break; -- 1.7.11.4
On 09/28/2012 08:58 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
The qemuMonitorOpen method only needs a virDomainObjPtr in order to access the QEMU pid. This is not critical when detecting the QEMU capabilties, so can easily be skipped
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_monitor.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
ACK. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
From: "Daniel P. Berrange" <berrange@redhat.com> After calling qemuMonitorClose(), it is still possible for the QEMU monitor I/O event callback to get invoked. This will trigger an error message because mon->fd has been set to -1 at this point. Silently ignore the case where mon->fd is -1, likewise for mon->watch being zero. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_monitor.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index cb121e8..6c3f09e 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -538,6 +538,9 @@ static void qemuMonitorUpdateWatch(qemuMonitorPtr mon) VIR_EVENT_HANDLE_HANGUP | VIR_EVENT_HANDLE_ERROR; + if (!mon->watch) + return; + if (mon->lastError.code == VIR_ERR_OK) { events |= VIR_EVENT_HANDLE_READABLE; @@ -563,6 +566,11 @@ qemuMonitorIO(int watch, int fd, int events, void *opaque) { #if DEBUG_IO VIR_DEBUG("Monitor %p I/O on watch %d fd %d events %d", mon, watch, fd, events); #endif + if (mon->fd == -1 || mon->watch == 0) { + qemuMonitorUnlock(mon); + virObjectUnref(mon); + return; + } if (mon->fd != fd || mon->watch != watch) { if (events & (VIR_EVENT_HANDLE_HANGUP | VIR_EVENT_HANDLE_ERROR)) @@ -830,9 +838,12 @@ void qemuMonitorClose(qemuMonitorPtr mon) "mon=%p refs=%d", mon, mon->object.refs); if (mon->fd >= 0) { - if (mon->watch) + if (mon->watch) { virEventRemoveHandle(mon->watch); + mon->watch = 0; + } VIR_FORCE_CLOSE(mon->fd); + fprintf(stderr, "Closing monitr\n"); } /* In case another thread is waiting for its monitor command to be -- 1.7.11.4
On 09/28/2012 08:58 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
After calling qemuMonitorClose(), it is still possible for the QEMU monitor I/O event callback to get invoked. This will trigger an error message because mon->fd has been set to -1 at this point. Silently ignore the case where mon->fd is -1, likewise for mon->watch being zero.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_monitor.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
@@ -830,9 +838,12 @@ void qemuMonitorClose(qemuMonitorPtr mon) "mon=%p refs=%d", mon, mon->object.refs);
if (mon->fd >= 0) { - if (mon->watch) + if (mon->watch) { virEventRemoveHandle(mon->watch); + mon->watch = 0; + } VIR_FORCE_CLOSE(mon->fd); + fprintf(stderr, "Closing monitr\n");
Oops. ACK with the debugging cruft elided. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
From: "Daniel P. Berrange" <berrange@redhat.com> Start a QEMU process using $QEMU -S -no-user-config -nodefaults \ -nographic -M none -qmp unix:/some/path,server,nowait and talk QMP over stdio to discover what capabilities the binary supports. This works for QEMU 1.2.0 or later and for older QEMU automatically fallback to the old approach of parsing -help and related command line args. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_capabilities.c | 394 +++++++++++++++++++++++++++++++++++++++---- 1 file changed, 358 insertions(+), 36 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index f0442ee..682d3e6 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -35,6 +35,7 @@ #include "command.h" #include "bitmap.h" #include "virnodesuspend.h" +#include "qemu_monitor.h" #include <sys/stat.h> #include <unistd.h> @@ -189,6 +190,8 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST, struct _qemuCaps { virObject object; + bool usedQMP; + char *binary; time_t mtime; @@ -1286,6 +1289,7 @@ static struct qemuCapsStringFlags qemuCapsObjectPropsVirtioNet[] = { }; static struct qemuCapsStringFlags qemuCapsObjectPropsPciAssign[] = { + { "rombar", QEMU_CAPS_PCI_ROMBAR }, { "configfd", QEMU_CAPS_PCI_CONFIGFD }, { "bootindex", QEMU_CAPS_PCI_BOOTINDEX }, }; @@ -1866,6 +1870,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); @@ -1898,11 +1906,117 @@ qemuCapsProbeQMPEvents(qemuCapsPtr caps, } +static int +qemuCapsProbeQMPObjects(qemuCapsPtr caps, + qemuMonitorPtr mon) +{ + int nvalues; + char **values; + size_t i; + + if ((nvalues = qemuMonitorGetObjectTypes(mon, &values)) < 0) + return -1; + qemuCapsProcessStringFlags(caps, + ARRAY_CARDINALITY(qemuCapsObjectTypes), + qemuCapsObjectTypes, + nvalues, values); + qemuCapsFreeStringList(nvalues, values); + + for (i = 0 ; i < ARRAY_CARDINALITY(qemuCapsObjectProps); i++) { + const char *type = qemuCapsObjectProps[i].type; + if ((nvalues = qemuMonitorGetObjectProps(mon, + type, + &values)) < 0) + return -1; + qemuCapsProcessStringFlags(caps, + qemuCapsObjectProps[i].nprops, + qemuCapsObjectProps[i].props, + nvalues, values); + qemuCapsFreeStringList(nvalues, values); + } + + /* Prefer -chardev spicevmc (detected earlier) over -device spicevmc */ + if (qemuCapsGet(caps, QEMU_CAPS_CHARDEV_SPICEVMC)) + qemuCapsClear(caps, QEMU_CAPS_DEVICE_SPICEVMC); + + return 0; +} + + +static int +qemuCapsProbeQMPMachineTypes(qemuCapsPtr caps, + qemuMonitorPtr mon) +{ + qemuMonitorMachineInfoPtr *machines = NULL; + int nmachines = 0; + int ret = -1; + size_t i; + + if ((nmachines = qemuMonitorGetMachines(mon, &machines)) < 0) + goto cleanup; + + if (VIR_ALLOC_N(caps->machineTypes, nmachines) < 0) { + virReportOOMError(); + goto cleanup; + } + if (VIR_ALLOC_N(caps->machineAliases, nmachines) < 0) { + virReportOOMError(); + goto cleanup; + } + + for (i = 0 ; i < nmachines ; i++) { + if (machines[i]->alias) { + if (!(caps->machineAliases[i] = strdup(machines[i]->name))) { + virReportOOMError(); + goto cleanup; + } + if (!(caps->machineTypes[i] = strdup(machines[i]->alias))) { + virReportOOMError(); + goto cleanup; + } + } else { + if (!(caps->machineTypes[i] = strdup(machines[i]->name))) { + virReportOOMError(); + goto cleanup; + } + } + } + + ret = 0; + +cleanup: + for (i = 0 ; i < nmachines ; i++) + qemuMonitorMachineInfoFree(machines[i]); + VIR_FREE(machines); + return ret; +} + + +static int +qemuCapsProbeQMPCPUDefinitions(qemuCapsPtr caps, + qemuMonitorPtr mon) +{ + int ncpuDefinitions; + char **cpuDefinitions; + + if ((ncpuDefinitions = qemuMonitorGetCPUDefinitions(mon, &cpuDefinitions)) < 0) + return -1; + + caps->ncpuDefinitions = ncpuDefinitions; + caps->cpuDefinitions = cpuDefinitions; + + return 0; +} + + int qemuCapsProbeQMP(qemuCapsPtr caps, qemuMonitorPtr mon) { VIR_DEBUG("caps=%p mon=%p", caps, mon); + if (caps->usedQMP) + return 0; + if (qemuCapsProbeQMPCommands(caps, mon) < 0) return -1; @@ -1915,20 +2029,19 @@ int qemuCapsProbeQMP(qemuCapsPtr caps, #define QEMU_SYSTEM_PREFIX "qemu-system-" -qemuCapsPtr qemuCapsNewForBinary(const char *binary) +static int +qemuCapsInitHelp(qemuCapsPtr caps) { - qemuCapsPtr caps = qemuCapsNew(); - const char *tmp; - struct utsname ut; + virCommandPtr cmd = NULL; unsigned int is_kvm; char *help = NULL; - virCommandPtr cmd = NULL; - struct stat sb; + int ret = -1; + const char *tmp; + struct utsname ut; - if (!(caps->binary = strdup(binary))) - goto no_memory; + VIR_DEBUG("caps=%p", caps); - tmp = strstr(binary, QEMU_SYSTEM_PREFIX); + tmp = strstr(caps->binary, QEMU_SYSTEM_PREFIX); if (tmp) { tmp += strlen(QEMU_SYSTEM_PREFIX); @@ -1939,39 +2052,25 @@ qemuCapsPtr qemuCapsNewForBinary(const char *binary) uname_normalize(&ut); tmp = ut.machine; } - if (!(caps->arch = strdup(tmp))) - goto no_memory; - - /* We would also want to check faccessat if we cared about ACLs, - * but we don't. */ - if (stat(binary, &sb) < 0) { - virReportSystemError(errno, _("Cannot check QEMU binary %s"), - binary); - goto error; - } - caps->mtime = sb.st_mtime; - - /* Make sure the binary we are about to try exec'ing exists. - * Technically we could catch the exec() failure, but that's - * in a sub-process so it's hard to feed back a useful error. - */ - if (!virFileIsExecutable(binary)) { - goto error; + if (!(caps->arch = strdup(tmp))) { + virReportOOMError(); + goto cleanup; } - cmd = qemuCapsProbeCommand(binary, NULL); + cmd = qemuCapsProbeCommand(caps->binary, NULL); virCommandAddArgList(cmd, "-help", NULL); virCommandSetOutputBuffer(cmd, &help); if (virCommandRun(cmd, NULL) < 0) - goto error; + goto cleanup; - if (qemuCapsParseHelpStr(binary, help, caps, + if (qemuCapsParseHelpStr(caps->binary, + help, caps, &caps->version, &is_kvm, &caps->kvmVersion, false) < 0) - goto error; + goto cleanup; /* Currently only x86_64 and i686 support PCI-multibus. */ if (STREQLEN(caps->arch, "x86_64", 6) || @@ -1988,18 +2087,241 @@ 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); VIR_FREE(help); + return ret; +} + + +static void qemuCapsMonitorNotify(qemuMonitorPtr mon ATTRIBUTE_UNUSED, + virDomainObjPtr vm ATTRIBUTE_UNUSED) +{ +} + +static qemuMonitorCallbacks callbacks = { + .eofNotify = qemuCapsMonitorNotify, + .errorNotify = qemuCapsMonitorNotify, +}; + + +/* Capabilities that we assume are always enabled + * for QEMU >= 1.2.0 + */ +static void +qemuCapsInitQMPBasic(qemuCapsPtr caps) +{ + qemuCapsSet(caps, QEMU_CAPS_VNC_COLON); + qemuCapsSet(caps, QEMU_CAPS_NO_REBOOT); + qemuCapsSet(caps, QEMU_CAPS_DRIVE); + qemuCapsSet(caps, QEMU_CAPS_NAME); + qemuCapsSet(caps, QEMU_CAPS_UUID); + qemuCapsSet(caps, QEMU_CAPS_VNET_HDR); + qemuCapsSet(caps, QEMU_CAPS_MIGRATE_QEMU_TCP); + qemuCapsSet(caps, QEMU_CAPS_MIGRATE_QEMU_EXEC); + qemuCapsSet(caps, QEMU_CAPS_DRIVE_CACHE_V2); + qemuCapsSet(caps, QEMU_CAPS_DRIVE_FORMAT); + qemuCapsSet(caps, QEMU_CAPS_VGA); + qemuCapsSet(caps, QEMU_CAPS_0_10); + qemuCapsSet(caps, QEMU_CAPS_MEM_PATH); + qemuCapsSet(caps, QEMU_CAPS_DRIVE_SERIAL); + qemuCapsSet(caps, QEMU_CAPS_MIGRATE_QEMU_UNIX); + qemuCapsSet(caps, QEMU_CAPS_CHARDEV); + qemuCapsSet(caps, QEMU_CAPS_ENABLE_KVM); + qemuCapsSet(caps, QEMU_CAPS_MONITOR_JSON); + qemuCapsSet(caps, QEMU_CAPS_BALLOON); + qemuCapsSet(caps, QEMU_CAPS_DEVICE); + qemuCapsSet(caps, QEMU_CAPS_SDL); + qemuCapsSet(caps, QEMU_CAPS_SMP_TOPOLOGY); + qemuCapsSet(caps, QEMU_CAPS_NETDEV); + qemuCapsSet(caps, QEMU_CAPS_RTC); + qemuCapsSet(caps, QEMU_CAPS_VHOST_NET); + qemuCapsSet(caps, QEMU_CAPS_NO_HPET); + qemuCapsSet(caps, QEMU_CAPS_NODEFCONFIG); + qemuCapsSet(caps, QEMU_CAPS_BOOT_MENU); + qemuCapsSet(caps, QEMU_CAPS_FSDEV); + qemuCapsSet(caps, QEMU_CAPS_NESTING); + qemuCapsSet(caps, QEMU_CAPS_NAME_PROCESS); + qemuCapsSet(caps, QEMU_CAPS_DRIVE_READONLY); + qemuCapsSet(caps, QEMU_CAPS_SMBIOS_TYPE); + qemuCapsSet(caps, QEMU_CAPS_VGA_NONE); + qemuCapsSet(caps, QEMU_CAPS_MIGRATE_QEMU_FD); + qemuCapsSet(caps, QEMU_CAPS_DRIVE_AIO); + qemuCapsSet(caps, QEMU_CAPS_CHARDEV_SPICEVMC); + qemuCapsSet(caps, QEMU_CAPS_DEVICE_QXL_VGA); + qemuCapsSet(caps, QEMU_CAPS_DRIVE_CACHE_DIRECTSYNC); + qemuCapsSet(caps, QEMU_CAPS_NO_SHUTDOWN); + qemuCapsSet(caps, QEMU_CAPS_DRIVE_CACHE_UNSAFE); + qemuCapsSet(caps, QEMU_CAPS_NO_ACPI); + qemuCapsSet(caps, QEMU_CAPS_FSDEV_READONLY); + qemuCapsSet(caps, QEMU_CAPS_VIRTIO_BLK_SG_IO); + qemuCapsSet(caps, QEMU_CAPS_DRIVE_COPY_ON_READ); + qemuCapsSet(caps, QEMU_CAPS_CPU_HOST); + qemuCapsSet(caps, QEMU_CAPS_FSDEV_WRITEOUT); + qemuCapsSet(caps, QEMU_CAPS_DRIVE_IOTUNE); + qemuCapsSet(caps, QEMU_CAPS_WAKEUP); + qemuCapsSet(caps, QEMU_CAPS_NO_USER_CONFIG); + qemuCapsSet(caps, QEMU_CAPS_NETDEV_BRIDGE); +} + + +static int +qemuCapsInitQMP(qemuCapsPtr caps) +{ + int ret = -1; + virCommandPtr cmd = NULL; + qemuMonitorPtr mon = NULL; + int major, minor, micro; + char *package; + int status = 0; + virDomainChrSourceDef config; + + memset(&config, 0, sizeof(config)); + config.type = VIR_DOMAIN_CHR_TYPE_UNIX; + config.data.nix.path = (char*)"/tmp/qemu.sock"; + config.data.nix.listen = false; + + VIR_DEBUG("Try to get caps via QMP caps=%p", caps); + + cmd = virCommandNewArgList(caps->binary, + "-S", + "-no-user-config", + "-nodefaults", + "-nographic", + "-M", "none", + "-qmp", "unix:/tmp/qemu.sock,server,nowait", + "-daemonize", + NULL); + virCommandAddEnvPassCommon(cmd); + virCommandClearCaps(cmd); + + if (virCommandRun(cmd, &status) < 0) + goto cleanup; + + if (status != 0) { + ret = 0; + VIR_DEBUG("QEMU %s exited with status %d", caps->binary, status); + goto cleanup; + } + + if (!(mon = qemuMonitorOpen(NULL, &config, true, &callbacks))) + goto cleanup; + + qemuMonitorLock(mon); + + if (qemuMonitorSetCapabilities(mon) < 0) { + virErrorPtr err = virGetLastError(); + VIR_DEBUG("Failed to set monitor capabilities %s", + err ? err->message : "<unknown problem>"); + ret = 0; + goto cleanup; + } + + if (qemuMonitorGetVersion(mon, + &major, &minor, µ, + &package) < 0) { + virErrorPtr err = virGetLastError(); + VIR_DEBUG("Failed to query monitor version %s", + err ? err->message : "<unknown problem>"); + ret = 0; + goto cleanup; + } + + VIR_DEBUG("Got version %d.%d.%d (%s)", + major, minor, micro, NULLSTR(package)); + + if (!(major >= 1 && minor >= 1 && micro >= 90)) { + VIR_DEBUG("Not new enough for QMP capabilities detection"); + ret = 0; + goto cleanup; + } + + caps->usedQMP = true; + + qemuCapsInitQMPBasic(caps); + + if (!(caps->arch = qemuMonitorGetTargetArch(mon))) + goto cleanup; + + /* Currently only x86_64 and i686 support PCI-multibus. */ + if (STREQLEN(caps->arch, "x86_64", 6) || + STREQLEN(caps->arch, "i686", 4)) { + qemuCapsSet(caps, QEMU_CAPS_PCI_MULTIBUS); + } + + /* S390 and probably other archs do not support no-acpi - + maybe the qemu option parsing should be re-thought. */ + if (STRPREFIX(caps->arch, "s390")) + qemuCapsClear(caps, QEMU_CAPS_NO_ACPI); + + if (qemuCapsProbeQMPCommands(caps, mon) < 0) + goto cleanup; + if (qemuCapsProbeQMPEvents(caps, mon) < 0) + goto cleanup; + if (qemuCapsProbeQMPObjects(caps, mon) < 0) + goto cleanup; + if (qemuCapsProbeQMPMachineTypes(caps, mon) < 0) + goto cleanup; + if (qemuCapsProbeQMPCPUDefinitions(caps, mon) < 0) + goto cleanup; + + ret = 0; + +cleanup: + if (mon) + qemuMonitorUnlock(mon); + qemuMonitorClose(mon); + virCommandAbort(cmd); virCommandFree(cmd); + return ret; +} + + +qemuCapsPtr qemuCapsNewForBinary(const char *binary) +{ + qemuCapsPtr caps = qemuCapsNew(); + struct stat sb; + int rv; + + if (!(caps->binary = strdup(binary))) + goto no_memory; + + /* We would also want to check faccessat if we cared about ACLs, + * but we don't. */ + if (stat(binary, &sb) < 0) { + virReportSystemError(errno, _("Cannot check QEMU binary %s"), + binary); + goto error; + } + caps->mtime = sb.st_mtime; + + /* Make sure the binary we are about to try exec'ing exists. + * Technically we could catch the exec() failure, but that's + * in a sub-process so it's hard to feed back a useful error. + */ + if (!virFileIsExecutable(binary)) { + virReportSystemError(errno, _("QEMU binary %s is not executable"), + binary); + goto error; + } + + if ((rv = qemuCapsInitQMP(caps)) < 0) + goto error; + + if (!caps->usedQMP && + qemuCapsInitHelp(caps) < 0) + goto error; + return caps; no_memory: @@ -2007,7 +2329,7 @@ no_memory: error: virObjectUnref(caps); caps = NULL; - goto cleanup; + return NULL; } -- 1.7.11.4
On 09/28/2012 08:58 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Start a QEMU process using
$QEMU -S -no-user-config -nodefaults \ -nographic -M none -qmp unix:/some/path,server,nowait
and talk QMP over stdio to discover what capabilities the binary supports. This works for QEMU 1.2.0 or later and for older QEMU automatically fallback to the old approach of parsing -help and related command line args.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_capabilities.c | 394 +++++++++++++++++++++++++++++++++++++++---- 1 file changed, 358 insertions(+), 36 deletions(-)
+ +/* Capabilities that we assume are always enabled + * for QEMU >= 1.2.0 + */ +static void +qemuCapsInitQMPBasic(qemuCapsPtr caps) +{ + qemuCapsSet(caps, QEMU_CAPS_VNC_COLON); + qemuCapsSet(caps, QEMU_CAPS_NO_REBOOT);
I think qemuCapsSetList() would be nicer than setting one bit at a time, but that doesn't change semantics.
+ +static int +qemuCapsInitQMP(qemuCapsPtr caps) +{ + int ret = -1; + virCommandPtr cmd = NULL; + qemuMonitorPtr mon = NULL; + int major, minor, micro; + char *package; + int status = 0; + virDomainChrSourceDef config; + + memset(&config, 0, sizeof(config)); + config.type = VIR_DOMAIN_CHR_TYPE_UNIX; + config.data.nix.path = (char*)"/tmp/qemu.sock";
This does not seem like a safe file name (it is easily guessable). Should we instead be generating something under /var/run?
+ config.data.nix.listen = false; + + VIR_DEBUG("Try to get caps via QMP caps=%p", caps); + + cmd = virCommandNewArgList(caps->binary, + "-S", + "-no-user-config", + "-nodefaults", + "-nographic", + "-M", "none", + "-qmp", "unix:/tmp/qemu.sock,server,nowait",
And here, we should match whatever filename we actually use. Other than that, this looks nice. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Fri, Sep 28, 2012 at 09:20:01AM -0600, Eric Blake wrote:
On 09/28/2012 08:58 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Start a QEMU process using
$QEMU -S -no-user-config -nodefaults \ -nographic -M none -qmp unix:/some/path,server,nowait
and talk QMP over stdio to discover what capabilities the binary supports. This works for QEMU 1.2.0 or later and for older QEMU automatically fallback to the old approach of parsing -help and related command line args.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_capabilities.c | 394 +++++++++++++++++++++++++++++++++++++++---- 1 file changed, 358 insertions(+), 36 deletions(-)
+ +/* Capabilities that we assume are always enabled + * for QEMU >= 1.2.0 + */ +static void +qemuCapsInitQMPBasic(qemuCapsPtr caps) +{ + qemuCapsSet(caps, QEMU_CAPS_VNC_COLON); + qemuCapsSet(caps, QEMU_CAPS_NO_REBOOT);
I think qemuCapsSetList() would be nicer than setting one bit at a time, but that doesn't change semantics.
+ +static int +qemuCapsInitQMP(qemuCapsPtr caps) +{ + int ret = -1; + virCommandPtr cmd = NULL; + qemuMonitorPtr mon = NULL; + int major, minor, micro; + char *package; + int status = 0; + virDomainChrSourceDef config; + + memset(&config, 0, sizeof(config)); + config.type = VIR_DOMAIN_CHR_TYPE_UNIX; + config.data.nix.path = (char*)"/tmp/qemu.sock";
This does not seem like a safe file name (it is easily guessable). Should we instead be generating something under /var/run?
Of course it isn't :-( I blame the end of week exhaustion. Locally I have an addition to make it use driver->libDir, as we do for normal monitor sockets for actual VMs. 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 :|
From: "Daniel P. Berrange" <berrange@redhat.com> Start a QEMU process using $QEMU -S -no-user-config -nodefaults \ -nographic -M none -qmp unix:/some/path,server,nowait and talk QMP over stdio to discover what capabilities the binary supports. This works for QEMU 1.2.0 or later and for older QEMU automatically fallback to the old approach of parsing -help and related command line args. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_capabilities.c | 419 +++++++++++++++++++++++++++++++++++++++---- src/qemu/qemu_capabilities.h | 5 +- src/qemu/qemu_driver.c | 2 +- 3 files changed, 385 insertions(+), 41 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index f0442ee..c977e25 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -35,6 +35,7 @@ #include "command.h" #include "bitmap.h" #include "virnodesuspend.h" +#include "qemu_monitor.h" #include <sys/stat.h> #include <unistd.h> @@ -189,6 +190,8 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST, struct _qemuCaps { virObject object; + bool usedQMP; + char *binary; time_t mtime; @@ -210,6 +213,7 @@ struct _qemuCaps { struct _qemuCapsCache { virMutex lock; virHashTablePtr binaries; + char *libDir; }; @@ -1286,6 +1290,7 @@ static struct qemuCapsStringFlags qemuCapsObjectPropsVirtioNet[] = { }; static struct qemuCapsStringFlags qemuCapsObjectPropsPciAssign[] = { + { "rombar", QEMU_CAPS_PCI_ROMBAR }, { "configfd", QEMU_CAPS_PCI_CONFIGFD }, { "bootindex", QEMU_CAPS_PCI_BOOTINDEX }, }; @@ -1866,6 +1871,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); @@ -1898,11 +1907,117 @@ qemuCapsProbeQMPEvents(qemuCapsPtr caps, } +static int +qemuCapsProbeQMPObjects(qemuCapsPtr caps, + qemuMonitorPtr mon) +{ + int nvalues; + char **values; + size_t i; + + if ((nvalues = qemuMonitorGetObjectTypes(mon, &values)) < 0) + return -1; + qemuCapsProcessStringFlags(caps, + ARRAY_CARDINALITY(qemuCapsObjectTypes), + qemuCapsObjectTypes, + nvalues, values); + qemuCapsFreeStringList(nvalues, values); + + for (i = 0 ; i < ARRAY_CARDINALITY(qemuCapsObjectProps); i++) { + const char *type = qemuCapsObjectProps[i].type; + if ((nvalues = qemuMonitorGetObjectProps(mon, + type, + &values)) < 0) + return -1; + qemuCapsProcessStringFlags(caps, + qemuCapsObjectProps[i].nprops, + qemuCapsObjectProps[i].props, + nvalues, values); + qemuCapsFreeStringList(nvalues, values); + } + + /* Prefer -chardev spicevmc (detected earlier) over -device spicevmc */ + if (qemuCapsGet(caps, QEMU_CAPS_CHARDEV_SPICEVMC)) + qemuCapsClear(caps, QEMU_CAPS_DEVICE_SPICEVMC); + + return 0; +} + + +static int +qemuCapsProbeQMPMachineTypes(qemuCapsPtr caps, + qemuMonitorPtr mon) +{ + qemuMonitorMachineInfoPtr *machines = NULL; + int nmachines = 0; + int ret = -1; + size_t i; + + if ((nmachines = qemuMonitorGetMachines(mon, &machines)) < 0) + goto cleanup; + + if (VIR_ALLOC_N(caps->machineTypes, nmachines) < 0) { + virReportOOMError(); + goto cleanup; + } + if (VIR_ALLOC_N(caps->machineAliases, nmachines) < 0) { + virReportOOMError(); + goto cleanup; + } + + for (i = 0 ; i < nmachines ; i++) { + if (machines[i]->alias) { + if (!(caps->machineAliases[i] = strdup(machines[i]->name))) { + virReportOOMError(); + goto cleanup; + } + if (!(caps->machineTypes[i] = strdup(machines[i]->alias))) { + virReportOOMError(); + goto cleanup; + } + } else { + if (!(caps->machineTypes[i] = strdup(machines[i]->name))) { + virReportOOMError(); + goto cleanup; + } + } + } + + ret = 0; + +cleanup: + for (i = 0 ; i < nmachines ; i++) + qemuMonitorMachineInfoFree(machines[i]); + VIR_FREE(machines); + return ret; +} + + +static int +qemuCapsProbeQMPCPUDefinitions(qemuCapsPtr caps, + qemuMonitorPtr mon) +{ + int ncpuDefinitions; + char **cpuDefinitions; + + if ((ncpuDefinitions = qemuMonitorGetCPUDefinitions(mon, &cpuDefinitions)) < 0) + return -1; + + caps->ncpuDefinitions = ncpuDefinitions; + caps->cpuDefinitions = cpuDefinitions; + + return 0; +} + + int qemuCapsProbeQMP(qemuCapsPtr caps, qemuMonitorPtr mon) { VIR_DEBUG("caps=%p mon=%p", caps, mon); + if (caps->usedQMP) + return 0; + if (qemuCapsProbeQMPCommands(caps, mon) < 0) return -1; @@ -1915,20 +2030,19 @@ int qemuCapsProbeQMP(qemuCapsPtr caps, #define QEMU_SYSTEM_PREFIX "qemu-system-" -qemuCapsPtr qemuCapsNewForBinary(const char *binary) +static int +qemuCapsInitHelp(qemuCapsPtr caps) { - qemuCapsPtr caps = qemuCapsNew(); - const char *tmp; - struct utsname ut; + virCommandPtr cmd = NULL; unsigned int is_kvm; char *help = NULL; - virCommandPtr cmd = NULL; - struct stat sb; + int ret = -1; + const char *tmp; + struct utsname ut; - if (!(caps->binary = strdup(binary))) - goto no_memory; + VIR_DEBUG("caps=%p", caps); - tmp = strstr(binary, QEMU_SYSTEM_PREFIX); + tmp = strstr(caps->binary, QEMU_SYSTEM_PREFIX); if (tmp) { tmp += strlen(QEMU_SYSTEM_PREFIX); @@ -1939,39 +2053,25 @@ qemuCapsPtr qemuCapsNewForBinary(const char *binary) uname_normalize(&ut); tmp = ut.machine; } - if (!(caps->arch = strdup(tmp))) - goto no_memory; - - /* We would also want to check faccessat if we cared about ACLs, - * but we don't. */ - if (stat(binary, &sb) < 0) { - virReportSystemError(errno, _("Cannot check QEMU binary %s"), - binary); - goto error; - } - caps->mtime = sb.st_mtime; - - /* Make sure the binary we are about to try exec'ing exists. - * Technically we could catch the exec() failure, but that's - * in a sub-process so it's hard to feed back a useful error. - */ - if (!virFileIsExecutable(binary)) { - goto error; + if (!(caps->arch = strdup(tmp))) { + virReportOOMError(); + goto cleanup; } - cmd = qemuCapsProbeCommand(binary, NULL); + cmd = qemuCapsProbeCommand(caps->binary, NULL); virCommandAddArgList(cmd, "-help", NULL); virCommandSetOutputBuffer(cmd, &help); if (virCommandRun(cmd, NULL) < 0) - goto error; + goto cleanup; - if (qemuCapsParseHelpStr(binary, help, caps, + if (qemuCapsParseHelpStr(caps->binary, + help, caps, &caps->version, &is_kvm, &caps->kvmVersion, false) < 0) - goto error; + goto cleanup; /* Currently only x86_64 and i686 support PCI-multibus. */ if (STREQLEN(caps->arch, "x86_64", 6) || @@ -1988,18 +2088,256 @@ 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); VIR_FREE(help); + return ret; +} + + +static void qemuCapsMonitorNotify(qemuMonitorPtr mon ATTRIBUTE_UNUSED, + virDomainObjPtr vm ATTRIBUTE_UNUSED) +{ +} + +static qemuMonitorCallbacks callbacks = { + .eofNotify = qemuCapsMonitorNotify, + .errorNotify = qemuCapsMonitorNotify, +}; + + +/* Capabilities that we assume are always enabled + * for QEMU >= 1.2.0 + */ +static void +qemuCapsInitQMPBasic(qemuCapsPtr caps) +{ + qemuCapsSet(caps, QEMU_CAPS_VNC_COLON); + qemuCapsSet(caps, QEMU_CAPS_NO_REBOOT); + qemuCapsSet(caps, QEMU_CAPS_DRIVE); + qemuCapsSet(caps, QEMU_CAPS_NAME); + qemuCapsSet(caps, QEMU_CAPS_UUID); + qemuCapsSet(caps, QEMU_CAPS_VNET_HDR); + qemuCapsSet(caps, QEMU_CAPS_MIGRATE_QEMU_TCP); + qemuCapsSet(caps, QEMU_CAPS_MIGRATE_QEMU_EXEC); + qemuCapsSet(caps, QEMU_CAPS_DRIVE_CACHE_V2); + qemuCapsSet(caps, QEMU_CAPS_DRIVE_FORMAT); + qemuCapsSet(caps, QEMU_CAPS_VGA); + qemuCapsSet(caps, QEMU_CAPS_0_10); + qemuCapsSet(caps, QEMU_CAPS_MEM_PATH); + qemuCapsSet(caps, QEMU_CAPS_DRIVE_SERIAL); + qemuCapsSet(caps, QEMU_CAPS_MIGRATE_QEMU_UNIX); + qemuCapsSet(caps, QEMU_CAPS_CHARDEV); + qemuCapsSet(caps, QEMU_CAPS_ENABLE_KVM); + qemuCapsSet(caps, QEMU_CAPS_MONITOR_JSON); + qemuCapsSet(caps, QEMU_CAPS_BALLOON); + qemuCapsSet(caps, QEMU_CAPS_DEVICE); + qemuCapsSet(caps, QEMU_CAPS_SDL); + qemuCapsSet(caps, QEMU_CAPS_SMP_TOPOLOGY); + qemuCapsSet(caps, QEMU_CAPS_NETDEV); + qemuCapsSet(caps, QEMU_CAPS_RTC); + qemuCapsSet(caps, QEMU_CAPS_VHOST_NET); + qemuCapsSet(caps, QEMU_CAPS_NO_HPET); + qemuCapsSet(caps, QEMU_CAPS_NODEFCONFIG); + qemuCapsSet(caps, QEMU_CAPS_BOOT_MENU); + qemuCapsSet(caps, QEMU_CAPS_FSDEV); + qemuCapsSet(caps, QEMU_CAPS_NESTING); + qemuCapsSet(caps, QEMU_CAPS_NAME_PROCESS); + qemuCapsSet(caps, QEMU_CAPS_DRIVE_READONLY); + qemuCapsSet(caps, QEMU_CAPS_SMBIOS_TYPE); + qemuCapsSet(caps, QEMU_CAPS_VGA_NONE); + qemuCapsSet(caps, QEMU_CAPS_MIGRATE_QEMU_FD); + qemuCapsSet(caps, QEMU_CAPS_DRIVE_AIO); + qemuCapsSet(caps, QEMU_CAPS_CHARDEV_SPICEVMC); + qemuCapsSet(caps, QEMU_CAPS_DEVICE_QXL_VGA); + qemuCapsSet(caps, QEMU_CAPS_DRIVE_CACHE_DIRECTSYNC); + qemuCapsSet(caps, QEMU_CAPS_NO_SHUTDOWN); + qemuCapsSet(caps, QEMU_CAPS_DRIVE_CACHE_UNSAFE); + qemuCapsSet(caps, QEMU_CAPS_NO_ACPI); + qemuCapsSet(caps, QEMU_CAPS_FSDEV_READONLY); + qemuCapsSet(caps, QEMU_CAPS_VIRTIO_BLK_SG_IO); + qemuCapsSet(caps, QEMU_CAPS_DRIVE_COPY_ON_READ); + qemuCapsSet(caps, QEMU_CAPS_CPU_HOST); + qemuCapsSet(caps, QEMU_CAPS_FSDEV_WRITEOUT); + qemuCapsSet(caps, QEMU_CAPS_DRIVE_IOTUNE); + qemuCapsSet(caps, QEMU_CAPS_WAKEUP); + qemuCapsSet(caps, QEMU_CAPS_NO_USER_CONFIG); + qemuCapsSet(caps, QEMU_CAPS_NETDEV_BRIDGE); +} + + +static int +qemuCapsInitQMP(qemuCapsPtr caps, + const char *libDir) +{ + int ret = -1; + virCommandPtr cmd = NULL; + qemuMonitorPtr mon = NULL; + int major, minor, micro; + char *package; + int status = 0; + virDomainChrSourceDef config; + char *monarg = NULL; + char *monpath = NULL; + + if (virAsprintf(&monpath, "%s/%s", libDir, "capabilities.monitor.sock") < 0) { + virReportOOMError(); + goto cleanup; + } + if (virAsprintf(&monarg, "unix:%s,server,nowait", monpath) < 0) { + virReportOOMError(); + goto cleanup; + } + + memset(&config, 0, sizeof(config)); + config.type = VIR_DOMAIN_CHR_TYPE_UNIX; + config.data.nix.path = monpath; + config.data.nix.listen = false; + + VIR_DEBUG("Try to get caps via QMP caps=%p", caps); + + cmd = virCommandNewArgList(caps->binary, + "-S", + "-no-user-config", + "-nodefaults", + "-nographic", + "-M", "none", + "-qmp", monarg, + "-daemonize", + NULL); + virCommandAddEnvPassCommon(cmd); + virCommandClearCaps(cmd); + + if (virCommandRun(cmd, &status) < 0) + goto cleanup; + + if (status != 0) { + ret = 0; + VIR_DEBUG("QEMU %s exited with status %d", caps->binary, status); + goto cleanup; + } + + if (!(mon = qemuMonitorOpen(NULL, &config, true, &callbacks))) + goto cleanup; + + qemuMonitorLock(mon); + + if (qemuMonitorSetCapabilities(mon) < 0) { + virErrorPtr err = virGetLastError(); + VIR_DEBUG("Failed to set monitor capabilities %s", + err ? err->message : "<unknown problem>"); + ret = 0; + goto cleanup; + } + + if (qemuMonitorGetVersion(mon, + &major, &minor, µ, + &package) < 0) { + virErrorPtr err = virGetLastError(); + VIR_DEBUG("Failed to query monitor version %s", + err ? err->message : "<unknown problem>"); + ret = 0; + goto cleanup; + } + + VIR_DEBUG("Got version %d.%d.%d (%s)", + major, minor, micro, NULLSTR(package)); + + if (!(major >= 1 || (major == 1 && minor >= 1))) { + VIR_DEBUG("Not new enough for QMP capabilities detection"); + ret = 0; + goto cleanup; + } + + caps->usedQMP = true; + + qemuCapsInitQMPBasic(caps); + + if (!(caps->arch = qemuMonitorGetTargetArch(mon))) + goto cleanup; + + /* Currently only x86_64 and i686 support PCI-multibus. */ + if (STREQLEN(caps->arch, "x86_64", 6) || + STREQLEN(caps->arch, "i686", 4)) { + qemuCapsSet(caps, QEMU_CAPS_PCI_MULTIBUS); + } + + /* S390 and probably other archs do not support no-acpi - + maybe the qemu option parsing should be re-thought. */ + if (STRPREFIX(caps->arch, "s390")) + qemuCapsClear(caps, QEMU_CAPS_NO_ACPI); + + if (qemuCapsProbeQMPCommands(caps, mon) < 0) + goto cleanup; + if (qemuCapsProbeQMPEvents(caps, mon) < 0) + goto cleanup; + if (qemuCapsProbeQMPObjects(caps, mon) < 0) + goto cleanup; + if (qemuCapsProbeQMPMachineTypes(caps, mon) < 0) + goto cleanup; + if (qemuCapsProbeQMPCPUDefinitions(caps, mon) < 0) + goto cleanup; + + ret = 0; + +cleanup: + if (mon) + qemuMonitorUnlock(mon); + qemuMonitorClose(mon); + virCommandAbort(cmd); virCommandFree(cmd); + VIR_FREE(monarg); + VIR_FREE(monpath); + return ret; +} + + +qemuCapsPtr qemuCapsNewForBinary(const char *binary, + const char *libDir) +{ + qemuCapsPtr caps = qemuCapsNew(); + struct stat sb; + int rv; + + if (!(caps->binary = strdup(binary))) + goto no_memory; + + /* We would also want to check faccessat if we cared about ACLs, + * but we don't. */ + if (stat(binary, &sb) < 0) { + virReportSystemError(errno, _("Cannot check QEMU binary %s"), + binary); + goto error; + } + caps->mtime = sb.st_mtime; + + /* Make sure the binary we are about to try exec'ing exists. + * Technically we could catch the exec() failure, but that's + * in a sub-process so it's hard to feed back a useful error. + */ + if (!virFileIsExecutable(binary)) { + virReportSystemError(errno, _("QEMU binary %s is not executable"), + binary); + goto error; + } + + if ((rv = qemuCapsInitQMP(caps, libDir)) < 0) + goto error; + + if (!caps->usedQMP && + qemuCapsInitHelp(caps) < 0) + goto error; + return caps; no_memory: @@ -2007,7 +2345,7 @@ no_memory: error: virObjectUnref(caps); caps = NULL; - goto cleanup; + return NULL; } @@ -2033,7 +2371,7 @@ qemuCapsHashDataFree(void *payload, const void *key ATTRIBUTE_UNUSED) qemuCapsCachePtr -qemuCapsCacheNew(void) +qemuCapsCacheNew(const char *libDir) { qemuCapsCachePtr cache; @@ -2051,6 +2389,10 @@ qemuCapsCacheNew(void) if (!(cache->binaries = virHashCreate(10, qemuCapsHashDataFree))) goto error; + if (!(cache->libDir = strdup(libDir))) { + virReportOOMError(); + goto error; + } return cache; @@ -2076,7 +2418,7 @@ qemuCapsCacheLookup(qemuCapsCachePtr cache, const char *binary) if (!ret) { VIR_DEBUG("Creating capabilities for %s", binary); - ret = qemuCapsNewForBinary(binary); + ret = qemuCapsNewForBinary(binary, cache->libDir); if (ret) { VIR_DEBUG("Caching capabilities %p for %s", ret, binary); @@ -2114,6 +2456,7 @@ qemuCapsCacheFree(qemuCapsCachePtr cache) if (!cache) return; + VIR_FREE(cache->libDir); virHashFree(cache->binaries); virMutexDestroy(&cache->lock); VIR_FREE(cache); diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index c8207c4..89f351c 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -162,7 +162,8 @@ typedef qemuCapsCache *qemuCapsCachePtr; qemuCapsPtr qemuCapsNew(void); qemuCapsPtr qemuCapsNewCopy(qemuCapsPtr caps); -qemuCapsPtr qemuCapsNewForBinary(const char *binary); +qemuCapsPtr qemuCapsNewForBinary(const char *binary, + const char *libDir); int qemuCapsProbeQMP(qemuCapsPtr caps, qemuMonitorPtr mon); @@ -200,7 +201,7 @@ int qemuCapsGetMachineTypesCaps(qemuCapsPtr caps, bool qemuCapsIsValid(qemuCapsPtr caps); -qemuCapsCachePtr qemuCapsCacheNew(void); +qemuCapsCachePtr qemuCapsCacheNew(const char *libDir); qemuCapsPtr qemuCapsCacheLookup(qemuCapsCachePtr cache, const char *binary); qemuCapsPtr qemuCapsCacheLookupCopy(qemuCapsCachePtr cache, const char *binary); void qemuCapsCacheFree(qemuCapsCachePtr cache); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 95a30e6..b0a0bb5 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -767,7 +767,7 @@ qemudStartup(int privileged) { if (qemuSecurityInit(qemu_driver) < 0) goto error; - if ((qemu_driver->capsCache = qemuCapsCacheNew()) == NULL) + if ((qemu_driver->capsCache = qemuCapsCacheNew(qemu_driver->libDir)) == NULL) goto error; if ((qemu_driver->caps = qemuCreateCapabilities(qemu_driver)) == NULL) -- 1.7.11.4
On 09/28/2012 09:43 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Start a QEMU process using
$QEMU -S -no-user-config -nodefaults \ -nographic -M none -qmp unix:/some/path,server,nowait
and talk QMP over stdio to discover what capabilities the binary supports. This works for QEMU 1.2.0 or later and for older QEMU automatically fallback to the old approach of parsing -help and related command line args.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_capabilities.c | 419 +++++++++++++++++++++++++++++++++++++++---- src/qemu/qemu_capabilities.h | 5 +- src/qemu/qemu_driver.c | 2 +- 3 files changed, 385 insertions(+), 41 deletions(-)
+ char *monarg = NULL; + char *monpath = NULL; + + if (virAsprintf(&monpath, "%s/%s", libDir, "capabilities.monitor.sock") < 0) { + virReportOOMError(); + goto cleanup; + } + if (virAsprintf(&monarg, "unix:%s,server,nowait", monpath) < 0) { + virReportOOMError(); + goto cleanup; + }
I would have used virCommandAddArgFormat() instead of creating my own temporary string here, but that's minor. ACK; you fixed my concerns from the previous version. Also, be sure you run 'make check'; somewhere between commit de29867 and 7022b09, you broke qemuhelptest (I'm assuming it will get fixed soon, perhaps by this commit, so I'm not chasing which commit did the actual breakage): 16) QEMU Help String Parsing qemu-1.2.0 ... qemu-1.2.0: computed flags do not match: got 0x0000b0f2f1bffefff4cffd62fffddc6e, expected 0x0000b0f2f1bffefff4effd72fffddc6e Missing flag 36 Missing flag 53 FAILED -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
From: "Daniel P. Berrange" <berrange@redhat.com> Some architectures provide the query-cpu-definitions command, but are set to always return a "GenericError" from it :-( Catch this & treat it as if there was an empty list of CPUs returned Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_capabilities.c | 2 +- src/qemu/qemu_monitor.c | 1 - src/qemu/qemu_monitor_json.c | 12 ++++++++++++ 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 682d3e6..2ae6fec 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2240,7 +2240,7 @@ qemuCapsInitQMP(qemuCapsPtr caps) VIR_DEBUG("Got version %d.%d.%d (%s)", major, minor, micro, NULLSTR(package)); - if (!(major >= 1 && minor >= 1 && micro >= 90)) { + if (!(major >= 1 || (major == 1 && minor >= 1))) { VIR_DEBUG("Not new enough for QMP capabilities detection"); ret = 0; goto cleanup; diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 6c3f09e..85b0bc2 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -843,7 +843,6 @@ void qemuMonitorClose(qemuMonitorPtr mon) mon->watch = 0; } VIR_FORCE_CLOSE(mon->fd); - fprintf(stderr, "Closing monitr\n"); } /* In case another thread is waiting for its monitor command to be diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 166c431..bd52ce4 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3957,6 +3957,18 @@ int qemuMonitorJSONGetCPUDefinitions(qemuMonitorPtr mon, ret = qemuMonitorJSONCommand(mon, cmd, &reply); + if (ret == 0) { + /* Urgh, some QEMU architectures have the query-cpu-definitions + * command, but return 'GenericError' with string "Not supported", + * instead of simply omitting the command entirely :-( + */ + if (qemuMonitorJSONHasError(reply, "GenericError")) { + ret = 0; + goto cleanup; + } + ret = qemuMonitorJSONCheckError(cmd, reply); + } + if (ret == 0) ret = qemuMonitorJSONCheckError(cmd, reply); -- 1.7.11.4
On 09/28/2012 08:58 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Some architectures provide the query-cpu-definitions command, but are set to always return a "GenericError" from it :-( Catch this & treat it as if there was an empty list of CPUs returned
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_capabilities.c | 2 +- src/qemu/qemu_monitor.c | 1 - src/qemu/qemu_monitor_json.c | 12 ++++++++++++ 3 files changed, 13 insertions(+), 2 deletions(-)
ACK.
+++ b/src/qemu/qemu_capabilities.c @@ -2240,7 +2240,7 @@ qemuCapsInitQMP(qemuCapsPtr caps) VIR_DEBUG("Got version %d.%d.%d (%s)", major, minor, micro, NULLSTR(package));
- if (!(major >= 1 && minor >= 1 && micro >= 90)) { + if (!(major >= 1 || (major == 1 && minor >= 1))) { VIR_DEBUG("Not new enough for QMP capabilities detection"); ret = 0; goto cleanup;
This needs to be squashed into 3/4.
+++ b/src/qemu/qemu_monitor.c @@ -843,7 +843,6 @@ void qemuMonitorClose(qemuMonitorPtr mon) mon->watch = 0; } VIR_FORCE_CLOSE(mon->fd); - fprintf(stderr, "Closing monitr\n"); }
This needs to be squashed into 2/4. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Fri, Sep 28, 2012 at 03:58:23PM +0100, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Some architectures provide the query-cpu-definitions command, but are set to always return a "GenericError" from it :-( Catch this & treat it as if there was an empty list of CPUs returned
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_capabilities.c | 2 +- src/qemu/qemu_monitor.c | 1 - src/qemu/qemu_monitor_json.c | 12 ++++++++++++ 3 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 682d3e6..2ae6fec 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2240,7 +2240,7 @@ qemuCapsInitQMP(qemuCapsPtr caps) VIR_DEBUG("Got version %d.%d.%d (%s)", major, minor, micro, NULLSTR(package));
- if (!(major >= 1 && minor >= 1 && micro >= 90)) { + if (!(major >= 1 || (major == 1 && minor >= 1))) { VIR_DEBUG("Not new enough for QMP capabilities detection"); ret = 0; goto cleanup;
Opps, this was intended for the previous patch. 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 :|
participants (2)
-
Daniel P. Berrange -
Eric Blake