Re: [RFC] qemu: Check for changes in qemu modules directory

[Jim Fehlig]

Hi All,

Any comments on this RFC patch? Is this something we would like to see in the 
6.7.0 release?

On 8/20/20 5:03 PM, Jim Fehlig wrote:
Add a configuration option for specifying location of the qemu modules
directory, defaulting to /usr/lib64/qemu. Then use this location to
check for changes in the directory, indicating that a qemu module has
changed and capabilities need to be reprobed.

Signed-off-by: Jim Fehlig <jfeh...@suse.com>
---

This is a mildly tested impl of the idea discussed here (and elsewhere
in the thread)

https://www.redhat.com/archives/libvir-list/2020-August/msg00684.html

I've checked that both system and session caches are updated when a
qemu modules package is updated. I'll do some more testing but wanted
to send the patch for early comments. Testing in other environments
would be much appreciated. Thanks!

I've tried some more configurations and haven't noticed any problems with the 
patch thus far. The request for additional eyeballs and testing still valid 
though :-).

Regards,
Jim


  meson.build                  |  6 ++++++
  meson_options.txt            |  1 +
  src/qemu/qemu_capabilities.c | 35 +++++++++++++++++++++++++++++++++++
  3 files changed, 42 insertions(+)

diff --git a/meson.build b/meson.build
index a72d0c0e85..16928482aa 100644
--- a/meson.build
+++ b/meson.build
@@ -1745,6 +1745,12 @@ if not get_option('driver_qemu').disabled()
    if use_qemu
      conf.set('WITH_QEMU', 1)
  
+    qemu_moddir = get_option('qemu_moddir')
+    if qemu_moddir == ''
+      qemu_moddir = '/usr/lib64/qemu'
+    endif
+    conf.set_quoted('QEMU_MODDIR', qemu_moddir)
+
      if host_machine.system() in ['freebsd', 'darwin']
        default_qemu_user = 'root'
        default_qemu_group = 'wheel'
diff --git a/meson_options.txt b/meson_options.txt
index c538d323c1..cedbd79491 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -59,6 +59,7 @@ option('driver_openvz', type: 'feature', value: 'auto', 
description: 'OpenVZ dri
  option('driver_qemu', type: 'feature', value: 'auto', description: 'QEMU/KVM 
driver')
  option('qemu_user', type: 'string', value: '', description: 'username to run 
QEMU system instance as')
  option('qemu_group', type: 'string', value: '', description: 'groupname to 
run QEMU system instance as')
+option('qemu_moddir', type: 'string', value: '', description: 'set the 
directory where QEMU modules are located')
  option('driver_remote', type: 'feature', value: 'enabled', description: 
'remote driver')
  option('remote_default_mode', type: 'combo', choices: ['legacy', 'direct'], 
value: 'legacy', description: 'remote driver default mode')
  option('driver_secrets', type: 'feature', value: 'auto', description: 'local 
secrets management driver')
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index ff6ba8c9e9..a00853c753 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -677,6 +677,7 @@ struct _virQEMUCaps {
      char *binary;
      time_t ctime;
      time_t libvirtCtime;
+    time_t modDirMtime;
      bool invalidation;
  
      virBitmapPtr flags;
@@ -4194,6 +4195,7 @@ virQEMUCapsParseSEVInfo(virQEMUCapsPtr qemuCaps, 
xmlXPathContextPtr ctxt)
   * <qemuCaps>
   *   <emulator>/some/path</emulator>
   *   <qemuctime>234235253</qemuctime>
+ *   <qemumoddirmtime>234235253</qemumoddirmtime>
   *   <selfctime>234235253</selfctime>
   *   <selfvers>1002016</selfvers>
   *   <flag name='foo'/>
@@ -4283,6 +4285,9 @@ virQEMUCapsLoadCache(virArch hostArch,
      }
      qemuCaps->ctime = (time_t)l;
  
+    if (virXPathLongLong("string(./qemumoddirmtime)", ctxt, &l) == 0)
+        qemuCaps->modDirMtime = (time_t)l;
+
      if ((n = virXPathNodeSet("./flag", ctxt, &nodes)) < 0) {
          virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                         _("failed to parse qemu capabilities flags"));
@@ -4615,6 +4620,10 @@ virQEMUCapsFormatCache(virQEMUCapsPtr qemuCaps)
                            qemuCaps->binary);
      virBufferAsprintf(&buf, "<qemuctime>%llu</qemuctime>\n",
                        (long long)qemuCaps->ctime);
+    if (qemuCaps->modDirMtime > 0) {
+        virBufferAsprintf(&buf, "<qemumoddirmtime>%llu</qemumoddirmtime>\n",
+                          (long long)qemuCaps->modDirMtime);
+    }
      virBufferAsprintf(&buf, "<selfctime>%llu</selfctime>\n",
                        (long long)qemuCaps->libvirtCtime);
      virBufferAsprintf(&buf, "<selfvers>%lu</selfvers>\n",
@@ -4881,6 +4890,23 @@ virQEMUCapsIsValid(void *data,
      if (!qemuCaps->binary)
          return true;
  
+    if (virFileExists(QEMU_MODDIR)) {
+        if (stat(QEMU_MODDIR, &sb) < 0) {
+            VIR_DEBUG("Failed to stat QEMU module directory '%s': %s",
+                      QEMU_MODDIR,
+                      g_strerror(errno));
+            return false;
+        }
+
+        if (sb.st_mtime != qemuCaps->modDirMtime) {
+            VIR_DEBUG("Outdated capabilities for '%s': QEMU modules "
+                      "directory '%s' changed (%lld vs %lld)",
+                      qemuCaps->binary, QEMU_MODDIR,
+                      (long long)sb.st_mtime, (long 
long)qemuCaps->modDirMtime);
+            return false;
+        }
+    }
+
      if (qemuCaps->libvirtCtime != virGetSelfLastChanged() ||
          qemuCaps->libvirtVersion != LIBVIR_VERSION_NUMBER) {
          VIR_DEBUG("Outdated capabilities for '%s': libvirt changed "
@@ -5463,6 +5489,15 @@ virQEMUCapsNewForBinaryInternal(virArch hostArch,
          goto error;
      }
  
+    if (virFileExists(QEMU_MODDIR)) {
+        if (stat(QEMU_MODDIR, &sb) < 0) {
+            virReportSystemError(errno, _("Cannot check QEMU module directory 
%s"),
+                                 QEMU_MODDIR);
+            goto error;
+        }
+        qemuCaps->modDirMtime = sb.st_mtime;
+    }
+
      if (virQEMUCapsInitQMP(qemuCaps, libDir, runUid, runGid) < 0)
          goto error;