On Tue, Mar 10, 2020 at 10:39 AM Stefan Hajnoczi <stefa...@gmail.com> wrote:
> On Fri, Mar 06, 2020 at 02:26:48PM +0100, Christian Ehrhardt wrote: > > On upgrades the old .so files usually are replaced. But on the other > > hand since a qemu process represents a guest instance it is usually kept > > around. > > > > That makes late addition of dynamic features e.g. 'hot-attach of a ceph > > disk' fail by trying to load a new version of e.f. block-rbd.so into an > > old still running qemu binary. > > > > This adds a fallback to also load modules from a versioned directory in > the > > temporary /var/run path. That way qemu is providing a way for packaging > > to store modules of an upgraded qemu package as needed until the next > reboot. > > > > An example how that can then be used in packaging can be seen in: > > > https://git.launchpad.net/~paelzer/ubuntu/+source/qemu/log/?h=bug-1847361-miss-old-so-on-upgrade-UBUNTU > > > > Fixes: https://bugs.launchpad.net/ubuntu/+source/qemu/+bug/1847361 > > Signed-off-by: Christian Ehrhardt <christian.ehrha...@canonical.com> > > --- > > util/module.c | 7 +++++++ > > 1 file changed, 7 insertions(+) > > CCing Debian, Fedora, and Red Hat package maintainers in case they have > comments. > Yeah that seems worth, thanks Stefan. The use of /var/run makes me a little uneasy. I guess it's related to > wanting to uninstall the old package so the .so in their original > location cannot be used (even if they had a versioned path)? > Yes you'd want to uninstall the old bits from disk - even if there would be a versioned path. /var/run was considered a nice place as it is auto-cleaned on a reboot avoiding a massive and most likely broken logic trying to detect which qemu versions still have running binaries. And no distro will have so many qemu updates that N*~<100k for the .so files will really grow too big. Also if the path would end up to be the major concern and we can't agree on a single one we can consider making this: 1. not checking an extra path if not configured 2. on configure packaging can set a path of their choice I have not done so yet as I was hoping for a simpler patch and everyone knowing what to expect across e.g. distros. It also will make isolation easier for example I also have apparmor changes for libvirt allowing that path which is more complex if it ends up configurable. And finally this has to be considered an "offer" by qemu to the packagers to fix a real field issue. The packaging does not "have to" exploit this, every Distro is free to just ignore it. I'm not a package maintainer though so I hope the others will make > suggestions if there are other solutions :). > > > > > diff --git a/util/module.c b/util/module.c > > index 236a7bb52a..d2446104be 100644 > > --- a/util/module.c > > +++ b/util/module.c > > @@ -19,6 +19,7 @@ > > #endif > > #include "qemu/queue.h" > > #include "qemu/module.h" > > +#include "qemu-version.h" > > > > typedef struct ModuleEntry > > { > > @@ -170,6 +171,7 @@ bool module_load_one(const char *prefix, const char > *lib_name) > > #ifdef CONFIG_MODULES > > char *fname = NULL; > > char *exec_dir; > > + char *version_dir; > > const char *search_dir; > > char *dirs[4]; > > char *module_name; > > @@ -201,6 +203,11 @@ bool module_load_one(const char *prefix, const char > *lib_name) > > dirs[n_dirs++] = g_strdup_printf("%s", CONFIG_QEMU_MODDIR); > > dirs[n_dirs++] = g_strdup_printf("%s/..", exec_dir ? : ""); > > dirs[n_dirs++] = g_strdup_printf("%s", exec_dir ? : ""); > > + version_dir = g_strcanon(g_strdup(QEMU_PKGVERSION), > > + G_CSET_A_2_Z G_CSET_a_2_z G_CSET_DIGITS > "+-.~", > > + '_'); > > + dirs[n_dirs++] = g_strdup_printf("/var/run/qemu/%s", version_dir); > > + > > assert(n_dirs <= ARRAY_SIZE(dirs)); > > > > g_free(exec_dir); > > -- > > 2.25.1 > > > > > -- Christian Ehrhardt Staff Engineer, Ubuntu Server Canonical Ltd
