qemu modularization of qemu-5.1 vs libvirt domcapabilities cache?

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 @@commandline qemu-device-usb-redirect.x86_64 2:5.1.0-0.1.rc2.fc32 @@commandline 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? The domcapabilities cache will be automatically updated only under certain conditions such as the qemu binary ctime changing - but that isn't triggering any action here? With the devices broken out into modules, such as the Fedora rawhide RPM .spec file is proposing, this allows the devices to be individually installed or uninstalled at any time, and causes libvirt domcapabilities cache to be out-of-date. I was able to sometimes see it work by downgrading to qemu-5.0, upgrading to qemu-5.1rc2, and installing the device packages prior to calling "virsh domcapabilities" (or otherwise using them). I was also able to do the same by removing the libvirt cache files and restarted libvirtd service. Both of these are pretty non-obvious actions for a user. I'm wondering how to codify this when I use it for real on a production system. The upgrade path here seems unreliable, especially given that libvirt domcapabilities cache may even persist across reboots? This means I need to be very careful about the procedure to upgrade, and also I need to make sure to never install or remove any of the device packages without checking the procedure. Ouch. :-( Thoughts? -- Mark Mielke <mark.mielke@gmail.com>

On Wed, Aug 05, 2020 at 02:09:46 -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 @@commandline qemu-device-usb-redirect.x86_64 2:5.1.0-0.1.rc2.fc32 @@commandline
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?
Does the original qemu package still install everything? If a package is split, the original package still should install everything for compatibility.
The domcapabilities cache will be automatically updated only under certain conditions such as the qemu binary ctime changing - but that isn't triggering any action here? With the devices broken out into modules, such as the Fedora rawhide RPM .spec file is proposing, this allows the devices to be individually installed or uninstalled at any time, and causes libvirt domcapabilities cache to be out-of-date.
I was able to sometimes see it work by downgrading to qemu-5.0, upgrading to qemu-5.1rc2, and installing the device packages prior to calling "virsh domcapabilities" (or otherwise using them). I was also able to do the same by removing the libvirt cache files and restarted libvirtd service. Both of these are pretty non-obvious actions for a user.
I'm wondering how to codify this when I use it for real on a production system. The upgrade path here seems unreliable, especially given that libvirt domcapabilities cache may even persist across reboots? This means I need to be very careful about the procedure to upgrade, and also I need to make sure to never install or remove any of the device packages without checking the procedure. Ouch. :-(
Well, libvirt certainly needs to include the list of modules and their ctimes in the cache now and trigger a re-query if this changes. The cache itself is meant as an optimization so users shouldn't need to do anything or limit what's being done.

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. -- Andrea Bolognani / Red Hat / Virtualization

On Wed, Aug 5, 2020 at 4:19 AM Andrea Bolognani <abologna@redhat.com> wrote:
On Wed, 2020-08-05 at 02:09 -0400, Mark Mielke wrote:
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.
I was wondering if checking /usr/lib64/qemu (the module directory) might be sufficient? Adding or removing content from the modules directory would potentially change the domcapabilities? I don't know if there is a reliable way of determining this directory from libvirt? In general - a user might add or remove such packages at any time, and expect it to automatically work. I don't think having a list of modules would be effective, but if just checking the module directory itself worked, that could be the right compromise? 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. Thanks, -- Mark Mielke <mark.mielke@gmail.com>

On Wed, Aug 5, 2020 at 4:31 AM Mark Mielke <mark.mielke@gmail.com> wrote:
On Wed, Aug 5, 2020 at 4:19 AM Andrea Bolognani <abologna@redhat.com> 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.
The Fedora package owner agreed, and will be correcting it so that the default will include these packages. This addresses the upgrade case, and the surprise factor for users merely upgrading from Fedora 32 to Fedora 33 resulting in libvirt breaking for them. However, it does not address the exposed problem - which is that I can add or remove individual Qemu module packages at any time, and libvirt will not be aware of this change until some other event occurs which might not ever occur in this workflow (even reboot!). So, I think it is important to include the Qemu module directory in the list of timestamps to check to determine if the domcapabilities cache should be invalidated or not. If a module gets added or removed, the directory timestamp should change. I think the idea of libvirt probing the system and guessing when to invalidate the cache based upon only a few select data points, including ones like "qemu binary timestamp" are embedded with assumptions, is going to continue to be a problem. But, adding the above check would close an additional set of scenarios as covered. -- Mark Mielke <mark.mielke@gmail.com>

On 8/5/20 2:19 AM, Andrea Bolognani wrote:
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.
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? Checking the ctime of the directory would work if modules have been added or removed, but not for modules that get updated. Maybe it's not a problem if capabilities don't change on module updates, but I suspect that is not the case. It seems perfectly reasonable for an updated module to introduce a new capability. Regards, Jim

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.
I wonder if it is possible to use inotify to receive notification of changes to the modules directory?
That would certainly be possible, but it'd be a new feature (getting aware of qemu capability changes during runtime), while we're currently discussing an existing feature (getting qemu capabilities on startup) which is broken. I believe these issues should be discussed separately. Regards, Martin

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

On Thu, 2020-08-20 at 10:57 +0100, Daniel P. Berrangé wrote:
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.
Capability caching could be turned into a build-time option, optimized for the target audience. Or we could enable caching in general, but always rescan capabilites at libvirtd startup. That way startup of VMs wouldn't be slowed down. No? The latter would have the additional advantage that people who (de)install qemu modules would simply need to restart libvirtd in order to get a working system back. I suppose that would make sense to most users, and probably even occur to them as a workaround. Currently users have to locate and manually delete the cache files, not to mention that they need to figure this out first, which is far from easy. Regards Martin

On Thu, Aug 20, 2020 at 12:31:15PM +0200, Martin Wilck wrote:
On Thu, 2020-08-20 at 10:57 +0100, Daniel P. Berrangé wrote:
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.
Capability caching could be turned into a build-time option, optimized for the target audience.
It is rare at build time to know what your target audience's apps are going to be doing when you are a OS distro. So it isn't a decision that can usefully be made at build time.
Or we could enable caching in general, but always rescan capabilites at libvirtd startup. That way startup of VMs wouldn't be slowed down. No?
Scanning at libvirtd startup is something we work very hard to avoid. When you have 20 QEMU system emulators installed, it makes libvirtd startup incredibly slow which is a big problem when we are using auto-start + auto-shutdown for libvirtd. 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 :|

On Wed, Aug 05, 2020 at 10:19:26AM +0200, Andrea Bolognani wrote:
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.
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. 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 :|

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 Thu, Aug 20, 2020 at 12:43 PM Martin Wilck <mwilck@suse.com> 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. [1]: https://github.com/qemu/qemu/commit/bd83c861c0628a64997b7bd95c3bcc2e916baf2e
Cheers, Martin
-- Christian Ehrhardt Staff Engineer, Ubuntu Server Canonical Ltd

On 8/20/20 6:54 AM, Christian Ehrhardt wrote:
On Thu, Aug 20, 2020 at 12:43 PM Martin Wilck <mwilck@suse.com> 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 The daemon could then check changes to this file when validating the cache. I'm not familiar enough with deb packaging to know if there is a similar mechanism to filetrigger. Regards, Jim

On 8/20/20 8:56 AM, Jim Fehlig wrote:
On 8/20/20 6:54 AM, Christian Ehrhardt wrote:
On Thu, Aug 20, 2020 at 12:43 PM Martin Wilck <mwilck@suse.com> 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
Hmm, this is still problematic for session mode. I think your suggestion covers both system and session modes nicely. If we can agree on an approach I'd be happy to take a stab at implementing it. Regards, Jim

On Thu, Aug 20, 2020 at 09:05:56AM -0600, Jim Fehlig wrote:
On 8/20/20 8:56 AM, Jim Fehlig wrote:
On 8/20/20 6:54 AM, Christian Ehrhardt wrote:
On Thu, Aug 20, 2020 at 12:43 PM Martin Wilck <mwilck@suse.com> 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
Hmm, this is still problematic for session mode. I think your suggestion covers both system and session modes nicely.
Hmm, yeah, session mode ought not to be looking at the /var/cache directory tree ideally, as we like to keep the daemons fully isolated. I wonder if we really do just need to traverse the module directory. readdir+stat'ing 10-20 files is still waaaay faster than not having any caching at all. 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 :|

On Thu, Aug 20, 2020 at 8:55 AM Christian Ehrhardt < christian.ehrhardt@canonical.com> wrote:
On Thu, Aug 20, 2020 at 12:43 PM Martin Wilck <mwilck@suse.com> 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
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? -- Mark Mielke <mark.mielke@gmail.com>

On Thu, Aug 20, 2020 at 5:15 PM Mark Mielke <mark.mielke@gmail.com> wrote:
On Thu, Aug 20, 2020 at 8:55 AM Christian Ehrhardt <christian.ehrhardt@canonical.com> wrote:
On Thu, Aug 20, 2020 at 12:43 PM Martin Wilck <mwilck@suse.com> 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
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?
-- Mark Mielke <mark.mielke@gmail.com>
-- Christian Ehrhardt Staff Engineer, Ubuntu Server Canonical Ltd

On Thu, Aug 20, 2020 at 11:48 AM Christian Ehrhardt < christian.ehrhardt@canonical.com> wrote: > On Thu, Aug 20, 2020 at 5:15 PM Mark Mielke <mark.mielke@gmail.com> wrote: > > On Thu, Aug 20, 2020 at 8:55 AM Christian Ehrhardt < > 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". > But I guess there were reasons against it in the discussion before? > I don't think there was a follow up discussion on the idea. I think that is here. :-) -- Mark Mielke <mark.mielke@gmail.com>

On 8/20/20 10:54 AM, Mark Mielke wrote:
On Thu, Aug 20, 2020 at 11:48 AM Christian Ehrhardt <christian.ehrhardt@canonical.com <mailto:christian.ehrhardt@canonical.com>> wrote:
On Thu, Aug 20, 2020 at 5:15 PM Mark Mielke <mark.mielke@gmail.com <mailto:mark.mielke@gmail.com>> wrote: > On Thu, Aug 20, 2020 at 8:55 AM Christian Ehrhardt <christian.ehrhardt@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. Regards, Jim

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@canonical.com <mailto:christian.ehrhardt@canonical.com>> wrote:
On Thu, Aug 20, 2020 at 5:15 PM Mark Mielke <mark.mielke@gmail.com <mailto:mark.mielke@gmail.com>> wrote: > On Thu, Aug 20, 2020 at 8:55 AM Christian Ehrhardt <christian.ehrhardt@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. Not sure what a "make install" will do with QEMU, but since QEMU is about to switch to meson, we might get lucky in having the same temp+rename behaviour. 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 :|

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@canonical.com <mailto:christian.ehrhardt@canonical.com>> wrote:
On Thu, Aug 20, 2020 at 5:15 PM Mark Mielke <mark.mielke@gmail.com <mailto:mark.mielke@gmail.com>> wrote: > On Thu, Aug 20, 2020 at 8:55 AM Christian Ehrhardt <christian.ehrhardt@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? Regards, Jim

On 8/20/20 2:07 PM, Jim Fehlig wrote:
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@canonical.com <mailto:christian.ehrhardt@canonical.com>> wrote:
On Thu, Aug 20, 2020 at 5:15 PM Mark Mielke <mark.mielke@gmail.com <mailto:mark.mielke@gmail.com>> wrote: > On Thu, Aug 20, 2020 at 8:55 AM Christian Ehrhardt <christian.ehrhardt@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?
I've sent a patch that includes a 'qemu_moddir' configure option https://www.redhat.com/archives/libvir-list/2020-August/msg00688.html Regards, Jim

On Thu, Aug 20, 2020 at 2:01 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
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.
Yep, correct.
Not sure what a "make install" will do with QEMU, but since QEMU is about to switch to meson, we might get lucky in having the same temp+rename behaviour.
I think this is the relevant section of the Makefile: ifneq ($(CONFIG_MODULES),) $(INSTALL_DIR) "$(DESTDIR)$(qemu_moddir)" for s in $(modules-m:.mo=$(DSOSUF)); do \ t="$(DESTDIR)$(qemu_moddir)/$$(echo $$s | tr / -)"; \ $(INSTALL_LIB) $$s "$$t"; \ test -z "$(STRIP)" || $(STRIP) "$$t"; \ done endif The INSTALL_LIB is detected by configure, and will normally be: config-host.mak:INSTALL_LIB=install -c -m 0644 You can see here that "install" results in a new inode, which indicates a new file: $ cd /tmp $ install -c -m 0644 /etc/motd motd_temp $ ls -li motd_temp *3544372* -rw-r--r-- 1 mark mark 0 Aug 20 17:25 motd_temp $ install -c -m 0644 /etc/motd motd_temp $ ls -li motd_temp *3561572* -rw-r--r-- 1 mark mark 0 Aug 20 17:25 motd_temp Once upon a time, this wasn't done - and the consequence is that things like "make install" would break running programs with various undefined behaviour. Now it is done "correctly" for so long, that people may have forgotten the old days. :-) -- Mark Mielke <mark.mielke@gmail.com>
participants (7)
-
Andrea Bolognani
-
Christian Ehrhardt
-
Daniel P. Berrangé
-
Jim Fehlig
-
Mark Mielke
-
Martin Wilck
-
Peter Krempa