Re: [libvirt] [RFC PATCH 2/2] qemu: Force capabilities cache read if libvirtd date is different

On Wed, May 20, 2015 at 08:52:57AM -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. > > This could lead to issues with respect to how the order of libvirtd images > is created for maintenance or patch branches where if someone had a libvirtd > created on 'date x' that was created from (a) backported patch(es) followed > by a subsequent install of the primary release which would have 'date y' > where if 'date x' was greater than 'date y', then features in a primary > release branch may not be available. > > Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; > --- > src/qemu/qemu_capabilities.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c > index 2757636..51bbf55 100644 > --- a/src/qemu/qemu_capabilities.c > +++ b/src/qemu/qemu_capabilities.c > @@ -2979,9 +2979,11 @@ virQEMUCapsInitCached(virQEMUCapsPtr qemuCaps, const char *cacheDir) > goto cleanup; > } > > - /* Discard if cache is older that QEMU binary */ > + /* Discard if cache is older that QEMU binary or > + * if libvirtd changed > + */ This comment is actually run for QEMU already > if (qemuctime != qemuCaps->ctime || As we're re-generating if QEMU changed, not if it is older

> - selfctime < virGetSelfLastChanged()) { > + selfctime != virGetSelfLastChanged()) { > VIR_DEBUG("Outdated cached capabilities '%s' for '%s' " > "(%lld vs %lld, %lld vs %lld)", > capsfile, qemuCaps->binary, At first glance this looks reasonable, because it aligns behaviour for QEMU and libvirtd timestamp checks. I'm trying to understand why I used < for libvirtd check, but != for the QEMU check when i first wrote this, in case there was some edge case I've forgotten though.