Hi all: In testing qemu-5.1rc2 on my Fedora 32 home system, I found that the Fedora rawhide package has broken out both the QXL display device and the USB redirect device into separate RPM modules: qemu-device-display-qxl.x86_64 2:5.1.0-0.1.rc2.fc32 qemu-device-usb-redirect.x86_64 2:5.1.0-0.1.rc2.fc32 The upgrade from 5.0.0 to 5.1.0 does not treat these sub-packages as mandatory packages, therefore a straight upgrade of packages causes these domcapabilities to disappear. If the user tries to use libvirt after this, then existing domains using QXL display device or USB redirect device will fail to start. If the user then investigates and realizes that they now need to install the above packages separately, they find that qemu-kvm recognizes the modules right away, but libvirt does not. This looks to be due to the libvirt domcapabilities cache?

On Wed, Aug 5, 2020 at 4:19 AM Andrea Bolognani <abologna(a)redhat.com&gt; wrote: > That said, if upgrading QEMU results in losing features, even though > you can recover them through additional steps I would argue that's a > bug in the packaging that should be addressed on the QEMU side. > I had the same thought. However, I'm expecting this will be part of Fedora 33 (not yet out), and the QXL display driver is possibly becoming optional? Within the same release of Fedora, I expect the default module list should be stable, but between releases it might not be? But, what about long-lived major releases like RHEL / CentOS? Or people who "upgrade" from the distro release to the "advanced virtualization" release (RHEL / CentOS 8)? I also would expect functionality which seems pretty default - to stay default, although perhaps it could be a weak package dependency or similar, to permit people to uninstall it? They also moved qemu-device-usb-smartcart to optional along with the already mentioned qemu-device-display-qxl and qemu-device-usb-redirect, and I believe this has happened in the past with some of the block device drivers, only I might not use them, so they might not have affected me? I think everything in the module directory is really in scope.

On 8/5/20 2:19 AM, Andrea Bolognani wrote: > > I guess we need to start checking the modules directory in addition > to the main QEMU binary, and regenerate capabilities every time its > contents change. We recently received reports of this issue on Tumbleweed, which just got the modularized qemu 5.1 https://bugzilla.opensuse.org/show_bug.cgi?id=1175320 Mark, are you working on a patch to invalidate the cache on changes to the qemu modules directory? I suppose it needs to be handled similar to the qemu binaries. E.g. when building the cache include a list of all qemu modules found. When validating the cache see if any modules have disappeared, if any have been added, and if the ctime of any have changed. Yikes, sounds a little more complex than the binaries :-).

I wonder if it is possible to use inotify to receive notification of changes to the modules directory?

On Thu, Aug 20, 2020 at 11:32:03AM +0200, Martin Wilck wrote: > On Tue, 2020-08-18 at 15:15 -0600, Jim Fehlig wrote: > > On 8/5/20 2:19 AM, Andrea Bolognani wrote: > > > I guess we need to start checking the modules directory in > > > addition > > > to the main QEMU binary, and regenerate capabilities every time > > > its > > > contents change. > > > > We recently received reports of this issue on Tumbleweed, which > > just > > got the > > modularized qemu 5.1 > > > > https://bugzilla.opensuse.org/show_bug.cgi?id=1175320 > > > > Mark, are you working on a patch to invalidate the cache on > > changes > > to the qemu > > modules directory? I suppose it needs to be handled similar to > > the > > qemu > > binaries. E.g. when building the cache include a list of all qemu > > modules found. > > When validating the cache see if any modules have disappeared, if > > any > > have been > > added, and if the ctime of any have changed. Yikes, sounds a > > little > > more complex > > than the binaries :-). > > I'd like to question whether this effort is justified for an > optimization that matters only at libvirtd startup time, and even > there > saves no more than a few seconds. > > I propose to simply disable the caching of qemu capabilities (or > provide a build-time option to do so). Optimizations that produce > wrong > results should be avoided. Whether the time matters depends on your use case for QEMU. For heavy data center apps like OpenStack you won't notice it because OpenStack itself adds soo much overhead to the system. For cases where the VM is used as "embedded" infrastructure startup time can be critical. Not caching capabilities adds easily 300-500 ms to startup of a single VM, which is very significant when the current minimum startup time of a VM can be as low as 150 ms. IOW removing caching is not a viable option.

