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