On 09/12/2014 06:25 PM, Daniel P. Berrange wrote:
On Fri, Sep 12, 2014 at 06:10:44PM +0200, Pavel Hrdina wrote:
There was a bug that if libvirtd binary has been updated than the
capability file wasn't reloaded therefore new capabilities introduced
in libvirt cannot be used because the cached version was loaded.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1135431
That bug is all about FIPS support.
Yes it's about FIPS support but it's already in libvirt. I've tested it
and actually by removing cached file to force detect new capabilities
and after that it worked.
Now I realized that even checking the selfctime during start of libvirtd
isn't sufficient because you can enable the FIPS support for kenrel
without updating the libvirtd binary.
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 81ada48..dae89aa 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -286,6 +286,7 @@ struct _virQEMUCaps {
char *binary;
time_t ctime;
+ time_t selfctime;
virBitmapPtr flags;
@@ -2689,7 +2690,7 @@ virQEMUCapsSaveCache(virQEMUCapsPtr qemuCaps, const char *filename)
virBufferAsprintf(&buf, "<qemuctime>%llu</qemuctime>\n",
(long long)qemuCaps->ctime);
virBufferAsprintf(&buf, "<selfctime>%llu</selfctime>\n",
- (long long)virGetSelfLastChanged());
+ (long long)qemuCaps->selfctime);
if (qemuCaps->usedQMP)
virBufferAddLit(&buf, "<usedQMP/>\n");
@@ -2743,7 +2744,7 @@ virQEMUCapsSaveCache(virQEMUCapsPtr qemuCaps, const char *filename)
VIR_DEBUG("Saved caps '%s' for '%s' with (%lld, %lld)",
filename, qemuCaps->binary,
(long long)qemuCaps->ctime,
- (long long)virGetSelfLastChanged());
+ (long long)qemuCaps->selfctime);
ret = 0;
cleanup:
@@ -2871,12 +2872,12 @@ virQEMUCapsInitCached(virQEMUCapsPtr qemuCaps, const char *cacheDir)
/* Discard if cache is older that QEMU binary */
if (qemuctime != qemuCaps->ctime ||
- selfctime < virGetSelfLastChanged()) {
+ selfctime < qemuCaps->selfctime) {
VIR_DEBUG("Outdated cached capabilities '%s' for '%s' "
"(%lld vs %lld, %lld vs %lld)",
capsfile, qemuCaps->binary,
(long long)qemuctime, (long long)qemuCaps->ctime,
- (long long)selfctime, (long long)virGetSelfLastChanged());
+ (long long)selfctime, (long long)qemuCaps->selfctime);
ignore_value(unlink(capsfile));
virQEMUCapsReset(qemuCaps);
ret = 0;
@@ -3371,6 +3372,7 @@ virQEMUCapsPtr virQEMUCapsNewForBinary(const char *binary,
goto error;
}
qemuCaps->ctime = sb.st_ctime;
+ qemuCaps->selfctime = virGetSelfLastChanged();
/* Make sure the binary we are about to try exec'ing exists.
* Technically we could catch the exec() failure, but that's
@@ -3420,7 +3422,8 @@ bool virQEMUCapsIsValid(virQEMUCapsPtr qemuCaps)
if (stat(qemuCaps->binary, &sb) < 0)
return false;
- return sb.st_ctime == qemuCaps->ctime;
+ return sb.st_ctime == qemuCaps->ctime &&
+ virGetSelfLastChanged() >= qemuCaps->selfctime;
}
Huh, this doesn't make any sense.
The virQEMUCapsIsValid() method is used to invalidate the cache if
QEMU binary changes while libvirtd is running. It is not relevant
whether libvirtd itself has changed here, because changes to the
libvirtd binary on disk have no effect until libvirtd is restarted.
That's a good point and I've missed that.
When libvirtd is (re)started, the virQEMUCapsInitCached will already
check the cache selfctime vs its own virGetSelfLastChanged() and
throw away the cache file if stale.
As I've wrote above, there is still issue with the FIPS that it can be
enabled without updating libvirtd. And yes you are right that QEMU
should autodetect it, but it seems they are not planning to do anything
about that and probably we have to deal with it and somehow make it work.
I'll try to come up with different patch.
Thanks,
Pavel
So I don't see any bug here that needs fixing.
Regards,
Daniel
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list