On Wed, 2020-08-05 at 02:09 -0400, Mark Mielke wrote: > Hi all: > > In testing qemu-5.1rc2 on my Fedora 32 home system, I found that > the Fedora rawhide package has broken out both the QXL display > device and the USB redirect device into separate RPM modules: > > qemu-device-display-qxl.x86_64 2:5.1.0-0.1.rc2.fc32 > qemu-device-usb-redirect.x86_64 2:5.1.0-0.1.rc2.fc32 > > The upgrade from 5.0.0 to 5.1.0 does not treat these sub-packages > as mandatory packages, therefore a straight upgrade of packages > causes these domcapabilities to disappear. > > If the user tries to use libvirt after this, then existing domains > using QXL display device or USB redirect device will fail to start. > If the user then investigates and realizes that they now need to > install the above packages separately, they find that qemu-kvm > recognizes the modules right away, but libvirt does not. This looks > to be due to the libvirt domcapabilities cache? I guess we need to start checking the modules directory in addition to the main QEMU binary, and regenerate capabilities every time its contents change. That said, if upgrading QEMU results in losing features, even though you can recover them through additional steps I would argue that's a bug in the packaging that should be addressed on the QEMU side.

On Thu, 2020-08-20 at 11:00 +0100, Daniel P. Berrangé wrote: > On Wed, Aug 05, 2020 at 10:19:26AM +0200, Andrea Bolognani wrote: > > contents change. > > > > That said, if upgrading QEMU results in losing features, even > > though > > you can recover them through additional steps I would argue that's > > a > > bug in the packaging that should be addressed on the QEMU side. > > Potentially we can consider this a distro packaging problem and fix > it at the RPM level. > > eg the libvirt RPM can define a trigger that runs when *any* of the > qemu-device* RPMs is installed/updated. This trigger can simply > touch a file on disk somewhere, and libvirtd can monitor this one > file, instead of having to monitor every module. The simplest approach is to touch the qemu binaries. We discussed this already. It has the drawback that it makes "rpm -V" complain about wrong timestamps. It might also confuse backup software. Still, it might be a viable short-term workaround if nothing else is available.

Cheers, Martin

On 8/20/20 6:54 AM, Christian Ehrhardt wrote: > On Thu, Aug 20, 2020 at 12:43 PM Martin Wilck <mwilck(a)suse.com&gt; wrote: >> >> On Thu, 2020-08-20 at 11:00 +0100, Daniel P. Berrangé wrote: >>> On Wed, Aug 05, 2020 at 10:19:26AM +0200, Andrea Bolognani wrote: >>>> contents change. >>>> >>>> That said, if upgrading QEMU results in losing features, even >>>> though >>>> you can recover them through additional steps I would argue that's >>>> a >>>> bug in the packaging that should be addressed on the QEMU side. >>> >>> Potentially we can consider this a  distro packaging problem and fix >>> it at the RPM level. >>> >>> eg the libvirt RPM can define a trigger that runs when *any* of the >>> qemu-device*  RPMs is installed/updated. This trigger can simply >>> touch a file on disk somewhere, and libvirtd can monitor this one >>> file, instead of having to monitor every module. >> >> The simplest approach is to touch the qemu binaries. We discussed this >> already. It has the drawback that it makes "rpm -V" complain about >> wrong timestamps. It might also confuse backup software. Still, it >> might be a viable short-term workaround if nothing else is available. > > Qemu already allows to save modules in /var/run/qemu/ [1] to better handle > module upgrades which is already used in Debian and Ubuntu to avoid > late module load errors after upgrades. > > This was meant for upgrades, but if libvirt would define a known path in > there like /var/run/qemu/last_packaging_change the packages could easily > touch it on any install/remove/update as Daniel suggested and libvirt could > check this path like it does with the date of the qemu binary already. That would require changes in libvirt and the various module packages right? Are you fine with Daniel's suggestion of handling this all within libvirt? E.g. on the packaging side libvirt could define a trigger to be notified when the modules dir is updated and touch a well-known file %filetriggerin -- %{_libdir}/qemu touch /var/cache/libvirt/qemu/capabilities/module-changed %filetriggerun -- %{_libdir}/qemu touch /var/cache/libvirt/qemu/capabilities/module-changed

