[libvirt] [PATCH 07/11] qemu: don't pass qemuctime into virQEMUCapsIsValid

[Jiri Denemark]

On Mon, Jul 10, 2017 at 14:46:46 +0200, Pavel Hrdina wrote:
> It's not required and following patches will change the code.
> 
> Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
> ---
>  src/qemu/qemu_capabilities.c | 29 +++++++++++------------------
>  1 file changed, 11 insertions(+), 18 deletions(-)
> 
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 4d8890aaaf..4d26e04fc3 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -4284,11 +4284,11 @@ virQEMUCapsReset(virQEMUCapsPtr qemuCaps)
>  
>  static bool
>  virQEMUCapsIsValid(virQEMUCapsPtr qemuCaps,
> -                   time_t qemuctime,
>                     uid_t runUid,
>                     gid_t runGid)
>  {
>      bool kvmUsable;
> +    struct stat sb;
>  
>      if (!qemuCaps->binary)
>          return true;
> @@ -4305,24 +4305,19 @@ virQEMUCapsIsValid(virQEMUCapsPtr qemuCaps,
>          return false;
>      }
>  
> -    if (!qemuctime) {
> -        struct stat sb;
> -
> -        if (stat(qemuCaps->binary, &sb) < 0) {
> -            char ebuf[1024];
> -            VIR_DEBUG("Failed to stat QEMU binary '%s': %s",
> -                      qemuCaps->binary,
> -                      virStrerror(errno, ebuf, sizeof(ebuf)));
> -            return false;
> -        }
> -        qemuctime = sb.st_ctime;
> +    if (stat(qemuCaps->binary, &sb) < 0) {
> +        char ebuf[1024];
> +        VIR_DEBUG("Failed to stat QEMU binary '%s': %s",
> +                  qemuCaps->binary,
> +                  virStrerror(errno, ebuf, sizeof(ebuf)));
> +        return false;
>      }
>  
> -    if (qemuctime != qemuCaps->ctime) {
> +    if (sb.st_ctime != qemuCaps->ctime) {
>          VIR_DEBUG("Outdated capabilities for '%s': QEMU binary changed "
>                    "(%lld vs %lld)",
>                    qemuCaps->binary,
> -                  (long long) qemuctime, (long long) qemuCaps->ctime);
> +                  (long long) sb.st_ctime, (long long) qemuCaps->ctime);
>          return false;
>      }
>  
> @@ -4362,7 +4357,6 @@ virQEMUCapsInitCached(virCapsPtr caps,
>      int ret = -1;
>      char *binaryhash = NULL;
>      struct stat sb;
> -    time_t qemuctime = qemuCaps->ctime;
>  
>      if (virAsprintf(&capsdir, "%s/capabilities", cacheDir) < 0)
>          goto cleanup;

This hunk is wrong or you are missing some code which would handle this
differently. virQEMUCapsNewForBinaryInternal sets qemuCaps->ctime and
calls virQEMUCapsInitCached, which tries to load cached data in
qemuCaps. Once loaded from the cache, qemuCaps->ctime is set to the
cached one and if the cache is found to be invalid, the cached ctime
remains stored in qemuCaps->ctime instead of the real one.

> @@ -4402,7 +4396,7 @@ virQEMUCapsInitCached(virCapsPtr caps,
>          goto discard;
>      }
>  
> -    if (!virQEMUCapsIsValid(qemuCaps, qemuctime, runUid, runGid))
> +    if (!virQEMUCapsIsValid(qemuCaps, runUid, runGid))
>          goto discard;
>  
>      VIR_DEBUG("Loaded '%s' for '%s' ctime %lld usedQMP=%d",
> @@ -4411,7 +4405,6 @@ virQEMUCapsInitCached(virCapsPtr caps,
>  
>      ret = 1;
>   cleanup:
> -    qemuCaps->ctime = qemuctime;
>      VIR_FREE(binaryhash);
>      VIR_FREE(capsfile);
>      VIR_FREE(capsdir);

And this hunk is wrong too. However, the next patch fixes the code and
these two hunks are not needed anymore. Just move the hunks from 7/11 to
8/11.

> @@ -5413,7 +5406,7 @@ virQEMUCapsCacheValidate(virQEMUCapsCachePtr cache,
>                           virQEMUCapsPtr *qemuCaps)
>  {
>      if (*qemuCaps &&
> -        !virQEMUCapsIsValid(*qemuCaps, 0, cache->runUid, cache->runGid)) {
> +        !virQEMUCapsIsValid(*qemuCaps, cache->runUid, cache->runGid)) {
>          VIR_DEBUG("Cached capabilities %p no longer valid for %s",
>                    *qemuCaps, binary);
>          virHashRemoveEntry(cache->binaries, binary);

The rest of the patch looks OK.

Jirka