On Sat, May 23, 2015 at 10:33:30AM -0400, John Ferlan wrote:

https://bugzilla.redhat.com/show_bug.cgi?id=1195882

Original commit id 'cbde3589' indicates that the cache file would be discarded if either the QEMU binary or libvirtd 'ctime' changes; however, the code only discarded if the QEMU binary time didn't match or if the new libvirtd ctime was later than what created the cache file.

Since many factors come into play with 'ctime' adjustments (including perhaps turning back the hands of time), change the logic to also force a refresh if the ctime of libvirt is different than what's in the cache.

Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_capabilities.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 375df22..a6fae38 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2981,9 +2981,9 @@ virQEMUCapsInitCached(virQEMUCapsPtr qemuCaps, const char *cacheDir) goto cleanup; }

- /* Discard if cache is older that QEMU binary */ + /* Discard cache if QEMU binary or libvirtd changed */ if (qemuctime != qemuCaps->ctime || - selfctime < virGetSelfLastChanged()) { + selfctime != virGetSelfLastChanged()) { VIR_DEBUG("Outdated cached capabilities '%s' for '%s' " "(%lld vs %lld, %lld vs %lld)", capsfile, qemuCaps->binary,

ACK 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 :|

On Sat, May 23, 2015 at 10:33:31AM -0400, John Ferlan wrote:

Rather than an algorithm based solely on libvirtd ctime to refresh the capabilities add the element of the libvirt build version into the equation. Since that version wouldn't be there prior to this code being run - don't fail on reading the capabilities if not found. In this case, the cache will always be read when a new libvirt version is installed.

You meant 'rebuilt' not 'read', right? Anyway, this makes complete sense, ACK.

Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_capabilities.c | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index a6fae38..960afa4 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2605,6 +2605,7 @@ int virQEMUCapsProbeQMP(virQEMUCapsPtr qemuCaps, * <qemuCaps> * <qemuctime>234235253</qemuctime> * <selfctime>234235253</selfctime> + * <selfvers>1002016</selfvers> * <usedQMP/> * <flag name='foo'/> * <flag name='bar'/> @@ -2617,7 +2618,8 @@ int virQEMUCapsProbeQMP(virQEMUCapsPtr qemuCaps, */ static int virQEMUCapsLoadCache(virQEMUCapsPtr qemuCaps, const char *filename, - time_t *qemuctime, time_t *selfctime) + time_t *qemuctime, time_t *selfctime, + unsigned long *selfvers) { xmlDocPtr doc = NULL; int ret = -1; @@ -2627,6 +2629,7 @@ virQEMUCapsLoadCache(virQEMUCapsPtr qemuCaps, const char *filename, xmlXPathContextPtr ctxt = NULL; char *str = NULL; long long int l; + unsigned long lu;

if (!(doc = virXMLParseFile(filename))) goto cleanup; @@ -2660,6 +2663,10 @@ virQEMUCapsLoadCache(virQEMUCapsPtr qemuCaps, const char *filename, } *selfctime = (time_t)l;

+ *selfvers = 0; + if (virXPathULong("string(./selfvers)", ctxt, &lu) == 0) + *selfvers = lu; + qemuCaps->usedQMP = virXPathBoolean("count(./usedQMP) > 0", ctxt) > 0;

@@ -2798,6 +2805,8 @@ virQEMUCapsSaveCache(virQEMUCapsPtr qemuCaps, const char *filename) (long long)qemuCaps->ctime); virBufferAsprintf(&buf, "<selfctime>%llu</selfctime>\n", (long long)virGetSelfLastChanged()); + virBufferAsprintf(&buf, "<selfvers>%lu</selfvers>\n", + (unsigned long)LIBVIR_VERSION_NUMBER);

if (qemuCaps->usedQMP) virBufferAddLit(&buf, "<usedQMP/>\n"); @@ -2938,6 +2947,7 @@ virQEMUCapsInitCached(virQEMUCapsPtr qemuCaps, const char *cacheDir) struct stat sb; time_t qemuctime; time_t selfctime; + unsigned long selfvers;

if (virAsprintf(&capsdir, "%s/capabilities", cacheDir) < 0) goto cleanup; @@ -2970,7 +2980,8 @@ virQEMUCapsInitCached(virQEMUCapsPtr qemuCaps, const char *cacheDir) goto cleanup; }

- if (virQEMUCapsLoadCache(qemuCaps, capsfile, &qemuctime, &selfctime) < 0) { + if (virQEMUCapsLoadCache(qemuCaps, capsfile, &qemuctime, &selfctime, + &selfvers) < 0) { virErrorPtr err = virGetLastError(); VIR_WARN("Failed to load cached caps from '%s' for '%s': %s", capsfile, qemuCaps->binary, err ? NULLSTR(err->message) : @@ -2983,12 +2994,14 @@ virQEMUCapsInitCached(virQEMUCapsPtr qemuCaps, const char *cacheDir)

/* Discard cache if QEMU binary or libvirtd changed */ if (qemuctime != qemuCaps->ctime || - selfctime != virGetSelfLastChanged()) { + selfctime != virGetSelfLastChanged() || + selfvers != LIBVIR_VERSION_NUMBER) { VIR_DEBUG("Outdated cached capabilities '%s' for '%s' " - "(%lld vs %lld, %lld vs %lld)", + "(%lld vs %lld, %lld vs %lld, %lu vs %lu)", capsfile, qemuCaps->binary, (long long)qemuctime, (long long)qemuCaps->ctime, - (long long)selfctime, (long long)virGetSelfLastChanged()); + (long long)selfctime, (long long)virGetSelfLastChanged(), + selfvers, (unsigned long)LIBVIR_VERSION_NUMBER); ignore_value(unlink(capsfile)); virQEMUCapsReset(qemuCaps); ret = 0; -- 2.1.0

-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list