On Thu, Aug 20, 2020 at 12:43 PM Martin Wilck <mwilck(a)suse.com&gt; wrote: > The simplest approach is to touch the qemu binaries. We discussed this > already. It has the drawback that it makes "rpm -V" complain about > wrong timestamps. It might also confuse backup software. Still, it > might be a viable short-term workaround if nothing else is available. Qemu already allows to save modules in /var/run/qemu/ [1] to better handle module upgrades which is already used in Debian and Ubuntu to avoid late module load errors after upgrades. This was meant for upgrades, but if libvirt would define a known path in there like /var/run/qemu/last_packaging_change the packages could easily touch it on any install/remove/update as Daniel suggested and libvirt could check this path like it does with the date of the qemu binary already. [1]: https://github.com/qemu/qemu/commit/bd83c861c0628a64997b7bd95c3bcc2e916baf2e

On Thu, Aug 20, 2020 at 5:15 PM Mark Mielke <mark.mielke(a)gmail.com&gt; wrote: > On Thu, Aug 20, 2020 at 8:55 AM Christian Ehrhardt < christian.ehrhardt(a)canonical.com&gt; wrote: >> This was meant for upgrades, but if libvirt would define a known path in >> there like /var/run/qemu/last_packaging_change the packages could easily >> touch it on any install/remove/update as Daniel suggested and libvirt could >> check this path like it does with the date of the qemu binary already. > Earlier in this thread - I think one or two of us had asked about the timestamp on the directory that contains the modules. > I'm wondering if a "last_packaging_change" provides any value over and above the timestamp of the directory itself? Wouldn't the directory be best - as it would work automatically for both distro packaging as well as custom installs? Sure, if - "list of files in module dir" - "stat dates of files in module dir" would be checked by libvirt that would also be a no-op for packaging and thereby land faster.

But I guess there were reasons against it in the discussion before?

On 8/20/20 10:54 AM, Mark Mielke wrote: > On Thu, Aug 20, 2020 at 11:48 AM Christian Ehrhardt > <christian.ehrhardt(a)canonical.com > <mailto:christian.ehrhardt@canonical.com>> wrote: > > On Thu, Aug 20, 2020 at 5:15 PM Mark Mielke <mark.mielke(a)gmail.com > <mailto:mark.mielke@gmail.com>> wrote: > > On Thu, Aug 20, 2020 at 8:55 AM Christian Ehrhardt > <christian.ehrhardt(a)canonical.com <mailto:christian.ehrhardt@canonical.com>> > wrote: > >> This was meant for upgrades, but if libvirt would define a known path in > >> there like /var/run/qemu/last_packaging_change the packages could easily > >> touch it on any install/remove/update as Daniel suggested and libvirt could > >> check this path like it does with the date of the qemu binary already. > > Earlier in this thread - I think one or two of us had asked about the > timestamp on the directory that contains the modules. > > I'm wondering if a "last_packaging_change" provides any value over and > above the timestamp of the directory itself? Wouldn't the directory be best > - as it would work automatically for both distro packaging as well as custom > installs? > Sure, if > - "list of files in module dir" > - "stat dates of files in module dir" > would be checked by libvirt that would also be a no-op for packaging > and thereby land faster. > > > Why is the list of files and stat dates needed? Any change done by RPM > to add or remove a file from the directory, should cause the mtime for > the directory to be updated. It doesn't really matter what the change is > - it only matters that the change is detected. > > The case for needing to check the files *in* the directory, would be a > concern about the modules keeping the same name, but being updated in > place. I'm doubtful that this ever occurs for real, as it would cause > breakage for running programs. Existing running programs would mmap() or > similar the binaries into memory, and cannot be updated in place. > Instead, the inode remains alive, and a new file is created with the > same name, to replace the old file, and once all file descriptors are > closed - the inode is deleted. > > So, unless there is a hierarchical directory structure - I believe > checking the modules directory timestamp is near 100% accuracy for > whether modules were added, removed, or updated using "safe" deployment > practices either by RPM or "make install". I wrote a small test program that checks mtime (also tried ctime) and it only changes on the directory when files are added and deleted. There is no change to either if a file in the directory has been updated. It would be really convenient to check only the directory to see if its' contents have changed but I'm not aware of how to do that other than with something like inotify. Suggestions welcomed.

