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 <[email protected]>
> ---
> 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 :|
--
libvir-list mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/libvir-list
