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

[Daniel P. Berrange]

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 at redhat.com>
> ---
>  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.

Tentative ACK if others agree

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