[libvirt] [PATCH v2 0/6] qemu capabilities cleanup and preparation for virFileCache

Pavel Hrdina (6): util/virhash: add name parameter to virHashSearch qemu: move libvirt ctime and version into _virQEMUCaps struct qemu: move virQEMUCapsIsValid before its usage and make it static qemu: move libvirt ctime and version check into virQEMUCapsIsValid qemu: don't pass qemuctime into virQEMUCapsIsValid qemu: separate virQEMUCapsInitCached out of virQEMUCapsNewForBinaryInternal src/conf/virdomainobjlist.c | 2 +- src/conf/virnetworkobj.c | 4 +- src/conf/virsecretobj.c | 2 +- src/qemu/qemu_capabilities.c | 282 ++++++++++++++++++++----------------------- src/qemu/qemu_capabilities.h | 5 - src/qemu/qemu_capspriv.h | 9 +- src/util/virhash.c | 11 +- src/util/virhash.h | 2 +- src/xen/xm_internal.c | 5 +- tests/qemucapabilitiestest.c | 4 +- tests/qemucapsprobe.c | 2 +- tests/testutilsqemu.c | 5 +- tests/virhashtest.c | 2 +- 13 files changed, 151 insertions(+), 184 deletions(-) -- 2.13.3

While searching for an element using a function it may be desirable to know the element key for future operation. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com> --- src/conf/virdomainobjlist.c | 2 +- src/conf/virnetworkobj.c | 4 ++-- src/conf/virsecretobj.c | 2 +- src/qemu/qemu_capabilities.c | 5 +++-- src/util/virhash.c | 11 ++++++++--- src/util/virhash.h | 2 +- src/xen/xm_internal.c | 5 +++-- tests/virhashtest.c | 2 +- 8 files changed, 20 insertions(+), 13 deletions(-) diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c index c121382bcb..7bd4cd29b9 100644 --- a/src/conf/virdomainobjlist.c +++ b/src/conf/virdomainobjlist.c @@ -119,7 +119,7 @@ virDomainObjListFindByIDInternal(virDomainObjListPtr doms, { virDomainObjPtr obj; virObjectLock(doms); - obj = virHashSearch(doms->objs, virDomainObjListSearchID, &id); + obj = virHashSearch(doms->objs, virDomainObjListSearchID, &id, NULL); if (ref) { virObjectRef(obj); virObjectUnlock(doms); diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c index 88e42b5b3e..ccde72e727 100644 --- a/src/conf/virnetworkobj.c +++ b/src/conf/virnetworkobj.c @@ -208,7 +208,7 @@ virNetworkObjFindByNameLocked(virNetworkObjListPtr nets, { virNetworkObjPtr ret = NULL; - ret = virHashSearch(nets->objs, virNetworkObjSearchName, name); + ret = virHashSearch(nets->objs, virNetworkObjSearchName, name, NULL); if (ret) virObjectRef(ret); return ret; @@ -980,7 +980,7 @@ virNetworkObjBridgeInUse(virNetworkObjListPtr nets, struct virNetworkObjBridgeInUseHelperData data = {bridge, skipname}; virObjectLock(nets); - obj = virHashSearch(nets->objs, virNetworkObjBridgeInUseHelper, &data); + obj = virHashSearch(nets->objs, virNetworkObjBridgeInUseHelper, &data, NULL); virObjectUnlock(nets); return obj != NULL; diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c index e3bcbe593d..792ceb1f15 100644 --- a/src/conf/virsecretobj.c +++ b/src/conf/virsecretobj.c @@ -247,7 +247,7 @@ virSecretObjListFindByUsageLocked(virSecretObjListPtr secrets, struct virSecretSearchData data = { .usageType = usageType, .usageID = usageID }; - obj = virHashSearch(secrets->objs, virSecretObjSearchName, &data); + obj = virHashSearch(secrets->objs, virSecretObjSearchName, &data, NULL); if (obj) virObjectRef(obj); return obj; diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 04aa8d53c0..dbebf61961 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -5502,14 +5502,15 @@ virQEMUCapsCacheLookupByArch(virCapsPtr caps, struct virQEMUCapsSearchData data = { .arch = arch }; virMutexLock(&cache->lock); - ret = virHashSearch(cache->binaries, virQEMUCapsCompareArch, &data); + ret = virHashSearch(cache->binaries, virQEMUCapsCompareArch, &data, NULL); if (!ret) { /* If the first attempt at finding capabilities has failed, try * again using the QEMU target as lookup key instead */ target = virQEMUCapsFindTarget(virArchFromHost(), data.arch); if (target != data.arch) { data.arch = target; - ret = virHashSearch(cache->binaries, virQEMUCapsCompareArch, &data); + ret = virHashSearch(cache->binaries, virQEMUCapsCompareArch, + &data, NULL); } } diff --git a/src/util/virhash.c b/src/util/virhash.c index a8e0edfd3b..7fa2992f18 100644 --- a/src/util/virhash.c +++ b/src/util/virhash.c @@ -702,15 +702,18 @@ virHashRemoveAll(virHashTablePtr table) * @table: the hash table to search * @iter: an iterator to identify the desired element * @data: extra opaque information passed to the iter + * @name: the name of found user data, pass NULL to ignore * * Iterates over the hash table calling the 'iter' callback * for each element. The first element for which the iter * returns non-zero will be returned by this function. - * The elements are processed in a undefined order + * The elements are processed in a undefined order. Caller is + * responsible for freeing the @name. */ void *virHashSearch(const virHashTable *ctable, virHashSearcher iter, - const void *data) + const void *data, + void **name) { size_t i; @@ -730,6 +733,8 @@ void *virHashSearch(const virHashTable *ctable, for (entry = table->table[i]; entry; entry = entry->next) { if (iter(entry->payload, entry->name, data)) { table->iterating = false; + if (name) + *name = table->keyCopy(entry->name); return entry->payload; } } @@ -824,7 +829,7 @@ bool virHashEqual(const virHashTable *table1, virHashSize(table1) != virHashSize(table2)) return false; - virHashSearch(table1, virHashEqualSearcher, &data); + virHashSearch(table1, virHashEqualSearcher, &data, NULL); return data.equal; } diff --git a/src/util/virhash.h b/src/util/virhash.h index 61c172b9e0..00b2550e70 100644 --- a/src/util/virhash.h +++ b/src/util/virhash.h @@ -194,7 +194,7 @@ bool virHashEqual(const virHashTable *table1, int virHashForEach(virHashTablePtr table, virHashIterator iter, void *data); ssize_t virHashRemoveSet(virHashTablePtr table, virHashSearcher iter, const void *data); void *virHashSearch(const virHashTable *table, virHashSearcher iter, - const void *data); + const void *data, void **name); /* Convenience for when VIR_FREE(value) is sufficient as a data freer. */ void virHashValueFree(void *value, const void *name); diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c index fba814aa19..123f379572 100644 --- a/src/xen/xm_internal.c +++ b/src/xen/xm_internal.c @@ -880,7 +880,8 @@ xenXMDomainLookupByUUID(virConnectPtr conn, const unsigned char *uuid) if (!xenInotifyActive(conn) && xenXMConfigCacheRefresh(conn) < 0) goto cleanup; - if (!(entry = virHashSearch(priv->configCache, xenXMDomainSearchForUUID, (const void *)uuid))) + if (!(entry = virHashSearch(priv->configCache, xenXMDomainSearchForUUID, + (const void *)uuid, NULL))) goto cleanup; ret = virDomainDefNewFull(entry->def->name, uuid, -1); @@ -971,7 +972,7 @@ xenXMDomainDefineXML(virConnectPtr conn, virDomainDefPtr def) * it has the same name */ if ((entry = virHashSearch(priv->configCache, xenXMDomainSearchForUUID, - (const void *)&(def->uuid))) != NULL) { + (const void *)&(def->uuid), NULL)) != NULL) { if ((entry->def != NULL) && (entry->def->name != NULL) && (STRNEQ(def->name, entry->def->name))) { char uuidstr[VIR_UUID_STRING_BUFLEN]; diff --git a/tests/virhashtest.c b/tests/virhashtest.c index dbf07ae7f7..9407f98c4b 100644 --- a/tests/virhashtest.c +++ b/tests/virhashtest.c @@ -436,7 +436,7 @@ testHashSearch(const void *data ATTRIBUTE_UNUSED) if (!(hash = testHashInit(0))) return -1; - entry = virHashSearch(hash, testHashSearchIter, NULL); + entry = virHashSearch(hash, testHashSearchIter, NULL, NULL); if (!entry || STRNEQ(uuids_subset[testSearchIndex], entry)) { VIR_TEST_VERBOSE("\nvirHashSearch didn't find entry '%s'\n", -- 2.13.3

Cleanups the code a little bit and reduces amount of arguments passed throughout the functions. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_capabilities.c | 44 +++++++++++++++++++++----------------------- src/qemu/qemu_capspriv.h | 8 ++------ tests/qemucapabilitiestest.c | 4 ++-- tests/testutilsqemu.c | 5 +---- 4 files changed, 26 insertions(+), 35 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index dbebf61961..61dc5f4a82 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -475,11 +475,13 @@ struct _virQEMUCaps { char *binary; time_t ctime; + time_t libvirtCtime; virBitmapPtr flags; unsigned int version; unsigned int kvmVersion; + unsigned int libvirtVersion; char *package; virArch arch; @@ -3782,9 +3784,7 @@ virQEMUCapsLoadCPUModels(virQEMUCapsPtr qemuCaps, int virQEMUCapsLoadCache(virCapsPtr caps, virQEMUCapsPtr qemuCaps, - const char *filename, - time_t *selfctime, - unsigned long *selfvers) + const char *filename) { xmlDocPtr doc = NULL; int ret = -1; @@ -3826,11 +3826,11 @@ virQEMUCapsLoadCache(virCapsPtr caps, _("missing selfctime in QEMU capabilities XML")); goto cleanup; } - *selfctime = (time_t)l; + qemuCaps->libvirtCtime = (time_t)l; - *selfvers = 0; + qemuCaps->libvirtVersion = 0; if (virXPathULong("string(./selfvers)", ctxt, &lu) == 0) - *selfvers = lu; + qemuCaps->libvirtVersion = lu; qemuCaps->usedQMP = virXPathBoolean("count(./usedQMP) > 0", ctxt) > 0; @@ -4103,9 +4103,7 @@ virQEMUCapsFormatCPUModels(virQEMUCapsPtr qemuCaps, char * -virQEMUCapsFormatCache(virQEMUCapsPtr qemuCaps, - time_t selfCTime, - unsigned long selfVersion) +virQEMUCapsFormatCache(virQEMUCapsPtr qemuCaps) { virBuffer buf = VIR_BUFFER_INITIALIZER; char *ret = NULL; @@ -4117,9 +4115,9 @@ virQEMUCapsFormatCache(virQEMUCapsPtr qemuCaps, virBufferAsprintf(&buf, "<qemuctime>%llu</qemuctime>\n", (long long) qemuCaps->ctime); virBufferAsprintf(&buf, "<selfctime>%llu</selfctime>\n", - (long long) selfCTime); + (long long) qemuCaps->libvirtCtime); virBufferAsprintf(&buf, "<selfvers>%lu</selfvers>\n", - (unsigned long) selfVersion); + (unsigned long) qemuCaps->libvirtVersion); if (qemuCaps->usedQMP) virBufferAddLit(&buf, "<usedQMP/>\n"); @@ -4194,9 +4192,7 @@ virQEMUCapsSaveCache(virQEMUCapsPtr qemuCaps, const char *filename) char *xml = NULL; int ret = -1; - xml = virQEMUCapsFormatCache(qemuCaps, - virGetSelfLastChanged(), - LIBVIR_VERSION_NUMBER); + xml = virQEMUCapsFormatCache(qemuCaps); if (virFileWriteStr(filename, xml, 0600) < 0) { virReportSystemError(errno, @@ -4208,7 +4204,7 @@ virQEMUCapsSaveCache(virQEMUCapsPtr qemuCaps, const char *filename) VIR_DEBUG("Saved caps '%s' for '%s' with (%lld, %lld)", filename, qemuCaps->binary, (long long)qemuCaps->ctime, - (long long)virGetSelfLastChanged()); + (long long)qemuCaps->libvirtCtime); ret = 0; cleanup: @@ -4298,8 +4294,6 @@ virQEMUCapsInitCached(virCapsPtr caps, char *binaryhash = NULL; struct stat sb; time_t qemuctime = qemuCaps->ctime; - time_t selfctime; - unsigned long selfvers; if (virAsprintf(&capsdir, "%s/capabilities", cacheDir) < 0) goto cleanup; @@ -4332,8 +4326,7 @@ virQEMUCapsInitCached(virCapsPtr caps, goto cleanup; } - if (virQEMUCapsLoadCache(caps, qemuCaps, capsfile, - &selfctime, &selfvers) < 0) { + if (virQEMUCapsLoadCache(caps, qemuCaps, capsfile) < 0) { VIR_WARN("Failed to load cached caps from '%s' for '%s': %s", capsfile, qemuCaps->binary, virGetLastErrorMessage()); virResetLastError(); @@ -4344,13 +4337,15 @@ virQEMUCapsInitCached(virCapsPtr caps, goto discard; /* Discard cache if QEMU binary or libvirtd changed */ - if (selfctime != virGetSelfLastChanged() || - selfvers != LIBVIR_VERSION_NUMBER) { + if (qemuCaps->libvirtCtime != virGetSelfLastChanged() || + qemuCaps->libvirtVersion != LIBVIR_VERSION_NUMBER) { VIR_DEBUG("Outdated capabilities for '%s': libvirt changed " "(%lld vs %lld, %lu vs %lu)", qemuCaps->binary, - (long long)selfctime, (long long)virGetSelfLastChanged(), - selfvers, (unsigned long)LIBVIR_VERSION_NUMBER); + (long long)qemuCaps->libvirtCtime, + (long long)virGetSelfLastChanged(), + (unsigned long)qemuCaps->libvirtVersion, + (unsigned long)LIBVIR_VERSION_NUMBER); goto discard; } @@ -5246,6 +5241,9 @@ virQEMUCapsNewForBinaryInternal(virCapsPtr caps, goto error; } + qemuCaps->libvirtCtime = virGetSelfLastChanged(); + qemuCaps->libvirtVersion = LIBVIR_VERSION_NUMBER; + if (cacheDir && virQEMUCapsRememberCached(qemuCaps, cacheDir) < 0) goto error; diff --git a/src/qemu/qemu_capspriv.h b/src/qemu/qemu_capspriv.h index 94fa75b960..1162e0b284 100644 --- a/src/qemu/qemu_capspriv.h +++ b/src/qemu/qemu_capspriv.h @@ -50,12 +50,8 @@ virQEMUCapsNewForBinaryInternal(virCapsPtr caps, int virQEMUCapsLoadCache(virCapsPtr caps, virQEMUCapsPtr qemuCaps, - const char *filename, - time_t *selfctime, - unsigned long *selfvers); -char *virQEMUCapsFormatCache(virQEMUCapsPtr qemuCaps, - time_t selfCTime, - unsigned long selfVersion); + const char *filename); +char *virQEMUCapsFormatCache(virQEMUCapsPtr qemuCaps); int virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, diff --git a/tests/qemucapabilitiestest.c b/tests/qemucapabilitiestest.c index 037a4279dd..d254adf27c 100644 --- a/tests/qemucapabilitiestest.c +++ b/tests/qemucapabilitiestest.c @@ -66,7 +66,7 @@ testQemuCaps(const void *opaque) qemuMonitorTestGetMonitor(mon)) < 0) goto cleanup; - if (!(actual = virQEMUCapsFormatCache(capsActual, 0, 0))) + if (!(actual = virQEMUCapsFormatCache(capsActual))) goto cleanup; if (virTestCompareToFile(actual, capsFile) < 0) @@ -108,7 +108,7 @@ testQemuCapsCopy(const void *opaque) if (!(copy = virQEMUCapsNewCopy(orig))) goto cleanup; - if (!(actual = virQEMUCapsFormatCache(copy, 0, 0))) + if (!(actual = virQEMUCapsFormatCache(copy))) goto cleanup; if (virTestCompareToFile(actual, capsFile) < 0) diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index ee4853841c..88e11ba14a 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -568,12 +568,9 @@ qemuTestParseCapabilities(virCapsPtr caps, const char *capsFile) { virQEMUCapsPtr qemuCaps = NULL; - time_t selfctime; - unsigned long version; if (!(qemuCaps = virQEMUCapsNew()) || - virQEMUCapsLoadCache(caps, qemuCaps, capsFile, - &selfctime, &version) < 0) + virQEMUCapsLoadCache(caps, qemuCaps, capsFile) < 0) goto error; return qemuCaps; -- 2.13.3

Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_capabilities.c | 112 +++++++++++++++++++++---------------------- src/qemu/qemu_capabilities.h | 5 -- 2 files changed, 56 insertions(+), 61 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 61dc5f4a82..1eaba204e3 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -4281,6 +4281,62 @@ virQEMUCapsReset(virQEMUCapsPtr qemuCaps) } +static bool +virQEMUCapsIsValid(virQEMUCapsPtr qemuCaps, + time_t qemuctime, + uid_t runUid, + gid_t runGid) +{ + bool kvmUsable; + + if (!qemuCaps->binary) + return true; + + if (!qemuctime) { + struct stat sb; + + if (stat(qemuCaps->binary, &sb) < 0) { + char ebuf[1024]; + VIR_DEBUG("Failed to stat QEMU binary '%s': %s", + qemuCaps->binary, + virStrerror(errno, ebuf, sizeof(ebuf))); + return false; + } + qemuctime = sb.st_ctime; + } + + if (qemuctime != qemuCaps->ctime) { + VIR_DEBUG("Outdated capabilities for '%s': QEMU binary changed " + "(%lld vs %lld)", + qemuCaps->binary, + (long long) qemuctime, (long long) qemuCaps->ctime); + return false; + } + + kvmUsable = virFileAccessibleAs("/dev/kvm", R_OK | W_OK, + runUid, runGid) == 0; + + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM) && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_ENABLE_KVM) && + kvmUsable) { + VIR_DEBUG("KVM was not enabled when probing '%s', " + "but it should be usable now", + qemuCaps->binary); + return false; + } + + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM) && + !kvmUsable) { + VIR_DEBUG("KVM was enabled when probing '%s', " + "but it is not available now", + qemuCaps->binary); + return false; + } + + return true; +} + + static int virQEMUCapsInitCached(virCapsPtr caps, virQEMUCapsPtr qemuCaps, @@ -5275,62 +5331,6 @@ virQEMUCapsNewForBinary(virCapsPtr caps, } -bool -virQEMUCapsIsValid(virQEMUCapsPtr qemuCaps, - time_t qemuctime, - uid_t runUid, - gid_t runGid) -{ - bool kvmUsable; - - if (!qemuCaps->binary) - return true; - - if (!qemuctime) { - struct stat sb; - - if (stat(qemuCaps->binary, &sb) < 0) { - char ebuf[1024]; - VIR_DEBUG("Failed to stat QEMU binary '%s': %s", - qemuCaps->binary, - virStrerror(errno, ebuf, sizeof(ebuf))); - return false; - } - qemuctime = sb.st_ctime; - } - - if (qemuctime != qemuCaps->ctime) { - VIR_DEBUG("Outdated capabilities for '%s': QEMU binary changed " - "(%lld vs %lld)", - qemuCaps->binary, - (long long) qemuctime, (long long) qemuCaps->ctime); - return false; - } - - kvmUsable = virFileAccessibleAs("/dev/kvm", R_OK | W_OK, - runUid, runGid) == 0; - - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM) && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_ENABLE_KVM) && - kvmUsable) { - VIR_DEBUG("KVM was not enabled when probing '%s', " - "but it should be usable now", - qemuCaps->binary); - return false; - } - - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM) && - !kvmUsable) { - VIR_DEBUG("KVM was enabled when probing '%s', " - "but it is not available now", - qemuCaps->binary); - return false; - } - - return true; -} - - struct virQEMUCapsMachineTypeFilter { const char *machineType; virQEMUCapsFlags *flags; diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 8250b57b46..bd44b90002 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -495,11 +495,6 @@ int virQEMUCapsGetMachineTypesCaps(virQEMUCapsPtr qemuCaps, size_t *nmachines, virCapsGuestMachinePtr **machines); -bool virQEMUCapsIsValid(virQEMUCapsPtr qemuCaps, - time_t ctime, - uid_t runUid, - gid_t runGid); - void virQEMUCapsFilterByMachineType(virQEMUCapsPtr qemuCaps, const char *machineType); -- 2.13.3

Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_capabilities.c | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 1eaba204e3..dd52a5f688 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -4292,6 +4292,18 @@ virQEMUCapsIsValid(virQEMUCapsPtr qemuCaps, if (!qemuCaps->binary) return true; + if (qemuCaps->libvirtCtime != virGetSelfLastChanged() || + qemuCaps->libvirtVersion != LIBVIR_VERSION_NUMBER) { + VIR_DEBUG("Outdated capabilities for '%s': libvirt changed " + "(%lld vs %lld, %lu vs %lu)", + qemuCaps->binary, + (long long)qemuCaps->libvirtCtime, + (long long)virGetSelfLastChanged(), + (unsigned long)qemuCaps->libvirtVersion, + (unsigned long)LIBVIR_VERSION_NUMBER); + return false; + } + if (!qemuctime) { struct stat sb; @@ -4392,19 +4404,6 @@ virQEMUCapsInitCached(virCapsPtr caps, if (!virQEMUCapsIsValid(qemuCaps, qemuctime, runUid, runGid)) goto discard; - /* Discard cache if QEMU binary or libvirtd changed */ - if (qemuCaps->libvirtCtime != virGetSelfLastChanged() || - qemuCaps->libvirtVersion != LIBVIR_VERSION_NUMBER) { - VIR_DEBUG("Outdated capabilities for '%s': libvirt changed " - "(%lld vs %lld, %lu vs %lu)", - qemuCaps->binary, - (long long)qemuCaps->libvirtCtime, - (long long)virGetSelfLastChanged(), - (unsigned long)qemuCaps->libvirtVersion, - (unsigned long)LIBVIR_VERSION_NUMBER); - goto discard; - } - VIR_DEBUG("Loaded '%s' for '%s' ctime %lld usedQMP=%d", capsfile, qemuCaps->binary, (long long)qemuCaps->ctime, qemuCaps->usedQMP); -- 2.13.3

It's not required and following patches will change the code. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_capabilities.c | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index dd52a5f688..948a7d900b 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -4283,11 +4283,11 @@ virQEMUCapsReset(virQEMUCapsPtr qemuCaps) static bool virQEMUCapsIsValid(virQEMUCapsPtr qemuCaps, - time_t qemuctime, uid_t runUid, gid_t runGid) { bool kvmUsable; + struct stat sb; if (!qemuCaps->binary) return true; @@ -4304,24 +4304,19 @@ virQEMUCapsIsValid(virQEMUCapsPtr qemuCaps, return false; } - if (!qemuctime) { - struct stat sb; - - if (stat(qemuCaps->binary, &sb) < 0) { - char ebuf[1024]; - VIR_DEBUG("Failed to stat QEMU binary '%s': %s", - qemuCaps->binary, - virStrerror(errno, ebuf, sizeof(ebuf))); - return false; - } - qemuctime = sb.st_ctime; + if (stat(qemuCaps->binary, &sb) < 0) { + char ebuf[1024]; + VIR_DEBUG("Failed to stat QEMU binary '%s': %s", + qemuCaps->binary, + virStrerror(errno, ebuf, sizeof(ebuf))); + return false; } - if (qemuctime != qemuCaps->ctime) { + if (sb.st_ctime != qemuCaps->ctime) { VIR_DEBUG("Outdated capabilities for '%s': QEMU binary changed " "(%lld vs %lld)", qemuCaps->binary, - (long long) qemuctime, (long long) qemuCaps->ctime); + (long long) sb.st_ctime, (long long) qemuCaps->ctime); return false; } @@ -4401,7 +4396,7 @@ virQEMUCapsInitCached(virCapsPtr caps, goto discard; } - if (!virQEMUCapsIsValid(qemuCaps, qemuctime, runUid, runGid)) + if (!virQEMUCapsIsValid(qemuCaps, runUid, runGid)) goto discard; VIR_DEBUG("Loaded '%s' for '%s' ctime %lld usedQMP=%d", @@ -5411,7 +5406,7 @@ virQEMUCapsCacheValidate(virQEMUCapsCachePtr cache, virQEMUCapsPtr *qemuCaps) { if (*qemuCaps && - !virQEMUCapsIsValid(*qemuCaps, 0, cache->runUid, cache->runGid)) { + !virQEMUCapsIsValid(*qemuCaps, cache->runUid, cache->runGid)) { VIR_DEBUG("Cached capabilities %p no longer valid for %s", *qemuCaps, binary); virHashRemoveEntry(cache->binaries, binary); -- 2.13.3

On Wed, Jul 19, 2017 at 17:09:50 +0200, Pavel Hrdina wrote:
It's not required and following patches will change the code.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_capabilities.c | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-)
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>

Preparation for switching to virFileCache where there are two callbacks, one to get a new data and second one to load a cached data. This also removes virQEMUCapsReset which is no longer required. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_capabilities.c | 143 +++++++++++++++++++------------------------ src/qemu/qemu_capspriv.h | 1 - tests/qemucapsprobe.c | 2 +- 3 files changed, 63 insertions(+), 83 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 948a7d900b..b7351322bc 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -4250,37 +4250,6 @@ virQEMUCapsRememberCached(virQEMUCapsPtr qemuCaps, const char *cacheDir) } -static void -virQEMUCapsReset(virQEMUCapsPtr qemuCaps) -{ - size_t i; - - virBitmapClearAll(qemuCaps->flags); - qemuCaps->version = qemuCaps->kvmVersion = 0; - VIR_FREE(qemuCaps->package); - qemuCaps->arch = VIR_ARCH_NONE; - qemuCaps->usedQMP = false; - - virObjectUnref(qemuCaps->kvmCPUModels); - qemuCaps->kvmCPUModels = NULL; - virObjectUnref(qemuCaps->tcgCPUModels); - qemuCaps->tcgCPUModels = NULL; - - for (i = 0; i < qemuCaps->nmachineTypes; i++) { - VIR_FREE(qemuCaps->machineTypes[i].name); - VIR_FREE(qemuCaps->machineTypes[i].alias); - } - VIR_FREE(qemuCaps->machineTypes); - qemuCaps->nmachineTypes = 0; - - VIR_FREE(qemuCaps->gicCapabilities); - qemuCaps->ngicCapabilities = 0; - - virQEMUCapsHostCPUDataClear(&qemuCaps->kvmCPU); - virQEMUCapsHostCPUDataClear(&qemuCaps->tcgCPU); -} - - static bool virQEMUCapsIsValid(virQEMUCapsPtr qemuCaps, uid_t runUid, @@ -4346,7 +4315,8 @@ virQEMUCapsIsValid(virQEMUCapsPtr qemuCaps, static int virQEMUCapsInitCached(virCapsPtr caps, - virQEMUCapsPtr qemuCaps, + virQEMUCapsPtr *qemuCaps, + const char *binary, const char *cacheDir, uid_t runUid, gid_t runGid) @@ -4356,14 +4326,12 @@ virQEMUCapsInitCached(virCapsPtr caps, int ret = -1; char *binaryhash = NULL; struct stat sb; - time_t qemuctime = qemuCaps->ctime; + virQEMUCapsPtr qemuCapsNew = NULL; if (virAsprintf(&capsdir, "%s/capabilities", cacheDir) < 0) goto cleanup; - if (virCryptoHashString(VIR_CRYPTO_HASH_SHA256, - qemuCaps->binary, - &binaryhash) < 0) + if (virCryptoHashString(VIR_CRYPTO_HASH_SHA256, binary, &binaryhash) < 0) goto cleanup; if (virAsprintf(&capsfile, "%s/%s.xml", capsdir, binaryhash) < 0) @@ -4379,33 +4347,39 @@ virQEMUCapsInitCached(virCapsPtr caps, if (stat(capsfile, &sb) < 0) { if (errno == ENOENT) { VIR_DEBUG("No cached capabilities '%s' for '%s'", - capsfile, qemuCaps->binary); + capsfile, binary); ret = 0; goto cleanup; } virReportSystemError(errno, _("Unable to access cache '%s' for '%s'"), - capsfile, qemuCaps->binary); + capsfile, binary); goto cleanup; } - if (virQEMUCapsLoadCache(caps, qemuCaps, capsfile) < 0) { + if (!(qemuCapsNew = virQEMUCapsNew())) + goto cleanup; + + if (VIR_STRDUP(qemuCapsNew->binary, binary) < 0) + goto discard; + + if (virQEMUCapsLoadCache(caps, qemuCapsNew, capsfile) < 0) { VIR_WARN("Failed to load cached caps from '%s' for '%s': %s", - capsfile, qemuCaps->binary, virGetLastErrorMessage()); + capsfile, qemuCapsNew->binary, virGetLastErrorMessage()); virResetLastError(); goto discard; } - if (!virQEMUCapsIsValid(qemuCaps, runUid, runGid)) + if (!virQEMUCapsIsValid(qemuCapsNew, runUid, runGid)) goto discard; VIR_DEBUG("Loaded '%s' for '%s' ctime %lld usedQMP=%d", - capsfile, qemuCaps->binary, - (long long)qemuCaps->ctime, qemuCaps->usedQMP); + capsfile, qemuCapsNew->binary, + (long long)qemuCapsNew->ctime, qemuCapsNew->usedQMP); ret = 1; + *qemuCaps = qemuCapsNew; cleanup: - qemuCaps->ctime = qemuctime; VIR_FREE(binaryhash); VIR_FREE(capsfile); VIR_FREE(capsdir); @@ -4413,9 +4387,9 @@ virQEMUCapsInitCached(virCapsPtr caps, discard: VIR_DEBUG("Dropping cached capabilities '%s' for '%s'", - capsfile, qemuCaps->binary); + capsfile, qemuCapsNew->binary); ignore_value(unlink(capsfile)); - virQEMUCapsReset(qemuCaps); + virObjectUnref(qemuCapsNew); ret = 0; goto cleanup; } @@ -5230,14 +5204,12 @@ virQEMUCapsPtr virQEMUCapsNewForBinaryInternal(virCapsPtr caps, const char *binary, const char *libDir, - const char *cacheDir, uid_t runUid, gid_t runGid, bool qmpOnly) { virQEMUCapsPtr qemuCaps; struct stat sb; - int rv; char *qmperr = NULL; if (!(qemuCaps = virQEMUCapsNew())) @@ -5265,42 +5237,30 @@ virQEMUCapsNewForBinaryInternal(virCapsPtr caps, goto error; } - if (!cacheDir) - rv = 0; - else if ((rv = virQEMUCapsInitCached(caps, qemuCaps, cacheDir, - runUid, runGid)) < 0) + if (virQEMUCapsInitQMP(qemuCaps, libDir, runUid, runGid, &qmperr) < 0) { + virQEMUCapsLogProbeFailure(binary); goto error; + } - if (rv == 0) { - if (virQEMUCapsInitQMP(qemuCaps, libDir, runUid, runGid, &qmperr) < 0) { - virQEMUCapsLogProbeFailure(binary); - goto error; - } - - if (qmpOnly && !qemuCaps->usedQMP) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to probe QEMU binary with QMP: %s"), - qmperr ? qmperr : _("unknown error")); - virQEMUCapsLogProbeFailure(binary); - goto error; - } - - if (!qemuCaps->usedQMP && - virQEMUCapsInitHelp(qemuCaps, runUid, runGid, qmperr) < 0) { - virQEMUCapsLogProbeFailure(binary); - goto error; - } + if (qmpOnly && !qemuCaps->usedQMP) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to probe QEMU binary with QMP: %s"), + qmperr ? qmperr : _("unknown error")); + virQEMUCapsLogProbeFailure(binary); + goto error; + } - qemuCaps->libvirtCtime = virGetSelfLastChanged(); - qemuCaps->libvirtVersion = LIBVIR_VERSION_NUMBER; + if (!qemuCaps->usedQMP && + virQEMUCapsInitHelp(qemuCaps, runUid, runGid, qmperr) < 0) { + virQEMUCapsLogProbeFailure(binary); + goto error; + } - if (cacheDir && - virQEMUCapsRememberCached(qemuCaps, cacheDir) < 0) - goto error; + qemuCaps->libvirtCtime = virGetSelfLastChanged(); + qemuCaps->libvirtVersion = LIBVIR_VERSION_NUMBER; - virQEMUCapsInitHostCPUModel(qemuCaps, caps, VIR_DOMAIN_VIRT_KVM); - virQEMUCapsInitHostCPUModel(qemuCaps, caps, VIR_DOMAIN_VIRT_QEMU); - } + virQEMUCapsInitHostCPUModel(qemuCaps, caps, VIR_DOMAIN_VIRT_KVM); + virQEMUCapsInitHostCPUModel(qemuCaps, caps, VIR_DOMAIN_VIRT_QEMU); cleanup: VIR_FREE(qmperr); @@ -5320,8 +5280,29 @@ virQEMUCapsNewForBinary(virCapsPtr caps, uid_t runUid, gid_t runGid) { - return virQEMUCapsNewForBinaryInternal(caps, binary, libDir, cacheDir, - runUid, runGid, false); + int rv; + virQEMUCapsPtr qemuCaps = NULL; + + if ((rv = virQEMUCapsInitCached(caps, &qemuCaps, binary, cacheDir, + runUid, runGid)) < 0) + goto error; + + if (rv == 0) { + if (!(qemuCaps = virQEMUCapsNewForBinaryInternal(caps, binary, + libDir, runUid, + runGid, false))) { + goto error; + } + + if (virQEMUCapsRememberCached(qemuCaps, cacheDir) < 0) + goto error; + } + + return qemuCaps; + + error: + virObjectUnref(qemuCaps); + return NULL; } diff --git a/src/qemu/qemu_capspriv.h b/src/qemu/qemu_capspriv.h index 1162e0b284..3458fc0700 100644 --- a/src/qemu/qemu_capspriv.h +++ b/src/qemu/qemu_capspriv.h @@ -43,7 +43,6 @@ virQEMUCapsPtr virQEMUCapsNewForBinaryInternal(virCapsPtr caps, const char *binary, const char *libDir, - const char *cacheDir, uid_t runUid, gid_t runGid, bool qmpOnly); diff --git a/tests/qemucapsprobe.c b/tests/qemucapsprobe.c index 662c7036ed..581ac38465 100644 --- a/tests/qemucapsprobe.c +++ b/tests/qemucapsprobe.c @@ -70,7 +70,7 @@ main(int argc, char **argv) if (virThreadCreate(&thread, false, eventLoop, NULL) < 0) return EXIT_FAILURE; - if (!(caps = virQEMUCapsNewForBinaryInternal(NULL, argv[1], "/tmp", NULL, + if (!(caps = virQEMUCapsNewForBinaryInternal(NULL, argv[1], "/tmp", -1, -1, true))) return EXIT_FAILURE; -- 2.13.3

On Wed, Jul 19, 2017 at 17:09:51 +0200, Pavel Hrdina wrote:
Preparation for switching to virFileCache where there are two callbacks, one to get a new data and second one to load a cached data.
This also removes virQEMUCapsReset which is no longer required.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_capabilities.c | 143 +++++++++++++++++++------------------------ src/qemu/qemu_capspriv.h | 1 - tests/qemucapsprobe.c | 2 +- 3 files changed, 63 insertions(+), 83 deletions(-)
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>

On Thu, Jul 20, 2017 at 01:32:36PM +0200, Jiri Denemark wrote:
On Wed, Jul 19, 2017 at 17:09:51 +0200, Pavel Hrdina wrote:
Preparation for switching to virFileCache where there are two callbacks, one to get a new data and second one to load a cached data.
This also removes virQEMUCapsReset which is no longer required.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_capabilities.c | 143 +++++++++++++++++++------------------------ src/qemu/qemu_capspriv.h | 1 - tests/qemucapsprobe.c | 2 +- 3 files changed, 63 insertions(+), 83 deletions(-)
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
I'll push the series shortly. Thanks, Pavel
participants (2)
-
Jiri Denemark
-
Pavel Hrdina