11:54 a.m.
A followup to
https://www.redhat.com/archives/libvir-list/2014-March/msg00297.html
Probing capabilities takes 200-300ms per binary and we have as many
as 26 binaries. This noticably slows down libvirtd startup. It does
not look like performance of probing QEMU can be improved, so this
series introduces caching of the capabilities information. So the
first time libvirtd starts it'll be slow, but thereafter it is fast.
The cache is invalidated any time the QEMU binary timestamp changes
or the libvirtd binary or driver module timestamp changes.
In v2:
- Store timestamps in XML file instead of non-portable utimes()
- Use ctime instead of mtime since the latter can be faked
by package managers to go backwards in time.
Daniel P. Berrange (3):
Add helper APIs to track if libvirtd or loadable modules have changed
Change QEMU capabilities cache to check ctime instead of mtime
Cache result of QEMU capabilities extraction
daemon/libvirtd.c | 2 +
src/driver.c | 2 +
src/libvirt_private.syms | 2 +
src/qemu/qemu_capabilities.c | 442 +++++++++++++++++++++++++++++++++++++++++--
src/qemu/qemu_capabilities.h | 2 +
src/qemu/qemu_driver.c | 1 +
src/util/virutil.c | 23 +++
src/util/virutil.h | 4 +
8 files changed, 467 insertions(+), 11 deletions(-)
--
1.8.5.3
11:54 a.m.
New subject: [libvirt] [PATCH v2 1/3] Add helper APIs to track if libvirtd or loadable modules have changed
The future QEMU capabilities cache needs to be able to invalidate
itself if the libvirtd binary or any loadable modules are changed
on disk. Record the 'ctime' value for these binaries and provide
helper APIs to query it. This approach assumes that if libvirt.so
is changed, then libvirtd will also change, which should usually
be the case with libtool's wrapper scripts that cause libvirtd to
get re-linked
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
daemon/libvirtd.c | 2 ++
src/driver.c | 2 ++
src/libvirt_private.syms | 2 ++
src/util/virutil.c | 23 +++++++++++++++++++++++
src/util/virutil.h | 4 ++++
5 files changed, 33 insertions(+)
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index 72f0e81..36adaf0 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -1152,6 +1152,8 @@ int main(int argc, char **argv) {
exit(EXIT_FAILURE);
}
+ virUpdateSelfLastChanged(argv[0]);
+
if (strstr(argv[0], "lt-libvirtd") ||
strstr(argv[0], "/daemon/.libs/libvirtd")) {
char *tmp = strrchr(argv[0], '/');
diff --git a/src/driver.c b/src/driver.c
index ab2a253..721cbeb 100644
--- a/src/driver.c
+++ b/src/driver.c
@@ -74,6 +74,8 @@ virDriverLoadModule(const char *name)
goto cleanup;
}
+ virUpdateSelfLastChanged(modfile);
+
handle = dlopen(modfile, RTLD_NOW | RTLD_GLOBAL);
if (!handle) {
VIR_ERROR(_("failed to load module %s %s"), modfile, dlerror());
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 00e3d9c..f1607cd 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1954,6 +1954,7 @@ virGetGroupID;
virGetGroupList;
virGetGroupName;
virGetHostname;
+virGetSelfLastChanged;
virGetUnprivSGIOSysfsPath;
virGetUserCacheDirectory;
virGetUserConfigDirectory;
@@ -1983,6 +1984,7 @@ virSetNonBlock;
virSetUIDGID;
virSetUIDGIDWithCaps;
virStrIsPrint;
+virUpdateSelfLastChanged;
virValidateWWN;
diff --git a/src/util/virutil.c b/src/util/virutil.c
index 7a2fbb0..b6106fc 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -2173,3 +2173,26 @@ bool virIsSUID(void)
{
return getuid() != geteuid();
}
+
+
+static time_t selfLastChanged;
+
+time_t virGetSelfLastChanged(void)
+{
+ return selfLastChanged;
+}
+
+
+void virUpdateSelfLastChanged(const char *path)
+{
+ struct stat sb;
+
+ if (stat(path, &sb) < 0)
+ return;
+
+ if (sb.st_ctime > selfLastChanged) {
+ VIR_DEBUG("Setting self last changed to %lld for '%s'",
+ (long long)sb.st_ctime, path);
+ selfLastChanged = sb.st_ctime;
+ }
+}
diff --git a/src/util/virutil.h b/src/util/virutil.h
index 029265c..cffe1ed 100644
--- a/src/util/virutil.h
+++ b/src/util/virutil.h
@@ -194,4 +194,8 @@ const char *virGetEnvBlockSUID(const char *name);
const char *virGetEnvAllowSUID(const char *name);
bool virIsSUID(void);
+
+time_t virGetSelfLastChanged(void);
+void virUpdateSelfLastChanged(const char *path);
+
#endif /* __VIR_UTIL_H__ */
--
1.8.5.3
1:39 p.m.
New subject: [libvirt] [PATCH v2 1/3] Add helper APIs to track if libvirtd or loadable modules have changed
On 03/10/2014 10:54 AM, Daniel P. Berrange wrote:
The future QEMU capabilities cache needs to be able to invalidate
itself if the libvirtd binary or any loadable modules are changed
on disk. Record the 'ctime' value for these binaries and provide
helper APIs to query it. This approach assumes that if libvirt.so
is changed, then libvirtd will also change, which should usually
be the case with libtool's wrapper scripts that cause libvirtd to
get re-linked
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
+void virUpdateSelfLastChanged(const char *path)
+{
+ struct stat sb;
+
+ if (stat(path, &sb) < 0)
+ return;
+
+ if (sb.st_ctime > selfLastChanged) {
+ VIR_DEBUG("Setting self last changed to %lld for '%s'",
+ (long long)sb.st_ctime, path);
+ selfLastChanged = sb.st_ctime;
No sub-second resolution? I suppose it's okay, although gnulib gives
you functions to access and compare full timestamp resolution.
ACK.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
11:54 a.m.
New subject: [libvirt] [PATCH v2 2/3] Change QEMU capabilities cache to check ctime instead of mtime
Debian's package manager will preserve mtime timesatmp on binaries
from the time they are built, rather than installed. So if a
user downgrades their QEMU dpkg, the libvirt capabilities
cache will not refresh. The fix is to use ctime instead of mtime
since it cannot be faked.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/qemu/qemu_capabilities.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index cae25e0..df5581f 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -259,7 +259,7 @@ struct _virQEMUCaps {
bool usedQMP;
char *binary;
- time_t mtime;
+ time_t ctime;
virBitmapPtr flags;
@@ -2796,7 +2796,7 @@ virQEMUCapsPtr virQEMUCapsNewForBinary(const char *binary,
binary);
goto error;
}
- qemuCaps->mtime = sb.st_mtime;
+ qemuCaps->ctime = sb.st_ctime;
/* Make sure the binary we are about to try exec'ing exists.
* Technically we could catch the exec() failure, but that's
@@ -2838,7 +2838,7 @@ bool virQEMUCapsIsValid(virQEMUCapsPtr qemuCaps)
if (stat(qemuCaps->binary, &sb) < 0)
return false;
- return sb.st_mtime == qemuCaps->mtime;
+ return sb.st_ctime == qemuCaps->ctime;
}
--
1.8.5.3
1:40 p.m.
New subject: [libvirt] [PATCH v2 2/3] Change QEMU capabilities cache to check ctime instead of mtime
On 03/10/2014 10:54 AM, Daniel P. Berrange wrote:
Debian's package manager will preserve mtime timesatmp on
binaries
from the time they are built, rather than installed. So if a
user downgrades their QEMU dpkg, the libvirt capabilities
cache will not refresh. The fix is to use ctime instead of mtime
since it cannot be faked.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/qemu/qemu_capabilities.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
ACK.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
5:26 a.m.
New subject: [libvirt] [PATCH v2 2/3] Change QEMU capabilities cache to check ctime instead of mtime
On Mon, Mar 10, 2014 at 04:54:29PM +0000, Daniel P. Berrange wrote:
Debian's package manager will preserve mtime timesatmp on
binaries
There's a 'timestamp' typo here if this hasn't been pushet yet.
Christophe
11:54 a.m.
New subject: [libvirt] [PATCH v2 3/3] Cache result of QEMU capabilities extraction
Extracting capabilities from QEMU takes a notable amount of time
when all QEMU binaries are installed. Each system emulator
needs about 200-300ms multiplied by 26 binaries == ~5-8 seconds.
This change causes the QEMU driver to save an XML file containing
the content of the virQEMUCaps object instance in the cache
dir eg /var/cache/libvirt/qemu/capabilities/$SHA256(binarypath).xml
or $HOME/.cache/libvirt/qemu/cache/capabilities/$SHA256(binarypath).xml
We attempt to load this and only if it fails, do we fallback to
probing the QEMU binary. The ctime of the QEMU binary and libvirtd
are stored in the cached file and its data discarded if either
of them change.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/qemu/qemu_capabilities.c | 436 ++++++++++++++++++++++++++++++++++++++++++-
src/qemu/qemu_capabilities.h | 2 +
src/qemu/qemu_driver.c | 1 +
3 files changed, 431 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index df5581f..db95377 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -25,6 +25,7 @@
#include "qemu_capabilities.h"
#include "viralloc.h"
+#include "vircrypto.h"
#include "virlog.h"
#include "virerror.h"
#include "virfile.h"
@@ -253,6 +254,14 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
"usb-kbd", /* 165 */
);
+
+/*
+ * Update the XML parser/formatter when adding more
+ * information to this struct so that it gets cached
+ * correctly. It does not have to be ABI-stable. A
+ * newer libvirtd will simply discard the cache if
+ * it fails to parse the XML doc and save a new one.
+ */
struct _virQEMUCaps {
virObject object;
@@ -281,7 +290,7 @@ struct _virQEMUCapsCache {
virMutex lock;
virHashTablePtr binaries;
char *libDir;
- char *runDir;
+ char *cacheDir;
uid_t runUid;
gid_t runGid;
};
@@ -2339,6 +2348,403 @@ int virQEMUCapsProbeQMP(virQEMUCapsPtr qemuCaps,
}
+/*
+ * Parsing a doc that looks like
+ *
+ * <qemuCaps>
+ * <usedQMP/>
+ * <flag name='foo'/>
+ * <flag name='bar'/>
+ * ...
+ * <cpu name="pentium3"/>
+ * ...
+ * <machine name="pc-1.0" alias="pc" maxCpus="4"/>
+ * ...
+ * </qemuCaps>
+ */
+static int
+virQEMUCapsLoadCache(virQEMUCapsPtr qemuCaps, const char *filename,
+ time_t *qemuctime, time_t *selfctime)
+{
+ xmlDocPtr doc = NULL;
+ int ret = -1;
+ size_t i;
+ int n;
+ xmlNodePtr *nodes = NULL;
+ xmlXPathContextPtr ctxt = NULL;
+ char *str;
+ long long int l;
+
+ if (!(doc = virXMLParseFile(filename)))
+ goto cleanup;
+
+ if (!(ctxt = xmlXPathNewContext(doc))) {
+ virReportOOMError();
+ goto cleanup;
+ }
+
+ ctxt->node = xmlDocGetRootElement(doc);
+
+ if (STRNEQ((const char *)ctxt->node->name, "qemuCaps")) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("unexpected root element <%s>, "
+ "expecting <qemuCaps>"),
+ ctxt->node->name);
+ goto cleanup;
+ }
+
+ if (virXPathLongLong("string(./qemuctime)", ctxt, &l) < 0) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("missing qemuctime in QEMU capabilities XML"));
+ goto cleanup;
+ }
+ *qemuctime = (time_t)l;
+
+ if (virXPathLongLong("string(./selfctime)", ctxt, &l) < 0) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("missing selfctime in QEMU capabilities XML"));
+ goto cleanup;
+ }
+ *selfctime = (time_t)l;
+
+ qemuCaps->usedQMP = virXPathBoolean("count(.//usedQMP) > 0",
+ ctxt) > 0;
+
+ if ((n = virXPathNodeSet(".//flag", ctxt, &nodes)) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("failed to parse qemu capabilities flags"));
+ goto cleanup;
+ }
+ VIR_DEBUG("Got flags %d", n);
+ if (n > 0) {
+ for (i = 0; i < n; i++) {
+ int flag;
+ if (!(str = virXMLPropString(nodes[i], "name"))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("missing flag name in QEMU capabilities
cache"));
+ goto cleanup;
+ }
+ flag = virQEMUCapsTypeFromString(str);
+ if (flag < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Unknown qemu capabilities flag %s"), str);
+ VIR_FREE(str);
+ goto cleanup;
+ }
+ VIR_FREE(str);
+ virQEMUCapsSet(qemuCaps, flag);
+ }
+ }
+ VIR_FREE(nodes);
+
+ if (virXPathUInt("string(.//version)", ctxt, &qemuCaps->version)
< 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("missing version in QEMU capabilities cache"));
+ goto cleanup;
+ }
+
+ if (virXPathUInt("string(.//kvmVersion)", ctxt,
&qemuCaps->kvmVersion) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("missing version in QEMU capabilities cache"));
+ goto cleanup;
+ }
+
+ if (!(str = virXPathString("string(.//arch)", ctxt))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("missing arch in QEMU capabilities cache"));
+ goto cleanup;
+ }
+ if (!(qemuCaps->arch = virArchFromString(str))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("unknown arch %s in QEMU capabilities cache"), str);
+ goto cleanup;
+ }
+
+ if ((n = virXPathNodeSet(".//cpu", ctxt, &nodes)) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("failed to parse qemu capabilities cpus"));
+ goto cleanup;
+ }
+ if (n > 0) {
+ qemuCaps->ncpuDefinitions = n;
+ if (VIR_ALLOC_N(qemuCaps->cpuDefinitions,
+ qemuCaps->ncpuDefinitions) < 0)
+ goto cleanup;
+
+ for (i = 0; i < n; i++) {
+ if (!(str = virXMLPropString(nodes[i], "name"))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("missing cpu name in QEMU capabilities
cache"));
+ goto cleanup;
+ }
+ qemuCaps->cpuDefinitions[i] = str;
+ }
+ }
+ VIR_FREE(nodes);
+
+
+ if ((n = virXPathNodeSet(".//machine", ctxt, &nodes)) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("failed to parse qemu capabilities machines"));
+ goto cleanup;
+ }
+ if (n > 0) {
+ qemuCaps->nmachineTypes = n;
+ if (VIR_ALLOC_N(qemuCaps->machineTypes,
+ qemuCaps->nmachineTypes) < 0 ||
+ VIR_ALLOC_N(qemuCaps->machineAliases,
+ qemuCaps->nmachineTypes) < 0 ||
+ VIR_ALLOC_N(qemuCaps->machineMaxCpus,
+ qemuCaps->nmachineTypes) < 0)
+ goto cleanup;
+
+ for (i = 0; i < n; i++) {
+ if (!(str = virXMLPropString(nodes[i], "name"))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("missing machine name in QEMU capabilities
cache"));
+ goto cleanup;
+ }
+ qemuCaps->machineTypes[i] = str;
+
+ qemuCaps->machineAliases[i] = virXMLPropString(nodes[i],
"alias");
+
+ str = virXMLPropString(nodes[i], "maxCpus");
+ if (str &&
+ virStrToLong_ui(str, NULL, 10, &(qemuCaps->machineMaxCpus[i]))
< 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("malformed machine cpu count in QEMU capabilities
cache"));
+ goto cleanup;
+ }
+ }
+ }
+ VIR_FREE(nodes);
+
+ ret = 0;
+ cleanup:
+ VIR_FREE(nodes);
+ xmlXPathFreeContext(ctxt);
+ xmlFreeDoc(doc);
+ return ret;
+}
+
+
+static int
+virQEMUCapsSaveCache(virQEMUCapsPtr qemuCaps, const char *filename)
+{
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
+ const char *xml = NULL;
+ int ret = -1;
+ size_t i;
+
+ virBufferAddLit(&buf, "<qemuCaps>\n");
+
+ virBufferAsprintf(&buf, " <qemuctime>%llu</qemuctime>\n",
+ (long long)qemuCaps->ctime);
+ virBufferAsprintf(&buf, " <selfctime>%llu</selfctime>\n",
+ (long long)virGetSelfLastChanged());
+
+ if (qemuCaps->usedQMP)
+ virBufferAddLit(&buf, " <usedQMP/>\n");
+
+ for (i = 0; i < QEMU_CAPS_LAST; i++) {
+ if (virQEMUCapsGet(qemuCaps, i)) {
+ virBufferAsprintf(&buf, " <flag name='%s'/>\n",
+ virQEMUCapsTypeToString(i));
+ }
+ }
+
+ virBufferAsprintf(&buf, " <version>%d</version>\n",
+ qemuCaps->version);
+
+ virBufferAsprintf(&buf, " <kvmVersion>%d</kvmVersion>\n",
+ qemuCaps->kvmVersion);
+
+ virBufferAsprintf(&buf, " <arch>%s</arch>\n",
+ virArchToString(qemuCaps->arch));
+
+ for (i = 0; i < qemuCaps->ncpuDefinitions; i++) {
+ virBufferEscapeString(&buf, " <cpu name='%s'/>\n",
+ qemuCaps->cpuDefinitions[i]);
+ }
+
+ for (i = 0; i < qemuCaps->nmachineTypes; i++) {
+ virBufferEscapeString(&buf, " <machine name='%s'",
+ qemuCaps->machineTypes[i]);
+ if (qemuCaps->machineAliases[i])
+ virBufferEscapeString(&buf, " alias='%s'",
+ qemuCaps->machineAliases[i]);
+ virBufferAsprintf(&buf, " maxCpus='%u'/>\n",
+ qemuCaps->machineMaxCpus[i]);
+ }
+
+ virBufferAddLit(&buf, "</qemuCaps>\n");
+
+ if (virBufferError(&buf))
+ goto cleanup;
+
+ xml = virBufferContentAndReset(&buf);
+
+ if (virFileWriteStr(filename, xml, 0600) < 0) {
+ virReportSystemError(errno,
+ _("Failed to save '%s' for
'%s'"),
+ filename, qemuCaps->binary);
+ goto cleanup;
+ }
+
+ VIR_DEBUG("Saved caps '%s' for '%s' with (%lld, %lld)",
+ filename, qemuCaps->binary,
+ (long long)qemuCaps->ctime,
+ (long long)virGetSelfLastChanged());
+
+ ret = 0;
+ cleanup:
+ VIR_FREE(xml);
+ return ret;
+}
+
+static int
+virQEMUCapsRememberCached(virQEMUCapsPtr qemuCaps, const char *cacheDir)
+{
+ char *capsdir = NULL;
+ char *capsfile = NULL;
+ int ret = -1;
+ char *binaryhash = NULL;
+
+ if (virAsprintf(&capsdir, "%s/capabilities", cacheDir) < 0)
+ goto cleanup;
+
+ if (virCryptoHashString(VIR_CRYPTO_HASH_SHA256,
+ qemuCaps->binary,
+ &binaryhash) < 0)
+ goto cleanup;
+
+ if (virAsprintf(&capsfile, "%s/%s.xml", capsdir, binaryhash) < 0)
+ goto cleanup;
+
+ if (virFileMakePath(capsdir) < 0) {
+ virReportSystemError(errno,
+ _("Unable to create directory '%s'"),
+ capsdir);
+ goto cleanup;
+ }
+
+ if (virQEMUCapsSaveCache(qemuCaps, capsfile) < 0)
+ goto cleanup;
+
+ ret = 0;
+ cleanup:
+ VIR_FREE(binaryhash);
+ VIR_FREE(capsfile);
+ VIR_FREE(capsdir);
+ return ret;
+}
+
+
+static void
+virQEMUCapsReset(virQEMUCapsPtr qemuCaps)
+{
+ size_t i;
+
+ virBitmapClearAll(qemuCaps->flags);
+ qemuCaps->version = qemuCaps->kvmVersion = 0;
+ qemuCaps->arch = VIR_ARCH_NONE;
+
+ for (i = 0; i < qemuCaps->ncpuDefinitions; i++) {
+ VIR_FREE(qemuCaps->cpuDefinitions[i]);
+ }
+ VIR_FREE(qemuCaps->cpuDefinitions);
+ qemuCaps->ncpuDefinitions = 0;
+
+ for (i = 0; i < qemuCaps->nmachineTypes; i++) {
+ VIR_FREE(qemuCaps->machineTypes[i]);
+ VIR_FREE(qemuCaps->machineAliases[i]);
+ }
+ VIR_FREE(qemuCaps->machineTypes);
+ VIR_FREE(qemuCaps->machineAliases);
+ qemuCaps->nmachineTypes = 0;
+}
+
+
+static int
+virQEMUCapsInitCached(virQEMUCapsPtr qemuCaps, const char *cacheDir)
+{
+ char *capsdir = NULL;
+ char *capsfile = NULL;
+ int ret = -1;
+ char *binaryhash = NULL;
+ struct stat sb;
+ time_t qemuctime;
+ time_t selfctime;
+
+ if (virAsprintf(&capsdir, "%s/capabilities", cacheDir) < 0)
+ goto cleanup;
+
+ if (virCryptoHashString(VIR_CRYPTO_HASH_SHA256,
+ qemuCaps->binary,
+ &binaryhash) < 0)
+ goto cleanup;
+
+ if (virAsprintf(&capsfile, "%s/%s.xml", capsdir, binaryhash) < 0)
+ goto cleanup;
+
+ if (virFileMakePath(capsdir) < 0) {
+ virReportSystemError(errno,
+ _("Unable to create directory '%s'"),
+ capsdir);
+ goto cleanup;
+ }
+
+ if (stat(capsfile, &sb) < 0) {
+ if (errno == ENOENT) {
+ VIR_DEBUG("No cached capabilities '%s' for '%s'",
+ capsfile, qemuCaps->binary);
+ ret = 0;
+ goto cleanup;
+ }
+ virReportSystemError(errno,
+ _("Unable to access cache '%s' for
'%s'"),
+ capsfile, qemuCaps->binary);
+ goto cleanup;
+ }
+
+ if (virQEMUCapsLoadCache(qemuCaps, capsfile, &qemuctime, &selfctime) < 0)
{
+ virErrorPtr err = virGetLastError();
+ VIR_WARN("Failed to load cached caps from '%s' for '%s':
%s",
+ capsfile, qemuCaps->binary, err ? NULLSTR(err->message) :
+ _("unknown error"));
+ virResetLastError();
+ ret = 0;
+ virQEMUCapsReset(qemuCaps);
+ goto cleanup;
+ }
+
+ /* Discard if cache is older that QEMU binary */
+ if (qemuctime != qemuCaps->ctime ||
+ selfctime < virGetSelfLastChanged()) {
+ VIR_DEBUG("Outdated cached capabilities '%s' for '%s'
"
+ "(%lld vs %lld, %lld vs %lld)",
+ capsfile, qemuCaps->binary,
+ (long long)qemuctime, (long long)qemuCaps->ctime,
+ (long long)selfctime, (long long)virGetSelfLastChanged());
+ ignore_value(unlink(capsfile));
+ virQEMUCapsReset(qemuCaps);
+ ret = 0;
+ goto cleanup;
+ }
+
+ VIR_DEBUG("Loaded '%s' for '%s' ctime %lld usedQMP=%d",
+ capsfile, qemuCaps->binary,
+ (long long)qemuCaps->ctime, qemuCaps->usedQMP);
+
+ ret = 1;
+ cleanup:
+ VIR_FREE(binaryhash);
+ VIR_FREE(capsfile);
+ VIR_FREE(capsdir);
+ return ret;
+}
+
+
#define QEMU_SYSTEM_PREFIX "qemu-system-"
static int
@@ -2779,6 +3185,7 @@ virQEMUCapsLogProbeFailure(const char *binary)
virQEMUCapsPtr virQEMUCapsNewForBinary(const char *binary,
const char *libDir,
+ const char *cacheDir,
uid_t runUid,
gid_t runGid)
{
@@ -2808,15 +3215,23 @@ virQEMUCapsPtr virQEMUCapsNewForBinary(const char *binary,
goto error;
}
- if ((rv = virQEMUCapsInitQMP(qemuCaps, libDir, runUid, runGid)) < 0) {
- virQEMUCapsLogProbeFailure(binary);
+ if ((rv = virQEMUCapsInitCached(qemuCaps, cacheDir)) < 0)
goto error;
- }
- if (!qemuCaps->usedQMP &&
- virQEMUCapsInitHelp(qemuCaps, runUid, runGid) < 0) {
- virQEMUCapsLogProbeFailure(binary);
- goto error;
+ if (rv == 0) {
+ if (virQEMUCapsInitQMP(qemuCaps, libDir, runUid, runGid) < 0) {
+ virQEMUCapsLogProbeFailure(binary);
+ goto error;
+ }
+
+ if (!qemuCaps->usedQMP &&
+ virQEMUCapsInitHelp(qemuCaps, runUid, runGid) < 0) {
+ virQEMUCapsLogProbeFailure(binary);
+ goto error;
+ }
+
+ if (virQEMUCapsRememberCached(qemuCaps, cacheDir) < 0)
+ goto error;
}
return qemuCaps;
@@ -2851,6 +3266,7 @@ virQEMUCapsHashDataFree(void *payload, const void *key
ATTRIBUTE_UNUSED)
virQEMUCapsCachePtr
virQEMUCapsCacheNew(const char *libDir,
+ const char *cacheDir,
uid_t runUid,
gid_t runGid)
{
@@ -2870,6 +3286,8 @@ virQEMUCapsCacheNew(const char *libDir,
goto error;
if (VIR_STRDUP(cache->libDir, libDir) < 0)
goto error;
+ if (VIR_STRDUP(cache->cacheDir, cacheDir) < 0)
+ goto error;
cache->runUid = runUid;
cache->runGid = runGid;
@@ -2899,6 +3317,7 @@ virQEMUCapsCacheLookup(virQEMUCapsCachePtr cache, const char
*binary)
VIR_DEBUG("Creating capabilities for %s",
binary);
ret = virQEMUCapsNewForBinary(binary, cache->libDir,
+ cache->cacheDir,
cache->runUid, cache->runGid);
if (ret) {
VIR_DEBUG("Caching capabilities %p for %s",
@@ -2938,6 +3357,7 @@ virQEMUCapsCacheFree(virQEMUCapsCachePtr cache)
return;
VIR_FREE(cache->libDir);
+ VIR_FREE(cache->cacheDir);
virHashFree(cache->binaries);
virMutexDestroy(&cache->lock);
VIR_FREE(cache);
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index b5445e7..a9082d5 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -218,6 +218,7 @@ virQEMUCapsPtr virQEMUCapsNew(void);
virQEMUCapsPtr virQEMUCapsNewCopy(virQEMUCapsPtr qemuCaps);
virQEMUCapsPtr virQEMUCapsNewForBinary(const char *binary,
const char *libDir,
+ const char *cacheDir,
uid_t runUid,
gid_t runGid);
@@ -262,6 +263,7 @@ bool virQEMUCapsIsValid(virQEMUCapsPtr qemuCaps);
virQEMUCapsCachePtr virQEMUCapsCacheNew(const char *libDir,
+ const char *cacheDir,
uid_t uid, gid_t gid);
virQEMUCapsPtr virQEMUCapsCacheLookup(virQEMUCapsCachePtr cache,
const char *binary);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 8a54b8a..7fea07c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -744,6 +744,7 @@ qemuStateInitialize(bool privileged,
}
qemu_driver->qemuCapsCache = virQEMUCapsCacheNew(cfg->libDir,
+ cfg->cacheDir,
run_uid,
run_gid);
if (!qemu_driver->qemuCapsCache)
--
1.8.5.3
1:50 p.m.
New subject: [libvirt] [PATCH v2 3/3] Cache result of QEMU capabilities extraction
On 03/10/2014 10:54 AM, Daniel P. Berrange wrote:
Extracting capabilities from QEMU takes a notable amount of time
when all QEMU binaries are installed. Each system emulator
needs about 200-300ms multiplied by 26 binaries == ~5-8 seconds.
This change causes the QEMU driver to save an XML file containing
the content of the virQEMUCaps object instance in the cache
dir eg /var/cache/libvirt/qemu/capabilities/$SHA256(binarypath).xml
or $HOME/.cache/libvirt/qemu/cache/capabilities/$SHA256(binarypath).xml
Good, http://www.tldp.org/LDP/Linux-Filesystem-Hierarchy/html/var.html
states that /var/cache normally persists across reboots.
We attempt to load this and only if it fails, do we fallback to
probing the QEMU binary. The ctime of the QEMU binary and libvirtd
are stored in the cached file and its data discarded if either
of them change.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/qemu/qemu_capabilities.c | 436 ++++++++++++++++++++++++++++++++++++++++++-
src/qemu/qemu_capabilities.h | 2 +
src/qemu/qemu_driver.c | 1 +
3 files changed, 431 insertions(+), 8 deletions(-)
+
+/*
+ * Update the XML parser/formatter when adding more
+ * information to this struct so that it gets cached
+ * correctly. It does not have to be ABI-stable. A
+ * newer libvirtd will simply discard the cache if
+ * it fails to parse the XML doc and save a new one.
A newer libvirtd will discard the cache anyways, because the timestamp
differs from the cached ctime.
+/*
+ * Parsing a doc that looks like
+ *
+ * <qemuCaps>
+ * <usedQMP/>
+ * <flag name='foo'/>
+ * <flag name='bar'/>
+ * ...
+ * <cpu name="pentium3"/>
+ * ...
+ * <machine name="pc-1.0" alias="pc"
maxCpus="4"/>
+ * ...
+ * </qemuCaps>
Ought to mention the cached ctime elements.
+ */
+static int
+virQEMUCapsLoadCache(virQEMUCapsPtr qemuCaps, const char *filename,
+ time_t *qemuctime, time_t *selfctime)
+{
+ if (virXPathLongLong("string(./qemuctime)", ctxt,
&l) < 0) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("missing qemuctime in QEMU capabilities XML"));
+ qemuCaps->usedQMP = virXPathBoolean("count(.//usedQMP)
> 0",
Why ./qemuctime but .//usedQMP? What difference does // make in XPath?
+
+static int
+virQEMUCapsSaveCache(virQEMUCapsPtr qemuCaps, const char *filename)
+{
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
+ const char *xml = NULL;
+ int ret = -1;
+ size_t i;
+
+ virBufferAddLit(&buf, "<qemuCaps>\n");
+
+ virBufferAsprintf(&buf, " <qemuctime>%llu</qemuctime>\n",
Conflicts with Laine's work to require virBufferAddIndent() rather than
hard-coding indentation. Will be interesting to see who gets in first :)
+static void
+virQEMUCapsReset(virQEMUCapsPtr qemuCaps)
+{
+ size_t i;
+
+ virBitmapClearAll(qemuCaps->flags);
+ qemuCaps->version = qemuCaps->kvmVersion = 0;
+ qemuCaps->arch = VIR_ARCH_NONE;
Shouldn't you also reset qemuCaps->usedQMP and qemuCaps->ctime, to get
the struct back to a known-default state? Or is this only ever going to
be used just before freeing the struct, at which point all the
reset-to-0 code is wasted CPU cycles?
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
5:09 a.m.
New subject: [libvirt] [PATCH v2 3/3] Cache result of QEMU capabilities extraction
On Mon, Mar 10, 2014 at 12:50:20PM -0600, Eric Blake wrote:
On 03/10/2014 10:54 AM, Daniel P. Berrange wrote:
> Extracting capabilities from QEMU takes a notable amount of time
> when all QEMU binaries are installed. Each system emulator
> needs about 200-300ms multiplied by 26 binaries == ~5-8 seconds.
>
> This change causes the QEMU driver to save an XML file containing
> the content of the virQEMUCaps object instance in the cache
> dir eg /var/cache/libvirt/qemu/capabilities/$SHA256(binarypath).xml
> or $HOME/.cache/libvirt/qemu/cache/capabilities/$SHA256(binarypath).xml
> + */
> +static int
> +virQEMUCapsLoadCache(virQEMUCapsPtr qemuCaps, const char *filename,
> + time_t *qemuctime, time_t *selfctime)
> +{
> + if (virXPathLongLong("string(./qemuctime)", ctxt, &l) < 0) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("missing qemuctime in QEMU capabilities XML"));
> + qemuCaps->usedQMP = virXPathBoolean("count(.//usedQMP) > 0",
Why ./qemuctime but .//usedQMP? What difference does // make in XPath?
That's a bug, but thanks to xpath it just happens to still work.
./foo means match '<foo>' as an immediate child of current node.
.//foo means match '<foo>' as an arbitrarily deep child of the
current node.
> +static int
> +virQEMUCapsSaveCache(virQEMUCapsPtr qemuCaps, const char *filename)
> +{
> + virBuffer buf = VIR_BUFFER_INITIALIZER;
> + const char *xml = NULL;
> + int ret = -1;
> + size_t i;
> +
> + virBufferAddLit(&buf, "<qemuCaps>\n");
> +
> + virBufferAsprintf(&buf, "
<qemuctime>%llu</qemuctime>\n",
Conflicts with Laine's work to require virBufferAddIndent() rather than
hard-coding indentation. Will be interesting to see who gets in first :)
Well it doesn't conflict so much as mean Laine has more work todo :-P
> +static void
> +virQEMUCapsReset(virQEMUCapsPtr qemuCaps)
> +{
> + size_t i;
> +
> + virBitmapClearAll(qemuCaps->flags);
> + qemuCaps->version = qemuCaps->kvmVersion = 0;
> + qemuCaps->arch = VIR_ARCH_NONE;
Shouldn't you also reset qemuCaps->usedQMP and qemuCaps->ctime, to get
the struct back to a known-default state? Or is this only ever going to
be used just before freeing the struct, at which point all the
reset-to-0 code is wasted CPU cycles?
It should reset usedQMP, but not ctime - the ctime value always
matches the current QEMU binary and we don't overwrite that when
loading the XML.
Regards,
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 :|
3909
days inactive
3910
days old
8 comments
3 participants
participants (3)
-
Christophe Fergeau -
Daniel P. Berrange -
Eric Blake