src/qemu/qemu_capabilities.c | 56 ++++++++++++++++++++++++++++++++++-- 1 file changed, 54 insertions(+), 2 deletions(-)
Introduce caching whether /dev/kvm is usable as the QEMU user:QEMU
group. This reduces the overhead of the QEMU capabilities cache
lookup. Before this patch there were many fork() calls used for
checking whether /dev/kvm is accessible. Now we store the result
whether /dev/kvm is accessible or not and we only need to re-run the
virFileAccessibleAs check if the ctime of /dev/kvm has changed.
Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
---
src/qemu/qemu_capabilities.c | 56 ++++++++++++++++++++++++++++++++++--
1 file changed, 54 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index e228f52ec0bb..ea95915f0f71 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -3238,6 +3238,10 @@ struct _virQEMUCapsCachePriv {
virArch hostArch;
unsigned int microcodeVersion;
char *kernelVersion;
+
+ /* cache whether /dev/kvm is usable as runUid:runGuid */
+ virTristateBool kvmUsable;
+ time_t kvmCtime;
};
typedef struct _virQEMUCapsCachePriv virQEMUCapsCachePriv;
typedef virQEMUCapsCachePriv *virQEMUCapsCachePrivPtr;
@@ -3824,6 +3828,54 @@ virQEMUCapsSaveFile(void *data,
}
+/* Determine whether '/dev/kvm' is usable as QEMU user:QEMU group. */
+static bool
+virQEMUCapsKVMUsable(virQEMUCapsCachePrivPtr priv)
+{
+ struct stat sb;
+ static const char *kvm_device = "/dev/kvm";
+ virTristateBool value;
+ virTristateBool cached_value = priv->kvmUsable;
+ time_t kvm_ctime;
+ time_t cached_kvm_ctime = priv->kvmCtime;
+
+ if (stat(kvm_device, &sb) < 0) {
+ virReportSystemError(errno,
+ _("Failed to stat %s"), kvm_device);
+ return false;
+ }
+ kvm_ctime = sb.st_ctime;
+
+ if (cached_value != VIR_TRISTATE_BOOL_ABSENT) {
+ if (kvm_ctime != cached_kvm_ctime) {
+ VIR_DEBUG("%s has changed (%lld vs %lld)", kvm_device,
+ (long long)kvm_ctime, (long long)cached_kvm_ctime);
+ goto update;
+ }
+
+ return cached_value == VIR_TRISTATE_BOOL_YES;
+ }
+
+ update:
+ if (virFileAccessibleAs(kvm_device, R_OK | W_OK,
+ priv->runUid, priv->runGid) == 0) {
+ value = VIR_TRISTATE_BOOL_YES;
+ } else {
+ value = VIR_TRISTATE_BOOL_NO;
+ }
+
+ /* There is a race window between 'stat' and
+ * 'virFileAccessibleAs'. However, since we're only interested in
+ * detecting changes *after* the virFileAccessibleAs check, we can
+ * neglect this here.
+ */
+ priv->kvmCtime = kvm_ctime;
+ priv->kvmUsable = value;
+
+ return value == VIR_TRISTATE_BOOL_YES;
+}
+
+
static bool
virQEMUCapsIsValid(void *data,
void *privData)
@@ -3872,8 +3924,7 @@ virQEMUCapsIsValid(void *data,
return true;
}
- kvmUsable = virFileAccessibleAs("/dev/kvm", R_OK | W_OK,
- priv->runUid, priv->runGid) == 0;
+ kvmUsable = virQEMUCapsKVMUsable(priv);
if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM) &&
kvmUsable) {
@@ -4684,6 +4735,7 @@ virQEMUCapsCacheNew(const char *libDir,
priv->runUid = runUid;
priv->runGid = runGid;
priv->microcodeVersion = microcodeVersion;
+ priv->kvmUsable = VIR_TRISTATE_BOOL_ABSENT;
if (uname(&uts) == 0 &&
virAsprintf(&priv->kernelVersion, "%s %s", uts.release, uts.version) < 0)
--
2.17.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Oct 29, 2018 at 05:53:58PM +0100, Marc Hartmayer wrote: > Introduce caching whether /dev/kvm is usable as the QEMU user:QEMU > group. This reduces the overhead of the QEMU capabilities cache > lookup. Before this patch there were many fork() calls used for > checking whether /dev/kvm is accessible. Now we store the result > whether /dev/kvm is accessible or not and we only need to re-run the > virFileAccessibleAs check if the ctime of /dev/kvm has changed. > > Suggested-by: Daniel P. Berrangé <berrange@redhat.com> > Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com> > --- > src/qemu/qemu_capabilities.c | 56 ++++++++++++++++++++++++++++++++++-- > 1 file changed, 54 insertions(+), 2 deletions(-) > > diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c > index e228f52ec0bb..ea95915f0f71 100644 > --- a/src/qemu/qemu_capabilities.c > +++ b/src/qemu/qemu_capabilities.c > @@ -3238,6 +3238,10 @@ struct _virQEMUCapsCachePriv { > virArch hostArch; > unsigned int microcodeVersion; > char *kernelVersion; > + > + /* cache whether /dev/kvm is usable as runUid:runGuid */ > + virTristateBool kvmUsable; > + time_t kvmCtime; > }; > typedef struct _virQEMUCapsCachePriv virQEMUCapsCachePriv; > typedef virQEMUCapsCachePriv *virQEMUCapsCachePrivPtr; > @@ -3824,6 +3828,54 @@ virQEMUCapsSaveFile(void *data, > } > > > +/* Determine whether '/dev/kvm' is usable as QEMU user:QEMU group. */ > +static bool > +virQEMUCapsKVMUsable(virQEMUCapsCachePrivPtr priv) > +{ > + struct stat sb; > + static const char *kvm_device = "/dev/kvm"; > + virTristateBool value; > + virTristateBool cached_value = priv->kvmUsable; > + time_t kvm_ctime; > + time_t cached_kvm_ctime = priv->kvmCtime; > + > + if (stat(kvm_device, &sb) < 0) { > + virReportSystemError(errno, > + _("Failed to stat %s"), kvm_device); > + return false; > + } > + kvm_ctime = sb.st_ctime; > + > + if (cached_value != VIR_TRISTATE_BOOL_ABSENT) { > + if (kvm_ctime != cached_kvm_ctime) { > + VIR_DEBUG("%s has changed (%lld vs %lld)", kvm_device, > + (long long)kvm_ctime, (long long)cached_kvm_ctime); > + goto update; > + } > + > + return cached_value == VIR_TRISTATE_BOOL_YES; > + } > + > + update: I don't like this use of goto's. It could be simplified thus to give a linear flow: if (kvm_ctime != cached_kvm_ctime) { VIR_DEBUG("%s has changed (%lld vs %lld)", kvm_device, (long long)kvm_ctime, (long long)cached_kvm_ctime); cached_value = VIR_TRISTATE_BOOL_ABSENT; } if (cached_value != VIR_TRISTATE_BOOL_ABSENT) { return cached_value == VIR_TRISTATE_BOOL_YES; } > + if (virFileAccessibleAs(kvm_device, R_OK | W_OK, > + priv->runUid, priv->runGid) == 0) { > + value = VIR_TRISTATE_BOOL_YES; > + } else { > + value = VIR_TRISTATE_BOOL_NO; > + } > + > + /* There is a race window between 'stat' and > + * 'virFileAccessibleAs'. However, since we're only interested in > + * detecting changes *after* the virFileAccessibleAs check, we can > + * neglect this here. > + */ > + priv->kvmCtime = kvm_ctime; > + priv->kvmUsable = value; > + > + return value == VIR_TRISTATE_BOOL_YES; > +} Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2024 Red Hat, Inc.