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