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

[John Ferlan]

On 05/20/2015 09:28 AM, Daniel P. Berrange wrote:
> 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
> 

Re-reading the comment is odd - I can change it to:

"Discard cache if QEMU binary or libvirtd changed"


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

I assumed it perhaps was related to in release upstream related work
which generally wouldn't need the update unless of course someone
updated QEMU as well. The v1 check was mtime based - perhaps that's the
reason?  I dunno!

John