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@xxxxxxxxxx>
> ---
> 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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
