GNULIB provides APIs for calculating md5 and sha256 hashes, but these APIs only return you raw byte arrays. Most users in libvirt want the hash in printable string format. Add some helper APIs in util/vircrypto.{c,h} for doing this. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- .gitignore | 1 + include/libvirt/virterror.h | 1 + po/POTFILES.in | 1 + src/Makefile.am | 1 + src/libvirt_private.syms | 4 ++ src/util/vircrypto.c | 80 ++++++++++++++++++++++++++++++++++++++++ src/util/vircrypto.h | 40 ++++++++++++++++++++ src/util/virerror.c | 1 + tests/Makefile.am | 5 +++ tests/vircryptotest.c | 90 +++++++++++++++++++++++++++++++++++++++++++++ 10 files changed, 224 insertions(+) create mode 100644 src/util/vircrypto.c create mode 100644 src/util/vircrypto.h create mode 100644 tests/vircryptotest.c diff --git a/.gitignore b/.gitignore index cb60734..1973640 100644 --- a/.gitignore +++ b/.gitignore @@ -198,6 +198,7 @@ /tests/virbuftest /tests/vircapstest /tests/vircgrouptest +/tests/vircryptotest /tests/virdbustest /tests/virdrivermoduletest /tests/virendiantest diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h index cc6569d..495c121 100644 --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -121,6 +121,7 @@ typedef enum { VIR_FROM_ACCESS = 55, /* Error from access control manager */ VIR_FROM_SYSTEMD = 56, /* Error from systemd code */ VIR_FROM_BHYVE = 57, /* Error from bhyve driver */ + VIR_FROM_CRYPTO = 58, /* Error from crypto code */ # ifdef VIR_ENUM_SENTINELS VIR_ERR_DOMAIN_LAST diff --git a/po/POTFILES.in b/po/POTFILES.in index bcc0ee0..a8a5975 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -158,6 +158,7 @@ src/util/vircgroup.c src/util/virclosecallbacks.c src/util/vircommand.c src/util/virconf.c +src/util/vircrypto.c src/util/virdbus.c src/util/virdnsmasq.c src/util/vireventpoll.c diff --git a/src/Makefile.am b/src/Makefile.am index 25d0370..4bc2df4 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -93,6 +93,7 @@ UTIL_SOURCES = \ util/virclosecallbacks.c util/virclosecallbacks.h \ util/vircommand.c util/vircommand.h \ util/virconf.c util/virconf.h \ + util/vircrypto.c util/vircrypto.h \ util/virdbus.c util/virdbus.h util/virdbuspriv.h \ util/virdnsmasq.c util/virdnsmasq.h \ util/virebtables.c util/virebtables.h \ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 80070c5..00e3d9c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1149,6 +1149,10 @@ virConfWriteFile; virConfWriteMem; +# util/vircrypto.h +virCryptoHashString; + + # util/virdbus.h virDBusCallMethod; virDBusCloseSystemBus; diff --git a/src/util/vircrypto.c b/src/util/vircrypto.c new file mode 100644 index 0000000..bd00644 --- /dev/null +++ b/src/util/vircrypto.c @@ -0,0 +1,80 @@ +/* + * vircrypto.c: cryptographic helper APIs + * + * Copyright (C) 2014 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>;. + */ + +#include <config.h> + +#include "vircrypto.h" +#include "virerror.h" +#include "viralloc.h" + +#include "md5.h" +#include "sha256.h" + +#define VIR_FROM_THIS VIR_FROM_CRYPTO + +static const char hex[] = { '0', '1', '2', '3', '4', '5', '6', '7', + '8', '9', 'a', 'b', 'c', 'd', 'e', 'f' }; + +struct virHashInfo { + void *(*func)(const char *buf, size_t len, void *res); + size_t hashlen; +} hashinfo[] = { + { md5_buffer, MD5_DIGEST_SIZE }, + { sha256_buffer, SHA256_DIGEST_SIZE }, +}; + +#define VIR_CRYPTO_LARGEST_DIGEST_SIZE SHA256_DIGEST_SIZE + +verify(ARRAY_CARDINALITY(hashinfo) == VIR_CRYPTO_HASH_LAST); + +int +virCryptoHashString(virCryptoHash hash, + const char *input, + char **output) +{ + unsigned char buf[VIR_CRYPTO_LARGEST_DIGEST_SIZE]; + size_t hashstrlen; + size_t i; + + if (hash >= VIR_CRYPTO_HASH_LAST) { + virReportError(VIR_ERR_INVALID_ARG, + _("Unknown crypto hash %d"), hash); + return -1; + } + + hashstrlen = (hashinfo[hash].hashlen * 2) + 1; + + if (!(hashinfo[hash].func(input, strlen(input), buf))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to compute hash of data")); + return -1; + } + + if (VIR_ALLOC_N(*output, hashstrlen) < 0) + return -1; + + for (i = 0; i < hashinfo[hash].hashlen; i++) { + (*output)[i*2] = hex[(buf[i] >> 4) & 0xf]; + (*output)[(i*2)+1] = hex[buf[i] & 0xf]; + } + (*output)[hashstrlen] = '\0'; + + return 0; +} diff --git a/src/util/vircrypto.h b/src/util/vircrypto.h new file mode 100644 index 0000000..f67d49d --- /dev/null +++ b/src/util/vircrypto.h @@ -0,0 +1,40 @@ +/* + * vircrypto.h: cryptographic helper APIs + * + * Copyright (C) 2014 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>;. + */ + +#ifndef __VIR_CRYPTO_H__ +# define __VIR_CRYPTO_H__ + +# include "internal.h" + +typedef enum { + VIR_CRYPTO_HASH_MD5, /* Don't use this except for historic compat */ + VIR_CRYPTO_HASH_SHA256, + + VIR_CRYPTO_HASH_LAST +} virCryptoHash; + +int +virCryptoHashString(virCryptoHash hash, + const char *input, + char **output) + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) + ATTRIBUTE_RETURN_CHECK; + +#endif /* __VIR_CRYPTO_H__ */ diff --git a/src/util/virerror.c b/src/util/virerror.c index 820e1ad..3eb4f5d 100644 --- a/src/util/virerror.c +++ b/src/util/virerror.c @@ -125,6 +125,7 @@ VIR_ENUM_IMPL(virErrorDomain, VIR_ERR_DOMAIN_LAST, "Access Manager", /* 55 */ "Systemd", "Bhyve", + "Crypto", ) diff --git a/tests/Makefile.am b/tests/Makefile.am index cd91734..67f6600 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -135,6 +135,7 @@ test_programs = virshtest sockettest \ virauthconfigtest \ virbitmaptest \ vircgrouptest \ + vircryptotest \ virpcitest \ virendiantest \ virfiletest \ @@ -812,6 +813,10 @@ vircgroupmock_la_CFLAGS = $(AM_CFLAGS) vircgroupmock_la_LDFLAGS = -module -avoid-version \ -rpath /evil/libtool/hack/to/force/shared/lib/creation +vircryptotest_SOURCES = \ + vircryptotest.c testutils.h testutils.c +vircryptotest_LDADD = $(LDADDS) + virpcitest_SOURCES = \ virpcitest.c testutils.h testutils.c virpcitest_LDADD = $(LDADDS) diff --git a/tests/vircryptotest.c b/tests/vircryptotest.c new file mode 100644 index 0000000..270d0a9 --- /dev/null +++ b/tests/vircryptotest.c @@ -0,0 +1,90 @@ +/* + * vircrypto.h: cryptographic helper APIs + * + * Copyright (C) 2014 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>;. + */ + +#include <config.h> + +#include "vircrypto.h" + +#include "testutils.h" + + +struct testCryptoHashData { + virCryptoHash hash; + const char *input; + const char *output; +}; + +static int +testCryptoHash(const void *opaque) +{ + const struct testCryptoHashData *data = opaque; + char *actual = NULL; + int ret = -1; + + if (virCryptoHashString(data->hash, data->input, &actual) < 0) { + fprintf(stderr, "Failed to generate crypto hash\n"); + goto cleanup; + } + + if (STRNEQ_NULLABLE(data->output, actual)) { + fprintf(stderr, "Expected hash '%s' but got '%s'\n", + data->output, NULLSTR(actual)); + goto cleanup; + } + + ret = 0; + cleanup: + VIR_FREE(actual); + return ret; +} + + +static int +mymain(void) +{ + int ret = 0; + +#define VIR_CRYPTO_HASH(h, i, o) \ + do { \ + struct testCryptoHashData data = { \ + .hash = h, \ + .input = i, \ + .output = o, \ + }; \ + if (virtTestRun("Hash " i, testCryptoHash, &data) < 0) \ + ret = -1; \ + } while (0) + + VIR_CRYPTO_HASH(VIR_CRYPTO_HASH_MD5, "", "d41d8cd98f00b204e9800998ecf8427e"); + VIR_CRYPTO_HASH(VIR_CRYPTO_HASH_SHA256, "", "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"); + + VIR_CRYPTO_HASH(VIR_CRYPTO_HASH_MD5, " ", "7215ee9c7d9dc229d2921a40e899ec5f"); + VIR_CRYPTO_HASH(VIR_CRYPTO_HASH_SHA256, " ", "36a9e7f1c95b82ffb99743e0c5c4ce95d83c9a430aac59f84ef3cbfab6145068"); + + VIR_CRYPTO_HASH(VIR_CRYPTO_HASH_MD5, "\n", "68b329da9893e34099c7d8ad5cb9c940"); + VIR_CRYPTO_HASH(VIR_CRYPTO_HASH_SHA256, "\n", "01ba4719c80b6fe911b091a7c05124b64eeece964e09c058ef8f9805daca546b"); + + VIR_CRYPTO_HASH(VIR_CRYPTO_HASH_MD5, "The quick brown fox", "a2004f37730b9445670a738fa0fc9ee5"); + VIR_CRYPTO_HASH(VIR_CRYPTO_HASH_SHA256, "The quick brown fox", "5cac4f980fedc3d3f1f99b4be3472c9b30d56523e632d151237ec9309048bda9"); + + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; +} + +VIRT_TEST_MAIN(mymain) -- 1.8.5.3

Conver the sanlock and lockd lock driver plugins over to use the new virCryptoHashString APIs instead of having their own duplicated code. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/locking/lock_driver_lockd.c | 32 ++--------------------------- src/locking/lock_driver_sanlock.c | 42 ++++++++++----------------------------- 2 files changed, 12 insertions(+), 62 deletions(-) diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c index f3b9467..1b92d68 100644 --- a/src/locking/lock_driver_lockd.c +++ b/src/locking/lock_driver_lockd.c @@ -24,6 +24,7 @@ #include "lock_driver.h" #include "virconf.h" #include "viralloc.h" +#include "vircrypto.h" #include "virlog.h" #include "viruuid.h" #include "virfile.h" @@ -31,7 +32,6 @@ #include "rpc/virnetclient.h" #include "lock_protocol.h" #include "configmake.h" -#include "sha256.h" #include "virstring.h" #define VIR_FROM_THIS VIR_FROM_LOCKING @@ -505,34 +505,6 @@ static int virLockManagerLockDaemonNew(virLockManagerPtr lock, } -static const char hex[] = { '0', '1', '2', '3', '4', '5', '6', '7', - '8', '9', 'a', 'b', 'c', 'd', 'e', 'f' }; - -static char *virLockManagerLockDaemonDiskLeaseName(const char *path) -{ - unsigned char buf[SHA256_DIGEST_SIZE]; - char *ret; - size_t i; - - if (!(sha256_buffer(path, strlen(path), buf))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Unable to compute sha256 checksum")); - return NULL; - } - - if (VIR_ALLOC_N(ret, (SHA256_DIGEST_SIZE * 2) + 1) < 0) - return NULL; - - for (i = 0; i < SHA256_DIGEST_SIZE; i++) { - ret[i*2] = hex[(buf[i] >> 4) & 0xf]; - ret[(i*2)+1] = hex[buf[i] & 0xf]; - } - ret[(SHA256_DIGEST_SIZE * 2) + 1] = '\0'; - - return ret; -} - - static int virLockManagerLockDaemonAddResource(virLockManagerPtr lock, unsigned int type, const char *name, @@ -605,7 +577,7 @@ static int virLockManagerLockDaemonAddResource(virLockManagerPtr lock, if (driver->fileLockSpaceDir) { if (VIR_STRDUP(newLockspace, driver->fileLockSpaceDir) < 0) goto error; - if (!(newName = virLockManagerLockDaemonDiskLeaseName(name))) + if (virCryptoHashString(VIR_CRYPTO_HASH_SHA256, name, &newName) < 0) goto error; autoCreate = true; VIR_DEBUG("Using indirect lease %s for %s", newName, name); diff --git a/src/locking/lock_driver_sanlock.c b/src/locking/lock_driver_sanlock.c index f11f3c6..0c87048 100644 --- a/src/locking/lock_driver_sanlock.c +++ b/src/locking/lock_driver_sanlock.c @@ -40,8 +40,8 @@ #include "virlog.h" #include "virerror.h" #include "viralloc.h" +#include "vircrypto.h" #include "virfile.h" -#include "md5.h" #include "virconf.h" #include "virstring.h" @@ -509,36 +509,6 @@ static void virLockManagerSanlockFree(virLockManagerPtr lock) } -static const char hex[] = { '0', '1', '2', '3', '4', '5', '6', '7', - '8', '9', 'a', 'b', 'c', 'd', 'e', 'f' }; - -static int virLockManagerSanlockDiskLeaseName(const char *path, - char *str, - size_t strbuflen) -{ - unsigned char buf[MD5_DIGEST_SIZE]; - size_t i; - - if (strbuflen < ((MD5_DIGEST_SIZE * 2) + 1)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("String length too small to store md5 checksum")); - return -1; - } - - if (!(md5_buffer(path, strlen(path), buf))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Unable to compute md5 checksum")); - return -1; - } - - for (i = 0; i < MD5_DIGEST_SIZE; i++) { - str[i*2] = hex[(buf[i] >> 4) & 0xf]; - str[(i*2)+1] = hex[buf[i] & 0xf]; - } - str[(MD5_DIGEST_SIZE*2)+1] = '\0'; - return 0; -} - static int virLockManagerSanlockAddLease(virLockManagerPtr lock, const char *name, size_t nparams, @@ -606,6 +576,7 @@ static int virLockManagerSanlockAddDisk(virLockManagerPtr lock, int ret = -1; struct sanlk_resource *res = NULL; char *path = NULL; + char *hash = NULL; if (nparams) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -618,8 +589,14 @@ static int virLockManagerSanlockAddDisk(virLockManagerPtr lock, res->flags = shared ? SANLK_RES_SHARED : 0; res->num_disks = 1; - if (virLockManagerSanlockDiskLeaseName(name, res->name, SANLK_NAME_LEN) < 0) + if (virCryptoHashString(VIR_CRYPTO_HASH_MD5, name, &hash) < 0) + goto cleanup; + if (!virStrcpy(res->name, hash, SANLK_NAME_LEN)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("MD5 hash '%s' unexpectedly larger than %d characters"), + hash, (SANLK_NAME_LEN - 1)); goto cleanup; + } if (virAsprintf(&path, "%s/%s", driver->autoDiskLeasePath, res->name) < 0) @@ -649,6 +626,7 @@ cleanup: if (ret == -1) VIR_FREE(res); VIR_FREE(path); + VIR_FREE(hash); return ret; } -- 1.8.5.3

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 timestamp of the file is compared to the timestamp of the QEMU binary and discarded if the QEMU binary is newer. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/qemu/qemu_capabilities.c | 412 ++++++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_capabilities.h | 2 + src/qemu/qemu_driver.c | 1 + 3 files changed, 407 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index cae25e0..e3ed960 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" @@ -44,6 +45,7 @@ #include <unistd.h> #include <sys/wait.h> #include <stdarg.h> +#include <utime.h> #define VIR_FROM_THIS VIR_FROM_QEMU @@ -281,7 +283,7 @@ struct _virQEMUCapsCache { virMutex lock; virHashTablePtr binaries; char *libDir; - char *runDir; + char *cacheDir; uid_t runUid; gid_t runGid; }; @@ -2339,6 +2341,386 @@ 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) +{ + xmlDocPtr doc = NULL; + int ret = -1; + size_t i; + int n; + xmlNodePtr *nodes = NULL; + xmlXPathContextPtr ctxt = NULL; + char *str; + + 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; + } + + 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; + struct utimbuf ut; + + virBufferAddLit(&buf, "<qemuCaps>\n"); + + 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; + } + + ut.actime = qemuCaps->mtime; + ut.modtime = qemuCaps->mtime; + if (utime(filename, &ut) < 0) { + virReportSystemError(errno, + _("Failed to set mtime on '%s' for '%s'"), + filename, qemuCaps->binary); + ignore_value(unlink(filename)); + goto cleanup; + } + + VIR_DEBUG("Saved caps '%s' for '%s' with %lld", + filename, qemuCaps->binary, (long long)qemuCaps->mtime); + + 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; + + 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; + } + + /* Discard if cache is older that QEMU binary */ + /* XXX must also compare to libvirtd timestamp */ + if (sb.st_mtime < qemuCaps->mtime) { + VIR_DEBUG("Outdated cached capabilities '%s' for '%s' (%lld vs %lld)", + capsfile, qemuCaps->binary, + (long long)sb.st_mtime, (long long)qemuCaps->mtime); + ignore_value(unlink(capsfile)); + ret = 0; + goto cleanup; + } + + if (virQEMUCapsLoadCache(qemuCaps, capsfile) < 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; + } + + VIR_DEBUG("Loaded '%s' for '%s' mtime %lld usedQMP=%d", + capsfile, qemuCaps->binary, + (long long)qemuCaps->mtime, 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 +3161,7 @@ virQEMUCapsLogProbeFailure(const char *binary) virQEMUCapsPtr virQEMUCapsNewForBinary(const char *binary, const char *libDir, + const char *cacheDir, uid_t runUid, gid_t runGid) { @@ -2808,15 +3191,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 +3242,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 +3262,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 +3293,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 +3333,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 9aad2dc..51c9ed0 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

On 03/05/2014 10:53 AM, Daniel P. Berrange wrote: <utime.h> (and the utime() function) is deprecated by POSIX and therefore no longer portable because it corrupts sub-second timestamps. Better is to use gnulib's utimensat (or futimens). But what timestamps do we actually have to munge? Why not just store the qemu binary timestamp in the XML, rather than playing games with the mtime of the xml file? This ignores subsecond timestamps; you may want to use gnulib's stat-time module to do a more accurate check. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Currently the QEMU capabilities cache files on disk are only invalidated if the QEMU binary changes. New versions of libvirt, however, may want to extract more information from existing binaries. Thus we must take into account modification time of the libvirtd daemon and/or any loadable driver modules when checking cache validity Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- daemon/libvirtd.c | 2 ++ src/driver.c | 2 ++ src/libvirt_private.syms | 2 ++ src/qemu/qemu_capabilities.c | 24 +++++++++++++++--------- src/util/virutil.c | 23 +++++++++++++++++++++++ src/util/virutil.h | 4 ++++ 6 files changed, 48 insertions(+), 9 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 72f0e81..d8226a4 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1152,6 +1152,8 @@ int main(int argc, char **argv) { exit(EXIT_FAILURE); } + virUpdateSelfLastModified(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..2acf39c 100644 --- a/src/driver.c +++ b/src/driver.c @@ -74,6 +74,8 @@ virDriverLoadModule(const char *name) goto cleanup; } + virUpdateSelfLastModified(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..82a8918 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1954,6 +1954,7 @@ virGetGroupID; virGetGroupList; virGetGroupName; virGetHostname; +virGetSelfLastModified; virGetUnprivSGIOSysfsPath; virGetUserCacheDirectory; virGetUserConfigDirectory; @@ -1983,6 +1984,7 @@ virSetNonBlock; virSetUIDGID; virSetUIDGIDWithCaps; virStrIsPrint; +virUpdateSelfLastModified; virValidateWWN; diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index e3ed960..3dfaf48 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2564,18 +2564,21 @@ virQEMUCapsSaveCache(virQEMUCapsPtr qemuCaps, const char *filename) goto cleanup; } - ut.actime = qemuCaps->mtime; - ut.modtime = qemuCaps->mtime; + ut.modtime = virGetSelfLastModified(); + if (qemuCaps->mtime > ut.modtime) + ut.modtime = qemuCaps->mtime; + + ut.actime = ut.modtime; if (utime(filename, &ut) < 0) { virReportSystemError(errno, - _("Failed to set mtime on '%s' for '%s'"), - filename, qemuCaps->binary); + _("Failed to set mtime on '%s' for '%s' to %lld"), + filename, qemuCaps->binary, (long long)ut.modtime); ignore_value(unlink(filename)); goto cleanup; } VIR_DEBUG("Saved caps '%s' for '%s' with %lld", - filename, qemuCaps->binary, (long long)qemuCaps->mtime); + filename, qemuCaps->binary, (long long)ut.modtime); ret = 0; cleanup: @@ -2654,6 +2657,7 @@ virQEMUCapsInitCached(virQEMUCapsPtr qemuCaps, const char *cacheDir) int ret = -1; char *binaryhash = NULL; struct stat sb; + time_t selfmtime = virGetSelfLastModified(); if (virAsprintf(&capsdir, "%s/capabilities", cacheDir) < 0) goto cleanup; @@ -2687,11 +2691,13 @@ virQEMUCapsInitCached(virQEMUCapsPtr qemuCaps, const char *cacheDir) } /* Discard if cache is older that QEMU binary */ - /* XXX must also compare to libvirtd timestamp */ - if (sb.st_mtime < qemuCaps->mtime) { - VIR_DEBUG("Outdated cached capabilities '%s' for '%s' (%lld vs %lld)", + if (sb.st_mtime < qemuCaps->mtime || + sb.st_mtime < selfmtime) { + VIR_DEBUG("Outdated cached capabilities '%s' for '%s' (%lld,%lld vs %lld)", capsfile, qemuCaps->binary, - (long long)sb.st_mtime, (long long)qemuCaps->mtime); + (long long)sb.st_mtime, + (long long)selfmtime, + (long long)qemuCaps->mtime); ignore_value(unlink(capsfile)); ret = 0; goto cleanup; diff --git a/src/util/virutil.c b/src/util/virutil.c index 7a2fbb0..0e3fbd7 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -2173,3 +2173,26 @@ bool virIsSUID(void) { return getuid() != geteuid(); } + + +static time_t selfLastModified; + +time_t virGetSelfLastModified(void) +{ + return selfLastModified; +} + + +void virUpdateSelfLastModified(const char *path) +{ + struct stat sb; + + if (stat(path, &sb) < 0) + return; + + if (sb.st_mtime > selfLastModified) { + VIR_DEBUG("Setting self last modified to %lld for '%s'", + (long long)sb.st_mtime, path); + selfLastModified = sb.st_mtime; + } +} diff --git a/src/util/virutil.h b/src/util/virutil.h index 029265c..0bf7687 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 virGetSelfLastModified(void); +void virUpdateSelfLastModified(const char *path); + #endif /* __VIR_UTIL_H__ */ -- 1.8.5.3