[libvirt] [PATCH v3 00/20] Add support for detecting QEMU capabilities via QM
This is an update to: https://www.redhat.com/archives/libvir-list/2012-September/msg01051.html Changes in v3: - Already merged patches dropped - Rebased to latest git
From: "Daniel P. Berrange" <berrange@redhat.com> Introduce a qemuCapsCachePtr object to provide a global cache of capabilities for QEMU binaries. The cache auto-populates on first request for capabilities about a binary, and will auto-refresh if the binary has changed since a previous cache was populated Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_capabilities.c | 99 ++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_capabilities.h | 9 ++++ 2 files changed, 108 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 3b08ef8..1163dd8 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -206,6 +206,11 @@ struct _qemuCaps { char **machineAliases; }; +struct _qemuCapsCache { + virMutex lock; + virHashTablePtr binaries; +}; + static virClassPtr qemuCapsClass; static void qemuCapsDispose(void *obj); @@ -1940,6 +1945,10 @@ qemuCapsPtr qemuCapsNewForBinary(const char *binary) tmp = strstr(binary, QEMU_SYSTEM_PREFIX); if (tmp) { tmp += strlen(QEMU_SYSTEM_PREFIX); + + /* For historical compat we uses 'itanium' as arch name */ + if (STREQ(tmp, "ia64")) + tmp = "itanium"; } else { uname_normalize(&ut); tmp = ut.machine; @@ -2048,3 +2057,93 @@ bool qemuCapsIsValid(qemuCapsPtr caps) return sb.st_mtime == caps->mtime; } + + +static void +qemuCapsHashDataFree(void *payload, const void *key ATTRIBUTE_UNUSED) +{ + virObjectUnref(payload); +} + +qemuCapsCachePtr qemuCapsCacheNew(void) +{ + qemuCapsCachePtr cache; + + if (VIR_ALLOC(cache) < 0) { + virReportOOMError(); + return NULL; + } + + if (virMutexInit(&cache->lock) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to initialize mutex")); + VIR_FREE(cache); + return NULL; + } + + if (!(cache->binaries = virHashCreate(10, qemuCapsHashDataFree))) + goto error; + + return cache; + +error: + qemuCapsCacheFree(cache); + return NULL; +} + + +qemuCapsPtr qemuCapsCacheLookup(qemuCapsCachePtr cache, const char *binary) +{ + qemuCapsPtr ret = NULL; + virMutexLock(&cache->lock); + ret = virHashLookup(cache->binaries, binary); + if (ret && + !qemuCapsIsValid(ret)) { + VIR_DEBUG("Cached capabilities %p no longer valid for %s", + ret, binary); + virHashRemoveEntry(cache->binaries, binary); + ret = NULL; + } + if (!ret) { + VIR_DEBUG("Creating capabilities for %s", + binary); + ret = qemuCapsNewForBinary(binary); + if (ret) { + VIR_DEBUG("Caching capabilities %p for %s", + ret, binary); + if (virHashAddEntry(cache->binaries, binary, ret) < 0) { + virObjectUnref(ret); + ret = NULL; + } + } + } + VIR_DEBUG("Returning caps %p for %s", ret, binary); + virObjectRef(ret); + virMutexUnlock(&cache->lock); + return ret; +} + + +qemuCapsPtr qemuCapsCacheLookupCopy(qemuCapsCachePtr cache, const char *binary) +{ + qemuCapsPtr caps = qemuCapsCacheLookup(cache, binary); + qemuCapsPtr ret; + + if (!caps) + return NULL; + + ret = qemuCapsNewCopy(caps); + virObjectUnref(caps); + return ret; +} + + +void qemuCapsCacheFree(qemuCapsCachePtr cache) +{ + if (!cache) + return; + + virHashFree(cache->binaries); + virMutexDestroy(&cache->lock); + VIR_FREE(cache); +} diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 485c297..27ed378 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -154,6 +154,9 @@ enum qemuCapsFlags { typedef struct _qemuCaps qemuCaps; typedef qemuCaps *qemuCapsPtr; +typedef struct _qemuCapsCache qemuCapsCache; +typedef qemuCapsCache *qemuCapsCachePtr; + qemuCapsPtr qemuCapsNew(void); qemuCapsPtr qemuCapsNewCopy(qemuCapsPtr caps); qemuCapsPtr qemuCapsNewForBinary(const char *binary); @@ -183,6 +186,12 @@ const char *qemuCapsGetCanonicalMachine(qemuCapsPtr caps, bool qemuCapsIsValid(qemuCapsPtr caps); + +qemuCapsCachePtr qemuCapsCacheNew(void); +qemuCapsPtr qemuCapsCacheLookup(qemuCapsCachePtr cache, const char *binary); +qemuCapsPtr qemuCapsCacheLookupCopy(qemuCapsCachePtr cache, const char *binary); +void qemuCapsCacheFree(qemuCapsCachePtr cache); + virCapsPtr qemuCapsInit(virCapsPtr old_caps); int qemuCapsProbeMachineTypes(const char *binary, -- 1.7.11.4
On 09/25/2012 11:59 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Introduce a qemuCapsCachePtr object to provide a global cache of capabilities for QEMU binaries. The cache auto-populates on first request for capabilities about a binary, and will auto-refresh if the binary has changed since a previous cache was populated
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_capabilities.c | 99 ++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_capabilities.h | 9 ++++ 2 files changed, 108 insertions(+)
@@ -1940,6 +1945,10 @@ qemuCapsPtr qemuCapsNewForBinary(const char *binary) tmp = strstr(binary, QEMU_SYSTEM_PREFIX); if (tmp) { tmp += strlen(QEMU_SYSTEM_PREFIX); + + /* For historical compat we uses 'itanium' as arch name */ + if (STREQ(tmp, "ia64")) + tmp = "itanium";
This hunk looks like it may have been rebased to the wrong patch, as omitting it makes no difference to the new caching code. ACK with that fixed. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Tue, Sep 25, 2012 at 01:43:30PM -0600, Eric Blake wrote:
On 09/25/2012 11:59 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Introduce a qemuCapsCachePtr object to provide a global cache of capabilities for QEMU binaries. The cache auto-populates on first request for capabilities about a binary, and will auto-refresh if the binary has changed since a previous cache was populated
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_capabilities.c | 99 ++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_capabilities.h | 9 ++++ 2 files changed, 108 insertions(+)
@@ -1940,6 +1945,10 @@ qemuCapsPtr qemuCapsNewForBinary(const char *binary) tmp = strstr(binary, QEMU_SYSTEM_PREFIX); if (tmp) { tmp += strlen(QEMU_SYSTEM_PREFIX); + + /* For historical compat we uses 'itanium' as arch name */ + if (STREQ(tmp, "ia64")) + tmp = "itanium";
This hunk looks like it may have been rebased to the wrong patch, as omitting it makes no difference to the new caching code.
Hmm, yes, it should have been included in a patch that was acked & pushed previously. Will apply it separately as a dedicated fix now
ACK with that fixed.
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 :|
On Tue, Sep 25, 2012 at 12:59 PM, Daniel P. Berrange <berrange@redhat.com> wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Introduce a qemuCapsCachePtr object to provide a global cache of capabilities for QEMU binaries. The cache auto-populates on first request for capabilities about a binary, and will auto-refresh if the binary has changed since a previous cache was populated
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_capabilities.c | 99 ++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_capabilities.h | 9 ++++ 2 files changed, 108 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 3b08ef8..1163dd8 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -206,6 +206,11 @@ struct _qemuCaps { char **machineAliases; };
+struct _qemuCapsCache { + virMutex lock; + virHashTablePtr binaries; +}; +
static virClassPtr qemuCapsClass; static void qemuCapsDispose(void *obj); @@ -1940,6 +1945,10 @@ qemuCapsPtr qemuCapsNewForBinary(const char *binary) tmp = strstr(binary, QEMU_SYSTEM_PREFIX); if (tmp) { tmp += strlen(QEMU_SYSTEM_PREFIX); + + /* For historical compat we uses 'itanium' as arch name */
s/uses/use/
+ if (STREQ(tmp, "ia64")) + tmp = "itanium"; } else { uname_normalize(&ut); tmp = ut.machine; @@ -2048,3 +2057,93 @@ bool qemuCapsIsValid(qemuCapsPtr caps)
return sb.st_mtime == caps->mtime; } + + +static void +qemuCapsHashDataFree(void *payload, const void *key ATTRIBUTE_UNUSED) +{ + virObjectUnref(payload); +} + +qemuCapsCachePtr qemuCapsCacheNew(void)
Return type on its own line if I'm recalling the coding std.
+{ + qemuCapsCachePtr cache; + + if (VIR_ALLOC(cache) < 0) { + virReportOOMError(); + return NULL; + } + + if (virMutexInit(&cache->lock) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to initialize mutex")); + VIR_FREE(cache); + return NULL; + } + + if (!(cache->binaries = virHashCreate(10, qemuCapsHashDataFree))) + goto error; + + return cache; + +error: + qemuCapsCacheFree(cache); + return NULL; +} + + +qemuCapsPtr qemuCapsCacheLookup(qemuCapsCachePtr cache, const char *binary)
Again.
+{ + qemuCapsPtr ret = NULL; + virMutexLock(&cache->lock); + ret = virHashLookup(cache->binaries, binary); + if (ret && + !qemuCapsIsValid(ret)) { + VIR_DEBUG("Cached capabilities %p no longer valid for %s", + ret, binary); + virHashRemoveEntry(cache->binaries, binary); + ret = NULL; + } + if (!ret) { + VIR_DEBUG("Creating capabilities for %s", + binary); + ret = qemuCapsNewForBinary(binary); + if (ret) { + VIR_DEBUG("Caching capabilities %p for %s", + ret, binary); + if (virHashAddEntry(cache->binaries, binary, ret) < 0) { + virObjectUnref(ret); + ret = NULL; + } + } + } + VIR_DEBUG("Returning caps %p for %s", ret, binary); + virObjectRef(ret); + virMutexUnlock(&cache->lock); + return ret; +} + + +qemuCapsPtr qemuCapsCacheLookupCopy(qemuCapsCachePtr cache, const char *binary)
Again.
+{ + qemuCapsPtr caps = qemuCapsCacheLookup(cache, binary); + qemuCapsPtr ret; + + if (!caps) + return NULL; + + ret = qemuCapsNewCopy(caps); + virObjectUnref(caps); + return ret; +} + + +void qemuCapsCacheFree(qemuCapsCachePtr cache)
Again.
+{ + if (!cache) + return; + + virHashFree(cache->binaries); + virMutexDestroy(&cache->lock); + VIR_FREE(cache); +} diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 485c297..27ed378 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -154,6 +154,9 @@ enum qemuCapsFlags { typedef struct _qemuCaps qemuCaps; typedef qemuCaps *qemuCapsPtr;
+typedef struct _qemuCapsCache qemuCapsCache; +typedef qemuCapsCache *qemuCapsCachePtr; + qemuCapsPtr qemuCapsNew(void); qemuCapsPtr qemuCapsNewCopy(qemuCapsPtr caps); qemuCapsPtr qemuCapsNewForBinary(const char *binary); @@ -183,6 +186,12 @@ const char *qemuCapsGetCanonicalMachine(qemuCapsPtr caps,
bool qemuCapsIsValid(qemuCapsPtr caps);
+ +qemuCapsCachePtr qemuCapsCacheNew(void); +qemuCapsPtr qemuCapsCacheLookup(qemuCapsCachePtr cache, const char *binary); +qemuCapsPtr qemuCapsCacheLookupCopy(qemuCapsCachePtr cache, const char *binary); +void qemuCapsCacheFree(qemuCapsCachePtr cache); + virCapsPtr qemuCapsInit(virCapsPtr old_caps);
int qemuCapsProbeMachineTypes(const char *binary, -- 1.7.11.4
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- Doug Goldstein
From: "Daniel P. Berrange" <berrange@redhat.com> When building up a virCapsPtr instance, the QEMU driver was copying the list of machine types across from the previous virCapsPtr instance, if the QEMU binary had not changed. Replace this ad-hoc caching of data with use of the new qemuCapsCache global cache. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/conf/capabilities.h | 1 - src/qemu/qemu_capabilities.c | 228 +++++++++++++------------------------------ src/qemu/qemu_capabilities.h | 7 +- src/qemu/qemu_conf.h | 2 + src/qemu/qemu_driver.c | 15 +-- 5 files changed, 84 insertions(+), 169 deletions(-) diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h index b7c9f6c..0d56290 100644 --- a/src/conf/capabilities.h +++ b/src/conf/capabilities.h @@ -53,7 +53,6 @@ struct _virCapsGuestDomainInfo { char *loader; int nmachines; virCapsGuestMachinePtr *machines; - time_t emulator_mtime; /* do @machines need refreshing? */ }; typedef struct _virCapsGuestDomain virCapsGuestDomain; diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 1163dd8..cb5f63b 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -411,97 +411,6 @@ cleanup: return ret; } -static int -qemuCapsGetOldMachinesFromInfo(virCapsGuestDomainInfoPtr info, - const char *emulator, - time_t emulator_mtime, - virCapsGuestMachinePtr **machines, - size_t *nmachines) -{ - virCapsGuestMachinePtr *list; - int i; - - if (!info->nmachines) - return 0; - - if (!info->emulator || !STREQ(emulator, info->emulator)) - return 0; - - if (emulator_mtime != info->emulator_mtime) { - VIR_DEBUG("mtime on %s has changed, refreshing machine types", - info->emulator); - return 0; - } - - if (VIR_ALLOC_N(list, info->nmachines) < 0) { - virReportOOMError(); - return 0; - } - - for (i = 0; i < info->nmachines; i++) { - if (VIR_ALLOC(list[i]) < 0) { - goto no_memory; - } - if (info->machines[i]->name && - !(list[i]->name = strdup(info->machines[i]->name))) { - goto no_memory; - } - if (info->machines[i]->canonical && - !(list[i]->canonical = strdup(info->machines[i]->canonical))) { - goto no_memory; - } - } - - *machines = list; - *nmachines = info->nmachines; - - return 1; - - no_memory: - virReportOOMError(); - virCapabilitiesFreeMachines(list, info->nmachines); - return 0; -} - -static int -qemuCapsGetOldMachines(const char *ostype, - const char *arch, - int wordsize, - const char *emulator, - time_t emulator_mtime, - virCapsPtr old_caps, - virCapsGuestMachinePtr **machines, - size_t *nmachines) -{ - int i; - - for (i = 0; i < old_caps->nguests; i++) { - virCapsGuestPtr guest = old_caps->guests[i]; - int j; - - if (!STREQ(ostype, guest->ostype) || - !STREQ(arch, guest->arch.name) || - wordsize != guest->arch.wordsize) - continue; - - for (j = 0; j < guest->arch.ndomains; j++) { - virCapsGuestDomainPtr dom = guest->arch.domains[j]; - - if (qemuCapsGetOldMachinesFromInfo(&dom->info, - emulator, emulator_mtime, - machines, nmachines)) - return 1; - } - - if (qemuCapsGetOldMachinesFromInfo(&guest->arch.defaultInfo, - emulator, emulator_mtime, - machines, nmachines)) - return 1; - } - - return 0; -} - typedef int (*qemuCapsParseCPUModels)(const char *output, @@ -705,7 +614,7 @@ cleanup: static int qemuCapsInitGuest(virCapsPtr caps, - virCapsPtr old_caps, + qemuCapsCachePtr cache, const char *hostmachine, const struct qemu_arch_info *info, int hvm) @@ -716,11 +625,8 @@ qemuCapsInitGuest(virCapsPtr caps, int haskqemu = 0; char *kvmbin = NULL; char *binary = NULL; - time_t binary_mtime; virCapsGuestMachinePtr *machines = NULL; size_t nmachines = 0; - struct stat st; - size_t ncpus; qemuCapsPtr qemubinCaps = NULL; qemuCapsPtr kvmbinCaps = NULL; int ret = -1; @@ -736,10 +642,12 @@ qemuCapsInitGuest(virCapsPtr caps, } /* Ignore binary if extracting version info fails */ - if (binary && - qemuCapsExtractVersionInfo(binary, info->arch, - false, NULL, &qemubinCaps) < 0) - VIR_FREE(binary); + if (binary) { + if (!(qemubinCaps = qemuCapsCacheLookup(cache, binary))) { + virResetLastError(); + VIR_FREE(binary); + } + } /* qemu-kvm/kvm binaries can only be used if * - host & guest arches match @@ -759,9 +667,8 @@ qemuCapsInitGuest(virCapsPtr caps, if (!kvmbin) continue; - if (qemuCapsExtractVersionInfo(kvmbin, info->arch, - false, NULL, - &kvmbinCaps) < 0) { + if (!(kvmbinCaps = qemuCapsCacheLookup(cache, kvmbin))) { + virResetLastError(); VIR_FREE(kvmbin); continue; } @@ -789,15 +696,6 @@ qemuCapsInitGuest(virCapsPtr caps, qemuCapsGet(qemubinCaps, QEMU_CAPS_KQEMU)) haskqemu = 1; - if (stat(binary, &st) == 0) { - binary_mtime = st.st_mtime; - } else { - char ebuf[1024]; - VIR_WARN("Failed to stat %s, most peculiar : %s", - binary, virStrerror(errno, ebuf, sizeof(ebuf))); - binary_mtime = 0; - } - if (info->machine) { virCapsGuestMachinePtr machine; @@ -820,14 +718,7 @@ qemuCapsInitGuest(virCapsPtr caps, machines[0] = machine; } else { - int probe = 1; - if (old_caps && binary_mtime) - probe = !qemuCapsGetOldMachines(hvm ? "hvm" : "xen", info->arch, - info->wordsize, binary, binary_mtime, - old_caps, &machines, &nmachines); - if (probe && - qemuCapsProbeMachineTypes(binary, qemubinCaps, - &machines, &nmachines) < 0) + if (qemuCapsGetMachineTypesCaps(qemubinCaps, &nmachines, &machines) < 0) goto error; } @@ -846,11 +737,8 @@ qemuCapsInitGuest(virCapsPtr caps, machines = NULL; nmachines = 0; - guest->arch.defaultInfo.emulator_mtime = binary_mtime; - if (caps->host.cpu && - qemuCapsProbeCPUModels(binary, NULL, info->arch, &ncpus, NULL) == 0 && - ncpus > 0 && + qemuCapsGetCPUDefinitions(qemubinCaps, NULL) > 0 && !virCapabilitiesAddGuestFeature(guest, "cpuselection", 1, 0)) goto error; @@ -879,28 +767,9 @@ qemuCapsInitGuest(virCapsPtr caps, if (haskvm) { virCapsGuestDomainPtr dom; - if (kvmbin) { - int probe = 1; - - if (stat(kvmbin, &st) == 0) { - binary_mtime = st.st_mtime; - } else { - char ebuf[1024]; - VIR_WARN("Failed to stat %s, most peculiar : %s", - binary, virStrerror(errno, ebuf, sizeof(ebuf))); - binary_mtime = 0; - } - - if (old_caps && binary_mtime) - probe = !qemuCapsGetOldMachines("hvm", info->arch, - info->wordsize, kvmbin, - binary_mtime, old_caps, - &machines, &nmachines); - if (probe && - qemuCapsProbeMachineTypes(kvmbin, kvmbinCaps, - &machines, &nmachines) < 0) - goto error; - } + if (kvmbin && + qemuCapsGetMachineTypesCaps(kvmbinCaps, &nmachines, &machines) < 0) + goto error; if ((dom = virCapabilitiesAddGuestDomain(guest, "kvm", @@ -914,7 +783,6 @@ qemuCapsInitGuest(virCapsPtr caps, machines = NULL; nmachines = 0; - dom->info.emulator_mtime = binary_mtime; } } else { if (virCapabilitiesAddGuestDomain(guest, @@ -958,7 +826,7 @@ error: static int qemuCapsInitCPU(virCapsPtr caps, - const char *arch) + const char *arch) { virCPUDefPtr cpu = NULL; union cpuData *data = NULL; @@ -1004,7 +872,7 @@ static int qemuDefaultConsoleType(const char *ostype ATTRIBUTE_UNUSED) } -virCapsPtr qemuCapsInit(virCapsPtr old_caps) +virCapsPtr qemuCapsInit(qemuCapsCachePtr cache) { struct utsname utsname; virCapsPtr caps; @@ -1030,14 +898,8 @@ virCapsPtr qemuCapsInit(virCapsPtr old_caps) VIR_WARN("Failed to query host NUMA topology, disabling NUMA capabilities"); } - if (old_caps == NULL || old_caps->host.cpu == NULL) { - if (qemuCapsInitCPU(caps, utsname.machine) < 0) - VIR_WARN("Failed to get host CPU"); - } - else { - caps->host.cpu = old_caps->host.cpu; - old_caps->host.cpu = NULL; - } + if (qemuCapsInitCPU(caps, utsname.machine) < 0) + VIR_WARN("Failed to get host CPU"); /* Add the power management features of the host */ @@ -1049,7 +911,7 @@ virCapsPtr qemuCapsInit(virCapsPtr old_caps) /* First the pure HVM guests */ for (i = 0 ; i < ARRAY_CARDINALITY(arch_info_hvm) ; i++) - if (qemuCapsInitGuest(caps, old_caps, + if (qemuCapsInitGuest(caps, cache, utsname.machine, &arch_info_hvm[i], 1) < 0) goto no_memory; @@ -1064,7 +926,7 @@ virCapsPtr qemuCapsInit(virCapsPtr old_caps) if (STREQ(arch_info_xen[i].arch, utsname.machine) || (STREQ(utsname.machine, "x86_64") && STREQ(arch_info_xen[i].arch, "i686"))) { - if (qemuCapsInitGuest(caps, old_caps, + if (qemuCapsInitGuest(caps, cache, utsname.machine, &arch_info_xen[i], 0) < 0) goto no_memory; @@ -1873,6 +1735,11 @@ qemuCapsGet(qemuCapsPtr caps, } +const char *qemuCapsGetBinary(qemuCapsPtr caps) +{ + return caps->binary; +} + const char *qemuCapsGetArch(qemuCapsPtr caps) { return caps->arch; @@ -1894,7 +1761,8 @@ unsigned int qemuCapsGetKVMVersion(qemuCapsPtr caps) size_t qemuCapsGetCPUDefinitions(qemuCapsPtr caps, char ***names) { - *names = caps->cpuDefinitions; + if (names) + *names = caps->cpuDefinitions; return caps->ncpuDefinitions; } @@ -1902,10 +1770,50 @@ size_t qemuCapsGetCPUDefinitions(qemuCapsPtr caps, size_t qemuCapsGetMachineTypes(qemuCapsPtr caps, char ***names) { - *names = caps->machineTypes; + if (names) + *names = caps->machineTypes; return caps->nmachineTypes; } +int qemuCapsGetMachineTypesCaps(qemuCapsPtr caps, + size_t *nmachines, + virCapsGuestMachinePtr **machines) +{ + size_t i; + + *nmachines = 0; + *machines = NULL; + if (VIR_ALLOC_N(*machines, caps->nmachineTypes) < 0) + goto no_memory; + *nmachines = caps->nmachineTypes; + + for (i = 0 ; i < caps->nmachineTypes ; i++) { + virCapsGuestMachinePtr mach; + if (VIR_ALLOC(mach) < 0) + goto no_memory; + if (caps->machineAliases[i]) { + if (!(mach->name = strdup(caps->machineAliases[i]))) + goto no_memory; + if (!(mach->canonical = strdup(caps->machineTypes[i]))) + goto no_memory; + } else { + if (!(mach->name = strdup(caps->machineTypes[i]))) + goto no_memory; + } + (*machines)[i] = mach; + } + + return 0; + +no_memory: + virCapabilitiesFreeMachines(*machines, *nmachines); + *nmachines = 0; + *machines = NULL; + return -1; +} + + + const char *qemuCapsGetCanonicalMachine(qemuCapsPtr caps, const char *name) diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 27ed378..942740d 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -174,6 +174,7 @@ bool qemuCapsGet(qemuCapsPtr caps, char *qemuCapsFlagsString(qemuCapsPtr caps); +const char *qemuCapsGetBinary(qemuCapsPtr caps); const char *qemuCapsGetArch(qemuCapsPtr caps); unsigned int qemuCapsGetVersion(qemuCapsPtr caps); unsigned int qemuCapsGetKVMVersion(qemuCapsPtr caps); @@ -184,6 +185,10 @@ size_t qemuCapsGetMachineTypes(qemuCapsPtr caps, const char *qemuCapsGetCanonicalMachine(qemuCapsPtr caps, const char *name); +int qemuCapsGetMachineTypesCaps(qemuCapsPtr caps, + size_t *nmachines, + virCapsGuestMachinePtr **machines); + bool qemuCapsIsValid(qemuCapsPtr caps); @@ -192,7 +197,7 @@ qemuCapsPtr qemuCapsCacheLookup(qemuCapsCachePtr cache, const char *binary); qemuCapsPtr qemuCapsCacheLookupCopy(qemuCapsCachePtr cache, const char *binary); void qemuCapsCacheFree(qemuCapsCachePtr cache); -virCapsPtr qemuCapsInit(virCapsPtr old_caps); +virCapsPtr qemuCapsInit(qemuCapsCachePtr cache); int qemuCapsProbeMachineTypes(const char *binary, qemuCapsPtr caps, diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 180c144..ca2f694 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -43,6 +43,7 @@ # include "command.h" # include "threadpool.h" # include "locking/lock_manager.h" +# include "qemu_capabilities.h" # define QEMUD_CPUMASK_LEN CPU_SETSIZE @@ -115,6 +116,7 @@ struct qemud_driver { int max_queued; virCapsPtr caps; + qemuCapsCachePtr capsCache; virDomainEventStatePtr domainEventState; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 226d499..e0c5cdb 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -378,8 +378,7 @@ error: static virCapsPtr -qemuCreateCapabilities(virCapsPtr oldcaps, - struct qemud_driver *driver) +qemuCreateCapabilities(struct qemud_driver *driver) { size_t i; virCapsPtr caps; @@ -388,7 +387,7 @@ qemuCreateCapabilities(virCapsPtr oldcaps, const char *doi, *model; /* Basic host arch / guest machine capabilities */ - if (!(caps = qemuCapsInit(oldcaps))) { + if (!(caps = qemuCapsInit(driver->capsCache))) { virReportOOMError(); return NULL; } @@ -768,8 +767,10 @@ qemudStartup(int privileged) { if (qemuSecurityInit(qemu_driver) < 0) goto error; - if ((qemu_driver->caps = qemuCreateCapabilities(NULL, - qemu_driver)) == NULL) + if ((qemu_driver->capsCache = qemuCapsCacheNew()) == NULL) + goto error; + + if ((qemu_driver->caps = qemuCreateCapabilities(qemu_driver)) == NULL) goto error; if ((qemu_driver->activePciHostdevs = pciDeviceListNew()) == NULL) @@ -985,6 +986,7 @@ qemudShutdown(void) { pciDeviceListFree(qemu_driver->inactivePciHostdevs); usbDeviceListFree(qemu_driver->activeUsbHostdevs); virCapabilitiesFree(qemu_driver->caps); + qemuCapsCacheFree(qemu_driver->capsCache); virDomainObjListDeinit(&qemu_driver->domains); virBitmapFree(qemu_driver->reservedRemotePorts); @@ -1224,8 +1226,7 @@ static char *qemudGetCapabilities(virConnectPtr conn) { qemuDriverLock(driver); - if ((caps = qemuCreateCapabilities(qemu_driver->caps, - qemu_driver)) == NULL) { + if ((caps = qemuCreateCapabilities(qemu_driver)) == NULL) { virCapabilitiesFree(caps); goto cleanup; } -- 1.7.11.4
On 09/25/2012 11:59 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
When building up a virCapsPtr instance, the QEMU driver was copying the list of machine types across from the previous virCapsPtr instance, if the QEMU binary had not changed. Replace this ad-hoc caching of data with use of the new qemuCapsCache global cache.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/conf/capabilities.h | 1 - src/qemu/qemu_capabilities.c | 228 +++++++++++++------------------------------ src/qemu/qemu_capabilities.h | 7 +- src/qemu/qemu_conf.h | 2 + src/qemu/qemu_driver.c | 15 +-- 5 files changed, 84 insertions(+), 169 deletions(-)
ACK. Nice to see twice as many lines deleted. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
From: "Daniel P. Berrange" <berrange@redhat.com> Remove all use of the existing APIs for querying QEMU capability flags. Instead obtain a qemuCapsPtr object from the global cache. This avoids the execution of 'qemu -help' (and related commands) when launching new guests. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_capabilities.c | 92 ++++---------------------------------------- src/qemu/qemu_capabilities.h | 11 ++---- src/qemu/qemu_command.c | 85 +++++++++------------------------------- src/qemu/qemu_command.h | 3 +- src/qemu/qemu_driver.c | 63 +++++++++++++++++++++--------- src/qemu/qemu_process.c | 21 +++------- tests/qemuxml2argvtest.c | 4 -- tests/qemuxmlnstest.c | 4 -- 8 files changed, 81 insertions(+), 202 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index cb5f63b..50d3e92 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1468,79 +1468,6 @@ qemuCapsParseDeviceStr(const char *str, qemuCapsPtr caps) return 0; } -int qemuCapsExtractVersionInfo(const char *qemu, - const char *arch, - bool check_yajl, - unsigned int *retversion, - qemuCapsPtr *retcaps) -{ - int ret = -1; - unsigned int version, is_kvm, kvm_version; - qemuCapsPtr caps = NULL; - char *help = NULL; - virCommandPtr cmd; - - if (retcaps) - *retcaps = NULL; - if (retversion) - *retversion = 0; - - /* 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(qemu)) { - virReportSystemError(errno, _("Cannot find QEMU binary %s"), qemu); - return -1; - } - - cmd = qemuCapsProbeCommand(qemu, NULL); - virCommandAddArgList(cmd, "-help", NULL); - virCommandSetOutputBuffer(cmd, &help); - - if (virCommandRun(cmd, NULL) < 0) - goto cleanup; - - if (!(caps = qemuCapsNew()) || - qemuCapsParseHelpStr(qemu, help, caps, - &version, &is_kvm, &kvm_version, - check_yajl) == -1) - goto cleanup; - - /* Currently only x86_64 and i686 support PCI-multibus. */ - if (STREQLEN(arch, "x86_64", 6) || - STREQLEN(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(arch, "s390")) - qemuCapsClear(caps, QEMU_CAPS_NO_ACPI); - - /* qemuCapsExtractDeviceStr will only set additional caps if qemu - * understands the 0.13.0+ notion of "-device driver,". */ - if (qemuCapsGet(caps, QEMU_CAPS_DEVICE) && - strstr(help, "-device driver,?") && - qemuCapsExtractDeviceStr(qemu, caps) < 0) - goto cleanup; - - if (retversion) - *retversion = version; - if (retcaps) { - *retcaps = caps; - caps = NULL; - } - - ret = 0; - -cleanup: - VIR_FREE(help); - virCommandFree(cmd); - virObjectUnref(caps); - - return ret; -} static void uname_normalize (struct utsname *ut) @@ -1556,12 +1483,13 @@ uname_normalize (struct utsname *ut) ut->machine[1] = '6'; } -int qemuCapsExtractVersion(virCapsPtr caps, - unsigned int *version) +int qemuCapsGetDefaultVersion(virCapsPtr caps, + qemuCapsCachePtr capsCache, + unsigned int *version) { const char *binary; - struct stat sb; struct utsname ut; + qemuCapsPtr qemucaps; if (*version > 0) return 0; @@ -1576,17 +1504,11 @@ int qemuCapsExtractVersion(virCapsPtr caps, return -1; } - if (stat(binary, &sb) < 0) { - virReportSystemError(errno, - _("Cannot find QEMU binary %s"), binary); - return -1; - } - - if (qemuCapsExtractVersionInfo(binary, ut.machine, false, - version, NULL) < 0) { + if (!(qemucaps = qemuCapsCacheLookup(capsCache, binary))) return -1; - } + *version = qemuCapsGetVersion(qemucaps); + virObjectUnref(qemucaps); return 0; } diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 942740d..4b96662 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -27,6 +27,7 @@ # include "virobject.h" # include "capabilities.h" # include "command.h" +# include "virobject.h" /* Internal flags to keep track of qemu command line capabilities */ enum qemuCapsFlags { @@ -210,13 +211,9 @@ int qemuCapsProbeCPUModels(const char *qemu, size_t *count, const char ***cpus); -int qemuCapsExtractVersion(virCapsPtr caps, - unsigned int *version); -int qemuCapsExtractVersionInfo(const char *qemu, - const char *arch, - bool check_yajl, - unsigned int *version, - qemuCapsPtr *retcaps); +int qemuCapsGetDefaultVersion(virCapsPtr caps, + qemuCapsCachePtr capsCache, + unsigned int *version); int qemuCapsParseHelpStr(const char *qemu, const char *str, diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index e7bb88e..dd043a1 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -810,33 +810,13 @@ qemuDomainPrimeS390VirtioDevices(virDomainDefPtr def, } -static int +static void qemuDomainAssignS390Addresses(virDomainDefPtr def, qemuCapsPtr caps) { - int ret = -1; - qemuCapsPtr localCaps = NULL; - - if (!caps) { - /* need to get information from real environment */ - if (qemuCapsExtractVersionInfo(def->emulator, def->os.arch, - false, NULL, - &localCaps) < 0) - goto cleanup; - caps = localCaps; - } - - if (qemuCapsGet(caps, QEMU_CAPS_VIRTIO_S390)) { - /* deal with legacy virtio-s390 */ + /* deal with legacy virtio-s390 */ + if (qemuCapsGet(caps, QEMU_CAPS_VIRTIO_S390)) qemuDomainPrimeS390VirtioDevices( def, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390); - } - - ret = 0; - -cleanup: - virObjectUnref(localCaps); - - return ret; } static int @@ -863,7 +843,7 @@ qemuAssignSpaprVIOAddress(virDomainDefPtr def, virDomainDeviceInfoPtr info, unsigned long long default_reg) { bool user_reg; - int rc; + int ret; if (info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO) return 0; @@ -875,8 +855,8 @@ qemuAssignSpaprVIOAddress(virDomainDefPtr def, virDomainDeviceInfoPtr info, info->addr.spaprvio.has_reg = true; } - rc = virDomainDeviceInfoIterate(def, qemuSpaprVIOFindByReg, info); - while (rc != 0) { + ret = virDomainDeviceInfoIterate(def, qemuSpaprVIOFindByReg, info); + while (ret != 0) { if (user_reg) { virReportError(VIR_ERR_XML_ERROR, _("spapr-vio address %#llx already in use"), @@ -886,7 +866,7 @@ qemuAssignSpaprVIOAddress(virDomainDefPtr def, virDomainDeviceInfoPtr info, /* We assigned the reg, so try a new value */ info->addr.spaprvio.reg += 0x1000; - rc = virDomainDeviceInfoIterate(def, qemuSpaprVIOFindByReg, info); + ret = virDomainDeviceInfoIterate(def, qemuSpaprVIOFindByReg, info); } return 0; @@ -895,45 +875,32 @@ qemuAssignSpaprVIOAddress(virDomainDefPtr def, virDomainDeviceInfoPtr info, int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def, qemuCapsPtr caps) { - int i, rc = -1; + int i, ret = -1; int model; - qemuCapsPtr localCaps = NULL; /* Default values match QEMU. See spapr_(llan|vscsi|vty).c */ - if (!caps) { - /* need to get information from real environment */ - if (qemuCapsExtractVersionInfo(def->emulator, def->os.arch, - false, NULL, - &localCaps) < 0) - goto cleanup; - caps = localCaps; - } - for (i = 0 ; i < def->nnets; i++) { if (def->nets[i]->model && STREQ(def->nets[i]->model, "spapr-vlan")) def->nets[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO; - rc = qemuAssignSpaprVIOAddress(def, &def->nets[i]->info, - 0x1000ul); - if (rc) + if (qemuAssignSpaprVIOAddress(def, &def->nets[i]->info, + 0x1000ul) < 0) goto cleanup; } for (i = 0 ; i < def->ncontrollers; i++) { model = def->controllers[i]->model; if (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) { - rc = qemuSetScsiControllerModel(def, caps, &model); - if (rc) + if (qemuSetScsiControllerModel(def, caps, &model) < 0) goto cleanup; } if (model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI && def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) def->controllers[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO; - rc = qemuAssignSpaprVIOAddress(def, &def->controllers[i]->info, - 0x2000ul); - if (rc) + if (qemuAssignSpaprVIOAddress(def, &def->controllers[i]->info, + 0x2000ul) < 0) goto cleanup; } @@ -943,19 +910,17 @@ int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def, STREQ(def->os.arch, "ppc64") && STREQ(def->os.machine, "pseries")) def->serials[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO; - rc = qemuAssignSpaprVIOAddress(def, &def->serials[i]->info, - 0x30000000ul); - if (rc) + if (qemuAssignSpaprVIOAddress(def, &def->serials[i]->info, + 0x30000000ul) < 0) goto cleanup; } /* No other devices are currently supported on spapr-vio */ - rc = 0; + ret = 0; cleanup: - virObjectUnref(localCaps); - return rc; + return ret; } #define QEMU_PCI_ADDRESS_LAST_SLOT 31 @@ -1070,20 +1035,9 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, virDomainObjPtr obj) { int ret = -1; - qemuCapsPtr localCaps = NULL; qemuDomainPCIAddressSetPtr addrs = NULL; qemuDomainObjPrivatePtr priv = NULL; - if (!caps) { - /* need to get information from real environment */ - if (qemuCapsExtractVersionInfo(def->emulator, def->os.arch, - false, - NULL, - &localCaps) < 0) - goto cleanup; - caps = localCaps; - } - if (qemuCapsGet(caps, QEMU_CAPS_DEVICE)) { if (!(addrs = qemuDomainPCIAddressSetCreate(def))) goto cleanup; @@ -1108,7 +1062,6 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, ret = 0; cleanup: - virObjectUnref(localCaps); qemuDomainPCIAddressSetFree(addrs); return ret; @@ -1124,9 +1077,7 @@ int qemuDomainAssignAddresses(virDomainDefPtr def, if (rc) return rc; - rc = qemuDomainAssignS390Addresses(def, caps); - if (rc) - return rc; + qemuDomainAssignS390Addresses(def, caps); return qemuDomainAssignPCIAddresses(def, caps, obj); } diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 939833d..37bef84 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -188,7 +188,8 @@ virDomainDefPtr qemuParseCommandLinePid(virCapsPtr caps, int qemuDomainAssignAddresses(virDomainDefPtr def, qemuCapsPtr caps, - virDomainObjPtr); + virDomainObjPtr obj) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def, qemuCapsPtr caps); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e0c5cdb..2b17c49 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1475,7 +1475,9 @@ static int qemudGetVersion(virConnectPtr conn, unsigned long *version) { int ret = -1; qemuDriverLock(driver); - if (qemuCapsExtractVersion(driver->caps, &driver->qemuVersion) < 0) + if (qemuCapsGetDefaultVersion(driver->caps, + driver->capsCache, + &driver->qemuVersion) < 0) goto cleanup; *version = driver->qemuVersion; @@ -1517,6 +1519,7 @@ static virDomainPtr qemudDomainCreate(virConnectPtr conn, const char *xml, virDomainEventPtr event = NULL; virDomainEventPtr event2 = NULL; unsigned int start_flags = VIR_QEMU_PROCESS_START_COLD; + qemuCapsPtr caps = NULL; virCheckFlags(VIR_DOMAIN_START_PAUSED | VIR_DOMAIN_START_AUTODESTROY, NULL); @@ -1538,10 +1541,13 @@ static virDomainPtr qemudDomainCreate(virConnectPtr conn, const char *xml, if (virDomainObjIsDuplicate(&driver->domains, def, 1) < 0) goto cleanup; + if (!(caps = qemuCapsCacheLookup(driver->capsCache, def->emulator))) + goto cleanup; + if (qemudCanonicalizeMachine(driver, def) < 0) goto cleanup; - if (qemuDomainAssignAddresses(def, NULL, NULL) < 0) + if (qemuDomainAssignAddresses(def, caps, NULL) < 0) goto cleanup; if (!(vm = virDomainAssignDef(driver->caps, @@ -1595,6 +1601,7 @@ cleanup: if (event2) qemuDomainEventQueue(driver, event2); } + virObjectUnref(caps); qemuDriverUnlock(driver); return dom; } @@ -5210,6 +5217,9 @@ static char *qemuDomainXMLToNative(virConnectPtr conn, if (!def) goto cleanup; + if (!(caps = qemuCapsCacheLookup(driver->capsCache, def->emulator))) + goto cleanup; + /* Since we're just exporting args, we can't do bridge/network/direct * setups, since libvirt will normally create TAP/macvtap devices * directly. We convert those configs into generic 'ethernet' @@ -5278,12 +5288,6 @@ static char *qemuDomainXMLToNative(virConnectPtr conn, net->info.bootIndex = bootIndex; } - if (qemuCapsExtractVersionInfo(def->emulator, def->os.arch, - false, - NULL, - &caps) < 0) - goto cleanup; - monitor_json = qemuCapsGet(caps, QEMU_CAPS_MONITOR_JSON); if (qemuProcessPrepareMonitorChr(driver, &monConfig, def->name) < 0) @@ -5577,6 +5581,7 @@ static virDomainPtr qemudDomainDefine(virConnectPtr conn, const char *xml) { virDomainObjPtr vm = NULL; virDomainPtr dom = NULL; virDomainEventPtr event = NULL; + qemuCapsPtr caps = NULL; int dupVM; qemuDriverLock(driver); @@ -5591,10 +5596,13 @@ static virDomainPtr qemudDomainDefine(virConnectPtr conn, const char *xml) { if ((dupVM = virDomainObjIsDuplicate(&driver->domains, def, 0)) < 0) goto cleanup; + if (!(caps = qemuCapsCacheLookup(driver->capsCache, def->emulator))) + goto cleanup; + if (qemudCanonicalizeMachine(driver, def) < 0) goto cleanup; - if (qemuDomainAssignAddresses(def, NULL, NULL) < 0) + if (qemuDomainAssignAddresses(def, caps, NULL) < 0) goto cleanup; /* We need to differentiate two cases: @@ -5657,6 +5665,7 @@ cleanup: virDomainObjUnlock(vm); if (event) qemuDomainEventQueue(driver, event); + virObjectUnref(caps); qemuDriverUnlock(driver); return dom; } @@ -6075,7 +6084,8 @@ qemuDomainUpdateDeviceLive(virDomainObjPtr vm, } static int -qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef, +qemuDomainAttachDeviceConfig(qemuCapsPtr caps, + virDomainDefPtr vmdef, virDomainDeviceDefPtr dev) { virDomainDiskDefPtr disk; @@ -6101,7 +6111,7 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef, if (disk->bus != VIR_DOMAIN_DISK_BUS_VIRTIO) if (virDomainDefAddImplicitControllers(vmdef) < 0) return -1; - if (qemuDomainAssignAddresses(vmdef, NULL, NULL) < 0) + if (qemuDomainAssignAddresses(vmdef, caps, NULL) < 0) return -1; break; @@ -6120,7 +6130,7 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef, return -1; } dev->data.net = NULL; - if (qemuDomainAssignAddresses(vmdef, NULL, NULL) < 0) + if (qemuDomainAssignAddresses(vmdef, caps, NULL) < 0) return -1; break; @@ -6136,7 +6146,7 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef, return -1; } dev->data.hostdev = NULL; - if (qemuDomainAssignAddresses(vmdef, NULL, NULL) < 0) + if (qemuDomainAssignAddresses(vmdef, caps, NULL) < 0) return -1; break; @@ -6168,7 +6178,7 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef, return -1; dev->data.controller = NULL; - if (qemuDomainAssignAddresses(vmdef, NULL, NULL) < 0) + if (qemuDomainAssignAddresses(vmdef, caps, NULL) < 0) return -1; break; @@ -6261,7 +6271,8 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef, } static int -qemuDomainUpdateDeviceConfig(virDomainDefPtr vmdef, +qemuDomainUpdateDeviceConfig(qemuCapsPtr caps, + virDomainDefPtr vmdef, virDomainDeviceDefPtr dev) { virDomainDiskDefPtr orig, disk; @@ -6321,7 +6332,7 @@ qemuDomainUpdateDeviceConfig(virDomainDefPtr vmdef, vmdef->nets[pos] = net; dev->data.net = NULL; - if (qemuDomainAssignAddresses(vmdef, NULL, NULL) < 0) + if (qemuDomainAssignAddresses(vmdef, caps, NULL) < 0) return -1; break; @@ -6352,6 +6363,8 @@ qemuDomainModifyDeviceFlags(virDomainPtr dom, const char *xml, bool force = (flags & VIR_DOMAIN_DEVICE_MODIFY_FORCE) != 0; int ret = -1; unsigned int affect; + qemuCapsPtr caps = NULL; + qemuDomainObjPrivatePtr priv; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG | @@ -6369,6 +6382,7 @@ qemuDomainModifyDeviceFlags(virDomainPtr dom, const char *xml, _("no domain with matching uuid '%s'"), uuidstr); goto cleanup; } + priv = vm->privateData; if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; @@ -6410,6 +6424,11 @@ qemuDomainModifyDeviceFlags(virDomainPtr dom, const char *xml, goto endjob; } + if (priv->caps) + caps = virObjectRef(priv->caps); + else if (!(caps = qemuCapsCacheLookup(driver->capsCache, vm->def->emulator))) + goto cleanup; + if (flags & VIR_DOMAIN_AFFECT_CONFIG) { if (virDomainDefCompatibleDevice(vm->def, dev) < 0) goto endjob; @@ -6420,13 +6439,13 @@ qemuDomainModifyDeviceFlags(virDomainPtr dom, const char *xml, goto endjob; switch (action) { case QEMU_DEVICE_ATTACH: - ret = qemuDomainAttachDeviceConfig(vmdef, dev); + ret = qemuDomainAttachDeviceConfig(caps, vmdef, dev); break; case QEMU_DEVICE_DETACH: ret = qemuDomainDetachDeviceConfig(vmdef, dev); break; case QEMU_DEVICE_UPDATE: - ret = qemuDomainUpdateDeviceConfig(vmdef, dev); + ret = qemuDomainUpdateDeviceConfig(caps, vmdef, dev); break; default: virReportError(VIR_ERR_INTERNAL_ERROR, @@ -6486,6 +6505,7 @@ endjob: vm = NULL; cleanup: + virObjectUnref(caps); virDomainDefFree(vmdef); if (dev != dev_copy) virDomainDeviceDefFree(dev_copy); @@ -12262,6 +12282,7 @@ static virDomainPtr qemuDomainAttach(virConnectPtr conn, bool monJSON = false; pid_t pid = pid_value; char *pidfile = NULL; + qemuCapsPtr caps = NULL; virCheckFlags(0, NULL); @@ -12291,13 +12312,16 @@ static virDomainPtr qemuDomainAttach(virConnectPtr conn, goto cleanup; } + if (!(caps = qemuCapsCacheLookup(driver->capsCache, def->emulator))) + goto cleanup; + if (virDomainObjIsDuplicate(&driver->domains, def, 1) < 0) goto cleanup; if (qemudCanonicalizeMachine(driver, def) < 0) goto cleanup; - if (qemuDomainAssignAddresses(def, NULL, NULL) < 0) + if (qemuDomainAssignAddresses(def, caps, NULL) < 0) goto cleanup; if (!(vm = virDomainAssignDef(driver->caps, @@ -12329,6 +12353,7 @@ endjob: cleanup: virDomainDefFree(def); + virObjectUnref(caps); virDomainChrSourceDefFree(monConfig); if (vm) virDomainObjUnlock(vm); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index ec312d1..38fb9a6 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3105,10 +3105,8 @@ qemuProcessReconnect(void *opaque) * caps in the domain status, so re-query them */ if (!priv->caps && - qemuCapsExtractVersionInfo(obj->def->emulator, obj->def->os.arch, - false, - NULL, - &priv->caps) < 0) + !(priv->caps = qemuCapsCacheLookupCopy(driver->capsCache, + obj->def->emulator))) goto error; /* In case the domain shutdown while we were not running, @@ -3505,11 +3503,8 @@ int qemuProcessStart(virConnectPtr conn, VIR_DEBUG("Determining emulator version"); virObjectUnref(priv->caps); - priv->caps = NULL; - if (qemuCapsExtractVersionInfo(vm->def->emulator, vm->def->os.arch, - true, - NULL, - &priv->caps) < 0) + if (!(priv->caps = qemuCapsCacheLookupCopy(driver->capsCache, + vm->def->emulator))) goto cleanup; if (qemuAssignDeviceAliases(vm->def, priv->caps) < 0) @@ -4270,12 +4265,8 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED, VIR_DEBUG("Determining emulator version"); virObjectUnref(priv->caps); - priv->caps = NULL; - if (qemuCapsExtractVersionInfo(vm->def->emulator, - vm->def->os.arch, - false, - NULL, - &priv->caps) < 0) + if (!(priv->caps = qemuCapsCacheLookupCopy(driver->capsCache, + vm->def->emulator))) goto cleanup; VIR_DEBUG("Preparing monitor state"); diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 90dad17..e35460a 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -157,10 +157,6 @@ static int testCompareXMLToArgvFiles(const char *xml, VIR_FREE(log); virResetLastError(); - /* We do not call qemuCapsExtractVersionInfo() before calling - * qemuBuildCommandLine(), so we should set QEMU_CAPS_PCI_MULTIBUS for - * x86_64 and i686 architectures here. - */ if (STREQLEN(vmdef->os.arch, "x86_64", 6) || STREQLEN(vmdef->os.arch, "i686", 4)) { qemuCapsSet(extraFlags, QEMU_CAPS_PCI_MULTIBUS); diff --git a/tests/qemuxmlnstest.c b/tests/qemuxmlnstest.c index 43b5fe6..dd56091 100644 --- a/tests/qemuxmlnstest.c +++ b/tests/qemuxmlnstest.c @@ -102,10 +102,6 @@ static int testCompareXMLToArgvFiles(const char *xml, VIR_FREE(log); virResetLastError(); - /* We do not call qemuCapsExtractVersionInfo() before calling - * qemuBuildCommandLine(), so we should set QEMU_CAPS_PCI_MULTIBUS for - * x86_64 and i686 architectures here. - */ if (STREQLEN(vmdef->os.arch, "x86_64", 6) || STREQLEN(vmdef->os.arch, "i686", 4)) { qemuCapsSet(extraFlags, QEMU_CAPS_PCI_MULTIBUS); -- 1.7.11.4
On 09/25/2012 11:59 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Remove all use of the existing APIs for querying QEMU capability flags. Instead obtain a qemuCapsPtr object from the global cache. This avoids the execution of 'qemu -help' (and related commands) when launching new guests.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_capabilities.c | 92 ++++---------------------------------------- src/qemu/qemu_capabilities.h | 11 ++---- src/qemu/qemu_command.c | 85 +++++++++------------------------------- src/qemu/qemu_command.h | 3 +- src/qemu/qemu_driver.c | 63 +++++++++++++++++++++--------- src/qemu/qemu_process.c | 21 +++------- tests/qemuxml2argvtest.c | 4 -- tests/qemuxmlnstest.c | 4 -- 8 files changed, 81 insertions(+), 202 deletions(-)
Another nice code reduction. ACK. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
From: "Daniel P. Berrange" <berrange@redhat.com> When XML for a new guest is received, the machine type is immediately canonicalized into the version specific name. This involves probing QEMU for supported machine types. Replace this probing with a lookup of the machine types in the (hopefully cached) qemuCapsPtr object Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_command.h | 3 -- src/qemu/qemu_driver.c | 133 ++++++++--------------------------------------- tests/qemuxml2argvtest.c | 12 ++++- tests/qemuxmlnstest.c | 3 -- 4 files changed, 33 insertions(+), 118 deletions(-) diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 37bef84..953fa72 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -162,9 +162,6 @@ int qemuOpenVhostNet(virDomainDefPtr def, qemuCapsPtr caps, int *vhostfd); -int qemudCanonicalizeMachine(struct qemud_driver *driver, - virDomainDefPtr def); - /* * NB: def->name can be NULL upon return and the caller * *must* decide how to fill in a name in this case diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2b17c49..fed7fa2 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1510,6 +1510,26 @@ static int qemudNumDomains(virConnectPtr conn) { return n; } + +static int +qemuCanonicalizeMachine(virDomainDefPtr def, qemuCapsPtr caps) +{ + const char *canon = qemuCapsGetCanonicalMachine(caps, def->os.machine); + + if (canon != def->os.machine) { + char *tmp; + if (!(tmp = strdup(canon))) { + virReportOOMError(); + return -1; + } + VIR_FREE(def->os.machine); + def->os.machine = tmp; + } + + return 0; +} + + static virDomainPtr qemudDomainCreate(virConnectPtr conn, const char *xml, unsigned int flags) { struct qemud_driver *driver = conn->privateData; @@ -1544,7 +1564,7 @@ static virDomainPtr qemudDomainCreate(virConnectPtr conn, const char *xml, if (!(caps = qemuCapsCacheLookup(driver->capsCache, def->emulator))) goto cleanup; - if (qemudCanonicalizeMachine(driver, def) < 0) + if (qemuCanonicalizeMachine(def, caps) < 0) goto cleanup; if (qemuDomainAssignAddresses(def, caps, NULL) < 0) @@ -5467,113 +5487,6 @@ qemuDomainStart(virDomainPtr dom) return qemuDomainStartWithFlags(dom, 0); } -static int -qemudCanonicalizeMachineFromInfo(virDomainDefPtr def, - virCapsGuestDomainInfoPtr info, - char **canonical) -{ - int i; - - *canonical = NULL; - - for (i = 0; i < info->nmachines; i++) { - virCapsGuestMachinePtr machine = info->machines[i]; - - if (!machine->canonical) - continue; - - if (def->os.machine && STRNEQ(def->os.machine, machine->name)) - continue; - - if (!(*canonical = strdup(machine->canonical))) { - virReportOOMError(); - return -1; - } - - break; - } - - return 0; -} - -static int -qemudCanonicalizeMachineDirect(virDomainDefPtr def, char **canonical) -{ - virCapsGuestMachinePtr *machines = NULL; - size_t i, nmachines = 0; - - /* XXX we should be checking emulator capabilities and pass them instead - * of NULL so that -nodefconfig or -no-user-config is properly added when - * probing machine types. Luckily, qemu does not support specifying new - * machine types in its configuration files yet, which means passing this - * additional parameter makes no difference now. - */ - if (qemuCapsProbeMachineTypes(def->emulator, NULL, - &machines, &nmachines) < 0) - return -1; - - for (i = 0; i < nmachines; i++) { - if (!machines[i]->canonical) - continue; - - if (def->os.machine && STRNEQ(def->os.machine, machines[i]->name)) - continue; - - *canonical = machines[i]->canonical; - machines[i]->canonical = NULL; - break; - } - - virCapabilitiesFreeMachines(machines, nmachines); - - return 0; -} - -int -qemudCanonicalizeMachine(struct qemud_driver *driver, virDomainDefPtr def) -{ - char *canonical = NULL; - int i; - - for (i = 0; i < driver->caps->nguests; i++) { - virCapsGuestPtr guest = driver->caps->guests[i]; - virCapsGuestDomainInfoPtr info; - int j; - - for (j = 0; j < guest->arch.ndomains; j++) { - info = &guest->arch.domains[j]->info; - - if (!info->emulator || !STREQ(info->emulator, def->emulator)) - continue; - - if (!info->nmachines) - info = &guest->arch.defaultInfo; - - if (qemudCanonicalizeMachineFromInfo(def, info, &canonical) < 0) - return -1; - goto out; - } - - info = &guest->arch.defaultInfo; - - if (info->emulator && STREQ(info->emulator, def->emulator)) { - if (qemudCanonicalizeMachineFromInfo(def, info, &canonical) < 0) - return -1; - goto out; - } - } - - if (qemudCanonicalizeMachineDirect(def, &canonical) < 0) - return -1; - -out: - if (canonical) { - VIR_FREE(def->os.machine); - def->os.machine = canonical; - } - return 0; -} - static virDomainPtr qemudDomainDefine(virConnectPtr conn, const char *xml) { struct qemud_driver *driver = conn->privateData; virDomainDefPtr def; @@ -5599,7 +5512,7 @@ static virDomainPtr qemudDomainDefine(virConnectPtr conn, const char *xml) { if (!(caps = qemuCapsCacheLookup(driver->capsCache, def->emulator))) goto cleanup; - if (qemudCanonicalizeMachine(driver, def) < 0) + if (qemuCanonicalizeMachine(def, caps) < 0) goto cleanup; if (qemuDomainAssignAddresses(def, caps, NULL) < 0) @@ -12318,7 +12231,7 @@ static virDomainPtr qemuDomainAttach(virConnectPtr conn, if (virDomainObjIsDuplicate(&driver->domains, def, 1) < 0) goto cleanup; - if (qemudCanonicalizeMachine(driver, def) < 0) + if (qemuCanonicalizeMachine(def, caps) < 0) goto cleanup; if (qemuDomainAssignAddresses(def, caps, NULL) < 0) diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index e35460a..1ef4e91 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -142,8 +142,12 @@ static int testCompareXMLToArgvFiles(const char *xml, QEMU_CAPS_NO_ACPI, QEMU_CAPS_LAST); - if (qemudCanonicalizeMachine(&driver, vmdef) < 0) - goto out; + if (STREQ(vmdef->os.machine, "pc") && + STREQ(vmdef->emulator, "/usr/bin/qemu-system-x86_64")) { + VIR_FREE(vmdef->os.machine); + if (!(vmdef->os.machine = strdup("pc-0.11"))) + goto out; + } if (qemuCapsGet(extraFlags, QEMU_CAPS_DEVICE)) { if (qemuDomainAssignAddresses(vmdef, extraFlags, NULL)) { @@ -157,6 +161,10 @@ static int testCompareXMLToArgvFiles(const char *xml, VIR_FREE(log); virResetLastError(); + /* We do not call qemuCapsExtractVersionInfo() before calling + * qemuBuildCommandLine(), so we should set QEMU_CAPS_PCI_MULTIBUS for + * x86_64 and i686 architectures here. + */ if (STREQLEN(vmdef->os.arch, "x86_64", 6) || STREQLEN(vmdef->os.arch, "i686", 4)) { qemuCapsSet(extraFlags, QEMU_CAPS_PCI_MULTIBUS); diff --git a/tests/qemuxmlnstest.c b/tests/qemuxmlnstest.c index dd56091..3b06539 100644 --- a/tests/qemuxmlnstest.c +++ b/tests/qemuxmlnstest.c @@ -92,9 +92,6 @@ static int testCompareXMLToArgvFiles(const char *xml, QEMU_CAPS_NO_ACPI, QEMU_CAPS_LAST); - if (qemudCanonicalizeMachine(&driver, vmdef) < 0) - goto fail; - if (qemuCapsGet(extraFlags, QEMU_CAPS_DEVICE)) qemuDomainAssignAddresses(vmdef, extraFlags, NULL); -- 1.7.11.4
On 09/25/2012 11:59 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
When XML for a new guest is received, the machine type is immediately canonicalized into the version specific name. This involves probing QEMU for supported machine types. Replace this probing with a lookup of the machine types in the (hopefully cached) qemuCapsPtr object
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_command.h | 3 -- src/qemu/qemu_driver.c | 133 ++++++++--------------------------------------- tests/qemuxml2argvtest.c | 12 ++++- tests/qemuxmlnstest.c | 3 -- 4 files changed, 33 insertions(+), 118 deletions(-)
Again, a nice cleanup. ACK
@@ -157,6 +161,10 @@ static int testCompareXMLToArgvFiles(const char *xml, VIR_FREE(log); virResetLastError();
+ /* We do not call qemuCapsExtractVersionInfo() before calling + * qemuBuildCommandLine(), so we should set QEMU_CAPS_PCI_MULTIBUS for + * x86_64 and i686 architectures here. + */ if (STREQLEN(vmdef->os.arch, "x86_64", 6) || STREQLEN(vmdef->os.arch, "i686", 4)) { qemuCapsSet(extraFlags, QEMU_CAPS_PCI_MULTIBUS);
You've got some churn; this comment was just removed in 3/20. Did you rebase wrong? -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
From: "Daniel P. Berrange" <berrange@redhat.com> When launching a QEMU guest the binary is probed to discover the list of supported CPU names. Remove this probing with a simple lookup of CPU models in the qemuCapsPtr object. This avoids another invocation of the QEMU binary during the startup path. As a nice benefit we can now remove all the nasty hacks from the test suite which were done to avoid having to exec QEMU on the test system. The building of the -cpu command line can just rely on data we pre-populate in qemuCapsPtr. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_capabilities.c | 18 ++++++++ src/qemu/qemu_capabilities.h | 2 + src/qemu/qemu_command.c | 18 ++------ tests/qemuxml2argvdata/qemu-lib.sh | 50 --------------------- tests/qemuxml2argvdata/qemu-supported-cpus.sh | 15 ------- tests/qemuxml2argvdata/qemu.sh | 15 ------- .../qemuxml2argv-cpu-eoi-disabled.args | 2 +- .../qemuxml2argv-cpu-eoi-disabled.xml | 2 +- .../qemuxml2argv-cpu-eoi-enabled.args | 2 +- .../qemuxml2argv-cpu-eoi-enabled.xml | 2 +- .../qemuxml2argvdata/qemuxml2argv-cpu-exact1.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-cpu-exact1.xml | 2 +- .../qemuxml2argv-cpu-exact2-nofallback.args | 2 +- .../qemuxml2argv-cpu-exact2-nofallback.xml | 2 +- .../qemuxml2argvdata/qemuxml2argv-cpu-exact2.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-cpu-exact2.xml | 2 +- .../qemuxml2argv-cpu-fallback.args | 2 +- .../qemuxml2argvdata/qemuxml2argv-cpu-fallback.xml | 2 +- .../qemuxml2argv-cpu-host-kvmclock.args | 2 +- .../qemuxml2argv-cpu-host-kvmclock.xml | 2 +- .../qemuxml2argv-cpu-host-model-fallback.args | 2 +- .../qemuxml2argv-cpu-host-model-fallback.xml | 2 +- .../qemuxml2argv-cpu-host-model-nofallback.xml | 2 +- .../qemuxml2argv-cpu-host-model.args | 2 +- .../qemuxml2argv-cpu-host-model.xml | 2 +- .../qemuxml2argv-cpu-host-passthrough.args | 2 +- .../qemuxml2argv-cpu-host-passthrough.xml | 2 +- .../qemuxml2argv-cpu-kvmclock.args | 2 +- .../qemuxml2argvdata/qemuxml2argv-cpu-kvmclock.xml | 2 +- .../qemuxml2argv-cpu-minimum1.args | 2 +- .../qemuxml2argvdata/qemuxml2argv-cpu-minimum1.xml | 2 +- .../qemuxml2argv-cpu-minimum2.args | 2 +- .../qemuxml2argvdata/qemuxml2argv-cpu-minimum2.xml | 2 +- .../qemuxml2argv-cpu-nofallback.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-cpu-numa2.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-cpu-numa2.xml | 2 +- .../qemuxml2argv-cpu-qemu-host-passthrough.xml | 2 +- .../qemuxml2argvdata/qemuxml2argv-cpu-strict1.args | 2 +- .../qemuxml2argvdata/qemuxml2argv-cpu-strict1.xml | 2 +- .../qemuxml2argv-cpu-topology1.args | 2 +- .../qemuxml2argv-cpu-topology1.xml | 2 +- .../qemuxml2argv-cpu-topology2.args | 2 +- .../qemuxml2argv-cpu-topology2.xml | 2 +- .../qemuxml2argv-cpu-topology3.args | 2 +- .../qemuxml2argv-cpu-topology3.xml | 2 +- .../qemuxml2argv-eoi-disabled.args | 2 +- .../qemuxml2argvdata/qemuxml2argv-eoi-disabled.xml | 2 +- .../qemuxml2argvdata/qemuxml2argv-eoi-enabled.args | 2 +- .../qemuxml2argvdata/qemuxml2argv-eoi-enabled.xml | 2 +- .../qemuxml2argv-graphics-spice-timeout.args | 2 +- .../qemuxml2argv-graphics-spice-timeout.xml | 2 +- tests/qemuxml2argvtest.c | 52 +++++++++++++--------- .../qemuxml2xmlout-graphics-spice-timeout.xml | 2 +- 55 files changed, 104 insertions(+), 162 deletions(-) delete mode 100644 tests/qemuxml2argvdata/qemu-lib.sh delete mode 100755 tests/qemuxml2argvdata/qemu-supported-cpus.sh delete mode 100755 tests/qemuxml2argvdata/qemu.sh diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 50d3e92..888c56c 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1680,6 +1680,24 @@ unsigned int qemuCapsGetKVMVersion(qemuCapsPtr caps) } +int qemuCapsAddCPUDefinition(qemuCapsPtr caps, + const char *name) +{ + char *tmp = strdup(name); + if (!tmp) { + virReportOOMError(); + return -1; + } + if (VIR_EXPAND_N(caps->cpuDefinitions, caps->ncpuDefinitions, 1) < 0) { + VIR_FREE(tmp); + virReportOOMError(); + return -1; + } + caps->cpuDefinitions[caps->ncpuDefinitions-1] = tmp; + return 0; +} + + size_t qemuCapsGetCPUDefinitions(qemuCapsPtr caps, char ***names) { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 4b96662..24591d8 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -179,6 +179,8 @@ const char *qemuCapsGetBinary(qemuCapsPtr caps); const char *qemuCapsGetArch(qemuCapsPtr caps); unsigned int qemuCapsGetVersion(qemuCapsPtr caps); unsigned int qemuCapsGetKVMVersion(qemuCapsPtr caps); +int qemuCapsAddCPUDefinition(qemuCapsPtr caps, + const char *name); size_t qemuCapsGetCPUDefinitions(qemuCapsPtr caps, char ***names); size_t qemuCapsGetMachineTypes(qemuCapsPtr caps, diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index dd043a1..c73034d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4067,7 +4067,7 @@ qemuBuildCpuArgStr(const struct qemud_driver *driver, virCPUDefPtr guest = NULL; virCPUDefPtr cpu = NULL; size_t ncpus = 0; - const char **cpus = NULL; + char **cpus = NULL; const char *default_model; union cpuData *data = NULL; bool have_cpu = false; @@ -4089,12 +4089,8 @@ qemuBuildCpuArgStr(const struct qemud_driver *driver, const char *preferred; int hasSVM; - if (host && - qemuCapsProbeCPUModels(emulator, caps, host->arch, - &ncpus, &cpus) < 0) - goto cleanup; - - if (!ncpus || !host) { + if (!host || + (ncpus = qemuCapsGetCPUDefinitions(caps, &cpus)) == 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("CPU specification not supported by hypervisor")); goto cleanup; @@ -4163,7 +4159,7 @@ qemuBuildCpuArgStr(const struct qemud_driver *driver, guest->type = VIR_CPU_TYPE_GUEST; guest->fallback = cpu->fallback; - if (cpuDecode(guest, data, cpus, ncpus, preferred) < 0) + if (cpuDecode(guest, data, (const char **)cpus, ncpus, preferred) < 0) goto cleanup; virBufferAdd(&buf, guest->model, -1); @@ -4244,12 +4240,6 @@ cleanup: virCPUDefFree(guest); virCPUDefFree(cpu); - if (cpus) { - for (i = 0; i < ncpus; i++) - VIR_FREE(cpus[i]); - VIR_FREE(cpus); - } - return ret; no_memory: diff --git a/tests/qemuxml2argvdata/qemu-lib.sh b/tests/qemuxml2argvdata/qemu-lib.sh deleted file mode 100644 index ba19119..0000000 --- a/tests/qemuxml2argvdata/qemu-lib.sh +++ /dev/null @@ -1,50 +0,0 @@ -candidates="/usr/bin/qemu-kvm - /usr/libexec/qemu-kvm - /usr/bin/qemu-system-x86_64 - /usr/bin/qemu" -qemu= -for candidate in $candidates; do - if test -x $candidate; then - qemu=$candidate - break - fi -done - -real_qemu() -{ - if test x$qemu != x; then - exec $qemu "$@" - else - return 1 - fi -} - -faked_machine() -{ - echo "pc" -} - -faked_cpu() -{ - cat <<EOF -x86 Opteron_G3 -x86 Opteron_G2 -x86 Opteron_G1 -x86 Nehalem -x86 Penryn -x86 Conroe -x86 [n270] -x86 [athlon] -x86 [pentium3] -x86 [pentium2] -x86 [pentium] -x86 [486] -x86 [coreduo] -x86 [qemu32] -x86 [kvm64] -x86 [core2duo] -x86 [phenom] -x86 [qemu64] -x86 [host] -EOF -} diff --git a/tests/qemuxml2argvdata/qemu-supported-cpus.sh b/tests/qemuxml2argvdata/qemu-supported-cpus.sh deleted file mode 100755 index 0204f51..0000000 --- a/tests/qemuxml2argvdata/qemu-supported-cpus.sh +++ /dev/null @@ -1,15 +0,0 @@ -#! /bin/sh - -. ${0%/*}/qemu-lib.sh - -case $* in -"-M ?") - faked_machine - ;; -"-cpu ?") - faked_cpu | grep -Fv '[' - ;; -*) - real_qemu "$@" - ;; -esac diff --git a/tests/qemuxml2argvdata/qemu.sh b/tests/qemuxml2argvdata/qemu.sh deleted file mode 100755 index 5928c1b..0000000 --- a/tests/qemuxml2argvdata/qemu.sh +++ /dev/null @@ -1,15 +0,0 @@ -#! /bin/sh - -. ${0%/*}/qemu-lib.sh - -case $* in -"-M ?") - faked_machine - ;; -"-cpu ?") - faked_cpu - ;; -*) - real_qemu "$@" - ;; -esac diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-eoi-disabled.args b/tests/qemuxml2argvdata/qemuxml2argv-cpu-eoi-disabled.args index 6d57f91..aee85c7 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-eoi-disabled.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-eoi-disabled.args @@ -1,4 +1,4 @@ -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test ./qemu.sh -S -M pc \ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc \ -cpu qemu32,-kvm_pv_eoi -m 214 -smp 6 -nographic -monitor \ unix:/tmp/test-monitor,server,nowait -boot n -net none -serial none \ -parallel none -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-eoi-disabled.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-eoi-disabled.xml index 467df30..5e5bc04 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-eoi-disabled.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-eoi-disabled.xml @@ -21,7 +21,7 @@ <on_reboot>restart</on_reboot> <on_crash>destroy</on_crash> <devices> - <emulator>/./qemu.sh</emulator> + <emulator>/usr/bin/qemu</emulator> <controller type='usb' index='0'/> <memballoon model='virtio'/> </devices> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-eoi-enabled.args b/tests/qemuxml2argvdata/qemuxml2argv-cpu-eoi-enabled.args index 3dc4310..3442caf 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-eoi-enabled.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-eoi-enabled.args @@ -1,4 +1,4 @@ -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test ./qemu.sh -S -M pc \ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc \ -cpu qemu32,+kvm_pv_eoi -m 214 -smp 6 -nographic -monitor \ unix:/tmp/test-monitor,server,nowait -boot n -net none -serial none \ -parallel none -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-eoi-enabled.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-eoi-enabled.xml index 1ed630a..ecc542e 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-eoi-enabled.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-eoi-enabled.xml @@ -21,7 +21,7 @@ <on_reboot>restart</on_reboot> <on_crash>destroy</on_crash> <devices> - <emulator>/./qemu.sh</emulator> + <emulator>/usr/bin/qemu</emulator> <controller type='usb' index='0'/> <memballoon model='virtio'/> </devices> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-exact1.args b/tests/qemuxml2argvdata/qemuxml2argv-cpu-exact1.args index 83f848f..fce9ad1 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-exact1.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-exact1.args @@ -1,4 +1,4 @@ -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test ./qemu.sh -S -M pc \ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc \ -cpu qemu64,-svm,-lm,-nx,-syscall,-clflush,-pse36,-mca -m 214 -smp 6 \ -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot n -net \ none -serial none -parallel none -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-exact1.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-exact1.xml index 9165fe3..ddd9d5a 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-exact1.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-exact1.xml @@ -23,6 +23,6 @@ <on_reboot>restart</on_reboot> <on_crash>destroy</on_crash> <devices> - <emulator>/./qemu.sh</emulator> + <emulator>/usr/bin/qemu</emulator> </devices> </domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-exact2-nofallback.args b/tests/qemuxml2argvdata/qemuxml2argv-cpu-exact2-nofallback.args index 198d0d8..7f91bc7 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-exact2-nofallback.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-exact2-nofallback.args @@ -1,4 +1,4 @@ -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test ./qemu.sh -S -M pc \ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc \ -cpu core2duo,+lahf_lm,+3dnowext,+xtpr,+ds_cpl,+tm,+ht,+ds,-nx -m 214 -smp 6 \ -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot n -net \ none -serial none -parallel none -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-exact2-nofallback.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-exact2-nofallback.xml index 67785a9..de4c8d2 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-exact2-nofallback.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-exact2-nofallback.xml @@ -30,6 +30,6 @@ <on_reboot>restart</on_reboot> <on_crash>destroy</on_crash> <devices> - <emulator>/./qemu.sh</emulator> + <emulator>/usr/bin/qemu</emulator> </devices> </domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-exact2.args b/tests/qemuxml2argvdata/qemuxml2argv-cpu-exact2.args index 198d0d8..7f91bc7 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-exact2.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-exact2.args @@ -1,4 +1,4 @@ -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test ./qemu.sh -S -M pc \ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc \ -cpu core2duo,+lahf_lm,+3dnowext,+xtpr,+ds_cpl,+tm,+ht,+ds,-nx -m 214 -smp 6 \ -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot n -net \ none -serial none -parallel none -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-exact2.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-exact2.xml index 575541a..e027e6f 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-exact2.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-exact2.xml @@ -30,6 +30,6 @@ <on_reboot>restart</on_reboot> <on_crash>destroy</on_crash> <devices> - <emulator>/./qemu.sh</emulator> + <emulator>/usr/bin/qemu</emulator> </devices> </domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-fallback.args b/tests/qemuxml2argvdata/qemuxml2argv-cpu-fallback.args index 658f141..fe1b470 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-fallback.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-fallback.args @@ -3,7 +3,7 @@ PATH=/bin \ HOME=/home/test \ USER=test \ LOGNAME=test \ -./qemu.sh \ +/usr/bin/qemu \ -S \ -M pc \ -cpu Penryn,-sse4.1 \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-fallback.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-fallback.xml index 8f5987b..6125f41 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-fallback.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-fallback.xml @@ -20,6 +20,6 @@ <on_reboot>restart</on_reboot> <on_crash>destroy</on_crash> <devices> - <emulator>/./qemu.sh</emulator> + <emulator>/usr/bin/qemu</emulator> </devices> </domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-host-kvmclock.args b/tests/qemuxml2argvdata/qemuxml2argv-cpu-host-kvmclock.args index 8472b8a..5e1e759 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-host-kvmclock.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-host-kvmclock.args @@ -1,4 +1,4 @@ -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test ./qemu.sh -S -M pc \ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc \ -cpu host,-kvmclock -enable-kvm -m 214 -smp 6 \ -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot n -net \ none -serial none -parallel none -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-host-kvmclock.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-host-kvmclock.xml index cfbf440..16d71a3 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-host-kvmclock.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-host-kvmclock.xml @@ -17,7 +17,7 @@ <on_reboot>restart</on_reboot> <on_crash>destroy</on_crash> <devices> - <emulator>/./qemu.sh</emulator> + <emulator>/usr/bin/qemu</emulator> <controller type='usb' index='0'/> <memballoon model='virtio'/> </devices> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-host-model-fallback.args b/tests/qemuxml2argvdata/qemuxml2argv-cpu-host-model-fallback.args index ac8ab1a..6ec201a 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-host-model-fallback.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-host-model-fallback.args @@ -3,7 +3,7 @@ PATH=/bin \ HOME=/home/test \ USER=test \ LOGNAME=test \ -./qemu-supported-cpus.sh \ +/usr/bin/qemu \ -S \ -M pc \ -cpu Penryn,+xtpr,+tm2,+est,+vmx,+ds_cpl,+monitor,+pbe,+tm,+ht,+ss,+acpi,+ds,+vme,-sse4.1 \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-host-model-fallback.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-host-model-fallback.xml index 41d455f..a1136e2 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-host-model-fallback.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-host-model-fallback.xml @@ -14,6 +14,6 @@ <on_reboot>restart</on_reboot> <on_crash>destroy</on_crash> <devices> - <emulator>/./qemu-supported-cpus.sh</emulator> + <emulator>/usr/bin/qemu</emulator> </devices> </domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-host-model-nofallback.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-host-model-nofallback.xml index e2b4b83..d0219d5 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-host-model-nofallback.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-host-model-nofallback.xml @@ -16,6 +16,6 @@ <on_reboot>restart</on_reboot> <on_crash>destroy</on_crash> <devices> - <emulator>/./qemu-supported-cpus.sh</emulator> + <emulator>/usr/bin/qemu</emulator> </devices> </domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-host-model.args b/tests/qemuxml2argvdata/qemuxml2argv-cpu-host-model.args index cf7eb2a..426c1c0 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-host-model.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-host-model.args @@ -3,7 +3,7 @@ PATH=/bin \ HOME=/home/test \ USER=test \ LOGNAME=test \ -./qemu.sh \ +/usr/bin/qemu \ -S \ -M pc \ -cpu core2duo,+lahf_lm,+xtpr,+cx16,+tm2,+est,+vmx,+ds_cpl,+pbe,+tm,+ht,+ss,+acpi,+ds \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-host-model.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-host-model.xml index b7b95d7..7e3f617 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-host-model.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-host-model.xml @@ -14,6 +14,6 @@ <on_reboot>restart</on_reboot> <on_crash>destroy</on_crash> <devices> - <emulator>/./qemu.sh</emulator> + <emulator>/usr/bin/qemu</emulator> </devices> </domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-host-passthrough.args b/tests/qemuxml2argvdata/qemuxml2argv-cpu-host-passthrough.args index c63ecce..a7c6a6a 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-host-passthrough.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-host-passthrough.args @@ -3,7 +3,7 @@ PATH=/bin \ HOME=/home/test \ USER=test \ LOGNAME=test \ -./qemu.sh \ +/usr/bin/qemu \ -S \ -M pc \ -cpu host \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-host-passthrough.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-host-passthrough.xml index f591a17..f1233e1 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-host-passthrough.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-host-passthrough.xml @@ -14,6 +14,6 @@ <on_reboot>restart</on_reboot> <on_crash>destroy</on_crash> <devices> - <emulator>/./qemu.sh</emulator> + <emulator>/usr/bin/qemu</emulator> </devices> </domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-kvmclock.args b/tests/qemuxml2argvdata/qemuxml2argv-cpu-kvmclock.args index 6816c00..16bcc44 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-kvmclock.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-kvmclock.args @@ -1,4 +1,4 @@ -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test ./qemu.sh -S -M pc \ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc \ -cpu core2duo,-kvmclock -enable-kvm -m 214 -smp 6 \ -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot n -net \ none -serial none -parallel none -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-kvmclock.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-kvmclock.xml index 304d88c..0bbe8e0 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-kvmclock.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-kvmclock.xml @@ -18,7 +18,7 @@ <on_reboot>restart</on_reboot> <on_crash>destroy</on_crash> <devices> - <emulator>/./qemu.sh</emulator> + <emulator>/usr/bin/qemu</emulator> <controller type='usb' index='0'/> <memballoon model='virtio'/> </devices> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-minimum1.args b/tests/qemuxml2argvdata/qemuxml2argv-cpu-minimum1.args index df57c48..22b6a1e 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-minimum1.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-minimum1.args @@ -1,4 +1,4 @@ -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test ./qemu.sh -S -M pc \ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc \ -cpu core2duo,+lahf_lm,+xtpr,+cx16,+tm2,+est,+vmx,+ds_cpl,+pbe,+tm,+ht,+ss,\ +acpi,+ds -m 214 -smp 6 -nographic -monitor unix:/tmp/test-monitor,server,\ nowait -no-acpi -boot n -net none -serial none -parallel none -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-minimum1.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-minimum1.xml index 42026ab..4ba5d0b 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-minimum1.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-minimum1.xml @@ -16,6 +16,6 @@ <on_reboot>restart</on_reboot> <on_crash>destroy</on_crash> <devices> - <emulator>/./qemu.sh</emulator> + <emulator>/usr/bin/qemu</emulator> </devices> </domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-minimum2.args b/tests/qemuxml2argvdata/qemuxml2argv-cpu-minimum2.args index 9fb1648..cd615a2 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-minimum2.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-minimum2.args @@ -1,4 +1,4 @@ -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test ./qemu.sh -S -M pc \ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc \ -cpu core2duo,+lahf_lm,+xtpr,+cx16,+tm2,+est,+vmx,+ds_cpl,+pbe,+tm,+ht,+ss,\ +acpi,+ds,-lm,-nx,-syscall -m 214 -smp 6 -nographic -monitor \ unix:/tmp/test-monitor,server,nowait -no-acpi -boot n -net none -serial none \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-minimum2.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-minimum2.xml index beb0551..c43bf4f 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-minimum2.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-minimum2.xml @@ -20,6 +20,6 @@ <on_reboot>restart</on_reboot> <on_crash>destroy</on_crash> <devices> - <emulator>/./qemu.sh</emulator> + <emulator>/usr/bin/qemu</emulator> </devices> </domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-nofallback.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-nofallback.xml index 31db010..4ae0be8 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-nofallback.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-nofallback.xml @@ -20,6 +20,6 @@ <on_reboot>restart</on_reboot> <on_crash>destroy</on_crash> <devices> - <emulator>/./qemu.sh</emulator> + <emulator>/usr/bin/qemu</emulator> </devices> </domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.args b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.args index 7c0dd30..db70657 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.args @@ -1,4 +1,4 @@ -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test ./qemu.sh -S -M pc \ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc \ -m 214 -smp 16 -numa node,nodeid=0,cpus=0-7,mem=107 \ -numa node,nodeid=1,cpus=8-15,mem=107 -nographic -monitor \ unix:/tmp/test-monitor,server,nowait -no-acpi -boot n -net none -serial none \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.xml index 53cc294..ee402c8 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.xml @@ -20,6 +20,6 @@ <on_reboot>restart</on_reboot> <on_crash>destroy</on_crash> <devices> - <emulator>/./qemu.sh</emulator> + <emulator>/usr/bin/qemu</emulator> </devices> </domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa2.args b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa2.args index 2ac2568..1b2154d 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa2.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa2.args @@ -1,4 +1,4 @@ -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test ./qemu.sh -S -M pc \ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc \ -m 214 -smp 16,sockets=2,cores=4,threads=2 \ -numa node,nodeid=0,cpus=0-7,mem=107 \ -numa node,nodeid=1,cpus=8-15,mem=107 -nographic -monitor \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa2.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa2.xml index 53cc294..ee402c8 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa2.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa2.xml @@ -20,6 +20,6 @@ <on_reboot>restart</on_reboot> <on_crash>destroy</on_crash> <devices> - <emulator>/./qemu.sh</emulator> + <emulator>/usr/bin/qemu</emulator> </devices> </domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-qemu-host-passthrough.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-qemu-host-passthrough.xml index b8fbc51..7652a97 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-qemu-host-passthrough.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-qemu-host-passthrough.xml @@ -14,6 +14,6 @@ <on_reboot>restart</on_reboot> <on_crash>destroy</on_crash> <devices> - <emulator>/./qemu.sh</emulator> + <emulator>/usr/bin/qemu</emulator> </devices> </domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-strict1.args b/tests/qemuxml2argvdata/qemuxml2argv-cpu-strict1.args index 3d0c61a..41a6749 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-strict1.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-strict1.args @@ -1,4 +1,4 @@ -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test ./qemu.sh -S -M pc \ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc \ -cpu core2duo,+lahf_lm,+3dnowext,+xtpr,+est,+vmx,+ds_cpl,+tm,+ht,+acpi,+ds,-nx \ -m 214 -smp 6 -nographic -monitor unix:/tmp/test-monitor,server,nowait \ -no-acpi -boot n -net none -serial none -parallel none -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-strict1.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-strict1.xml index 02df183..935f46f 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-strict1.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-strict1.xml @@ -33,6 +33,6 @@ <on_reboot>restart</on_reboot> <on_crash>destroy</on_crash> <devices> - <emulator>/./qemu.sh</emulator> + <emulator>/usr/bin/qemu</emulator> </devices> </domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-topology1.args b/tests/qemuxml2argvdata/qemuxml2argv-cpu-topology1.args index 25e56ab..89f07a9 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-topology1.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-topology1.args @@ -1,4 +1,4 @@ -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test ./qemu.sh -S -M pc \ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc \ -m 214 -smp 6,sockets=3,cores=2,threads=1 -nographic -monitor \ unix:/tmp/test-monitor,server,nowait -no-acpi -boot n -net none -serial none \ -parallel none -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-topology1.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-topology1.xml index 64783d1..6f70aa3 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-topology1.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-topology1.xml @@ -16,6 +16,6 @@ <on_reboot>restart</on_reboot> <on_crash>destroy</on_crash> <devices> - <emulator>/./qemu.sh</emulator> + <emulator>/usr/bin/qemu</emulator> </devices> </domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-topology2.args b/tests/qemuxml2argvdata/qemuxml2argv-cpu-topology2.args index 348b757..8a65ae2 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-topology2.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-topology2.args @@ -1,4 +1,4 @@ -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test ./qemu.sh -S -M pc \ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc \ -cpu core2duo -m 214 -smp 6,sockets=1,cores=2,threads=3 -nographic -monitor \ unix:/tmp/test-monitor,server,nowait -no-acpi -boot n -net none -serial none \ -parallel none -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-topology2.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-topology2.xml index 6f16308..ab561fd 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-topology2.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-topology2.xml @@ -17,6 +17,6 @@ <on_reboot>restart</on_reboot> <on_crash>destroy</on_crash> <devices> - <emulator>/./qemu.sh</emulator> + <emulator>/usr/bin/qemu</emulator> </devices> </domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-topology3.args b/tests/qemuxml2argvdata/qemuxml2argv-cpu-topology3.args index 57b2eea..21e51c7 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-topology3.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-topology3.args @@ -1,3 +1,3 @@ -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test ./qemu.sh -S -M pc \ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc \ -m 214 -smp 6 -nographic -monitor unix:/tmp/test-monitor,server,nowait \ -no-acpi -boot n -net none -serial none -parallel none -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-topology3.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-topology3.xml index 64783d1..6f70aa3 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-topology3.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-topology3.xml @@ -16,6 +16,6 @@ <on_reboot>restart</on_reboot> <on_crash>destroy</on_crash> <devices> - <emulator>/./qemu.sh</emulator> + <emulator>/usr/bin/qemu</emulator> </devices> </domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-eoi-disabled.args b/tests/qemuxml2argvdata/qemuxml2argv-eoi-disabled.args index 93475bd..1d610f6 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-eoi-disabled.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-eoi-disabled.args @@ -1,4 +1,4 @@ -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test ./qemu.sh -S -M pc \ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc \ -cpu qemu32,-kvm_pv_eoi -m 214 -smp 6 -nographic -monitor \ unix:/tmp/test-monitor,server,nowait -boot n -net none -serial \ none -parallel none -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-eoi-disabled.xml b/tests/qemuxml2argvdata/qemuxml2argv-eoi-disabled.xml index f84570e..3173a41 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-eoi-disabled.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-eoi-disabled.xml @@ -18,7 +18,7 @@ <on_reboot>restart</on_reboot> <on_crash>destroy</on_crash> <devices> - <emulator>/./qemu.sh</emulator> + <emulator>/usr/bin/qemu</emulator> <controller type='usb' index='0'/> <memballoon model='virtio'/> </devices> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-eoi-enabled.args b/tests/qemuxml2argvdata/qemuxml2argv-eoi-enabled.args index 13f570b..02c8da3 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-eoi-enabled.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-eoi-enabled.args @@ -1,4 +1,4 @@ -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test ./qemu.sh -S -M pc \ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc \ -cpu qemu32,+kvm_pv_eoi -m 214 -smp 6 -nographic -monitor \ unix:/tmp/test-monitor,server,nowait -boot n -net none -serial \ none -parallel none -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-eoi-enabled.xml b/tests/qemuxml2argvdata/qemuxml2argv-eoi-enabled.xml index 03b6b52..22f0803 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-eoi-enabled.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-eoi-enabled.xml @@ -18,7 +18,7 @@ <on_reboot>restart</on_reboot> <on_crash>destroy</on_crash> <devices> - <emulator>/./qemu.sh</emulator> + <emulator>/usr/bin/qemu</emulator> <controller type='usb' index='0'/> <memballoon model='virtio'/> </devices> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-timeout.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-timeout.args index ebda714..453805a 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-timeout.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-timeout.args @@ -1,5 +1,5 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=spice \ -./qemu.sh -S -M pc -cpu core2duo,+lahf_lm,+xtpr,+cx16,+tm2,\ +/usr/bin/qemu -S -M pc -cpu core2duo,+lahf_lm,+xtpr,+cx16,+tm2,\ +est,+vmx,+ds_cpl,+pbe,+tm,+ht,+ss,+acpi,+ds \ -m 1024 -smp 2 -nodefaults -monitor unix:/tmp/test-monitor,server,nowait \ -boot dc -device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x6 \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-timeout.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-timeout.xml index b4a75f4..f9fdf37 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-timeout.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-timeout.xml @@ -38,7 +38,7 @@ <on_reboot>restart</on_reboot> <on_crash>restart</on_crash> <devices> - <emulator>/./qemu.sh</emulator> + <emulator>/usr/bin/qemu</emulator> <disk type='file' device='disk'> <driver name='qemu' type='qcow2'/> <source file='/var/lib/libvirt/images/f14.img'/> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 1ef4e91..c42774b 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -106,26 +106,6 @@ static int testCompareXMLToArgvFiles(const char *xml, goto out; } - /* - * For test purposes, we may want to fake emulator's output by providing - * our own script instead of a real emulator. For this to work we need to - * specify a relative path in <emulator/> element, which, however, is not - * allowed by RelaxNG schema for domain XML. To work around it we add an - * extra '/' at the beginning of relative emulator path so that it looks - * like, e.g., "/./qemu.sh" or "/../emulator/qemu.sh" instead of - * "./qemu.sh" or "../emulator/qemu.sh" respectively. The following code - * detects such paths, strips the extra '/' and makes the path absolute. - */ - if (vmdef->emulator && STRPREFIX(vmdef->emulator, "/.")) { - if (!(emulator = strdup(vmdef->emulator + 1))) - goto out; - VIR_FREE(vmdef->emulator); - vmdef->emulator = NULL; - if (virAsprintf(&vmdef->emulator, "%s/qemuxml2argvdata/%s", - abs_srcdir, emulator) < 0) - goto out; - } - if (qemuCapsGet(extraFlags, QEMU_CAPS_DOMID)) vmdef->id = 6; else @@ -274,12 +254,40 @@ cleanup: } +static int +testAddCPUModels(qemuCapsPtr caps, bool skipLegacy) +{ + const char *newModels[] = { + "Opteron_G3", "Opteron_G2", "Opteron_G1", + "Nehalem", "Penryn", "Conroe", + }; + const char *legacyModels[] = { + "n270", "athlon", "pentium3", "pentium2", "pentium", + "486", "coreduo", "kvm32", "qemu32", "kvm64", + "core2duo", "phenom", "qemu64", + }; + size_t i; + + for (i = 0 ; i < ARRAY_CARDINALITY(newModels) ; i++) { + if (qemuCapsAddCPUDefinition(caps, newModels[i]) < 0) + return -1; + } + if (skipLegacy) + return 0; + for (i = 0 ; i < ARRAY_CARDINALITY(legacyModels) ; i++) { + if (qemuCapsAddCPUDefinition(caps, legacyModels[i]) < 0) + return -1; + } + return 0; +} + static int mymain(void) { int ret = 0; char *map = NULL; + bool skipLegacyCPUs = false; abs_top_srcdir = getenv("abs_top_srcdir"); if (!abs_top_srcdir) @@ -311,6 +319,8 @@ mymain(void) }; \ if (!(info.extraFlags = qemuCapsNew())) \ return EXIT_FAILURE; \ + if (testAddCPUModels(info.extraFlags, skipLegacyCPUs) < 0) \ + return EXIT_FAILURE; \ qemuCapsSetList(info.extraFlags, __VA_ARGS__, QEMU_CAPS_LAST); \ if (virtTestRun("QEMU XML-2-ARGV " name, \ 1, testCompareXMLToArgvHelper, &info) < 0) \ @@ -770,8 +780,10 @@ mymain(void) DO_TEST("cpu-numa1", NONE); DO_TEST("cpu-numa2", QEMU_CAPS_SMP_TOPOLOGY); DO_TEST("cpu-host-model", NONE); + skipLegacyCPUs = true; DO_TEST("cpu-host-model-fallback", NONE); DO_TEST_FAILURE("cpu-host-model-nofallback", NONE); + skipLegacyCPUs = false; DO_TEST("cpu-host-passthrough", QEMU_CAPS_KVM, QEMU_CAPS_CPU_HOST); DO_TEST_FAILURE("cpu-host-passthrough", NONE); DO_TEST_FAILURE("cpu-qemu-host-passthrough", diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-spice-timeout.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-spice-timeout.xml index 574d474..cd19b64 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-spice-timeout.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-spice-timeout.xml @@ -38,7 +38,7 @@ <on_reboot>restart</on_reboot> <on_crash>restart</on_crash> <devices> - <emulator>/./qemu.sh</emulator> + <emulator>/usr/bin/qemu</emulator> <disk type='file' device='disk'> <driver name='qemu' type='qcow2'/> <source file='/var/lib/libvirt/images/f14.img'/> -- 1.7.11.4
On Tue, Sep 25, 2012 at 18:59:58 +0100, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
When launching a QEMU guest the binary is probed to discover the list of supported CPU names. Remove this probing with a simple lookup of CPU models in the qemuCapsPtr object. This avoids another invocation of the QEMU binary during the startup path.
As a nice benefit we can now remove all the nasty hacks from the test suite which were done to avoid having to exec QEMU on the test system. The building of the -cpu command line can just rely on data we pre-populate in qemuCapsPtr.
Nice, I wanted to do this since I first implemented CPU model probing :-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index dd043a1..c73034d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4067,7 +4067,7 @@ qemuBuildCpuArgStr(const struct qemud_driver *driver, virCPUDefPtr guest = NULL; virCPUDefPtr cpu = NULL; size_t ncpus = 0; - const char **cpus = NULL; + char **cpus = NULL; const char *default_model; union cpuData *data = NULL; bool have_cpu = false; @@ -4089,12 +4089,8 @@ qemuBuildCpuArgStr(const struct qemud_driver *driver, const char *preferred; int hasSVM;
- if (host && - qemuCapsProbeCPUModels(emulator, caps, host->arch, - &ncpus, &cpus) < 0) - goto cleanup; - - if (!ncpus || !host) { + if (!host || + (ncpus = qemuCapsGetCPUDefinitions(caps, &cpus)) == 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("CPU specification not supported by hypervisor")); goto cleanup; @@ -4163,7 +4159,7 @@ qemuBuildCpuArgStr(const struct qemud_driver *driver,
guest->type = VIR_CPU_TYPE_GUEST; guest->fallback = cpu->fallback; - if (cpuDecode(guest, data, cpus, ncpus, preferred) < 0) + if (cpuDecode(guest, data, (const char **)cpus, ncpus, preferred) < 0)
Hmm, is this typecast really necessary?
goto cleanup;
virBufferAdd(&buf, guest->model, -1);
... ACK Jirka
From: "Daniel P. Berrange" <berrange@redhat.com> The qemuCapsProbeMachineTypes & qemuCapsProbeCPUModels methods do not need to be invoked directly anymore. Make them static and refactor them to directly populate the qemuCapsPtr object Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_capabilities.c | 238 ++++++++++++++++--------------------------- src/qemu/qemu_capabilities.h | 13 +-- 2 files changed, 90 insertions(+), 161 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 888c56c..7e4ea50 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -305,17 +305,16 @@ qemuCapsProbeCommand(const char *qemu, */ static int qemuCapsParseMachineTypesStr(const char *output, - virCapsGuestMachinePtr **machines, - size_t *nmachines) + qemuCapsPtr caps) { const char *p = output; const char *next; - virCapsGuestMachinePtr *list = NULL; - int nitems = 0; + size_t defIdx = 0; do { const char *t; - virCapsGuestMachinePtr machine; + char *name; + char *canonical = NULL; if ((next = strchr(p, '\n'))) ++next; @@ -326,56 +325,61 @@ qemuCapsParseMachineTypesStr(const char *output, if (!(t = strchr(p, ' ')) || (next && t >= next)) continue; - if (VIR_ALLOC(machine) < 0) + if (!(name = strndup(p, t - p))) goto no_memory; - if (!(machine->name = strndup(p, t - p))) { - VIR_FREE(machine); - goto no_memory; - } - - if (VIR_REALLOC_N(list, nitems + 1) < 0) { - VIR_FREE(machine->name); - VIR_FREE(machine); - goto no_memory; - } - - p = t; - if (!(t = strstr(p, "(default)")) || (next && t >= next)) { - list[nitems++] = machine; - } else { - /* put the default first in the list */ - memmove(list + 1, list, sizeof(*list) * nitems); - list[0] = machine; - nitems++; - } + if (strstr(p, "(default)")) + defIdx = caps->nmachineTypes; if ((t = strstr(p, "(alias of ")) && (!next || t < next)) { p = t + strlen("(alias of "); if (!(t = strchr(p, ')')) || (next && t >= next)) continue; - if (!(machine->canonical = strndup(p, t - p))) + if (!(canonical = strndup(p, t - p))) { + VIR_FREE(name); goto no_memory; + } + } + + if (VIR_REALLOC_N(caps->machineTypes, caps->nmachineTypes + 1) < 0 || + VIR_REALLOC_N(caps->machineAliases, caps->nmachineTypes + 1) < 0) { + VIR_FREE(name); + VIR_FREE(canonical); + goto no_memory; + } + caps->nmachineTypes++; + if (canonical) { + caps->machineTypes[caps->nmachineTypes-1] = canonical; + caps->machineAliases[caps->nmachineTypes-1] = name; + } else { + caps->machineTypes[caps->nmachineTypes-1] = name; } } while ((p = next)); - *machines = list; - *nmachines = nitems; + + if (defIdx != 0) { + char *name = caps->machineTypes[defIdx]; + char *alias = caps->machineAliases[defIdx]; + memmove(caps->machineTypes + 1, + caps->machineTypes, + sizeof(caps->machineTypes[0]) * defIdx); + memmove(caps->machineAliases + 1, + caps->machineAliases, + sizeof(caps->machineAliases[0]) * defIdx); + caps->machineTypes[0] = name; + caps->machineAliases[0] = alias; + } return 0; no_memory: virReportOOMError(); - virCapabilitiesFreeMachines(list, nitems); return -1; } -int -qemuCapsProbeMachineTypes(const char *binary, - qemuCapsPtr caps, - virCapsGuestMachinePtr **machines, - size_t *nmachines) +static int +qemuCapsProbeMachineTypes(qemuCapsPtr caps) { char *output; int ret = -1; @@ -386,12 +390,13 @@ qemuCapsProbeMachineTypes(const char *binary, * 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, _("Cannot find QEMU binary %s"), binary); + if (!virFileIsExecutable(caps->binary)) { + virReportSystemError(errno, _("Cannot find QEMU binary %s"), + caps->binary); return -1; } - cmd = qemuCapsProbeCommand(binary, caps); + cmd = qemuCapsProbeCommand(caps->binary, caps); virCommandAddArgList(cmd, "-M", "?", NULL); virCommandSetOutputBuffer(cmd, &output); @@ -399,7 +404,7 @@ qemuCapsProbeMachineTypes(const char *binary, if (virCommandRun(cmd, &status) < 0) goto cleanup; - if (qemuCapsParseMachineTypesStr(output, machines, nmachines) < 0) + if (qemuCapsParseMachineTypesStr(output, caps) < 0) goto cleanup; ret = 0; @@ -414,8 +419,7 @@ cleanup: typedef int (*qemuCapsParseCPUModels)(const char *output, - size_t *retcount, - const char ***retcpus); + qemuCapsPtr caps); /* Format: * <arch> <model> @@ -424,17 +428,15 @@ typedef int */ static int qemuCapsParseX86Models(const char *output, - size_t *retcount, - const char ***retcpus) + qemuCapsPtr caps) { const char *p = output; const char *next; - unsigned int count = 0; - const char **cpus = NULL; - int i; + int ret = -1; do { const char *t; + size_t len; if ((next = strchr(p, '\n'))) next++; @@ -452,47 +454,31 @@ qemuCapsParseX86Models(const char *output, if (*p == '\0' || *p == '\n') continue; - if (retcpus) { - unsigned int len; - - if (VIR_REALLOC_N(cpus, count + 1) < 0) { - virReportOOMError(); - goto error; - } + if (VIR_EXPAND_N(caps->cpuDefinitions, caps->ncpuDefinitions, 1) < 0) { + virReportOOMError(); + goto cleanup; + } - if (next) - len = next - p - 1; - else - len = strlen(p); + if (next) + len = next - p - 1; + else + len = strlen(p); - if (len > 2 && *p == '[' && p[len - 1] == ']') { - p++; - len -= 2; - } + if (len > 2 && *p == '[' && p[len - 1] == ']') { + p++; + len -= 2; + } - if (!(cpus[count] = strndup(p, len))) { - virReportOOMError(); - goto error; - } + if (!(caps->cpuDefinitions[caps->ncpuDefinitions - 1] = strndup(p, len))) { + virReportOOMError(); + goto cleanup; } - count++; } while ((p = next)); - if (retcount) - *retcount = count; - if (retcpus) - *retcpus = cpus; - - return 0; - -error: - if (cpus) { - for (i = 0; i < count; i++) - VIR_FREE(cpus[i]); - } - VIR_FREE(cpus); + ret = 0; - return -1; +cleanup: + return ret; } /* ppc64 parser. @@ -500,17 +486,15 @@ error: */ static int qemuCapsParsePPCModels(const char *output, - size_t *retcount, - const char ***retcpus) + qemuCapsPtr caps) { const char *p = output; const char *next; - unsigned int count = 0; - const char **cpus = NULL; - int i, ret = -1; + int ret = -1; do { const char *t; + size_t len; if ((next = strchr(p, '\n'))) next++; @@ -531,75 +515,52 @@ qemuCapsParsePPCModels(const char *output, if (*p == '\n') continue; - if (retcpus) { - unsigned int len; - - if (VIR_REALLOC_N(cpus, count + 1) < 0) { - virReportOOMError(); - goto cleanup; - } + if (VIR_EXPAND_N(caps->cpuDefinitions, caps->ncpuDefinitions, 1) < 0) { + virReportOOMError(); + goto cleanup; + } - len = t - p - 1; + len = t - p - 1; - if (!(cpus[count] = strndup(p, len))) { - virReportOOMError(); - goto cleanup; - } + if (!(caps->cpuDefinitions[caps->ncpuDefinitions - 1] = strndup(p, len))) { + virReportOOMError(); + goto cleanup; } - count++; } while ((p = next)); - if (retcount) - *retcount = count; - if (retcpus) { - *retcpus = cpus; - cpus = NULL; - } ret = 0; cleanup: - if (cpus) { - for (i = 0; i < count; i++) - VIR_FREE(cpus[i]); - VIR_FREE(cpus); - } return ret; } -int -qemuCapsProbeCPUModels(const char *qemu, - qemuCapsPtr caps, - const char *arch, - size_t *count, - const char ***cpus) +static int +qemuCapsProbeCPUModels(qemuCapsPtr caps) { char *output = NULL; int ret = -1; qemuCapsParseCPUModels parse; virCommandPtr cmd; - if (count) - *count = 0; - if (cpus) - *cpus = NULL; - - if (STREQ(arch, "i686") || STREQ(arch, "x86_64")) + if (STREQ(caps->arch, "i686") || + STREQ(caps->arch, "x86_64")) parse = qemuCapsParseX86Models; - else if (STREQ(arch, "ppc64")) + else if (STREQ(caps->arch, "ppc64")) parse = qemuCapsParsePPCModels; else { - VIR_DEBUG("don't know how to parse %s CPU models", arch); + VIR_DEBUG("don't know how to parse %s CPU models", + caps->arch); return 0; } - cmd = qemuCapsProbeCommand(qemu, caps); + cmd = qemuCapsProbeCommand(caps->binary, caps); virCommandAddArgList(cmd, "-cpu", "?", NULL); virCommandSetOutputBuffer(cmd, &output); if (virCommandRun(cmd, NULL) < 0) goto cleanup; - if (parse(output, count, cpus) < 0) + if (parse(output, caps) < 0) goto cleanup; ret = 0; @@ -1782,9 +1743,6 @@ qemuCapsPtr qemuCapsNewForBinary(const char *binary) unsigned int is_kvm; char *help = NULL; virCommandPtr cmd = NULL; - virCapsGuestMachinePtr *machines = NULL; - size_t nmachines; - size_t i; struct stat sb; if (!(caps->binary = strdup(binary))) @@ -1853,31 +1811,12 @@ qemuCapsPtr qemuCapsNewForBinary(const char *binary) qemuCapsExtractDeviceStr(binary, caps) < 0) goto error; - if (qemuCapsProbeCPUModels(binary, caps, caps->arch, - &caps->ncpuDefinitions, - (const char ***)&caps->cpuDefinitions) < 0) + if (qemuCapsProbeCPUModels(caps) < 0) goto error; - if (qemuCapsProbeMachineTypes(binary, caps, - &machines, &nmachines) < 0) + if (qemuCapsProbeMachineTypes(caps) < 0) goto error; - if (VIR_ALLOC_N(caps->machineTypes, nmachines) < 0) - goto no_memory; - if (VIR_ALLOC_N(caps->machineAliases, nmachines) < 0) - goto no_memory; - caps->nmachineTypes = nmachines; - - for (i = 0 ; i < caps->nmachineTypes ; i++) { - if (machines[i]->canonical) { - caps->machineTypes[i] = machines[i]->canonical; - caps->machineAliases[i] = machines[i]->name; - } else { - caps->machineTypes[i] = machines[i]->name; - } - } - VIR_FREE(machines); - cleanup: VIR_FREE(help); virCommandFree(cmd); @@ -1886,7 +1825,6 @@ cleanup: no_memory: virReportOOMError(); error: - virCapabilitiesFreeMachines(machines, nmachines); virObjectUnref(caps); caps = NULL; goto cleanup; diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 24591d8..c6e4dd8 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -202,21 +202,11 @@ void qemuCapsCacheFree(qemuCapsCachePtr cache); virCapsPtr qemuCapsInit(qemuCapsCachePtr cache); -int qemuCapsProbeMachineTypes(const char *binary, - qemuCapsPtr caps, - virCapsGuestMachinePtr **machines, - size_t *nmachines); - -int qemuCapsProbeCPUModels(const char *qemu, - qemuCapsPtr caps, - const char *arch, - size_t *count, - const char ***cpus); - int qemuCapsGetDefaultVersion(virCapsPtr caps, qemuCapsCachePtr capsCache, unsigned int *version); +/* Only for use by test suite */ int qemuCapsParseHelpStr(const char *qemu, const char *str, qemuCapsPtr caps, @@ -224,6 +214,7 @@ int qemuCapsParseHelpStr(const char *qemu, unsigned int *is_kvm, unsigned int *kvm_version, bool check_yajl); +/* Only for use by test suite */ int qemuCapsParseDeviceStr(const char *str, qemuCapsPtr caps); -- 1.7.11.4
On Tue, Sep 25, 2012 at 18:59:59 +0100, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
The qemuCapsProbeMachineTypes & qemuCapsProbeCPUModels methods do not need to be invoked directly anymore. Make them static and refactor them to directly populate the qemuCapsPtr object
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_capabilities.c | 238 ++++++++++++++++--------------------------- src/qemu/qemu_capabilities.h | 13 +-- 2 files changed, 90 insertions(+), 161 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 888c56c..7e4ea50 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -305,17 +305,16 @@ qemuCapsProbeCommand(const char *qemu, */ static int qemuCapsParseMachineTypesStr(const char *output, - virCapsGuestMachinePtr **machines, - size_t *nmachines) + qemuCapsPtr caps) { const char *p = output; const char *next; - virCapsGuestMachinePtr *list = NULL; - int nitems = 0; + size_t defIdx = 0;
do { const char *t; - virCapsGuestMachinePtr machine; + char *name; + char *canonical = NULL;
if ((next = strchr(p, '\n'))) ++next; @@ -326,56 +325,61 @@ qemuCapsParseMachineTypesStr(const char *output, if (!(t = strchr(p, ' ')) || (next && t >= next)) continue;
- if (VIR_ALLOC(machine) < 0) + if (!(name = strndup(p, t - p))) goto no_memory;
- if (!(machine->name = strndup(p, t - p))) { - VIR_FREE(machine); - goto no_memory; - } - - if (VIR_REALLOC_N(list, nitems + 1) < 0) { - VIR_FREE(machine->name); - VIR_FREE(machine); - goto no_memory; - } - - p = t; - if (!(t = strstr(p, "(default)")) || (next && t >= next)) { - list[nitems++] = machine; - } else { - /* put the default first in the list */ - memmove(list + 1, list, sizeof(*list) * nitems); - list[0] = machine; - nitems++; - } + if (strstr(p, "(default)")) + defIdx = caps->nmachineTypes;
While this will work because it sets defIdx to 1, 2, 4, ... until it stops at the one which is really default. Can we preserve the condition that checks if (default) was found at the current line rather than just somewhere in the rest of the qemu output to make this less magic? And I would even preserve the p = t assignment above.
if ((t = strstr(p, "(alias of ")) && (!next || t < next)) { p = t + strlen("(alias of "); if (!(t = strchr(p, ')')) || (next && t >= next)) continue;
...
@@ -531,75 +515,52 @@ qemuCapsParsePPCModels(const char *output, if (*p == '\n') continue;
- if (retcpus) { - unsigned int len; - - if (VIR_REALLOC_N(cpus, count + 1) < 0) { - virReportOOMError(); - goto cleanup; - } + if (VIR_EXPAND_N(caps->cpuDefinitions, caps->ncpuDefinitions, 1) < 0) { + virReportOOMError(); + goto cleanup; + }
- len = t - p - 1; + len = t - p - 1;
- if (!(cpus[count] = strndup(p, len))) { - virReportOOMError(); - goto cleanup; - } + if (!(caps->cpuDefinitions[caps->ncpuDefinitions - 1] = strndup(p, len))) { + virReportOOMError(); + goto cleanup; } - count++; } while ((p = next));
- if (retcount) - *retcount = count; - if (retcpus) { - *retcpus = cpus; - cpus = NULL; - } ret = 0;
cleanup: - if (cpus) { - for (i = 0; i < count; i++) - VIR_FREE(cpus[i]); - VIR_FREE(cpus); - } return ret; }
-int -qemuCapsProbeCPUModels(const char *qemu, - qemuCapsPtr caps, - const char *arch, - size_t *count, - const char ***cpus) +static int +qemuCapsProbeCPUModels(qemuCapsPtr caps) { char *output = NULL; int ret = -1; qemuCapsParseCPUModels parse; virCommandPtr cmd;
- if (count) - *count = 0; - if (cpus) - *cpus = NULL; - - if (STREQ(arch, "i686") || STREQ(arch, "x86_64")) + if (STREQ(caps->arch, "i686") || + STREQ(caps->arch, "x86_64")) parse = qemuCapsParseX86Models; - else if (STREQ(arch, "ppc64")) + else if (STREQ(caps->arch, "ppc64")) parse = qemuCapsParsePPCModels; else { - VIR_DEBUG("don't know how to parse %s CPU models", arch); + VIR_DEBUG("don't know how to parse %s CPU models", + caps->arch); return 0; }
It's already been there but we changed coding guidelines to all or nothing for {} usage in if statements. Thus, while you are changing this, you can add {} as well.
- cmd = qemuCapsProbeCommand(qemu, caps); + cmd = qemuCapsProbeCommand(caps->binary, caps); virCommandAddArgList(cmd, "-cpu", "?", NULL); virCommandSetOutputBuffer(cmd, &output);
if (virCommandRun(cmd, NULL) < 0) goto cleanup;
- if (parse(output, count, cpus) < 0) + if (parse(output, caps) < 0) goto cleanup;
ret = 0;
... ACK with the nits fixed. Jirka
On Wed, Sep 26, 2012 at 11:42:09AM +0200, Jiri Denemark wrote:
On Tue, Sep 25, 2012 at 18:59:59 +0100, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
The qemuCapsProbeMachineTypes & qemuCapsProbeCPUModels methods do not need to be invoked directly anymore. Make them static and refactor them to directly populate the qemuCapsPtr object
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_capabilities.c | 238 ++++++++++++++++--------------------------- src/qemu/qemu_capabilities.h | 13 +-- 2 files changed, 90 insertions(+), 161 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 888c56c..7e4ea50 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -305,17 +305,16 @@ qemuCapsProbeCommand(const char *qemu, */ static int qemuCapsParseMachineTypesStr(const char *output, - virCapsGuestMachinePtr **machines, - size_t *nmachines) + qemuCapsPtr caps) { const char *p = output; const char *next; - virCapsGuestMachinePtr *list = NULL; - int nitems = 0; + size_t defIdx = 0;
do { const char *t; - virCapsGuestMachinePtr machine; + char *name; + char *canonical = NULL;
if ((next = strchr(p, '\n'))) ++next; @@ -326,56 +325,61 @@ qemuCapsParseMachineTypesStr(const char *output, if (!(t = strchr(p, ' ')) || (next && t >= next)) continue;
- if (VIR_ALLOC(machine) < 0) + if (!(name = strndup(p, t - p))) goto no_memory;
- if (!(machine->name = strndup(p, t - p))) { - VIR_FREE(machine); - goto no_memory; - } - - if (VIR_REALLOC_N(list, nitems + 1) < 0) { - VIR_FREE(machine->name); - VIR_FREE(machine); - goto no_memory; - } - - p = t; - if (!(t = strstr(p, "(default)")) || (next && t >= next)) { - list[nitems++] = machine; - } else { - /* put the default first in the list */ - memmove(list + 1, list, sizeof(*list) * nitems); - list[0] = machine; - nitems++; - } + if (strstr(p, "(default)")) + defIdx = caps->nmachineTypes;
While this will work because it sets defIdx to 1, 2, 4, ... until it stops at the one which is really default. Can we preserve the condition that checks if (default) was found at the current line rather than just somewhere in the rest of the qemu output to make this less magic? And I would even preserve the p = t assignment above.
Ok, so IIUC the folowing change: diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index fed7a06..b519db7 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -328,7 +328,8 @@ qemuCapsParseMachineTypesStr(const char *output, if (!(name = strndup(p, t - p))) goto no_memory; - if (strstr(p, "(default)")) + p = t; + if (!(t = strstr(p, "(default)")) && (!next || t < next)) { defIdx = caps->nmachineTypes; if ((t = strstr(p, "(alias of ")) && (!next || t < next)) { 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 :|
On Wed, Sep 26, 2012 at 13:56:22 +0100, Daniel P. Berrange wrote:
On Wed, Sep 26, 2012 at 11:42:09AM +0200, Jiri Denemark wrote:
On Tue, Sep 25, 2012 at 18:59:59 +0100, Daniel P. Berrange wrote:
- p = t; - if (!(t = strstr(p, "(default)")) || (next && t >= next)) { - list[nitems++] = machine; - } else { - /* put the default first in the list */ - memmove(list + 1, list, sizeof(*list) * nitems); - list[0] = machine; - nitems++; - } + if (strstr(p, "(default)")) + defIdx = caps->nmachineTypes;
While this will work because it sets defIdx to 1, 2, 4, ... until it stops at the one which is really default. Can we preserve the condition that checks if (default) was found at the current line rather than just somewhere in the rest of the qemu output to make this less magic? And I would even preserve the p = t assignment above.
Ok, so IIUC the folowing change:
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index fed7a06..b519db7 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -328,7 +328,8 @@ qemuCapsParseMachineTypesStr(const char *output, if (!(name = strndup(p, t - p))) goto no_memory;
- if (strstr(p, "(default)")) + p = t; + if (!(t = strstr(p, "(default)")) && (!next || t < next)) { defIdx = caps->nmachineTypes;
if ((t = strstr(p, "(alias of ")) && (!next || t < next)) {
Right, that's what I had in mind. Jirka
From: "Daniel P. Berrange" <berrange@redhat.com> Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_capabilities.c | 158 +++++++++++++------------------------------ 1 file changed, 47 insertions(+), 111 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 7e4ea50..0a203c5 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -236,7 +236,6 @@ struct qemu_feature_flags { struct qemu_arch_info { const char *arch; int wordsize; - const char *machine; const char *binary; const char *altbinary; const struct qemu_feature_flags *flags; @@ -258,25 +257,20 @@ static const struct qemu_feature_flags const arch_info_x86_64_flags [] = { /* The archicture tables for supported QEMU archs */ static const struct qemu_arch_info const arch_info_hvm[] = { - { "i686", 32, NULL, "qemu", + { "i686", 32, "qemu", "qemu-system-x86_64", arch_info_i686_flags, 4 }, - { "x86_64", 64, NULL, "qemu-system-x86_64", + { "x86_64", 64, "qemu-system-x86_64", NULL, arch_info_x86_64_flags, 2 }, - { "arm", 32, NULL, "qemu-system-arm", NULL, NULL, 0 }, - { "microblaze", 32, NULL, "qemu-system-microblaze", NULL, NULL, 0 }, - { "microblazeel", 32, NULL, "qemu-system-microblazeel", NULL, NULL, 0 }, - { "mips", 32, NULL, "qemu-system-mips", NULL, NULL, 0 }, - { "mipsel", 32, NULL, "qemu-system-mipsel", NULL, NULL, 0 }, - { "sparc", 32, NULL, "qemu-system-sparc", NULL, NULL, 0 }, - { "ppc", 32, NULL, "qemu-system-ppc", NULL, NULL, 0 }, - { "ppc64", 64, NULL, "qemu-system-ppc64", NULL, NULL, 0 }, - { "itanium", 64, NULL, "qemu-system-ia64", NULL, NULL, 0 }, - { "s390x", 64, NULL, "qemu-system-s390x", NULL, NULL, 0 }, -}; - -static const struct qemu_arch_info const arch_info_xen[] = { - { "i686", 32, "xenner", "xenner", NULL, arch_info_i686_flags, 4 }, - { "x86_64", 64, "xenner", "xenner", NULL, arch_info_x86_64_flags, 2 }, + { "arm", 32, "qemu-system-arm", NULL, NULL, 0 }, + { "microblaze", 32, "qemu-system-microblaze", NULL, NULL, 0 }, + { "microblazeel", 32, "qemu-system-microblazeel", NULL, NULL, 0 }, + { "mips", 32, "qemu-system-mips", NULL, NULL, 0 }, + { "mipsel", 32, "qemu-system-mipsel", NULL, NULL, 0 }, + { "sparc", 32, "qemu-system-sparc", NULL, NULL, 0 }, + { "ppc", 32, "qemu-system-ppc", NULL, NULL, 0 }, + { "ppc64", 64, "qemu-system-ppc64", NULL, NULL, 0 }, + { "itanium", 64, "qemu-system-ia64", NULL, NULL, 0 }, + { "s390x", 64, "qemu-system-s390x", NULL, NULL, 0 }, }; @@ -577,8 +571,7 @@ static int qemuCapsInitGuest(virCapsPtr caps, qemuCapsCachePtr cache, const char *hostmachine, - const struct qemu_arch_info *info, - int hvm) + const struct qemu_arch_info *info) { virCapsGuestPtr guest; int i; @@ -657,36 +650,13 @@ qemuCapsInitGuest(virCapsPtr caps, qemuCapsGet(qemubinCaps, QEMU_CAPS_KQEMU)) haskqemu = 1; - if (info->machine) { - virCapsGuestMachinePtr machine; - - if (VIR_ALLOC(machine) < 0) { - goto no_memory; - } - - if (!(machine->name = strdup(info->machine))) { - VIR_FREE(machine); - goto no_memory; - } - - nmachines = 1; - - if (VIR_ALLOC_N(machines, nmachines) < 0) { - VIR_FREE(machine->name); - VIR_FREE(machine); - goto no_memory; - } - - machines[0] = machine; - } else { - if (qemuCapsGetMachineTypesCaps(qemubinCaps, &nmachines, &machines) < 0) - goto error; - } + if (qemuCapsGetMachineTypesCaps(qemubinCaps, &nmachines, &machines) < 0) + goto error; /* We register kvm as the base emulator too, since we can * just give -no-kvm to disable acceleration if required */ if ((guest = virCapabilitiesAddGuest(caps, - hvm ? "hvm" : "xen", + "hvm", info->arch, info->wordsize, binary, @@ -707,52 +677,42 @@ qemuCapsInitGuest(virCapsPtr caps, !virCapabilitiesAddGuestFeature(guest, "deviceboot", 1, 0)) goto error; - if (hvm) { - if (virCapabilitiesAddGuestDomain(guest, - "qemu", - NULL, - NULL, - 0, - NULL) == NULL) - goto error; + if (virCapabilitiesAddGuestDomain(guest, + "qemu", + NULL, + NULL, + 0, + NULL) == NULL) + goto error; - if (haskqemu && - virCapabilitiesAddGuestDomain(guest, - "kqemu", - NULL, - NULL, - 0, - NULL) == NULL) - goto error; + if (haskqemu && + virCapabilitiesAddGuestDomain(guest, + "kqemu", + NULL, + NULL, + 0, + NULL) == NULL) + goto error; - if (haskvm) { - virCapsGuestDomainPtr dom; + if (haskvm) { + virCapsGuestDomainPtr dom; - if (kvmbin && - qemuCapsGetMachineTypesCaps(kvmbinCaps, &nmachines, &machines) < 0) - goto error; + if (kvmbin && + qemuCapsGetMachineTypesCaps(kvmbinCaps, &nmachines, &machines) < 0) + goto error; - if ((dom = virCapabilitiesAddGuestDomain(guest, - "kvm", - kvmbin ? kvmbin : binary, - NULL, - nmachines, - machines)) == NULL) { - goto error; - } + if ((dom = virCapabilitiesAddGuestDomain(guest, + "kvm", + kvmbin ? kvmbin : binary, + NULL, + nmachines, + machines)) == NULL) { + goto error; + } - machines = NULL; - nmachines = 0; + machines = NULL; + nmachines = 0; - } - } else { - if (virCapabilitiesAddGuestDomain(guest, - "kvm", - NULL, - NULL, - 0, - NULL) == NULL) - goto error; } if (info->nflags) { @@ -775,9 +735,6 @@ cleanup: return ret; -no_memory: - virReportOOMError(); - error: virCapabilitiesFreeMachines(machines, nmachines); @@ -838,7 +795,6 @@ virCapsPtr qemuCapsInit(qemuCapsCachePtr cache) struct utsname utsname; virCapsPtr caps; int i; - char *xenner = NULL; /* Really, this never fails - look at the man-page. */ uname (&utsname); @@ -874,28 +830,9 @@ virCapsPtr qemuCapsInit(qemuCapsCachePtr cache) for (i = 0 ; i < ARRAY_CARDINALITY(arch_info_hvm) ; i++) if (qemuCapsInitGuest(caps, cache, utsname.machine, - &arch_info_hvm[i], 1) < 0) + &arch_info_hvm[i]) < 0) goto no_memory; - /* Then possibly the Xen paravirt guests (ie Xenner */ - xenner = virFindFileInPath("xenner"); - - if (xenner != NULL && virFileIsExecutable(xenner) == 0 && - access("/dev/kvm", F_OK) == 0) { - for (i = 0 ; i < ARRAY_CARDINALITY(arch_info_xen) ; i++) - /* Allow Xen 32-on-32, 32-on-64 and 64-on-64 */ - if (STREQ(arch_info_xen[i].arch, utsname.machine) || - (STREQ(utsname.machine, "x86_64") && - STREQ(arch_info_xen[i].arch, "i686"))) { - if (qemuCapsInitGuest(caps, cache, - utsname.machine, - &arch_info_xen[i], 0) < 0) - goto no_memory; - } - } - - VIR_FREE(xenner); - /* QEMU Requires an emulator in the XML */ virCapabilitiesSetEmulatorRequired(caps); @@ -904,7 +841,6 @@ virCapsPtr qemuCapsInit(qemuCapsCachePtr cache) return caps; no_memory: - VIR_FREE(xenner); virCapabilitiesFree(caps); return NULL; } -- 1.7.11.4
On Tue, Sep 25, 2012 at 19:00:00 +0100, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_capabilities.c | 158 +++++++++++++------------------------------ 1 file changed, 47 insertions(+), 111 deletions(-)
ACK Jirka
From: "Daniel P. Berrange" <berrange@redhat.com> Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_capabilities.c | 163 +++++++++++++++++++++++-------------------- 1 file changed, 89 insertions(+), 74 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 0a203c5..c173286 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -227,53 +227,6 @@ static int qemuCapsOnceInit(void) VIR_ONCE_GLOBAL_INIT(qemuCaps) -struct qemu_feature_flags { - const char *name; - const int default_on; - const int toggle; -}; - -struct qemu_arch_info { - const char *arch; - int wordsize; - const char *binary; - const char *altbinary; - const struct qemu_feature_flags *flags; - int nflags; -}; - -/* Feature flags for the architecture info */ -static const struct qemu_feature_flags const arch_info_i686_flags [] = { - { "pae", 1, 0 }, - { "nonpae", 1, 0 }, - { "acpi", 1, 1 }, - { "apic", 1, 0 }, -}; - -static const struct qemu_feature_flags const arch_info_x86_64_flags [] = { - { "acpi", 1, 1 }, - { "apic", 1, 0 }, -}; - -/* The archicture tables for supported QEMU archs */ -static const struct qemu_arch_info const arch_info_hvm[] = { - { "i686", 32, "qemu", - "qemu-system-x86_64", arch_info_i686_flags, 4 }, - { "x86_64", 64, "qemu-system-x86_64", - NULL, arch_info_x86_64_flags, 2 }, - { "arm", 32, "qemu-system-arm", NULL, NULL, 0 }, - { "microblaze", 32, "qemu-system-microblaze", NULL, NULL, 0 }, - { "microblazeel", 32, "qemu-system-microblazeel", NULL, NULL, 0 }, - { "mips", 32, "qemu-system-mips", NULL, NULL, 0 }, - { "mipsel", 32, "qemu-system-mipsel", NULL, NULL, 0 }, - { "sparc", 32, "qemu-system-sparc", NULL, NULL, 0 }, - { "ppc", 32, "qemu-system-ppc", NULL, NULL, 0 }, - { "ppc64", 64, "qemu-system-ppc64", NULL, NULL, 0 }, - { "itanium", 64, "qemu-system-ia64", NULL, NULL, 0 }, - { "s390x", 64, "qemu-system-s390x", NULL, NULL, 0 }, -}; - - static virCommandPtr qemuCapsProbeCommand(const char *qemu, qemuCapsPtr caps) @@ -348,6 +301,7 @@ qemuCapsParseMachineTypesStr(const char *output, caps->machineAliases[caps->nmachineTypes-1] = name; } else { caps->machineTypes[caps->nmachineTypes-1] = name; + caps->machineAliases[caps->nmachineTypes-1] = NULL; } } while ((p = next)); @@ -367,7 +321,7 @@ qemuCapsParseMachineTypesStr(const char *output, return 0; - no_memory: +no_memory: virReportOOMError(); return -1; } @@ -567,11 +521,70 @@ cleanup: } +static char * +qemuCapsFindBinaryForArch(const char *hostarch, + const char *guestarch) +{ + char *ret; + + if (STREQ(guestarch, "i686")) { + ret = virFindFileInPath("qemu-system-i386"); + if (ret && !virFileIsExecutable(ret)) + VIR_FREE(ret); + + if (!ret && STREQ(hostarch, "x86_64")) { + ret = virFindFileInPath("qemu-system-x86_64"); + if (ret && !virFileIsExecutable(ret)) + VIR_FREE(ret); + } + + if (!ret) + ret = virFindFileInPath("qemu"); + } else if (STREQ(guestarch, "itanium")) { + ret = virFindFileInPath("qemu-system-ia64"); + } else { + char *bin; + if (virAsprintf(&bin, "qemu-system-%s", guestarch) < 0) { + virReportOOMError(); + return NULL; + } + ret = virFindFileInPath(bin); + VIR_FREE(bin); + } + if (ret && !virFileIsExecutable(ret)) + VIR_FREE(ret); + return ret; +} + +static int +qemuCapsGetArchWordSize(const char *guestarch) +{ + if (STREQ(guestarch, "i686") || + STREQ(guestarch, "ppc") || + STREQ(guestarch, "sparc") || + STREQ(guestarch, "mips") || + STREQ(guestarch, "mipsel")) + return 32; + return 64; +} + +static bool +qemuCapsIsValidForKVM(const char *hostarch, + const char *guestarch) +{ + if (STREQ(hostarch, guestarch)) + return true; + if (STREQ(hostarch, "x86_64") && + STREQ(guestarch, "i686")) + return true; + return false; +} + static int qemuCapsInitGuest(virCapsPtr caps, qemuCapsCachePtr cache, - const char *hostmachine, - const struct qemu_arch_info *info) + const char *hostarch, + const char *guestarch) { virCapsGuestPtr guest; int i; @@ -588,12 +601,7 @@ qemuCapsInitGuest(virCapsPtr caps, /* Check for existance of base emulator, or alternate base * which can be used with magic cpu choice */ - binary = virFindFileInPath(info->binary); - - if (binary == NULL || !virFileIsExecutable(binary)) { - VIR_FREE(binary); - binary = virFindFileInPath(info->altbinary); - } + binary = qemuCapsFindBinaryForArch(hostarch, guestarch); /* Ignore binary if extracting version info fails */ if (binary) { @@ -609,8 +617,7 @@ qemuCapsInitGuest(virCapsPtr caps, * - hostarch is x86_64 and guest arch is i686 * The latter simply needs "-cpu qemu32" */ - if (STREQ(info->arch, hostmachine) || - (STREQ(hostmachine, "x86_64") && STREQ(info->arch, "i686"))) { + if (qemuCapsIsValidForKVM(hostarch, guestarch)) { const char *const kvmbins[] = { "/usr/libexec/qemu-kvm", /* RHEL */ "qemu-kvm", /* Fedora */ "kvm" }; /* Upstream .spec */ @@ -657,8 +664,8 @@ qemuCapsInitGuest(virCapsPtr caps, * just give -no-kvm to disable acceleration if required */ if ((guest = virCapabilitiesAddGuest(caps, "hvm", - info->arch, - info->wordsize, + guestarch, + qemuCapsGetArchWordSize(guestarch), binary, NULL, nmachines, @@ -715,15 +722,16 @@ qemuCapsInitGuest(virCapsPtr caps, } - if (info->nflags) { - for (i = 0 ; i < info->nflags ; i++) { - if (virCapabilitiesAddGuestFeature(guest, - info->flags[i].name, - info->flags[i].default_on, - info->flags[i].toggle) == NULL) - goto error; - } - } + if ((STREQ(guestarch, "i686") || + STREQ(guestarch, "x86_64")) && + (virCapabilitiesAddGuestFeature(guest, "acpi", 1, 1) == NULL || + virCapabilitiesAddGuestFeature(guest, "apic", 1, 0) == NULL)) + goto error; + + if (STREQ(guestarch, "i686") && + (virCapabilitiesAddGuestFeature(guest, "pae", 1, 0) == NULL || + virCapabilitiesAddGuestFeature(guest, "nonpae", 1, 0) == NULL)) + goto error; ret = 0; @@ -795,13 +803,20 @@ virCapsPtr qemuCapsInit(qemuCapsCachePtr cache) struct utsname utsname; virCapsPtr caps; int i; + const char *const arches[] = { + "i686", "x86_64", "arm", + "microblaze", "microblazeel", + "mips", "mipsel", "sparc", + "ppc", "ppc64", "itanium", + "s390x" + }; /* Really, this never fails - look at the man-page. */ uname (&utsname); if ((caps = virCapabilitiesNew(utsname.machine, 1, 1)) == NULL) - goto no_memory; + goto error; /* Using KVM's mac prefix for QEMU too */ virCapabilitiesSetMacPrefix(caps, (unsigned char[]){ 0x52, 0x54, 0x00 }); @@ -827,11 +842,11 @@ virCapsPtr qemuCapsInit(qemuCapsCachePtr cache) "tcp"); /* First the pure HVM guests */ - for (i = 0 ; i < ARRAY_CARDINALITY(arch_info_hvm) ; i++) + for (i = 0 ; i < ARRAY_CARDINALITY(arches) ; i++) if (qemuCapsInitGuest(caps, cache, utsname.machine, - &arch_info_hvm[i]) < 0) - goto no_memory; + arches[i]) < 0) + goto error; /* QEMU Requires an emulator in the XML */ virCapabilitiesSetEmulatorRequired(caps); @@ -840,7 +855,7 @@ virCapsPtr qemuCapsInit(qemuCapsCachePtr cache) return caps; - no_memory: +error: virCapabilitiesFree(caps); return NULL; } -- 1.7.11.4
On Tue, Sep 25, 2012 at 19:00:01 +0100, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_capabilities.c | 163 +++++++++++++++++++++++-------------------- 1 file changed, 89 insertions(+), 74 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 0a203c5..c173286 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -227,53 +227,6 @@ static int qemuCapsOnceInit(void)
VIR_ONCE_GLOBAL_INIT(qemuCaps)
-struct qemu_feature_flags { - const char *name; - const int default_on; - const int toggle; -}; - -struct qemu_arch_info { - const char *arch; - int wordsize; - const char *binary; - const char *altbinary; - const struct qemu_feature_flags *flags; - int nflags; -}; - -/* Feature flags for the architecture info */ -static const struct qemu_feature_flags const arch_info_i686_flags [] = { - { "pae", 1, 0 }, - { "nonpae", 1, 0 }, - { "acpi", 1, 1 }, - { "apic", 1, 0 }, -}; - -static const struct qemu_feature_flags const arch_info_x86_64_flags [] = { - { "acpi", 1, 1 }, - { "apic", 1, 0 }, -}; - -/* The archicture tables for supported QEMU archs */ -static const struct qemu_arch_info const arch_info_hvm[] = { - { "i686", 32, "qemu", - "qemu-system-x86_64", arch_info_i686_flags, 4 }, - { "x86_64", 64, "qemu-system-x86_64", - NULL, arch_info_x86_64_flags, 2 }, - { "arm", 32, "qemu-system-arm", NULL, NULL, 0 }, - { "microblaze", 32, "qemu-system-microblaze", NULL, NULL, 0 }, - { "microblazeel", 32, "qemu-system-microblazeel", NULL, NULL, 0 }, - { "mips", 32, "qemu-system-mips", NULL, NULL, 0 }, - { "mipsel", 32, "qemu-system-mipsel", NULL, NULL, 0 }, - { "sparc", 32, "qemu-system-sparc", NULL, NULL, 0 }, - { "ppc", 32, "qemu-system-ppc", NULL, NULL, 0 }, - { "ppc64", 64, "qemu-system-ppc64", NULL, NULL, 0 }, - { "itanium", 64, "qemu-system-ia64", NULL, NULL, 0 }, - { "s390x", 64, "qemu-system-s390x", NULL, NULL, 0 }, -}; - - static virCommandPtr qemuCapsProbeCommand(const char *qemu, qemuCapsPtr caps) @@ -348,6 +301,7 @@ qemuCapsParseMachineTypesStr(const char *output, caps->machineAliases[caps->nmachineTypes-1] = name; } else { caps->machineTypes[caps->nmachineTypes-1] = name; + caps->machineAliases[caps->nmachineTypes-1] = NULL; } } while ((p = next));
Looks like a hunk that should go into 6/20.
@@ -367,7 +321,7 @@ qemuCapsParseMachineTypesStr(const char *output,
return 0;
- no_memory: +no_memory: virReportOOMError(); return -1; }
And this one too. ... ACK Jirka
From: "Daniel P. Berrange" <berrange@redhat.com> Add a new qemuMonitorGetVersion() method to support invocation of the 'query-version' JSON monitor command. No HMP equivalent is provided, since this will only be used for QEMU >= 1.2 Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_monitor.c | 24 ++++++++++ src/qemu/qemu_monitor.h | 7 +++ src/qemu/qemu_monitor_json.c | 78 +++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 7 +++ tests/qemumonitorjsontest.c | 102 +++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 218 insertions(+) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index fa3b7b2..6fe12fe 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2997,3 +2997,27 @@ int qemuMonitorSystemWakeup(qemuMonitorPtr mon) return qemuMonitorJSONSystemWakeup(mon); } + +int qemuMonitorGetVersion(qemuMonitorPtr mon, + int *major, + int *minor, + int *micro, + char **package) +{ + VIR_DEBUG("mon=%p major=%p minor=%p micro=%p package=%p", + mon, major, minor, micro, package); + + if (!mon) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + + if (!mon->json) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("JSON monitor is required")); + return -1; + } + + return qemuMonitorJSONGetVersion(mon, major, minor, micro, package); +} diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 6b4eb6f..779fca4 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -569,6 +569,13 @@ int qemuMonitorGetBlockIoThrottle(qemuMonitorPtr mon, int qemuMonitorSystemWakeup(qemuMonitorPtr mon); +int qemuMonitorGetVersion(qemuMonitorPtr mon, + int *major, + int *minor, + int *micro, + char **package) + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4); + /** * When running two dd process and using <> redirection, we need a * shell that will not truncate files. These two strings serve that diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index f372199..11c7ff5 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3811,3 +3811,81 @@ int qemuMonitorJSONSystemWakeup(qemuMonitorPtr mon) virJSONValueFree(reply); return ret; } + +int qemuMonitorJSONGetVersion(qemuMonitorPtr mon, + int *major, + int *minor, + int *micro, + char **package) +{ + int ret; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + virJSONValuePtr data; + virJSONValuePtr qemu; + + *major = *minor = *micro = 0; + if (package) + *package = NULL; + + if (!(cmd = qemuMonitorJSONMakeCommand("query-version", NULL))) + return -1; + + ret = qemuMonitorJSONCommand(mon, cmd, &reply); + + if (ret == 0) + ret = qemuMonitorJSONCheckError(cmd, reply); + + if (ret < 0) + goto cleanup; + + ret = -1; + + if (!(data = virJSONValueObjectGet(reply, "return"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-version reply was missing 'return' data")); + goto cleanup; + } + + if (!(qemu = virJSONValueObjectGet(data, "qemu"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-version reply was missing 'qemu' data")); + goto cleanup; + } + + if (virJSONValueObjectGetNumberInt(qemu, "major", major) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-version reply was missing 'major' version")); + goto cleanup; + } + if (virJSONValueObjectGetNumberInt(qemu, "minor", minor) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-version reply was missing 'minor' version")); + goto cleanup; + } + if (virJSONValueObjectGetNumberInt(qemu, "micro", micro) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-version reply was missing 'micro' version")); + goto cleanup; + } + + if (package) { + const char *tmp; + if (!(tmp = virJSONValueObjectGetString(data, "package"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-version reply was missing 'package' version")); + goto cleanup; + } + if (!(*package = strdup(tmp))) { + virReportOOMError(); + goto cleanup; + } + } + + ret = 0; + +cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index e531eb1..c66ff4f 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -280,4 +280,11 @@ int qemuMonitorJSONGetBlockIoThrottle(qemuMonitorPtr mon, int qemuMonitorJSONSystemWakeup(qemuMonitorPtr mon); +int qemuMonitorJSONGetVersion(qemuMonitorPtr mon, + int *major, + int *minor, + int *micro, + char **package) + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4); + #endif /* QEMU_MONITOR_JSON_H */ diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 48e78e7..c3a0c45 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -123,6 +123,107 @@ cleanup: } static int +testQemuMonitorJSONGetVersion(const void *data) +{ + virCapsPtr caps = (virCapsPtr)data; + qemuMonitorTestPtr test = qemuMonitorTestNew(true, caps); + int ret = -1; + int major; + int minor; + int micro; + char *package; + + if (!test) + return -1; + + if (qemuMonitorTestAddItem(test, "query-version", + "{ " + " \"return\":{ " + " \"qemu\":{ " + " \"major\":1, " + " \"minor\":2, " + " \"micro\":3 " + " }," + " \"package\":\"\"" + " }" + "}") < 0) + goto cleanup; + + if (qemuMonitorTestAddItem(test, "query-version", + "{ " + " \"return\":{ " + " \"qemu\":{ " + " \"major\":0, " + " \"minor\":11, " + " \"micro\":6 " + " }," + " \"package\":\"2.283.el6\"" + " }" + "}") < 0) + goto cleanup; + + if (qemuMonitorGetVersion(qemuMonitorTestGetMonitor(test), + &major, &minor, µ, + &package) < 0) + goto cleanup; + + if (major != 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "Major %d was not 1", major); + goto cleanup; + } + if (minor != 2) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "Minor %d was not 2", major); + goto cleanup; + } + if (micro != 3) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "Micro %d was not 3", major); + goto cleanup; + } + + if (STRNEQ(package, "")) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "Package %s was not ''", package); + goto cleanup; + } + + if (qemuMonitorGetVersion(qemuMonitorTestGetMonitor(test), + &major, &minor, µ, + &package) < 0) + goto cleanup; + + if (major != 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "Major %d was not 0", major); + goto cleanup; + } + if (minor != 11) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "Minor %d was not 11", major); + goto cleanup; + } + if (micro != 6) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "Micro %d was not 6", major); + goto cleanup; + } + + if (STRNEQ(package, "2.283.el6")) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "Package %s was not '2.283.el6'", package); + goto cleanup; + } + + ret = 0; + +cleanup: + qemuMonitorTestFree(test); + return ret; +} + +static int mymain(void) { int ret = 0; @@ -141,6 +242,7 @@ mymain(void) ret = -1 DO_TEST(GetStatus); + DO_TEST(GetVersion); virCapabilitiesFree(caps); -- 1.7.11.4
On Tue, Sep 25, 2012 at 19:00:02 +0100, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Add a new qemuMonitorGetVersion() method to support invocation of the 'query-version' JSON monitor command. No HMP equivalent is provided, since this will only be used for QEMU >= 1.2
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_monitor.c | 24 ++++++++++ src/qemu/qemu_monitor.h | 7 +++ src/qemu/qemu_monitor_json.c | 78 +++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 7 +++ tests/qemumonitorjsontest.c | 102 +++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 218 insertions(+)
ACK Jirka
From: "Daniel P. Berrange" <berrange@redhat.com> Add a new qemuMonitorGetMachines() method to support invocation of the 'query-machines' JSON monitor command. No HMP equivalent is required, since this will only be present for QEMU >= 1.2 Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_monitor.c | 30 +++++++++++++ src/qemu/qemu_monitor.h | 15 +++++++ src/qemu/qemu_monitor_json.c | 101 +++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 4 ++ tests/qemumonitorjsontest.c | 76 ++++++++++++++++++++++++++++++++ 5 files changed, 226 insertions(+) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 6fe12fe..581fe41 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3021,3 +3021,33 @@ int qemuMonitorGetVersion(qemuMonitorPtr mon, return qemuMonitorJSONGetVersion(mon, major, minor, micro, package); } + +int qemuMonitorGetMachines(qemuMonitorPtr mon, + qemuMonitorMachineInfoPtr **machines) +{ + VIR_DEBUG("mon=%p machines=%p", + mon, machines); + + if (!mon) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + + if (!mon->json) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("JSON monitor is required")); + return -1; + } + + return qemuMonitorJSONGetMachines(mon, machines); +} + +void qemuMonitorMachineInfoFree(qemuMonitorMachineInfoPtr machine) +{ + if (!machine) + return; + VIR_FREE(machine->name); + VIR_FREE(machine->alias); + VIR_FREE(machine); +} diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 779fca4..20ff742 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -576,6 +576,21 @@ int qemuMonitorGetVersion(qemuMonitorPtr mon, char **package) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4); + +typedef struct _qemuMonitorMachineInfo qemuMonitorMachineInfo; +typedef qemuMonitorMachineInfo *qemuMonitorMachineInfoPtr; + +struct _qemuMonitorMachineInfo { + char *name; + bool isDefault; + char *alias; +}; + +int qemuMonitorGetMachines(qemuMonitorPtr mon, + qemuMonitorMachineInfoPtr **machines); + +void qemuMonitorMachineInfoFree(qemuMonitorMachineInfoPtr machine); + /** * When running two dd process and using <> redirection, we need a * shell that will not truncate files. These two strings serve that diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 11c7ff5..b4ae893 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3889,3 +3889,104 @@ cleanup: virJSONValueFree(reply); return ret; } + + +int qemuMonitorJSONGetMachines(qemuMonitorPtr mon, + qemuMonitorMachineInfoPtr **machines) +{ + int ret; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + virJSONValuePtr data; + qemuMonitorMachineInfoPtr *infolist = NULL; + int n = 0; + size_t i; + + *machines = NULL; + + if (!(cmd = qemuMonitorJSONMakeCommand("query-machines", NULL))) + return -1; + + ret = qemuMonitorJSONCommand(mon, cmd, &reply); + + if (ret == 0) + ret = qemuMonitorJSONCheckError(cmd, reply); + + if (ret < 0) + goto cleanup; + + ret = -1; + + if (!(data = virJSONValueObjectGet(reply, "return"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-machines reply was missing return data")); + goto cleanup; + } + + if ((n = virJSONValueArraySize(data)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-machines reply data was not an array")); + goto cleanup; + } + + if (VIR_ALLOC_N(infolist, n) < 0) { + virReportOOMError(); + goto cleanup; + } + + for (i = 0 ; i < n ; i++) { + virJSONValuePtr child = virJSONValueArrayGet(data, i); + const char *tmp; + qemuMonitorMachineInfoPtr info; + + if (VIR_ALLOC(info) < 0) { + virReportOOMError(); + goto cleanup; + } + + infolist[i] = info; + + if (!(tmp = virJSONValueObjectGetString(child, "name"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-machines reply data was missing 'name'")); + goto cleanup; + } + + if (!(info->name = strdup(tmp))) { + virReportOOMError(); + goto cleanup; + } + + if (virJSONValueObjectHasKey(child, "is-default") && + virJSONValueObjectGetBoolean(child, "is-default", &info->isDefault) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-machines reply has malformed 'is-default' data")); + goto cleanup; + } + + if (virJSONValueObjectHasKey(child, "alias")) { + if (!(tmp = virJSONValueObjectGetString(child, "alias"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-machines reply has malformed 'alias' data")); + goto cleanup; + } + if (!(info->alias = strdup(tmp))) { + virReportOOMError(); + goto cleanup; + } + } + } + + ret = n; + *machines = infolist; + +cleanup: + if (ret < 0 && infolist) { + for (i = 0 ; i < n ; i++) + qemuMonitorMachineInfoFree(infolist[i]); + VIR_FREE(infolist); + } + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index c66ff4f..f71f11c 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -287,4 +287,8 @@ int qemuMonitorJSONGetVersion(qemuMonitorPtr mon, char **package) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4); +int qemuMonitorJSONGetMachines(qemuMonitorPtr mon, + qemuMonitorMachineInfoPtr **machines) + ATTRIBUTE_NONNULL(2); + #endif /* QEMU_MONITOR_JSON_H */ diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index c3a0c45..ad095b1 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -224,6 +224,81 @@ cleanup: } static int +testQemuMonitorJSONGetMachines(const void *data) +{ + virCapsPtr caps = (virCapsPtr)data; + qemuMonitorTestPtr test = qemuMonitorTestNew(true, caps); + int ret = -1; + qemuMonitorMachineInfoPtr *info; + int ninfo; + const char *null = NULL; + + if (!test) + return -1; + + if (qemuMonitorTestAddItem(test, "query-machines", + "{ " + " \"return\": [ " + " { " + " \"name\": \"pc-1.0\" " + " }, " + " { " + " \"name\": \"pc-1.1\" " + " }, " + " { " + " \"name\": \"pc-1.2\", " + " \"is-default\": true, " + " \"alias\": \"pc\" " + " } " + " ]" + "}") < 0) + goto cleanup; + + if ((ninfo = qemuMonitorGetMachines(qemuMonitorTestGetMonitor(test), + &info)) < 0) + goto cleanup; + + if (ninfo != 3) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "ninfo %d is not 3", ninfo); + goto cleanup; + } + +#define CHECK(i, wantname, wantisDefault, wantalias) \ + do { \ + if (STRNEQ(info[i]->name, (wantname))) { \ + virReportError(VIR_ERR_INTERNAL_ERROR, \ + "name %s is not %s", \ + info[i]->name, (wantname)); \ + goto cleanup; \ + } \ + if (info[i]->isDefault != (wantisDefault)) { \ + virReportError(VIR_ERR_INTERNAL_ERROR, \ + "isDefault %d is not %d", \ + info[i]->isDefault, (wantisDefault)); \ + goto cleanup; \ + } \ + if (STRNEQ_NULLABLE(info[i]->alias, (wantalias))) { \ + virReportError(VIR_ERR_INTERNAL_ERROR, \ + "alias %s is not %s", \ + info[i]->alias, NULLSTR(wantalias)); \ + goto cleanup; \ + } \ + } while (0) + + CHECK(0, "pc-1.0", false, null); + CHECK(1, "pc-1.1", false, null); + CHECK(2, "pc-1.2", true, "pc"); + + ret = 0; + +cleanup: + qemuMonitorTestFree(test); + return ret; +} + + +static int mymain(void) { int ret = 0; @@ -243,6 +318,7 @@ mymain(void) DO_TEST(GetStatus); DO_TEST(GetVersion); + DO_TEST(GetMachines); virCapabilitiesFree(caps); -- 1.7.11.4
On Tue, Sep 25, 2012 at 19:00:03 +0100, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Add a new qemuMonitorGetMachines() method to support invocation of the 'query-machines' JSON monitor command. No HMP equivalent is required, since this will only be present for QEMU >= 1.2
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_monitor.c | 30 +++++++++++++ src/qemu/qemu_monitor.h | 15 +++++++ src/qemu/qemu_monitor_json.c | 101 +++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 4 ++ tests/qemumonitorjsontest.c | 76 ++++++++++++++++++++++++++++++++ 5 files changed, 226 insertions(+)
ACK Jirka
From: "Daniel P. Berrange" <berrange@redhat.com> Add a new qemuMonitorGetCPUDefinitions() method to support invocation of the 'query-cpu-definitions' JSON monitor command. No HMP equivalent is required, since this will only be present for QEMU >= 1.2 Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_monitor.c | 21 +++++++++++++ src/qemu/qemu_monitor.h | 4 +++ src/qemu/qemu_monitor_json.c | 75 ++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 4 +++ tests/qemumonitorjsontest.c | 63 +++++++++++++++++++++++++++++++++++++ 5 files changed, 167 insertions(+) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 581fe41..45bb32e 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3051,3 +3051,24 @@ void qemuMonitorMachineInfoFree(qemuMonitorMachineInfoPtr machine) VIR_FREE(machine->alias); VIR_FREE(machine); } + +int qemuMonitorGetCPUDefinitions(qemuMonitorPtr mon, + char ***cpus) +{ + VIR_DEBUG("mon=%p cpus=%p", + mon, cpus); + + if (!mon) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + + if (!mon->json) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("JSON monitor is required")); + return -1; + } + + return qemuMonitorJSONGetCPUDefinitions(mon, cpus); +} diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 20ff742..fa192d4 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -591,6 +591,10 @@ int qemuMonitorGetMachines(qemuMonitorPtr mon, void qemuMonitorMachineInfoFree(qemuMonitorMachineInfoPtr machine); +int qemuMonitorGetCPUDefinitions(qemuMonitorPtr mon, + char ***cpus); + + /** * When running two dd process and using <> redirection, we need a * shell that will not truncate files. These two strings serve that diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index b4ae893..876188d 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3990,3 +3990,78 @@ cleanup: virJSONValueFree(reply); return ret; } + + +int qemuMonitorJSONGetCPUDefinitions(qemuMonitorPtr mon, + char ***cpus) +{ + int ret; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + virJSONValuePtr data; + char **cpulist = NULL; + int n = 0; + size_t i; + + *cpus = NULL; + + if (!(cmd = qemuMonitorJSONMakeCommand("query-cpu-definitions", NULL))) + return -1; + + ret = qemuMonitorJSONCommand(mon, cmd, &reply); + + if (ret == 0) + ret = qemuMonitorJSONCheckError(cmd, reply); + + if (ret < 0) + goto cleanup; + + ret = -1; + + if (!(data = virJSONValueObjectGet(reply, "return"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-cpu-definitions reply was missing return data")); + goto cleanup; + } + + if ((n = virJSONValueArraySize(data)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-cpu-definitions reply data was not an array")); + goto cleanup; + } + + if (VIR_ALLOC_N(cpulist, n) < 0) { + virReportOOMError(); + goto cleanup; + } + + for (i = 0 ; i < n ; i++) { + virJSONValuePtr child = virJSONValueArrayGet(data, i); + const char *tmp; + + if (!(tmp = virJSONValueObjectGetString(child, "name"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-cpu-definitions reply data was missing 'name'")); + goto cleanup; + } + + if (!(cpulist[i] = strdup(tmp))) { + virReportOOMError(); + goto cleanup; + } + } + + ret = n; + *cpus = cpulist; + +cleanup: + if (ret < 0 && cpulist) { + for (i = 0 ; i < n ; i++) + VIR_FREE(cpulist[i]); + VIR_FREE(cpulist); + } + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} + diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index f71f11c..218cd02 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -291,4 +291,8 @@ int qemuMonitorJSONGetMachines(qemuMonitorPtr mon, qemuMonitorMachineInfoPtr **machines) ATTRIBUTE_NONNULL(2); +int qemuMonitorJSONGetCPUDefinitions(qemuMonitorPtr mon, + char ***cpus) + ATTRIBUTE_NONNULL(2); + #endif /* QEMU_MONITOR_JSON_H */ diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index ad095b1..1346441 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -290,6 +290,68 @@ testQemuMonitorJSONGetMachines(const void *data) CHECK(1, "pc-1.1", false, null); CHECK(2, "pc-1.2", true, "pc"); +#undef CHECK + ret = 0; + +cleanup: + qemuMonitorTestFree(test); + return ret; +} + + +static int +testQemuMonitorJSONGetCPUDefinitions(const void *data) +{ + virCapsPtr caps = (virCapsPtr)data; + qemuMonitorTestPtr test = qemuMonitorTestNew(true, caps); + int ret = -1; + char **cpus = NULL; + int ncpus; + + if (!test) + return -1; + + if (qemuMonitorTestAddItem(test, "query-cpu-definitions", + "{ " + " \"return\": [ " + " { " + " \"name\": \"qemu64\" " + " }, " + " { " + " \"name\": \"Opteron_G4\" " + " }, " + " { " + " \"name\": \"Westmere\" " + " } " + " ]" + "}") < 0) + goto cleanup; + + if ((ncpus = qemuMonitorGetCPUDefinitions(qemuMonitorTestGetMonitor(test), + &cpus)) < 0) + goto cleanup; + + if (ncpus != 3) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "ncpus %d is not 3", ncpus); + goto cleanup; + } + +#define CHECK(i, wantname) \ + do { \ + if (STRNEQ(cpus[i], (wantname))) { \ + virReportError(VIR_ERR_INTERNAL_ERROR, \ + "name %s is not %s", \ + cpus[i], (wantname)); \ + goto cleanup; \ + } \ + } while (0) + + CHECK(0, "qemu64"); + CHECK(1, "Opteron_G4"); + CHECK(2, "Westmere"); + +#undef CHECK ret = 0; cleanup: @@ -319,6 +381,7 @@ mymain(void) DO_TEST(GetStatus); DO_TEST(GetVersion); DO_TEST(GetMachines); + DO_TEST(GetCPUDefinitions); virCapabilitiesFree(caps); -- 1.7.11.4
On Tue, Sep 25, 2012 at 19:00:04 +0100, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Add a new qemuMonitorGetCPUDefinitions() method to support invocation of the 'query-cpu-definitions' JSON monitor command. No HMP equivalent is required, since this will only be present for QEMU >= 1.2
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_monitor.c | 21 +++++++++++++ src/qemu/qemu_monitor.h | 4 +++ src/qemu/qemu_monitor_json.c | 75 ++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 4 +++ tests/qemumonitorjsontest.c | 63 +++++++++++++++++++++++++++++++++++++ 5 files changed, 167 insertions(+)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 581fe41..45bb32e 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3051,3 +3051,24 @@ void qemuMonitorMachineInfoFree(qemuMonitorMachineInfoPtr machine) VIR_FREE(machine->alias); VIR_FREE(machine); } + +int qemuMonitorGetCPUDefinitions(qemuMonitorPtr mon, + char ***cpus) +{ + VIR_DEBUG("mon=%p cpus=%p", + mon, cpus); + + if (!mon) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + + if (!mon->json) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("JSON monitor is required")); + return -1; + } + + return qemuMonitorJSONGetCPUDefinitions(mon, cpus); +}
Hmm, this starts to be pretty boring. I guess we should come up with a macro or something that would save us from copying the same code over and over when adding new monitor commands. Anyway, this does not need to be solved by this capabilities series. ...
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index ad095b1..1346441 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -290,6 +290,68 @@ testQemuMonitorJSONGetMachines(const void *data) CHECK(1, "pc-1.1", false, null); CHECK(2, "pc-1.2", true, "pc");
+#undef CHECK
Since you added this #undef CHECK twice in this patch, one of them should rather go into the previous patch.
+ ret = 0; + +cleanup: + qemuMonitorTestFree(test); + return ret; +} + + +static int +testQemuMonitorJSONGetCPUDefinitions(const void *data) +{ + virCapsPtr caps = (virCapsPtr)data; + qemuMonitorTestPtr test = qemuMonitorTestNew(true, caps); + int ret = -1; + char **cpus = NULL; + int ncpus; + + if (!test) + return -1; + + if (qemuMonitorTestAddItem(test, "query-cpu-definitions", + "{ " + " \"return\": [ " + " { " + " \"name\": \"qemu64\" " + " }, " + " { " + " \"name\": \"Opteron_G4\" " + " }, " + " { " + " \"name\": \"Westmere\" " + " } " + " ]" + "}") < 0) + goto cleanup; + + if ((ncpus = qemuMonitorGetCPUDefinitions(qemuMonitorTestGetMonitor(test), + &cpus)) < 0) + goto cleanup; + + if (ncpus != 3) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "ncpus %d is not 3", ncpus); + goto cleanup; + } + +#define CHECK(i, wantname) \ + do { \ + if (STRNEQ(cpus[i], (wantname))) { \ + virReportError(VIR_ERR_INTERNAL_ERROR, \ + "name %s is not %s", \ + cpus[i], (wantname)); \ + goto cleanup; \ + } \ + } while (0) + + CHECK(0, "qemu64"); + CHECK(1, "Opteron_G4"); + CHECK(2, "Westmere"); + +#undef CHECK ret = 0;
cleanup: @@ -319,6 +381,7 @@ mymain(void) DO_TEST(GetStatus); DO_TEST(GetVersion); DO_TEST(GetMachines); + DO_TEST(GetCPUDefinitions);
virCapabilitiesFree(caps);
ACK Jirka
From: "Daniel P. Berrange" <berrange@redhat.com> Add a new qemuMonitorGetCPUCommands() method to support invocation of the 'query-cpu-definitions' JSON monitor command. No HMP equivalent is required, since this will only be used when JSON is available The existing qemuMonitorJSONCheckCommands() method is refactored to use this new method Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_monitor.c | 22 +++++++++ src/qemu/qemu_monitor.h | 3 ++ src/qemu/qemu_monitor_json.c | 114 +++++++++++++++++++++++++++++++------------ src/qemu/qemu_monitor_json.h | 4 ++ tests/qemumonitorjsontest.c | 62 +++++++++++++++++++++++ 5 files changed, 175 insertions(+), 30 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 45bb32e..172b826 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3072,3 +3072,25 @@ int qemuMonitorGetCPUDefinitions(qemuMonitorPtr mon, return qemuMonitorJSONGetCPUDefinitions(mon, cpus); } + + +int qemuMonitorGetCommands(qemuMonitorPtr mon, + char ***commands) +{ + VIR_DEBUG("mon=%p commands=%p", + mon, commands); + + if (!mon) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + + if (!mon->json) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("JSON monitor is required")); + return -1; + } + + return qemuMonitorJSONGetCommands(mon, commands); +} diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index fa192d4..40bce06 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -594,6 +594,9 @@ void qemuMonitorMachineInfoFree(qemuMonitorMachineInfoPtr machine); int qemuMonitorGetCPUDefinitions(qemuMonitorPtr mon, char ***cpus); +int qemuMonitorGetCommands(qemuMonitorPtr mon, + char ***commands); + /** * When running two dd process and using <> redirection, we need a diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 876188d..9b05412 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -976,32 +976,15 @@ int qemuMonitorJSONCheckCommands(qemuMonitorPtr mon, qemuCapsPtr caps) { - int ret = -1; - virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("query-commands", NULL); - virJSONValuePtr reply = NULL; - virJSONValuePtr data; - int i, n; - - if (!cmd) - return ret; - - if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0 || - qemuMonitorJSONCheckError(cmd, reply) < 0) - goto cleanup; - - if (!(data = virJSONValueObjectGet(reply, "return")) || - data->type != VIR_JSON_TYPE_ARRAY || - (n = virJSONValueArraySize(data)) <= 0) - goto cleanup; - - for (i = 0; i < n; i++) { - virJSONValuePtr entry; - const char *name; + char **commands = NULL; + int ncommands; + size_t i; - if (!(entry = virJSONValueArrayGet(data, i)) || - !(name = virJSONValueObjectGetString(entry, "name"))) - goto cleanup; + if ((ncommands = qemuMonitorJSONGetCommands(mon, &commands)) < 0) + return -1; + for (i = 0 ; i < ncommands ; i++) { + char *name = commands[i]; if (STREQ(name, "system_wakeup")) qemuCapsSet(caps, QEMU_CAPS_WAKEUP); else if (STREQ(name, "transaction")) @@ -1012,14 +995,11 @@ qemuMonitorJSONCheckCommands(qemuMonitorPtr mon, qemuCapsSet(caps, QEMU_CAPS_BLOCKJOB_ASYNC); else if (STREQ(name, "dump-guest-memory")) qemuCapsSet(caps, QEMU_CAPS_DUMP_GUEST_MEMORY); + VIR_FREE(name); } + VIR_FREE(commands); - ret = 0; - -cleanup: - virJSONValueFree(cmd); - virJSONValueFree(reply); - return ret; + return 0; } @@ -4065,3 +4045,77 @@ cleanup: return ret; } + +int qemuMonitorJSONGetCommands(qemuMonitorPtr mon, + char ***commands) +{ + int ret; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + virJSONValuePtr data; + char **commandlist = NULL; + int n = 0; + size_t i; + + *commands = NULL; + + if (!(cmd = qemuMonitorJSONMakeCommand("query-commands", NULL))) + return -1; + + ret = qemuMonitorJSONCommand(mon, cmd, &reply); + + if (ret == 0) + ret = qemuMonitorJSONCheckError(cmd, reply); + + if (ret < 0) + goto cleanup; + + ret = -1; + + if (!(data = virJSONValueObjectGet(reply, "return"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-commands reply was missing return data")); + goto cleanup; + } + + if ((n = virJSONValueArraySize(data)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-commands reply data was not an array")); + goto cleanup; + } + + if (VIR_ALLOC_N(commandlist, n) < 0) { + virReportOOMError(); + goto cleanup; + } + + for (i = 0 ; i < n ; i++) { + virJSONValuePtr child = virJSONValueArrayGet(data, i); + const char *tmp; + + if (!(tmp = virJSONValueObjectGetString(child, "name"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-commands reply data was missing 'name'")); + goto cleanup; + } + + if (!(commandlist[i] = strdup(tmp))) { + virReportOOMError(); + goto cleanup; + } + } + + ret = n; + *commands = commandlist; + +cleanup: + if (ret < 0 && commandlist) { + for (i = 0 ; i < n ; i++) + VIR_FREE(commandlist[i]); + VIR_FREE(commandlist); + } + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} + diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 218cd02..9a64511 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -295,4 +295,8 @@ int qemuMonitorJSONGetCPUDefinitions(qemuMonitorPtr mon, char ***cpus) ATTRIBUTE_NONNULL(2); +int qemuMonitorJSONGetCommands(qemuMonitorPtr mon, + char ***commands) + ATTRIBUTE_NONNULL(2); + #endif /* QEMU_MONITOR_JSON_H */ diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 1346441..fc386fe 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -361,6 +361,67 @@ cleanup: static int +testQemuMonitorJSONGetCommands(const void *data) +{ + virCapsPtr caps = (virCapsPtr)data; + qemuMonitorTestPtr test = qemuMonitorTestNew(true, caps); + int ret = -1; + char **commands = NULL; + int ncommands; + + if (!test) + return -1; + + if (qemuMonitorTestAddItem(test, "query-commands", + "{ " + " \"return\": [ " + " { " + " \"name\": \"system_wakeup\" " + " }, " + " { " + " \"name\": \"cont\" " + " }, " + " { " + " \"name\": \"quit\" " + " } " + " ]" + "}") < 0) + goto cleanup; + + if ((ncommands = qemuMonitorGetCommands(qemuMonitorTestGetMonitor(test), + &commands)) < 0) + goto cleanup; + + if (ncommands != 3) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "ncommands %d is not 3", ncommands); + goto cleanup; + } + +#define CHECK(i, wantname) \ + do { \ + if (STRNEQ(commands[i], (wantname))) { \ + virReportError(VIR_ERR_INTERNAL_ERROR, \ + "name %s is not %s", \ + commands[i], (wantname)); \ + goto cleanup; \ + } \ + } while (0) + + CHECK(0, "system_wakeup"); + CHECK(1, "cont"); + CHECK(2, "quit"); + +#undef CHECK + ret = 0; + +cleanup: + qemuMonitorTestFree(test); + return ret; +} + + +static int mymain(void) { int ret = 0; @@ -382,6 +443,7 @@ mymain(void) DO_TEST(GetVersion); DO_TEST(GetMachines); DO_TEST(GetCPUDefinitions); + DO_TEST(GetCommands); virCapabilitiesFree(caps); -- 1.7.11.4
On 09/25/2012 12:00 PM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Add a new qemuMonitorGetCPUCommands() method to support invocation of the 'query-cpu-definitions' JSON monitor command. No HMP equivalent
s/cpu-definitions/commands/
is required, since this will only be used when JSON is available
The existing qemuMonitorJSONCheckCommands() method is refactored to use this new method
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_monitor.c | 22 +++++++++ src/qemu/qemu_monitor.h | 3 ++ src/qemu/qemu_monitor_json.c | 114 +++++++++++++++++++++++++++++++------------ src/qemu/qemu_monitor_json.h | 4 ++ tests/qemumonitorjsontest.c | 62 +++++++++++++++++++++++ 5 files changed, 175 insertions(+), 30 deletions(-)
As qemu gains more and more commands, is it worth qsort()ing the list before returning it, so later users can bsearch() for O(logN) instead of O(N) searching for a particular command? But that can be a later patch. ACK once you fix this syntax check failure: prohibit_empty_lines_at_EOF src/qemu/qemu_monitor_json.c maint.mk: empty line(s) or no newline at EOF make: *** [sc_prohibit_empty_lines_at_EOF] Error 1 -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
From: "Daniel P. Berrange" <berrange@redhat.com> Add a new qemuMonitorGetEvents() method to support invocation of the 'query-events' JSON monitor command. No HMP equivalent is required, since this will only be used when JSON is available The existing qemuMonitorJSONCheckEvents() method is refactored to use this new method Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_monitor.c | 22 ++++++++ src/qemu/qemu_monitor.h | 2 + src/qemu/qemu_monitor_json.c | 119 ++++++++++++++++++++++++++++++------------- src/qemu/qemu_monitor_json.h | 3 ++ 4 files changed, 110 insertions(+), 36 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 172b826..9e4c3fb 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3094,3 +3094,25 @@ int qemuMonitorGetCommands(qemuMonitorPtr mon, return qemuMonitorJSONGetCommands(mon, commands); } + + +int qemuMonitorGetEvents(qemuMonitorPtr mon, + char ***events) +{ + VIR_DEBUG("mon=%p events=%p", + mon, events); + + if (!mon) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + + if (!mon->json) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("JSON monitor is required")); + return -1; + } + + return qemuMonitorJSONGetEvents(mon, events); +} diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 40bce06..17cda51 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -596,6 +596,8 @@ int qemuMonitorGetCPUDefinitions(qemuMonitorPtr mon, int qemuMonitorGetCommands(qemuMonitorPtr mon, char ***commands); +int qemuMonitorGetEvents(qemuMonitorPtr mon, + char ***events); /** diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 9b05412..275f7df 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1007,49 +1007,23 @@ int qemuMonitorJSONCheckEvents(qemuMonitorPtr mon, qemuCapsPtr caps) { - int ret = -1; - virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("query-events", NULL); - virJSONValuePtr reply = NULL; - virJSONValuePtr data; - int i, n; - - if (!cmd) - return ret; - - if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) - goto cleanup; - - if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { - ret = 0; - goto cleanup; - } - - if (qemuMonitorJSONCheckError(cmd, reply) < 0) - goto cleanup; - - if (!(data = virJSONValueObjectGet(reply, "return")) || - data->type != VIR_JSON_TYPE_ARRAY || - (n = virJSONValueArraySize(data)) <= 0) - goto cleanup; + char **events = NULL; + int nevents; + size_t i; - for (i = 0; i < n; i++) { - virJSONValuePtr entry; - const char *name; + if ((nevents = qemuMonitorJSONGetEvents(mon, &events)) < 0) + return -1; - if (!(entry = virJSONValueArrayGet(data, i)) || - !(name = virJSONValueObjectGetString(entry, "name"))) - goto cleanup; + for (i = 0 ; i < nevents ; i++) { + char *name = events[i]; if (STREQ(name, "BALLOON_CHANGE")) qemuCapsSet(caps, QEMU_CAPS_BALLOON_EVENT); + VIR_FREE(name); } + VIR_FREE(events); - ret = 0; - -cleanup: - virJSONValueFree(cmd); - virJSONValueFree(reply); - return ret; + return 0; } @@ -4119,3 +4093,76 @@ cleanup: return ret; } + +int qemuMonitorJSONGetEvents(qemuMonitorPtr mon, + char ***events) +{ + int ret; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + virJSONValuePtr data; + char **eventlist = NULL; + int n = 0; + size_t i; + + *events = NULL; + + if (!(cmd = qemuMonitorJSONMakeCommand("query-events", NULL))) + return -1; + + ret = qemuMonitorJSONCommand(mon, cmd, &reply); + + if (ret == 0) + ret = qemuMonitorJSONCheckError(cmd, reply); + + if (ret < 0) + goto cleanup; + + ret = -1; + + if (!(data = virJSONValueObjectGet(reply, "return"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-events reply was missing return data")); + goto cleanup; + } + + if ((n = virJSONValueArraySize(data)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-events reply data was not an array")); + goto cleanup; + } + + if (VIR_ALLOC_N(eventlist, n) < 0) { + virReportOOMError(); + goto cleanup; + } + + for (i = 0 ; i < n ; i++) { + virJSONValuePtr child = virJSONValueArrayGet(data, i); + const char *tmp; + + if (!(tmp = virJSONValueObjectGetString(child, "name"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-events reply data was missing 'name'")); + goto cleanup; + } + + if (!(eventlist[i] = strdup(tmp))) { + virReportOOMError(); + goto cleanup; + } + } + + ret = n; + *events = eventlist; + +cleanup: + if (ret < 0 && eventlist) { + for (i = 0 ; i < n ; i++) + VIR_FREE(eventlist[i]); + VIR_FREE(eventlist); + } + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 9a64511..4798d75 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -298,5 +298,8 @@ int qemuMonitorJSONGetCPUDefinitions(qemuMonitorPtr mon, int qemuMonitorJSONGetCommands(qemuMonitorPtr mon, char ***commands) ATTRIBUTE_NONNULL(2); +int qemuMonitorJSONGetEvents(qemuMonitorPtr mon, + char ***events) + ATTRIBUTE_NONNULL(2); #endif /* QEMU_MONITOR_JSON_H */ -- 1.7.11.4
On 09/25/2012 12:00 PM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Add a new qemuMonitorGetEvents() method to support invocation of the 'query-events' JSON monitor command. No HMP equivalent is required, since this will only be used when JSON is available
The existing qemuMonitorJSONCheckEvents() method is refactored to use this new method
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_monitor.c | 22 ++++++++ src/qemu/qemu_monitor.h | 2 + src/qemu/qemu_monitor_json.c | 119 ++++++++++++++++++++++++++++++------------- src/qemu/qemu_monitor_json.h | 3 ++ 4 files changed, 110 insertions(+), 36 deletions(-)
ACK. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
From: "Daniel P. Berrange" <berrange@redhat.com> Add a new qemuMonitorGetObjectTypes() method to support invocation of the 'qom-list' JSON monitor command. No HMP equivalent is required, since this will only be present for QEMU >= 1.2 Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_monitor.c | 22 +++++++++++++ src/qemu/qemu_monitor.h | 3 ++ src/qemu/qemu_monitor_json.c | 74 ++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 4 +++ 4 files changed, 103 insertions(+) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 9e4c3fb..4ae89bf 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3116,3 +3116,25 @@ int qemuMonitorGetEvents(qemuMonitorPtr mon, return qemuMonitorJSONGetEvents(mon, events); } + + +int qemuMonitorGetObjectTypes(qemuMonitorPtr mon, + char ***types) +{ + VIR_DEBUG("mon=%p types=%p", + mon, types); + + if (!mon) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + + if (!mon->json) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("JSON monitor is required")); + return -1; + } + + return qemuMonitorJSONGetObjectTypes(mon, types); +} diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 17cda51..69c554f 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -599,6 +599,9 @@ int qemuMonitorGetCommands(qemuMonitorPtr mon, int qemuMonitorGetEvents(qemuMonitorPtr mon, char ***events); +int qemuMonitorGetObjectTypes(qemuMonitorPtr mon, + char ***types); + /** * When running two dd process and using <> redirection, we need a diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 275f7df..c2bd533 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -4166,3 +4166,77 @@ cleanup: virJSONValueFree(reply); return ret; } + + +int qemuMonitorJSONGetObjectTypes(qemuMonitorPtr mon, + char ***types) +{ + int ret; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + virJSONValuePtr data; + char **typelist = NULL; + int n = 0; + size_t i; + + *types = NULL; + + if (!(cmd = qemuMonitorJSONMakeCommand("qom-list-types", NULL))) + return -1; + + ret = qemuMonitorJSONCommand(mon, cmd, &reply); + + if (ret == 0) + ret = qemuMonitorJSONCheckError(cmd, reply); + + if (ret < 0) + goto cleanup; + + ret = -1; + + if (!(data = virJSONValueObjectGet(reply, "return"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("qom-list-types reply was missing return data")); + goto cleanup; + } + + if ((n = virJSONValueArraySize(data)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("qom-list-types reply data was not an array")); + goto cleanup; + } + + if (VIR_ALLOC_N(typelist, n) < 0) { + virReportOOMError(); + goto cleanup; + } + + for (i = 0 ; i < n ; i++) { + virJSONValuePtr child = virJSONValueArrayGet(data, i); + const char *tmp; + + if (!(tmp = virJSONValueObjectGetString(child, "name"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("qom-list-types reply data was missing 'name'")); + goto cleanup; + } + + if (!(typelist[i] = strdup(tmp))) { + virReportOOMError(); + goto cleanup; + } + } + + ret = n; + *types = typelist; + +cleanup: + if (ret < 0 && typelist) { + for (i = 0 ; i < n ; i++) + VIR_FREE(typelist[i]); + VIR_FREE(typelist); + } + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 4798d75..87be22e 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -302,4 +302,8 @@ int qemuMonitorJSONGetEvents(qemuMonitorPtr mon, char ***events) ATTRIBUTE_NONNULL(2); +int qemuMonitorJSONGetObjectTypes(qemuMonitorPtr mon, + char ***types) + ATTRIBUTE_NONNULL(2); + #endif /* QEMU_MONITOR_JSON_H */ -- 1.7.11.4
On 09/25/2012 12:00 PM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Add a new qemuMonitorGetObjectTypes() method to support invocation of the 'qom-list' JSON monitor command. No HMP equivalent
'qom-list-types'
is required, since this will only be present for QEMU >= 1.2
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_monitor.c | 22 +++++++++++++ src/qemu/qemu_monitor.h | 3 ++ src/qemu/qemu_monitor_json.c | 74 ++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 4 +++ 4 files changed, 103 insertions(+)
ACK -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
From: "Daniel P. Berrange" <berrange@redhat.com> Add a new qemuMonitorGetObjectProps() method to support invocation of the 'device-list-properties' JSON monitor command. No HMP equivalent is required, since this will only be present for QEMU >= 1.2 Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_monitor.c | 23 +++++++++++++ src/qemu/qemu_monitor.h | 3 ++ src/qemu/qemu_monitor_json.c | 82 ++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 4 +++ 4 files changed, 112 insertions(+) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 4ae89bf..6da4ecf 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3138,3 +3138,26 @@ int qemuMonitorGetObjectTypes(qemuMonitorPtr mon, return qemuMonitorJSONGetObjectTypes(mon, types); } + + +int qemuMonitorGetObjectProps(qemuMonitorPtr mon, + const char *type, + char ***props) +{ + VIR_DEBUG("mon=%p type=%s props=%p", + mon, type, props); + + if (!mon) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + + if (!mon->json) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("JSON monitor is required")); + return -1; + } + + return qemuMonitorJSONGetObjectProps(mon, type, props); +} diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 69c554f..88ff15a 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -601,6 +601,9 @@ int qemuMonitorGetEvents(qemuMonitorPtr mon, int qemuMonitorGetObjectTypes(qemuMonitorPtr mon, char ***types); +int qemuMonitorGetObjectProps(qemuMonitorPtr mon, + const char *type, + char ***props); /** diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index c2bd533..bd4da23 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -4240,3 +4240,85 @@ cleanup: virJSONValueFree(reply); return ret; } + + +int qemuMonitorJSONGetObjectProps(qemuMonitorPtr mon, + const char *type, + char ***props) +{ + int ret; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + virJSONValuePtr data; + char **proplist = NULL; + int n = 0; + size_t i; + + *props = NULL; + + if (!(cmd = qemuMonitorJSONMakeCommand("device-list-properties", + "s:typename", type, + NULL))) + return -1; + + ret = qemuMonitorJSONCommand(mon, cmd, &reply); + + if (ret == 0 && + qemuMonitorJSONHasError(reply, "DeviceNotFound")) { + goto cleanup; + } + + if (ret == 0) + ret = qemuMonitorJSONCheckError(cmd, reply); + + if (ret < 0) + goto cleanup; + + ret = -1; + + if (!(data = virJSONValueObjectGet(reply, "return"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("device-list-properties reply was missing return data")); + goto cleanup; + } + + if ((n = virJSONValueArraySize(data)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("device-list-properties reply data was not an array")); + goto cleanup; + } + + if (VIR_ALLOC_N(proplist, n) < 0) { + virReportOOMError(); + goto cleanup; + } + + for (i = 0 ; i < n ; i++) { + virJSONValuePtr child = virJSONValueArrayGet(data, i); + const char *tmp; + + if (!(tmp = virJSONValueObjectGetString(child, "name"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("device-list-properties reply data was missing 'name'")); + goto cleanup; + } + + if (!(proplist[i] = strdup(tmp))) { + virReportOOMError(); + goto cleanup; + } + } + + ret = n; + *props = proplist; + +cleanup: + if (ret < 0 && proplist) { + for (i = 0 ; i < n ; i++) + VIR_FREE(proplist[i]); + VIR_FREE(proplist); + } + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 87be22e..16a3a8f 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -305,5 +305,9 @@ int qemuMonitorJSONGetEvents(qemuMonitorPtr mon, int qemuMonitorJSONGetObjectTypes(qemuMonitorPtr mon, char ***types) ATTRIBUTE_NONNULL(2); +int qemuMonitorJSONGetObjectProps(qemuMonitorPtr mon, + const char *type, + char ***props) + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); #endif /* QEMU_MONITOR_JSON_H */ -- 1.7.11.4
On Tue, Sep 25, 2012 at 19:00:08 +0100, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Add a new qemuMonitorGetObjectProps() method to support invocation of the 'device-list-properties' JSON monitor command. No HMP equivalent is required, since this will only be present for QEMU >= 1.2
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_monitor.c | 23 +++++++++++++ src/qemu/qemu_monitor.h | 3 ++ src/qemu/qemu_monitor_json.c | 82 ++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 4 +++ 4 files changed, 112 insertions(+)
ACK Jirka
From: "Daniel P. Berrange" <berrange@redhat.com> Add a new qemuMonitorGetTargetArch() method to support invocation of the 'query-target' JSON monitor command. No HMP equivalent is required, since this will only be present for QEMU >= 1.2 Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_monitor.c | 21 +++++++++++++++++++++ src/qemu/qemu_monitor.h | 2 +- src/qemu/qemu_monitor_json.c | 45 ++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 1 + 4 files changed, 68 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 6da4ecf..ca5380b 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3161,3 +3161,24 @@ int qemuMonitorGetObjectProps(qemuMonitorPtr mon, return qemuMonitorJSONGetObjectProps(mon, type, props); } + + +char *qemuMonitorGetTargetArch(qemuMonitorPtr mon) +{ + VIR_DEBUG("mon=%p", + mon); + + if (!mon) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return NULL; + } + + if (!mon->json) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("JSON monitor is required")); + return NULL; + } + + return qemuMonitorJSONGetTargetArch(mon); +} diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 88ff15a..3e69f64 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -604,7 +604,7 @@ int qemuMonitorGetObjectTypes(qemuMonitorPtr mon, int qemuMonitorGetObjectProps(qemuMonitorPtr mon, const char *type, char ***props); - +char *qemuMonitorGetTargetArch(qemuMonitorPtr mon); /** * When running two dd process and using <> redirection, we need a diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index bd4da23..70afd91 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -4322,3 +4322,48 @@ cleanup: virJSONValueFree(reply); return ret; } + + +char * +qemuMonitorJSONGetTargetArch(qemuMonitorPtr mon) +{ + char *ret = NULL; + int rv; + const char *arch; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + virJSONValuePtr data; + + if (!(cmd = qemuMonitorJSONMakeCommand("query-target", NULL))) + return NULL; + + rv = qemuMonitorJSONCommand(mon, cmd, &reply); + + if (rv == 0) + rv = qemuMonitorJSONCheckError(cmd, reply); + + if (rv < 0) + goto cleanup; + + if (!(data = virJSONValueObjectGet(reply, "return"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-status reply was missing return data")); + goto cleanup; + } + + if (!(arch = virJSONValueObjectGetString(data, "arch"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-status reply was missing arch data")); + goto cleanup; + } + + if (!(ret = strdup(arch))) { + virReportOOMError(); + goto cleanup; + } + +cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 16a3a8f..83773d2 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -309,5 +309,6 @@ int qemuMonitorJSONGetObjectProps(qemuMonitorPtr mon, const char *type, char ***props) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); +char *qemuMonitorJSONGetTargetArch(qemuMonitorPtr mon); #endif /* QEMU_MONITOR_JSON_H */ -- 1.7.11.4
On Tue, Sep 25, 2012 at 19:00:09 +0100, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Add a new qemuMonitorGetTargetArch() method to support invocation of the 'query-target' JSON monitor command. No HMP equivalent is required, since this will only be present for QEMU >= 1.2
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_monitor.c | 21 +++++++++++++++++++++ src/qemu/qemu_monitor.h | 2 +- src/qemu/qemu_monitor_json.c | 45 ++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 1 + 4 files changed, 68 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 6da4ecf..ca5380b 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3161,3 +3161,24 @@ int qemuMonitorGetObjectProps(qemuMonitorPtr mon,
return qemuMonitorJSONGetObjectProps(mon, type, props); } + + +char *qemuMonitorGetTargetArch(qemuMonitorPtr mon) +{ + VIR_DEBUG("mon=%p", + mon); + + if (!mon) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return NULL; + } + + if (!mon->json) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("JSON monitor is required")); + return NULL; + } + + return qemuMonitorJSONGetTargetArch(mon); +} diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 88ff15a..3e69f64 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -604,7 +604,7 @@ int qemuMonitorGetObjectTypes(qemuMonitorPtr mon, int qemuMonitorGetObjectProps(qemuMonitorPtr mon, const char *type, char ***props); - +char *qemuMonitorGetTargetArch(qemuMonitorPtr mon);
/** * When running two dd process and using <> redirection, we need a diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index bd4da23..70afd91 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -4322,3 +4322,48 @@ cleanup: virJSONValueFree(reply); return ret; } + + +char * +qemuMonitorJSONGetTargetArch(qemuMonitorPtr mon) +{ + char *ret = NULL; + int rv; + const char *arch; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + virJSONValuePtr data; + + if (!(cmd = qemuMonitorJSONMakeCommand("query-target", NULL))) + return NULL; + + rv = qemuMonitorJSONCommand(mon, cmd, &reply); + + if (rv == 0) + rv = qemuMonitorJSONCheckError(cmd, reply); + + if (rv < 0) + goto cleanup; + + if (!(data = virJSONValueObjectGet(reply, "return"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-status reply was missing return data"));
s/query-status/query-target/
+ goto cleanup; + } + + if (!(arch = virJSONValueObjectGetString(data, "arch"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-status reply was missing arch data")); s/query-status/query-target/
+ goto cleanup; + } + + if (!(ret = strdup(arch))) { + virReportOOMError(); + goto cleanup; + } + +cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 16a3a8f..83773d2 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -309,5 +309,6 @@ int qemuMonitorJSONGetObjectProps(qemuMonitorPtr mon, const char *type, char ***props) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); +char *qemuMonitorJSONGetTargetArch(qemuMonitorPtr mon);
#endif /* QEMU_MONITOR_JSON_H */
ACK with the two copy&paste issues fixed. Jirka
From: "Daniel P. Berrange" <berrange@redhat.com> The qemu monitor does not require qemu_conf.h, and the qemu capabilities code actually wants bitmap.h 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.h | 1 - 3 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index c173286..f7ed3e0 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -32,8 +32,8 @@ #include "nodeinfo.h" #include "cpu/cpu.h" #include "domain_conf.h" -#include "qemu_conf.h" #include "command.h" +#include "bitmap.h" #include "virnodesuspend.h" #include <sys/stat.h> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index ca5380b..a5b867c 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -31,7 +31,6 @@ #include "qemu_monitor.h" #include "qemu_monitor_text.h" #include "qemu_monitor_json.h" -#include "qemu_conf.h" #include "virterror_internal.h" #include "memory.h" #include "logging.h" diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 3e69f64..5762893 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -29,7 +29,6 @@ # include "qemu_capabilities.h" # include "domain_conf.h" -# include "qemu_conf.h" # include "bitmap.h" # include "virhash.h" # include "json.h" -- 1.7.11.4
On Tue, Sep 25, 2012 at 19:00:10 +0100, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
The qemu monitor does not require qemu_conf.h, and the qemu capabilities code actually wants bitmap.h
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.h | 1 - 3 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index c173286..f7ed3e0 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -32,8 +32,8 @@ #include "nodeinfo.h" #include "cpu/cpu.h" #include "domain_conf.h" -#include "qemu_conf.h" #include "command.h" +#include "bitmap.h" #include "virnodesuspend.h"
#include <sys/stat.h> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index ca5380b..a5b867c 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -31,7 +31,6 @@ #include "qemu_monitor.h" #include "qemu_monitor_text.h" #include "qemu_monitor_json.h" -#include "qemu_conf.h" #include "virterror_internal.h" #include "memory.h" #include "logging.h" diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 3e69f64..5762893 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -29,7 +29,6 @@
# include "qemu_capabilities.h" # include "domain_conf.h" -# include "qemu_conf.h" # include "bitmap.h" # include "virhash.h" # include "json.h"
I guess you compile-tested :-) ACK Jirka
From: "Daniel P. Berrange" <berrange@redhat.com> The qemuMonitorSetCapabilities() API is used to initialize the QMP protocol capabilities. It has since been abused to initialize some libvirt internal capabilities based on command/event existance too. Move the latter code out into qemuCapsProbeQMP() in the QEMU capabilities source file instead Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_capabilities.c | 70 ++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_capabilities.h | 4 +++ src/qemu/qemu_monitor.c | 11 +------ src/qemu/qemu_monitor.h | 4 +-- src/qemu/qemu_monitor_json.c | 59 ------------------------------------- src/qemu/qemu_monitor_json.h | 5 ---- src/qemu/qemu_process.c | 5 +++- 7 files changed, 80 insertions(+), 78 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index f7ed3e0..9c4f3be 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1684,6 +1684,76 @@ const char *qemuCapsGetCanonicalMachine(qemuCapsPtr caps, } +static int +qemuCapsProbeQMPCommands(qemuCapsPtr caps, + qemuMonitorPtr mon) +{ + char **commands = NULL; + int ncommands; + size_t i; + + if ((ncommands = qemuMonitorGetCommands(mon, &commands)) < 0) + return -1; + + for (i = 0 ; i < ncommands ; i++) { + char *name = commands[i]; + if (STREQ(name, "system_wakeup")) + qemuCapsSet(caps, QEMU_CAPS_WAKEUP); + else if (STREQ(name, "transaction")) + qemuCapsSet(caps, QEMU_CAPS_TRANSACTION); + else if (STREQ(name, "block_job_cancel")) + qemuCapsSet(caps, QEMU_CAPS_BLOCKJOB_SYNC); + else if (STREQ(name, "block-job-cancel")) + qemuCapsSet(caps, QEMU_CAPS_BLOCKJOB_ASYNC); + else if (STREQ(name, "dump-guest-memory")) + qemuCapsSet(caps, QEMU_CAPS_DUMP_GUEST_MEMORY); + VIR_FREE(name); + } + VIR_FREE(commands); + + return 0; +} + + +static int +qemuCapsProbeQMPEvents(qemuCapsPtr caps, + qemuMonitorPtr mon) +{ + char **events = NULL; + int nevents; + size_t i; + + if ((nevents = qemuMonitorGetEvents(mon, &events)) < 0) + return -1; + + for (i = 0 ; i < nevents ; i++) { + char *name = events[i]; + + if (STREQ(name, "BALLOON_CHANGE")) + qemuCapsSet(caps, QEMU_CAPS_BALLOON_EVENT); + VIR_FREE(name); + } + VIR_FREE(events); + + return 0; +} + + +int qemuCapsProbeQMP(qemuCapsPtr caps, + qemuMonitorPtr mon) +{ + VIR_DEBUG("caps=%p mon=%p", caps, mon); + + if (qemuCapsProbeQMPCommands(caps, mon) < 0) + return -1; + + if (qemuCapsProbeQMPEvents(caps, mon) < 0) + return -1; + + return 0; +} + + #define QEMU_SYSTEM_PREFIX "qemu-system-" qemuCapsPtr qemuCapsNewForBinary(const char *binary) diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index c6e4dd8..31e39db 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -28,6 +28,7 @@ # include "capabilities.h" # include "command.h" # include "virobject.h" +# include "qemu_monitor.h" /* Internal flags to keep track of qemu command line capabilities */ enum qemuCapsFlags { @@ -162,6 +163,9 @@ qemuCapsPtr qemuCapsNew(void); qemuCapsPtr qemuCapsNewCopy(qemuCapsPtr caps); qemuCapsPtr qemuCapsNewForBinary(const char *binary); +int qemuCapsProbeQMP(qemuCapsPtr caps, + qemuMonitorPtr mon); + void qemuCapsSet(qemuCapsPtr caps, enum qemuCapsFlags flag) ATTRIBUTE_NONNULL(1); diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index a5b867c..31c8f70 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1125,8 +1125,7 @@ int qemuMonitorEmitBalloonChange(qemuMonitorPtr mon, } -int qemuMonitorSetCapabilities(qemuMonitorPtr mon, - qemuCapsPtr caps) +int qemuMonitorSetCapabilities(qemuMonitorPtr mon) { int ret; VIR_DEBUG("mon=%p", mon); @@ -1141,14 +1140,6 @@ int qemuMonitorSetCapabilities(qemuMonitorPtr mon, ret = qemuMonitorJSONSetCapabilities(mon); if (ret < 0) goto cleanup; - - ret = qemuMonitorJSONCheckCommands(mon, caps); - if (ret < 0) - goto cleanup; - - ret = qemuMonitorJSONCheckEvents(mon, caps); - if (ret < 0) - goto cleanup; } else { ret = 0; } diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 5762893..2807aa4 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -27,7 +27,6 @@ # include "internal.h" -# include "qemu_capabilities.h" # include "domain_conf.h" # include "bitmap.h" # include "virhash.h" @@ -155,8 +154,7 @@ qemuMonitorPtr qemuMonitorOpenFD(virDomainObjPtr vm, void qemuMonitorClose(qemuMonitorPtr mon); -int qemuMonitorSetCapabilities(qemuMonitorPtr mon, - qemuCapsPtr caps); +int qemuMonitorSetCapabilities(qemuMonitorPtr mon); void qemuMonitorLock(qemuMonitorPtr mon); void qemuMonitorUnlock(qemuMonitorPtr mon); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 70afd91..285a227 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -968,65 +968,6 @@ qemuMonitorJSONSetCapabilities(qemuMonitorPtr mon) } -/* - * Returns: 0 if human-monitor-command is not supported, +1 if - * human-monitor-command worked or -1 on failure - */ -int -qemuMonitorJSONCheckCommands(qemuMonitorPtr mon, - qemuCapsPtr caps) -{ - char **commands = NULL; - int ncommands; - size_t i; - - if ((ncommands = qemuMonitorJSONGetCommands(mon, &commands)) < 0) - return -1; - - for (i = 0 ; i < ncommands ; i++) { - char *name = commands[i]; - if (STREQ(name, "system_wakeup")) - qemuCapsSet(caps, QEMU_CAPS_WAKEUP); - else if (STREQ(name, "transaction")) - qemuCapsSet(caps, QEMU_CAPS_TRANSACTION); - else if (STREQ(name, "block_job_cancel")) - qemuCapsSet(caps, QEMU_CAPS_BLOCKJOB_SYNC); - else if (STREQ(name, "block-job-cancel")) - qemuCapsSet(caps, QEMU_CAPS_BLOCKJOB_ASYNC); - else if (STREQ(name, "dump-guest-memory")) - qemuCapsSet(caps, QEMU_CAPS_DUMP_GUEST_MEMORY); - VIR_FREE(name); - } - VIR_FREE(commands); - - return 0; -} - - -int -qemuMonitorJSONCheckEvents(qemuMonitorPtr mon, - qemuCapsPtr caps) -{ - char **events = NULL; - int nevents; - size_t i; - - if ((nevents = qemuMonitorJSONGetEvents(mon, &events)) < 0) - return -1; - - for (i = 0 ; i < nevents ; i++) { - char *name = events[i]; - - if (STREQ(name, "BALLOON_CHANGE")) - qemuCapsSet(caps, QEMU_CAPS_BALLOON_EVENT); - VIR_FREE(name); - } - VIR_FREE(events); - - return 0; -} - - int qemuMonitorJSONStartCPUs(qemuMonitorPtr mon, virConnectPtr conn ATTRIBUTE_UNUSED) diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 83773d2..ce30130 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -42,11 +42,6 @@ int qemuMonitorJSONHumanCommandWithFd(qemuMonitorPtr mon, int qemuMonitorJSONSetCapabilities(qemuMonitorPtr mon); -int qemuMonitorJSONCheckCommands(qemuMonitorPtr mon, - qemuCapsPtr caps); -int qemuMonitorJSONCheckEvents(qemuMonitorPtr mon, - qemuCapsPtr caps); - int qemuMonitorJSONStartCPUs(qemuMonitorPtr mon, virConnectPtr conn); int qemuMonitorJSONStopCPUs(qemuMonitorPtr mon); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 38fb9a6..e898cd0 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1263,7 +1263,10 @@ qemuConnectMonitor(struct qemud_driver *driver, virDomainObjPtr vm) qemuDomainObjEnterMonitorWithDriver(driver, vm); - ret = qemuMonitorSetCapabilities(priv->mon, priv->caps); + ret = qemuMonitorSetCapabilities(priv->mon); + if (ret == 0 && + qemuCapsGet(priv->caps, QEMU_CAPS_MONITOR_JSON)) + ret = qemuCapsProbeQMP(priv->caps, priv->mon); qemuDomainObjExitMonitorWithDriver(driver, vm); error: -- 1.7.11.4
On Tue, Sep 25, 2012 at 19:00:11 +0100, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
The qemuMonitorSetCapabilities() API is used to initialize the QMP protocol capabilities. It has since been abused to initialize some libvirt internal capabilities based on command/event existance too. Move the latter code out into qemuCapsProbeQMP() in the QEMU capabilities source file instead
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_capabilities.c | 70 ++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_capabilities.h | 4 +++ src/qemu/qemu_monitor.c | 11 +------ src/qemu/qemu_monitor.h | 4 +-- src/qemu/qemu_monitor_json.c | 59 ------------------------------------- src/qemu/qemu_monitor_json.h | 5 ---- src/qemu/qemu_process.c | 5 +++- 7 files changed, 80 insertions(+), 78 deletions(-)
ACK Jirka
From: "Daniel P. Berrange" <berrange@redhat.com> Currently the qemuCapsParseDeviceStr method has a bunch of open coded string searches/comparisons to detect devices and their properties. Soon this data will be obtained from QMP queries instead of -device help output. Maintaining the list of device and properties in two places is undesirable. Thus the existing qemuCapsParseDeviceStr() method needs to be refactored to separate the device types and properties from the actual search code. Thus the -device help output is now parsed to construct a list of device names, and device properties. These are then checked against a set of datatables to set the capability flags Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_capabilities.c | 364 ++++++++++++++++++++++++++++++------------- src/qemu/qemu_capabilities.h | 3 +- tests/qemuhelptest.c | 2 +- 3 files changed, 259 insertions(+), 110 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 9c4f3be..c4d36f9 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1233,6 +1233,262 @@ cleanup: return -1; } + +struct qemuCapsStringFlags { + const char *value; + int flag; +}; + + +struct qemuCapsStringFlags qemuCapsObjectTypes[] = { + { "hda-duplex", QEMU_CAPS_HDA_DUPLEX }, + { "hda-micro", QEMU_CAPS_HDA_MICRO }, + { "ccid-card-emulated", QEMU_CAPS_CCID_EMULATED }, + { "ccid-card-passthru", QEMU_CAPS_CCID_PASSTHRU }, + { "piix3-usb-uhci", QEMU_CAPS_PIIX3_USB_UHCI }, + { "piix4-usb-uhci", QEMU_CAPS_PIIX4_USB_UHCI }, + { "usb-ehci", QEMU_CAPS_USB_EHCI }, + { "ich9-usb-ehci1", QEMU_CAPS_ICH9_USB_EHCI1 }, + { "vt82c686b-usb-uhci", QEMU_CAPS_VT82C686B_USB_UHCI }, + { "pci-ohci", QEMU_CAPS_PCI_OHCI }, + { "nec-usb-xhci", QEMU_CAPS_NEC_USB_XHCI }, + { "usb-redir", QEMU_CAPS_USB_REDIR }, + { "usb-hub", QEMU_CAPS_USB_HUB }, + { "ich9-ahci", QEMU_CAPS_ICH9_AHCI }, + { "virtio-blk-s390", QEMU_CAPS_VIRTIO_S390 }, + { "lsi53c895a", QEMU_CAPS_SCSI_LSI }, + { "virtio-scsi-pci", QEMU_CAPS_VIRTIO_SCSI_PCI }, + { "spicevmc", QEMU_CAPS_DEVICE_SPICEVMC }, + { "qxl-vga", QEMU_CAPS_DEVICE_QXL_VGA }, + { "sga", QEMU_CAPS_SGA }, + { "scsi-block", QEMU_CAPS_SCSI_BLOCK }, + { "scsi-cd", QEMU_CAPS_SCSI_CD }, + { "ide-cd", QEMU_CAPS_IDE_CD }, +}; + + +static struct qemuCapsStringFlags qemuCapsObjectPropsVirtioBlk[] = { + { "multifunction", QEMU_CAPS_PCI_MULTIFUNCTION }, + { "bootindex", QEMU_CAPS_BOOTINDEX }, + { "ioeventfd", QEMU_CAPS_VIRTIO_IOEVENTFD }, + { "event_idx", QEMU_CAPS_VIRTIO_BLK_EVENT_IDX }, + { "scsi", QEMU_CAPS_VIRTIO_BLK_SCSI }, + { "logical_block_size", QEMU_CAPS_BLOCKIO }, +}; + +static struct qemuCapsStringFlags qemuCapsObjectPropsVirtioNet[] = { + { "tx", QEMU_CAPS_VIRTIO_TX_ALG }, + { "event_idx", QEMU_CAPS_VIRTIO_NET_EVENT_IDX }, +}; + +static struct qemuCapsStringFlags qemuCapsObjectPropsPciAssign[] = { + { "configfd", QEMU_CAPS_PCI_CONFIGFD }, + { "bootindex", QEMU_CAPS_PCI_BOOTINDEX }, +}; + +static struct qemuCapsStringFlags qemuCapsObjectPropsScsiDisk[] = { + { "channel", QEMU_CAPS_SCSI_DISK_CHANNEL }, + { "wwn", QEMU_CAPS_SCSI_DISK_WWN }, +}; + +static struct qemuCapsStringFlags qemuCapsObjectPropsIDEDrive[] = { + { "wwn", QEMU_CAPS_IDE_DRIVE_WWN }, +}; + +static struct qemuCapsStringFlags qemuCapsObjectPropsPixx4PM[] = { + { "disable_s3", QEMU_CAPS_DISABLE_S3 }, + { "disable_s4", QEMU_CAPS_DISABLE_S4 }, +}; + +static struct qemuCapsStringFlags qemuCapsObjectPropsUsbRedir[] = { + { "filter", QEMU_CAPS_USB_REDIR_FILTER }, +}; + +struct qemuCapsObjectTypeProps { + const char *type; + struct qemuCapsStringFlags *props; + size_t nprops; +}; + +static struct qemuCapsObjectTypeProps qemuCapsObjectProps[] = { + { "virtio-blk-pci", qemuCapsObjectPropsVirtioBlk, + ARRAY_CARDINALITY(qemuCapsObjectPropsVirtioBlk) }, + { "virtio-net-pci", qemuCapsObjectPropsVirtioNet, + ARRAY_CARDINALITY(qemuCapsObjectPropsVirtioNet) }, + { "pci-assign", qemuCapsObjectPropsPciAssign, + ARRAY_CARDINALITY(qemuCapsObjectPropsPciAssign) }, + { "scsi-disk", qemuCapsObjectPropsScsiDisk, + ARRAY_CARDINALITY(qemuCapsObjectPropsScsiDisk) }, + { "ide-drive", qemuCapsObjectPropsIDEDrive, + ARRAY_CARDINALITY(qemuCapsObjectPropsIDEDrive) }, + { "PIIX4_PM", qemuCapsObjectPropsPixx4PM, + ARRAY_CARDINALITY(qemuCapsObjectPropsPixx4PM) }, + { "usb-redir", qemuCapsObjectPropsUsbRedir, + ARRAY_CARDINALITY(qemuCapsObjectPropsUsbRedir) }, +}; + + +static void +qemuCapsProcessStringFlags(qemuCapsPtr caps, + size_t nflags, + struct qemuCapsStringFlags *flags, + size_t nvalues, + char *const*values) +{ + size_t i, j; + for (i = 0 ; i < nflags ; i++) { + for (j = 0 ; j < nvalues ; j++) { + if (STREQ(values[j], flags[i].value)) { + qemuCapsSet(caps, flags[i].flag); + break; + } + } + } +} + +#define OBJECT_TYPE_PREFIX "name \"" + +static int +qemuCapsParseDeviceStrObjectTypes(const char *str, + char ***types) +{ + const char *tmp = str; + int ret = -1; + size_t ntypelist = 0; + char **typelist = NULL; + + *types = NULL; + + while ((tmp = strstr(tmp, OBJECT_TYPE_PREFIX))) { + char *end; + tmp += strlen(OBJECT_TYPE_PREFIX); + end = strstr(tmp, "\""); + if (!end) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Malformed QEMU device list string, missing quote")); + goto cleanup; + } + + if (VIR_EXPAND_N(typelist, ntypelist, 1) < 0) { + virReportOOMError(); + goto cleanup; + } + if (!(typelist[ntypelist-1] = strndup(tmp, end-tmp))) { + virReportOOMError(); + goto cleanup; + } + } + + *types = typelist; + ret = ntypelist; + +cleanup: + return ret; +} + + +static int +qemuCapsParseDeviceStrObjectProps(const char *str, + const char *type, + char ***props) +{ + const char *tmp = str; + int ret = -1; + size_t nproplist = 0; + char **proplist = NULL; + + VIR_DEBUG("Extract type %s", type); + *props = NULL; + + while ((tmp = strchr(tmp, '\n'))) { + char *end; + tmp += 1; + + if (*tmp == '\0') + break; + + if (STRPREFIX(tmp, OBJECT_TYPE_PREFIX)) + continue; + + if (!STRPREFIX(tmp, type)) + continue; + + tmp += strlen(type); + if (*tmp != '.') + continue; + tmp++; + + end = strstr(tmp, "="); + if (!end) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Malformed QEMU device list string, missing '='")); + goto cleanup; + } + if (VIR_EXPAND_N(proplist, nproplist, 1) < 0) { + virReportOOMError(); + goto cleanup; + } + if (!(proplist[nproplist-1] = strndup(tmp, end-tmp))) { + virReportOOMError(); + goto cleanup; + } + } + + *props = proplist; + ret = nproplist; + +cleanup: + return ret; +} + + +static void +qemuCapsFreeStringList(size_t len, + char **values) +{ + size_t i; + for (i = 0 ; i < len ; i++) + VIR_FREE(values[i]); + VIR_FREE(values); +} + + +int +qemuCapsParseDeviceStr(qemuCapsPtr caps, const char *str) +{ + int nvalues; + char **values; + size_t i; + + if ((nvalues = qemuCapsParseDeviceStrObjectTypes(str, &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 = qemuCapsParseDeviceStrObjectProps(str, + 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 qemuCapsExtractDeviceStr(const char *qemu, qemuCapsPtr caps) @@ -1266,7 +1522,7 @@ qemuCapsExtractDeviceStr(const char *qemu, if (virCommandRun(cmd, NULL) < 0) goto cleanup; - ret = qemuCapsParseDeviceStr(output, caps); + ret = qemuCapsParseDeviceStr(caps, output); cleanup: VIR_FREE(output); @@ -1275,112 +1531,6 @@ cleanup: } -int -qemuCapsParseDeviceStr(const char *str, qemuCapsPtr caps) -{ - /* Which devices exist. */ - if (strstr(str, "name \"hda-duplex\"")) - qemuCapsSet(caps, QEMU_CAPS_HDA_DUPLEX); - if (strstr(str, "name \"hda-micro\"")) - qemuCapsSet(caps, QEMU_CAPS_HDA_MICRO); - if (strstr(str, "name \"ccid-card-emulated\"")) - qemuCapsSet(caps, QEMU_CAPS_CCID_EMULATED); - if (strstr(str, "name \"ccid-card-passthru\"")) - qemuCapsSet(caps, QEMU_CAPS_CCID_PASSTHRU); - - if (strstr(str, "name \"piix3-usb-uhci\"")) - qemuCapsSet(caps, QEMU_CAPS_PIIX3_USB_UHCI); - if (strstr(str, "name \"piix4-usb-uhci\"")) - qemuCapsSet(caps, QEMU_CAPS_PIIX4_USB_UHCI); - if (strstr(str, "name \"usb-ehci\"")) - qemuCapsSet(caps, QEMU_CAPS_USB_EHCI); - if (strstr(str, "name \"ich9-usb-ehci1\"")) - qemuCapsSet(caps, QEMU_CAPS_ICH9_USB_EHCI1); - if (strstr(str, "name \"vt82c686b-usb-uhci\"")) - qemuCapsSet(caps, QEMU_CAPS_VT82C686B_USB_UHCI); - if (strstr(str, "name \"pci-ohci\"")) - qemuCapsSet(caps, QEMU_CAPS_PCI_OHCI); - if (strstr(str, "name \"nec-usb-xhci\"")) - qemuCapsSet(caps, QEMU_CAPS_NEC_USB_XHCI); - if (strstr(str, "name \"usb-redir\"")) - qemuCapsSet(caps, QEMU_CAPS_USB_REDIR); - if (strstr(str, "usb-redir.filter")) - qemuCapsSet(caps, QEMU_CAPS_USB_REDIR_FILTER); - if (strstr(str, "name \"usb-hub\"")) - qemuCapsSet(caps, QEMU_CAPS_USB_HUB); - if (strstr(str, "name \"ich9-ahci\"")) - qemuCapsSet(caps, QEMU_CAPS_ICH9_AHCI); - if (strstr(str, "name \"virtio-blk-s390\"") || - strstr(str, "name \"virtio-net-s390\"") || - strstr(str, "name \"virtio-serial-s390\"")) - qemuCapsSet(caps, QEMU_CAPS_VIRTIO_S390); - - if (strstr(str, "name \"lsi53c895a\"")) - qemuCapsSet(caps, QEMU_CAPS_SCSI_LSI); - if (strstr(str, "name \"virtio-scsi-pci\"")) - qemuCapsSet(caps, QEMU_CAPS_VIRTIO_SCSI_PCI); - - /* Prefer -chardev spicevmc (detected earlier) over -device spicevmc */ - if (!qemuCapsGet(caps, QEMU_CAPS_CHARDEV_SPICEVMC) && - strstr(str, "name \"spicevmc\"")) - qemuCapsSet(caps, QEMU_CAPS_DEVICE_SPICEVMC); - - /* Features of given devices. */ - if (strstr(str, "pci-assign.configfd")) - qemuCapsSet(caps, QEMU_CAPS_PCI_CONFIGFD); - if (strstr(str, "virtio-blk-pci.multifunction")) - qemuCapsSet(caps, QEMU_CAPS_PCI_MULTIFUNCTION); - if (strstr(str, "virtio-blk-pci.bootindex")) { - qemuCapsSet(caps, QEMU_CAPS_BOOTINDEX); - if (strstr(str, "pci-assign.bootindex")) - qemuCapsSet(caps, QEMU_CAPS_PCI_BOOTINDEX); - } - if (strstr(str, "virtio-net-pci.tx=")) - qemuCapsSet(caps, QEMU_CAPS_VIRTIO_TX_ALG); - if (strstr(str, "name \"qxl-vga\"")) - qemuCapsSet(caps, QEMU_CAPS_DEVICE_QXL_VGA); - if (strstr(str, "virtio-blk-pci.ioeventfd")) - qemuCapsSet(caps, QEMU_CAPS_VIRTIO_IOEVENTFD); - if (strstr(str, "name \"sga\"")) - qemuCapsSet(caps, QEMU_CAPS_SGA); - if (strstr(str, "virtio-blk-pci.event_idx")) - qemuCapsSet(caps, QEMU_CAPS_VIRTIO_BLK_EVENT_IDX); - if (strstr(str, "virtio-net-pci.event_idx")) - qemuCapsSet(caps, QEMU_CAPS_VIRTIO_NET_EVENT_IDX); - if (strstr(str, "virtio-blk-pci.scsi")) - qemuCapsSet(caps, QEMU_CAPS_VIRTIO_BLK_SCSI); - if (strstr(str, "scsi-disk.channel")) - qemuCapsSet(caps, QEMU_CAPS_SCSI_DISK_CHANNEL); - if (strstr(str, "scsi-disk.wwn")) - qemuCapsSet(caps, QEMU_CAPS_SCSI_DISK_WWN); - if (strstr(str, "scsi-block")) - qemuCapsSet(caps, QEMU_CAPS_SCSI_BLOCK); - if (strstr(str, "scsi-cd")) - qemuCapsSet(caps, QEMU_CAPS_SCSI_CD); - if (strstr(str, "ide-cd")) - qemuCapsSet(caps, QEMU_CAPS_IDE_CD); - if (strstr(str, "ide-drive.wwn")) - qemuCapsSet(caps, QEMU_CAPS_IDE_DRIVE_WWN); - - /* - * the iolimit detection is not really straight forward: - * in qemu this is a capability of the block layer, if - * present any of -device scsi-disk, virtio-blk-*, ... - * will offer to specify logical and physical block size - * and other properties... - */ - if (strstr(str, ".logical_block_size") && - strstr(str, ".physical_block_size")) - qemuCapsSet(caps, QEMU_CAPS_BLOCKIO); - if (strstr(str, "PIIX4_PM.disable_s3=")) - qemuCapsSet(caps, QEMU_CAPS_DISABLE_S3); - if (strstr(str, "PIIX4_PM.disable_s4=")) - qemuCapsSet(caps, QEMU_CAPS_DISABLE_S4); - - return 0; -} - - static void uname_normalize (struct utsname *ut) { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 31e39db..059a4bc 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -219,8 +219,7 @@ int qemuCapsParseHelpStr(const char *qemu, unsigned int *kvm_version, bool check_yajl); /* Only for use by test suite */ -int qemuCapsParseDeviceStr(const char *str, - qemuCapsPtr caps); +int qemuCapsParseDeviceStr(qemuCapsPtr caps, const char *str); VIR_ENUM_DECL(qemuCaps); diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c index 079aef8..681f425 100644 --- a/tests/qemuhelptest.c +++ b/tests/qemuhelptest.c @@ -71,7 +71,7 @@ static int testHelpStrParsing(const void *data) if (virtTestLoadFile(path, &help) < 0) goto cleanup; - if (qemuCapsParseDeviceStr(help, flags) < 0) + if (qemuCapsParseDeviceStr(flags, help) < 0) goto cleanup; } -- 1.7.11.4
On Tue, Sep 25, 2012 at 19:00:12 +0100, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Currently the qemuCapsParseDeviceStr method has a bunch of open coded string searches/comparisons to detect devices and their properties. Soon this data will be obtained from QMP queries instead of -device help output. Maintaining the list of device and properties in two places is undesirable. Thus the existing qemuCapsParseDeviceStr() method needs to be refactored to separate the device types and properties from the actual search code.
Thus the -device help output is now parsed to construct a list of device names, and device properties. These are then checked against a set of datatables to set the capability flags
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_capabilities.c | 364 ++++++++++++++++++++++++++++++------------- src/qemu/qemu_capabilities.h | 3 +- tests/qemuhelptest.c | 2 +- 3 files changed, 259 insertions(+), 110 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 9c4f3be..c4d36f9 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1233,6 +1233,262 @@ cleanup: ... +qemuCapsParseDeviceStrObjectTypes(const char *str, + char ***types) +{ + const char *tmp = str; + int ret = -1; + size_t ntypelist = 0; + char **typelist = NULL; + + *types = NULL; + + while ((tmp = strstr(tmp, OBJECT_TYPE_PREFIX))) { + char *end; + tmp += strlen(OBJECT_TYPE_PREFIX); + end = strstr(tmp, "\""); + if (!end) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Malformed QEMU device list string, missing quote")); + goto cleanup; + } + + if (VIR_EXPAND_N(typelist, ntypelist, 1) < 0) { + virReportOOMError(); + goto cleanup; + } + if (!(typelist[ntypelist-1] = strndup(tmp, end-tmp))) { + virReportOOMError(); + goto cleanup; + } + } + + *types = typelist; + ret = ntypelist; + +cleanup: + return ret; +}
typelist and all string referenced from it are leaked in case of error
+ + +static int +qemuCapsParseDeviceStrObjectProps(const char *str, + const char *type, + char ***props) +{ + const char *tmp = str; + int ret = -1; + size_t nproplist = 0; + char **proplist = NULL; + + VIR_DEBUG("Extract type %s", type); + *props = NULL; + + while ((tmp = strchr(tmp, '\n'))) { + char *end; + tmp += 1; + + if (*tmp == '\0') + break; + + if (STRPREFIX(tmp, OBJECT_TYPE_PREFIX)) + continue; + + if (!STRPREFIX(tmp, type)) + continue; + + tmp += strlen(type); + if (*tmp != '.') + continue; + tmp++; + + end = strstr(tmp, "="); + if (!end) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Malformed QEMU device list string, missing '='")); + goto cleanup; + } + if (VIR_EXPAND_N(proplist, nproplist, 1) < 0) { + virReportOOMError(); + goto cleanup; + } + if (!(proplist[nproplist-1] = strndup(tmp, end-tmp))) { + virReportOOMError(); + goto cleanup; + } + } + + *props = proplist; + ret = nproplist; + +cleanup: + return ret; +}
Similar memory leak on error path. Jirka
From: "Daniel P. Berrange" <berrange@redhat.com> Start a QEMU process using $QEMU -S -no-user-config -nodefconfig -nodefaults \ -nographic -M none -qmp stdio 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 | 413 +++++++++++++++++++++++++++++++++++++++---- src/qemu/qemu_capabilities.h | 2 +- 2 files changed, 377 insertions(+), 38 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index c4d36f9..b4e824a 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> @@ -188,6 +189,8 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST, struct _qemuCaps { virObject object; + bool usedQMP; + char *binary; time_t mtime; @@ -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); @@ -1889,11 +1897,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; @@ -1906,20 +2020,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); @@ -1930,39 +2043,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) || @@ -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; +} + + + +static void qemuCapsMonitorEOFNotify(qemuMonitorPtr mon ATTRIBUTE_UNUSED, + virDomainObjPtr vm ATTRIBUTE_UNUSED) +{ +} + +static void qemuCapsMonitorErrorNotify(qemuMonitorPtr mon ATTRIBUTE_UNUSED, + virDomainObjPtr vm ATTRIBUTE_UNUSED) +{ +} + +static qemuMonitorCallbacks callbacks = { + .eofNotify = qemuCapsMonitorEOFNotify, + .errorNotify = qemuCapsMonitorErrorNotify, +}; + + +/* 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); +} + +/* + * Returns -1 on fatal error, 0 on success + */ +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; + + cmd = virCommandNewArgList(caps->binary, + "-S", + "-no-user-config", + "-nodefconfig", + "-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; + + VIR_FORCE_CLOSE(socks[1]); + + if (!(mon = qemuMonitorOpenFD(vm, socks[0], true, &callbacks))) + goto cleanup; + socks[0] = -1; + + 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: - VIR_FREE(help); + if (mon) + qemuMonitorUnlock(mon); + qemuMonitorClose(mon); + virCommandAbort(cmd); + VIR_FORCE_CLOSE(socks[0]); + virObjectUnref(vm); 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: @@ -1998,7 +2331,7 @@ no_memory: error: virObjectUnref(caps); caps = NULL; - goto cleanup; + return NULL; } @@ -2016,6 +2349,12 @@ bool qemuCapsIsValid(qemuCapsPtr caps) } +bool qemuCapsUsedQMP(qemuCapsPtr caps) +{ + return caps->usedQMP; +} + + static void qemuCapsHashDataFree(void *payload, const void *key ATTRIBUTE_UNUSED) { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 059a4bc..9078abb 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -197,7 +197,7 @@ int qemuCapsGetMachineTypesCaps(qemuCapsPtr caps, virCapsGuestMachinePtr **machines); bool qemuCapsIsValid(qemuCapsPtr caps); - +bool qemuCapsUsedQMP(qemuCapsPtr caps); qemuCapsCachePtr qemuCapsCacheNew(void); qemuCapsPtr qemuCapsCacheLookup(qemuCapsCachePtr cache, const char *binary); -- 1.7.11.4
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.
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 | 413 +++++++++++++++++++++++++++++++++++++++---- src/qemu/qemu_capabilities.h | 2 +- 2 files changed, 377 insertions(+), 38 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index c4d36f9..b4e824a 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> @@ -188,6 +189,8 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST, struct _qemuCaps { virObject object;
+ bool usedQMP; + char *binary; time_t mtime;
@@ -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? ...
@@ -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.
+ + +
I guess two empty lines would be sufficient? :-)
+static void qemuCapsMonitorEOFNotify(qemuMonitorPtr mon ATTRIBUTE_UNUSED, + virDomainObjPtr vm ATTRIBUTE_UNUSED)
Wrong indentation.
+{ +} + +static void qemuCapsMonitorErrorNotify(qemuMonitorPtr mon ATTRIBUTE_UNUSED, + virDomainObjPtr vm ATTRIBUTE_UNUSED)
Wrong indentation here as well.
+{ +} + +static qemuMonitorCallbacks callbacks = { + .eofNotify = qemuCapsMonitorEOFNotify, + .errorNotify = qemuCapsMonitorErrorNotify, +};
Anyway, can't we just do static void qemuCapsMonitorNotify(qemuMonitorPtr mon ATTRIBUTE_UNUSED, virDomainObjPtr vm ATTRIBUTE_UNUSED) { } static qemuMonitorCallbacks callbacks = { .eofNotify = qemuCapsMonitorNotify, .errorNotify = qemuCapsMonitorNotify, }; since both notifiers have the same prototype?
+ + +/* 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); +} + +/* + * Returns -1 on fatal error, 0 on success + */
As almost every function in libvirt does...
+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.
+ + 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. ... Jirka
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 :|
participants (4)
-
Daniel P. Berrange -
Doug Goldstein -
Eric Blake -
Jiri Denemark