[libvirt] [PATCH RFC 0/8] qemu: Cache results of parsing qemu help output
Stracing libvirtd shows that the qemu driver is executing 2 different qemu binaries 3 times each to fetch the version, capabilities [supported devices], and cpu models each time VM state is queried. E.g., [lines wrapped]: 6471 17:15:26.561890 execve("/usr/bin/qemu", ["/usr/bin/qemu", "-cpu", "?"], [/* 2 vars */]) = 0 6472 17:15:26.626668 execve("/usr/bin/qemu", ["/usr/bin/qemu", "-help"], [/* 2 vars */]) = 0 6473 17:15:26.698104 execve("/usr/bin/qemu", ["/usr/bin/qemu", "-device", "?", "-device", "pci-assign,?", "-device", "virtio-blk-pci,?"], [/* 2 vars */]) = 0 6484 17:15:27.267770 execve("/usr/bin/qemu-system-x86_64", ["/usr/bin/qemu-system-x86_64", "-cpu", "?"], /* 2 vars */]) = 0 6492 17:15:27.333177 execve("/usr/bin/qemu-system-x86_64", ["/usr/bin/qemu-system-x86_64", "-help"], [/* 2 vars */]) = 0 6496 17:15:27.402280 execve("/usr/bin/qemu-system-x86_64", ["/usr/bin/qemu-system-x86_64", "-device", "?", "-device", "pci-assign,?", "-device", "virtio-blk-pci,?"], [/* 2 vars */]) = 0 ~1sec per libvirt api call. Not a killer, but on a heavily loaded host -- several 10s of VMs -- a periodic query of all VM state, such as from a cloud compute manager, can take a couple of minutes to complete. Because the qemu binaries on the host do not change all that often, the results of parsing the qemu help output from the exec's above can be cached. The qemu driver already does some caching of capabilities, but it does not prevent the execs above. This series is a work in progress. I'm submitting it as an RFC because I saw Eric mention the frequent execing of qemu binaries and I have been working on this to eliminate the overhead shown above. The series caches the parse results of: + qemuCapsExtractVersionInfo + qemuCapsProbeMachineTypes + qemuCapsProbeCPUModels by splitting these functions into two parts. The existing function name fetches the cached parse results for the specified binary and returns them. The other half, named "qemuCapsCacheX", where X is one of ExtractVersionInfo, ProbeMachineTypes, and ProbeCPUModels, exec's the emulator binary and caches the results. The act of fetching the cached results will fill or refresh the cache as necessary in a new function qemuCapsCachedInfoGet(). A few auxilliary function have been added -- e.g., virCapabilitiesDupMachines() to duplicate a cached list of machine types and virBitmapDup() to duplicate cached capabilities flags. The series does not attempt to integrate with nor remove the existing capabilities caching. TBD. The series was developed and tested in the context of the Ubuntu 11.04 natty libvirt_0.8.8-1ubuntu6.7 package using quilt to manage patches in the debian/patches directory. In that context, it builds, passes all "make check" tests [under pbuilder] and some fairly heavy, overlapping VM launch tests where it does eliminate all but a few initial exec's of the various qemu* and kvm binaries. The version here, rebased to libvirt-0.9.10, builds cleanly under mock on Fedora 16 in the context of a modified libvirt-0.9.10-1.fc16 source package. I.e., no errors and warning-for-warning compatible with build of the libvirt-0.9.10 fc16 srpm downloaded from libvirt.org. I placed the modified spec file [applies the patches] and the build logs at: http://downloads.linux.hp.com/~lts/Libvirt/ I have installed the patched libvirt on a fedora 16 system and successfully defined and launched a vm. Testing in progress. I'll place an annotated test log ont the site above when complete. I also need to rebase atop the current mainline sources, but I wanted to get this series out for review to see if the overall approach would be acceptable. Comments?
Add support to duplicate a virCapsGuestMachine object -- e.g., from cached emulator information. --- src/conf/capabilities.c | 34 ++++++++++++++++++++++++++++++++++ src/conf/capabilities.h | 3 +++ src/libvirt_private.syms | 1 + 3 files changed, 38 insertions(+) Index: libvirt-0.9.10/src/conf/capabilities.h =================================================================== --- libvirt-0.9.10.orig/src/conf/capabilities.h +++ libvirt-0.9.10/src/conf/capabilities.h @@ -208,6 +208,9 @@ virCapabilitiesSetHostCPU(virCapsPtr cap extern virCapsGuestMachinePtr * virCapabilitiesAllocMachines(const char *const *names, int nnames); +extern virCapsGuestMachinePtr * +virCapabilitiesDupMachines(const virCapsGuestMachinePtr *smachines, + int nmachines); extern void virCapabilitiesFreeMachines(virCapsGuestMachinePtr *machines, int nmachines); Index: libvirt-0.9.10/src/conf/capabilities.c =================================================================== --- libvirt-0.9.10.orig/src/conf/capabilities.c +++ libvirt-0.9.10/src/conf/capabilities.c @@ -325,6 +325,40 @@ virCapabilitiesAllocMachines(const char } /** + * virCapabilitiesDupMachines: + * @smachines: table of virCapsGuestMachinePtr + * @nmachines: number of machine variants in table + * + * Allocate a table of virCapsGuestMachinePtr from the supplied table + * of virCapsGuestMachinePtr + */ +virCapsGuestMachinePtr * +virCapabilitiesDupMachines(const virCapsGuestMachinePtr *smachines, int nmachines) +{ + virCapsGuestMachinePtr *machines; + int i; + + if (VIR_ALLOC_N(machines, nmachines) < 0) + return NULL; + + for (i = 0; i < nmachines; i++) { + if (VIR_ALLOC(machines[i]) < 0 || + !(machines[i]->name = strdup(smachines[i]->name))) + goto error; + if (smachines[i]->canonical && + !(machines[i]->canonical = strdup(smachines[i]->canonical))) + goto error; + } + + return machines; + +error: + virCapabilitiesFreeMachines(machines, nmachines); + return NULL; + +} + +/** * virCapabilitiesFreeMachines: * @machines: table of vircapsGuestMachinePtr * Index: libvirt-0.9.10/src/libvirt_private.syms =================================================================== --- libvirt-0.9.10.orig/src/libvirt_private.syms +++ libvirt-0.9.10/src/libvirt_private.syms @@ -49,6 +49,7 @@ virCapabilitiesAllocMachines; virCapabilitiesDefaultGuestArch; virCapabilitiesDefaultGuestEmulator; virCapabilitiesDefaultGuestMachine; +virCapabilitiesDupMachines; virCapabilitiesFormatXML; virCapabilitiesFree; virCapabilitiesFreeMachines;
Add function to duplicate a virBitmap for returning cached qemuCaps[Flags] from qemuCapsExtractVersionInfo TODO: could factor the size calculation into a macro or inline function as it's used by both Dup and Alloc. --- src/libvirt_private.syms | 1 + src/util/bitmap.c | 24 ++++++++++++++++++++++++ src/util/bitmap.h | 6 ++++++ 3 files changed, 31 insertions(+) Index: libvirt-0.9.10/src/util/bitmap.c =================================================================== --- libvirt-0.9.10.orig/src/util/bitmap.c +++ libvirt-0.9.10/src/util/bitmap.c @@ -80,6 +80,30 @@ virBitmapPtr virBitmapAlloc(size_t size) } /** + * virBitmapDup: + * @sbm: source bitmap to duplicate + * + * Duplicate the source bitmap. + * + * Returns a pointer to the duplicated bitmap or NULL if + * memory cannot be allocated. + */ +virBitmapPtr virBitmapDup(virBitmapPtr sbm) +{ + virBitmapPtr bitmap; + + bitmap = virBitmapAlloc(sbm->size); + if (bitmap) { + size_t sz = (sbm->size + VIR_BITMAP_BITS_PER_UNIT - 1) / + VIR_BITMAP_BITS_PER_UNIT; + int i; + for (i = 0; i < sz; ++i) + bitmap->map[i] = sbm->map[i]; + } + return bitmap; +} + +/** * virBitmapFree: * @bitmap: previously allocated bitmap * Index: libvirt-0.9.10/src/util/bitmap.h =================================================================== --- libvirt-0.9.10.orig/src/util/bitmap.h +++ libvirt-0.9.10/src/util/bitmap.h @@ -37,6 +37,12 @@ typedef virBitmap *virBitmapPtr; virBitmapPtr virBitmapAlloc(size_t size) ATTRIBUTE_RETURN_CHECK; /* + * Duplicate the specified virBitmap + */ +virBitmapPtr virBitmapDup(virBitmapPtr src) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; + +/* * Free previously allocated bitmap */ void virBitmapFree(virBitmapPtr bitmap); Index: libvirt-0.9.10/src/libvirt_private.syms =================================================================== --- libvirt-0.9.10.orig/src/libvirt_private.syms +++ libvirt-0.9.10/src/libvirt_private.syms @@ -13,6 +13,7 @@ virRequestUsername; # bitmap.h virBitmapAlloc; virBitmapClearBit; +virBitmapDup; virBitmapFree; virBitmapGetBit; virBitmapSetBit;
Define a qemu emulator cache structure and function to lookup, create, refresh emulator cache objects. The cache "tags" are the paths to the emulator binaries. E.g., "/usr/bin/qemu" Subsequent patches will "hook up" the various extract/probe info functions to consult the cache. Notes/questions: 1) "qemuCapsProbes" converted to bitmap along with capabilities flags as part of rebase. Overkill? 2) Is it OK for the root of the cache and the nEmulators to be statically defined in qemu_capabilities.c as opposed to a field in the driver struct? It is private to that source file and I don't see an easy wait to get a handle on the driver struct therein. --- src/qemu/qemu_capabilities.c | 166 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 166 insertions(+) Index: libvirt-0.9.10/src/qemu/qemu_capabilities.c =================================================================== --- libvirt-0.9.10.orig/src/qemu/qemu_capabilities.c +++ libvirt-0.9.10/src/qemu/qemu_capabilities.c @@ -170,6 +170,46 @@ struct qemu_arch_info { int nflags; }; +/* + * * Flags to record which "probes" have been cached + * */ +enum qemuCapsProbes { + QEMU_PROBE_VERSION_INFO = 0, + QEMU_PROBE_MACHINE_TYPES = 1, + QEMU_PROBE_CPU_MODELS = 2, + QEMU_PROBE_SIZE +}; + +typedef struct _qemuEmulatorCache qemuEmulatorCache; +typedef qemuEmulatorCache* qemuEmulatorCachePtr; +struct _qemuEmulatorCache { + char *path; + char *arch; + time_t mtime; + virBitmapPtr cachedProbes; + + unsigned int version; + virBitmapPtr caps; + + virCapsGuestMachinePtr *machines; + int nmachines; + + char **cpus; + unsigned int ncpus; +}; + +static qemuEmulatorCachePtr *emulatorCache = NULL; +static int nEmulators; + +static qemuEmulatorCachePtr +qemuEmulatorCachedInfoGet(enum qemuCapsProbes, + const char *binary, + const char *arch); + +static void +qemuEmulatorCachedInfoRelease(qemuEmulatorCachePtr emulator ATTRIBUTE_UNUSED) +{ } + /* Feature flags for the architecture info */ static const struct qemu_feature_flags const arch_info_i686_flags [] = { { "pae", 1, 0 }, @@ -319,6 +359,12 @@ cleanup: } static int +qemuCapsCacheMachineTypes(qemuEmulatorCachePtr emulator) +{ + return emulator ? 0 : 1; +} + +static int qemuCapsGetOldMachinesFromInfo(virCapsGuestDomainInfoPtr info, const char *emulator, time_t emulator_mtime, @@ -612,6 +658,11 @@ cleanup: return ret; } +static int +qemuCapsCacheCPUModels(qemuEmulatorCachePtr emulator) +{ + return emulator ? 0 : 1; +} static int qemuCapsInitGuest(virCapsPtr caps, @@ -1510,6 +1561,12 @@ cleanup: return ret; } +static int +qemuCapsCacheVersionInfo(qemuEmulatorCachePtr emulator) +{ + return emulator ? 0 : 1; +} + static void uname_normalize (struct utsname *ut) { @@ -1610,3 +1667,112 @@ qemuCapsGet(virBitmapPtr caps, else return b; } + +static qemuEmulatorCachePtr +qemuEmulatorCachedInfoGet(enum qemuCapsProbes probe, + const char *binary, + const char *arch) +{ + qemuEmulatorCachePtr emulator = NULL; + struct stat st; + bool alreadyCached; + int i; + + if (stat(binary, &st) != 0) { + char ebuf[1024]; + VIR_INFO("Failed to stat emulator %s : %s", + binary, virStrerror(errno, ebuf, sizeof(ebuf))); + goto error; + } + + for (i = 0; i < nEmulators; ++i) { + emulator = emulatorCache[i]; + + if (!STREQ(binary, emulator->path)) + continue; + + if (arch && !emulator->arch) { + if (!(emulator->arch = strdup(arch))) + goto no_memory; + /* + * We have an 'arch' now, where we didn't before. + * So, even if we've already cached this probe, + * refresh the cache with the specified arch. + */ + break; + } + + if (st.st_mtime != emulator->mtime) + break; /* binary changed, refresh cache */ + + if (virBitmapGetBit(emulator->cachedProbes, probe, &alreadyCached) < 0) { + VIR_ERROR(_("Unrecognized probe id '%d'"), probe); + goto error; + } + if (!alreadyCached) + break; /* do it now */ + + return emulator; + } + + if (i == nEmulators) { + if (VIR_REALLOC_N(emulatorCache, nEmulators + 1) < 0) + goto no_memory; + if (VIR_ALLOC(emulator) < 0) + goto no_memory; + if (!(emulator->path = strdup(binary))) + goto no_memory_free_emulator; + if (arch && !(emulator->arch = strdup(arch))) + goto no_memory_free_emulator; + if (!(emulator->cachedProbes = virBitmapAlloc(QEMU_PROBE_SIZE))) + goto no_memory_free_emulator; + VIR_DEBUG("Adding emulator cache for %s - %s", + emulator->path, emulator->arch); + emulatorCache[nEmulators] = emulator; + nEmulators += 1; + } else { + VIR_DEBUG("Refreshing emulator cache for %s - %s", + emulator->path, emulator->arch); + } + + switch (probe) { + + case QEMU_PROBE_VERSION_INFO: + if (qemuCapsCacheVersionInfo(emulator) != 0 || + virBitmapSetBit(emulator->cachedProbes, QEMU_PROBE_VERSION_INFO) < 0) + goto error; + break; + + case QEMU_PROBE_MACHINE_TYPES: + if (qemuCapsCacheMachineTypes(emulator) != 0 || + virBitmapSetBit(emulator->cachedProbes, QEMU_PROBE_MACHINE_TYPES) < 0) + goto error; + break; + + case QEMU_PROBE_CPU_MODELS: + if (qemuCapsCacheCPUModels(emulator) != 0 || + virBitmapSetBit(emulator->cachedProbes, QEMU_PROBE_CPU_MODELS) < 0) + goto error; + break; + + default: + /* We REALLY shouldn't get here... */ + VIR_ERROR(_("Unrecognized probe id '%d'"), probe); + goto error; + } + + + emulator->mtime = st.st_mtime; + return emulator; + +no_memory_free_emulator: + VIR_FREE(emulator->path); + VIR_FREE(emulator->arch); + VIR_FREE(emulator); + +no_memory: + virReportOOMError(); + +error: + return NULL; +}
On Sun, Mar 11, 2012 at 02:45:04PM -0400, Lee Schermerhorn wrote:
Define a qemu emulator cache structure and function to lookup, create, refresh emulator cache objects. The cache "tags" are the paths to the emulator binaries. E.g., "/usr/bin/qemu"
Subsequent patches will "hook up" the various extract/probe info functions to consult the cache.
Notes/questions: 1) "qemuCapsProbes" converted to bitmap along with capabilities flags as part of rebase. Overkill? 2) Is it OK for the root of the cache and the nEmulators to be statically defined in qemu_capabilities.c as opposed to a field in the driver struct? It is private to that source file and I don't see an easy wait to get a handle on the driver struct therein.
--- src/qemu/qemu_capabilities.c | 166 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 166 insertions(+)
Index: libvirt-0.9.10/src/qemu/qemu_capabilities.c =================================================================== --- libvirt-0.9.10.orig/src/qemu/qemu_capabilities.c +++ libvirt-0.9.10/src/qemu/qemu_capabilities.c @@ -170,6 +170,46 @@ struct qemu_arch_info { int nflags; };
+/* + * * Flags to record which "probes" have been cached + * */ +enum qemuCapsProbes { + QEMU_PROBE_VERSION_INFO = 0, + QEMU_PROBE_MACHINE_TYPES = 1, + QEMU_PROBE_CPU_MODELS = 2, + QEMU_PROBE_SIZE +}; + +typedef struct _qemuEmulatorCache qemuEmulatorCache; +typedef qemuEmulatorCache* qemuEmulatorCachePtr; +struct _qemuEmulatorCache { + char *path; + char *arch; + time_t mtime; + virBitmapPtr cachedProbes; + + unsigned int version; + virBitmapPtr caps; + + virCapsGuestMachinePtr *machines; + int nmachines; + + char **cpus; + unsigned int ncpus; +}; + +static qemuEmulatorCachePtr *emulatorCache = NULL; +static int nEmulators; + +static qemuEmulatorCachePtr +qemuEmulatorCachedInfoGet(enum qemuCapsProbes, + const char *binary, + const char *arch); + +static void +qemuEmulatorCachedInfoRelease(qemuEmulatorCachePtr emulator ATTRIBUTE_UNUSED) +{ } + /* Feature flags for the architecture info */ static const struct qemu_feature_flags const arch_info_i686_flags [] = { { "pae", 1, 0 }, @@ -319,6 +359,12 @@ cleanup: }
static int +qemuCapsCacheMachineTypes(qemuEmulatorCachePtr emulator) +{ + return emulator ? 0 : 1; +} + +static int qemuCapsGetOldMachinesFromInfo(virCapsGuestDomainInfoPtr info, const char *emulator, time_t emulator_mtime, @@ -612,6 +658,11 @@ cleanup: return ret; }
+static int +qemuCapsCacheCPUModels(qemuEmulatorCachePtr emulator) +{ + return emulator ? 0 : 1; +}
static int qemuCapsInitGuest(virCapsPtr caps, @@ -1510,6 +1561,12 @@ cleanup: return ret; }
+static int +qemuCapsCacheVersionInfo(qemuEmulatorCachePtr emulator) +{ + return emulator ? 0 : 1; +} + static void uname_normalize (struct utsname *ut) { @@ -1610,3 +1667,112 @@ qemuCapsGet(virBitmapPtr caps, else return b; } + +static qemuEmulatorCachePtr +qemuEmulatorCachedInfoGet(enum qemuCapsProbes probe, + const char *binary, + const char *arch) +{ + qemuEmulatorCachePtr emulator = NULL; + struct stat st; + bool alreadyCached; + int i; + + if (stat(binary, &st) != 0) { + char ebuf[1024]; + VIR_INFO("Failed to stat emulator %s : %s", + binary, virStrerror(errno, ebuf, sizeof(ebuf))); + goto error;
Hum ... seems to me we should still go though the emulator cache here and discard cached info (if present) for that path
+ } + + for (i = 0; i < nEmulators; ++i) { + emulator = emulatorCache[i]; + + if (!STREQ(binary, emulator->path)) + continue; + + if (arch && !emulator->arch) { + if (!(emulator->arch = strdup(arch))) + goto no_memory; + /* + * We have an 'arch' now, where we didn't before. + * So, even if we've already cached this probe, + * refresh the cache with the specified arch. + */ + break; + } + + if (st.st_mtime != emulator->mtime) + break; /* binary changed, refresh cache */ + + if (virBitmapGetBit(emulator->cachedProbes, probe, &alreadyCached) < 0) { + VIR_ERROR(_("Unrecognized probe id '%d'"), probe); + goto error; + } + if (!alreadyCached) + break; /* do it now */ + + return emulator; + } + + if (i == nEmulators) { + if (VIR_REALLOC_N(emulatorCache, nEmulators + 1) < 0) + goto no_memory; + if (VIR_ALLOC(emulator) < 0) + goto no_memory; + if (!(emulator->path = strdup(binary))) + goto no_memory_free_emulator; + if (arch && !(emulator->arch = strdup(arch))) + goto no_memory_free_emulator; + if (!(emulator->cachedProbes = virBitmapAlloc(QEMU_PROBE_SIZE))) + goto no_memory_free_emulator; + VIR_DEBUG("Adding emulator cache for %s - %s", + emulator->path, emulator->arch); + emulatorCache[nEmulators] = emulator; + nEmulators += 1; + } else { + VIR_DEBUG("Refreshing emulator cache for %s - %s", + emulator->path, emulator->arch); + } + + switch (probe) { + + case QEMU_PROBE_VERSION_INFO: + if (qemuCapsCacheVersionInfo(emulator) != 0 || + virBitmapSetBit(emulator->cachedProbes, QEMU_PROBE_VERSION_INFO) < 0) + goto error; + break; + + case QEMU_PROBE_MACHINE_TYPES: + if (qemuCapsCacheMachineTypes(emulator) != 0 || + virBitmapSetBit(emulator->cachedProbes, QEMU_PROBE_MACHINE_TYPES) < 0) + goto error; + break; + + case QEMU_PROBE_CPU_MODELS: + if (qemuCapsCacheCPUModels(emulator) != 0 || + virBitmapSetBit(emulator->cachedProbes, QEMU_PROBE_CPU_MODELS) < 0) + goto error; + break; + + default: + /* We REALLY shouldn't get here... */ + VIR_ERROR(_("Unrecognized probe id '%d'"), probe); + goto error; + } + + + emulator->mtime = st.st_mtime; + return emulator; + +no_memory_free_emulator: + VIR_FREE(emulator->path); + VIR_FREE(emulator->arch); + VIR_FREE(emulator); + +no_memory: + virReportOOMError(); + +error: + return NULL; +}
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
Hook up qemuCapsExtractVersionInfo capabilities api to the emulator cache framework by splitting into two parts: - qemuCapsExtractVersionInfo() looks up emulator in cache and returns the cached version and flags. Cache look up may fill or refresh the cache. - wrap the part of the original qemuCapsExtractVersionInfo() with qemuCapsCacheVersionInfo() to run the specified binary and update the cached information for this binary. --- src/qemu/qemu_capabilities.c | 51 ++++++++++++++++++++++++++----------------- 1 file changed, 32 insertions(+), 19 deletions(-) Index: libvirt-0.9.10/src/qemu/qemu_capabilities.c =================================================================== --- libvirt-0.9.10.orig/src/qemu/qemu_capabilities.c +++ libvirt-0.9.10/src/qemu/qemu_capabilities.c @@ -1498,16 +1498,37 @@ int qemuCapsExtractVersionInfo(const cha unsigned int *retversion, virBitmapPtr *retflags) { + qemuEmulatorCachePtr emulator; + int ret = -1; + + emulator = qemuEmulatorCachedInfoGet(QEMU_PROBE_VERSION_INFO, qemu, arch); + if (emulator) { + if (retflags) + *retflags = virBitmapDup(emulator->caps); + if (retversion) + *retversion = emulator->version; + ret = 0; + } else { + if (retflags) + *retflags = 0; + if (retversion) + *retversion = 0; + } + + qemuEmulatorCachedInfoRelease(emulator); + return ret; +} + +static int +qemuCapsCacheVersionInfo(qemuEmulatorCachePtr emulator) +{ int ret = -1; unsigned int version, is_kvm, kvm_version; virBitmapPtr flags = NULL; char *help = NULL; + char *qemu = emulator->path, *arch = emulator->arch; virCommandPtr cmd; - - if (retflags) - *retflags = NULL; - if (retversion) - *retversion = 0; + VIR_DEBUG("Caching Version Info for %s - %s", qemu, arch ?: "no-arch"); /* Make sure the binary we are about to try exec'ing exists. * Technically we could catch the exec() failure, but that's @@ -1532,8 +1553,8 @@ int qemuCapsExtractVersionInfo(const cha goto cleanup; /* Currently only x86_64 and i686 support PCI-multibus. */ - if (STREQLEN(arch, "x86_64", 6) || - STREQLEN(arch, "i686", 4)) { + if (arch && (STREQLEN(arch, "x86_64", 6) || + STREQLEN(arch, "i686", 4))) { qemuCapsSet(flags, QEMU_CAPS_PCI_MULTIBUS); } @@ -1544,12 +1565,10 @@ int qemuCapsExtractVersionInfo(const cha qemuCapsExtractDeviceStr(qemu, flags) < 0) goto cleanup; - if (retversion) - *retversion = version; - if (retflags) { - *retflags = flags; - flags = NULL; - } + emulator->version = version; + qemuCapsFree(emulator->caps); /* for possible refresh */ + emulator->caps = flags; + flags = NULL; ret = 0; @@ -1561,12 +1580,6 @@ cleanup: return ret; } -static int -qemuCapsCacheVersionInfo(qemuEmulatorCachePtr emulator) -{ - return emulator ? 0 : 1; -} - static void uname_normalize (struct utsname *ut) {
Hook up qemuCapsProbeMachineTypes capabilities api to the emulator cache framework: - qemuCapsProbeMachineTypes() looks up emulator in cache and returns the version and flags. - wrap the part of the original qemuCapsProbeMachineTypes() with qemuCapsCacheMachineTypes() to run the specified binary and update the cached machine types supported by this binary. --- src/qemu/qemu_capabilities.c | 39 ++++++++++++++++++++++++++++++++------- 1 file changed, 32 insertions(+), 7 deletions(-) Index: libvirt-0.9.10/src/qemu/qemu_capabilities.c =================================================================== --- libvirt-0.9.10.orig/src/qemu/qemu_capabilities.c +++ libvirt-0.9.10/src/qemu/qemu_capabilities.c @@ -307,6 +307,11 @@ qemuCapsParseMachineTypesStr(const char } } while ((p = next)); + /* + * Free in case of possible cache refresh + */ + virCapabilitiesFreeMachines(*machines, *nmachines); + *machines = list; *nmachines = nitems; @@ -323,10 +328,36 @@ qemuCapsProbeMachineTypes(const char *bi virCapsGuestMachinePtr **machines, int *nmachines) { + qemuEmulatorCachePtr emulator; + int ret = -1; + + emulator = qemuEmulatorCachedInfoGet(QEMU_PROBE_MACHINE_TYPES, binary, NULL); + if (emulator) { + ret = 0; + if (machines) + *machines = virCapabilitiesDupMachines(emulator->machines, emulator->nmachines); + if (nmachines) + *nmachines = emulator->nmachines; + } else { + if (machines) + *machines = NULL; + if (nmachines) + *nmachines = 0; + } + + qemuEmulatorCachedInfoRelease(emulator); + return ret; +} + +static int +qemuCapsCacheMachineTypes(qemuEmulatorCachePtr emulator) +{ + char *binary = emulator->path; char *output; int ret = -1; virCommandPtr cmd; int status; + VIR_DEBUG("Caching Machine Types for %s", binary); /* Make sure the binary we are about to try exec'ing exists. * Technically we could catch the exec() failure, but that's @@ -346,7 +377,7 @@ qemuCapsProbeMachineTypes(const char *bi if (virCommandRun(cmd, &status) < 0) goto cleanup; - if (qemuCapsParseMachineTypesStr(output, machines, nmachines) < 0) + if (qemuCapsParseMachineTypesStr(output, &emulator->machines, &emulator->nmachines) < 0) goto cleanup; ret = 0; @@ -359,12 +390,6 @@ cleanup: } static int -qemuCapsCacheMachineTypes(qemuEmulatorCachePtr emulator) -{ - return emulator ? 0 : 1; -} - -static int qemuCapsGetOldMachinesFromInfo(virCapsGuestDomainInfoPtr info, const char *emulator, time_t emulator_mtime,
On Sun, Mar 11, 2012 at 02:44:44PM -0400, Lee Schermerhorn wrote:
Stracing libvirtd shows that the qemu driver is executing 2 different qemu binaries 3 times each to fetch the version, capabilities [supported devices], and cpu models each time VM state is queried. E.g., [lines wrapped]:
6471 17:15:26.561890 execve("/usr/bin/qemu", ["/usr/bin/qemu", "-cpu", "?"], [/* 2 vars */]) = 0 6472 17:15:26.626668 execve("/usr/bin/qemu", ["/usr/bin/qemu", "-help"], [/* 2 vars */]) = 0 6473 17:15:26.698104 execve("/usr/bin/qemu", ["/usr/bin/qemu", "-device", "?", "-device", "pci-assign,?", "-device", "virtio-blk-pci,?"], [/* 2 vars */]) = 0 6484 17:15:27.267770 execve("/usr/bin/qemu-system-x86_64", ["/usr/bin/qemu-system-x86_64", "-cpu", "?"], /* 2 vars */]) = 0 6492 17:15:27.333177 execve("/usr/bin/qemu-system-x86_64", ["/usr/bin/qemu-system-x86_64", "-help"], [/* 2 vars */]) = 0 6496 17:15:27.402280 execve("/usr/bin/qemu-system-x86_64", ["/usr/bin/qemu-system-x86_64", "-device", "?", "-device", "pci-assign,?", "-device", "virtio-blk-pci,?"], [/* 2 vars */]) = 0
~1sec per libvirt api call. Not a killer, but on a heavily loaded host -- several 10s of VMs -- a periodic query of all VM state, such as from a cloud compute manager, can take a couple of minutes to complete.
Because the qemu binaries on the host do not change all that often, the results of parsing the qemu help output from the exec's above can be cached. The qemu driver already does some caching of capabilities, but it does not prevent the execs above.
This series is a work in progress. I'm submitting it as an RFC because I saw Eric mention the frequent execing of qemu binaries and I have been working on this to eliminate the overhead shown above.
The series caches the parse results of:
+ qemuCapsExtractVersionInfo + qemuCapsProbeMachineTypes + qemuCapsProbeCPUModels
by splitting these functions into two parts. The existing function name fetches the cached parse results for the specified binary and returns them. The other half, named "qemuCapsCacheX", where X is one of ExtractVersionInfo, ProbeMachineTypes, and ProbeCPUModels, exec's the emulator binary and caches the results. The act of fetching the cached results will fill or refresh the cache as necessary in a new function qemuCapsCachedInfoGet(). A few auxilliary function have been added -- e.g., virCapabilitiesDupMachines() to duplicate a cached list of machine types and virBitmapDup() to duplicate cached capabilities flags.
yes, thanks, in general that's a point we really need to improve, thanks for going though this,
The series does not attempt to integrate with nor remove the existing capabilities caching. TBD.
The series was developed and tested in the context of the Ubuntu 11.04 natty libvirt_0.8.8-1ubuntu6.7 package using quilt to manage patches in the debian/patches directory. In that context, it builds, passes all "make check" tests [under pbuilder] and some fairly heavy, overlapping VM launch tests where it does eliminate all but a few initial exec's of the various qemu* and kvm binaries.
The version here, rebased to libvirt-0.9.10, builds cleanly under mock on Fedora 16 in the context of a modified libvirt-0.9.10-1.fc16 source package. I.e., no errors and warning-for-warning compatible with build of the libvirt-0.9.10 fc16 srpm downloaded from libvirt.org. I placed the modified spec file [applies the patches] and the build logs at:
http://downloads.linux.hp.com/~lts/Libvirt/
I have installed the patched libvirt on a fedora 16 system and successfully defined and launched a vm. Testing in progress. I'll place an annotated test log ont the site above when complete.
I also need to rebase atop the current mainline sources, but I wanted to get this series out for review to see if the overall approach would be acceptable.
Sounds a good idea, but we would really prefer patches against git head as that where development is really done. running make check and make syntax-check should be done to the patches in that context, thanks in advance ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
participants (2)
-
Daniel Veillard -
Lee Schermerhorn