On 8/20/20 12:01 PM, Daniel P. Berrangé wrote: > On Thu, Aug 20, 2020 at 11:42:45AM -0600, Jim Fehlig wrote: >> On 8/20/20 10:54 AM, Mark Mielke wrote: >>> On Thu, Aug 20, 2020 at 11:48 AM Christian Ehrhardt >>> <christian.ehrhardt(a)canonical.com >>> <mailto:christian.ehrhardt@canonical.com>> wrote: >>> >>>      On Thu, Aug 20, 2020 at 5:15 PM Mark Mielke <mark.mielke(a)gmail.com >>>      <mailto:mark.mielke@gmail.com>> wrote: >>>       > On Thu, Aug 20, 2020 at 8:55 AM Christian Ehrhardt >>>      <christian.ehrhardt(a)canonical.com >>> <mailto:christian.ehrhardt@canonical.com>> >>>      wrote: >>>       >> This was meant for upgrades, but if libvirt would define a known >>> path in >>>       >> there like /var/run/qemu/last_packaging_change the packages could >>> easily >>>       >> touch it on any install/remove/update as Daniel suggested and >>> libvirt could >>>       >> check this path like it does with the date of the qemu binary already. >>>       > Earlier in this thread - I think one or two of us had asked about the >>>      timestamp on the directory that contains the modules. >>>       > I'm wondering if a "last_packaging_change" provides any value over and >>>      above the timestamp of the directory itself? Wouldn't the directory be >>> best >>>      - as it would work automatically for both distro packaging as well as >>> custom >>>      installs? >>>      Sure, if >>>      - "list of files in module dir" >>>      - "stat dates of files in module dir" >>>      would be checked by libvirt that would also be a no-op for packaging >>>      and thereby land faster. >>> >>> >>> Why is the list of files and stat dates needed? Any change done by RPM >>> to add or remove a file from the directory, should cause the mtime for >>> the directory to be updated. It doesn't really matter what the change is >>> - it only matters that the change is detected. >>> >>> The case for needing to check the files *in* the directory, would be a >>> concern about the modules keeping the same name, but being updated in >>> place. I'm doubtful that this ever occurs for real, as it would cause >>> breakage for running programs. Existing running programs would mmap() or >>> similar the binaries into memory, and cannot be updated in place. >>> Instead, the inode remains alive, and a new file is created with the >>> same name, to replace the old file, and once all file descriptors are >>> closed - the inode is deleted. >>> >>> So, unless there is a hierarchical directory structure - I believe >>> checking the modules directory timestamp is near 100% accuracy for >>> whether modules were added, removed, or updated using "safe" deployment >>> practices either by RPM or "make install". >> >> I wrote a small test program that checks mtime (also tried ctime) and it >> only changes on the directory when files are added and deleted. There is no >> change to either if a file in the directory has been updated. It would be >> really convenient to check only the directory to see if its' contents have >> changed but I'm not aware of how to do that other than with something like >> inotify. Suggestions welcomed. > > IIUC, Mark's point is that an RPM update won't replace the file in-placec. > It will write out a new temporary file and then rename over the top of the > old file, which should trigger an update on the directory mtime. Ah, right. I wasn't considering the behavior of package mangers. My simple test of appending to an existing file doesn't simulate that. Indeed checking mtime on the directory should work. I can work on a patch, but one last question is the location of the directory? It looks like qemu searches all over the place for modules: QEMU_MODULE_DIR env var, CONFIG_QEMU_MODDIR, directory up from the executable location, directory of executable location, var/run/qemu. I suppose downstreams will go with CONFIG_QEMU_MODDIR for module location. Should libvirt have a config option to specify the path, so downstream can coordinate that between the packages?