[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



On Tue, Aug 25, 2020 at 11:23:13AM -0600, Jim Fehlig wrote:
> Hi All,
> 
> Any comments on this RFC patch? Is this something we would like to see in
> the 6.7.0 release?

Not done a full review yet, but it looks reasonable as an approach and
the code isn't too complex.

> 
> 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 <jfehlig 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;
> > 
> 

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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]