qemu-devel
[Top][All Lists]
Advanced

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

Re: [PATCH] modules: load modules from versioned /var/run dir


From: Christian Ehrhardt
Subject: Re: [PATCH] modules: load modules from versioned /var/run dir
Date: Tue, 10 Mar 2020 14:16:54 +0100



On Tue, Mar 10, 2020 at 1:10 PM Daniel P. Berrangé <address@hidden> wrote:
On Tue, Mar 10, 2020 at 09:39:10AM +0000, Stefan Hajnoczi 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 <address@hidden>
> > ---
> >  util/module.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
>
> CCing Debian, Fedora, and Red Hat package maintainers in case they have
> comments.
>
> 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)?
>
> I'm not a package maintainer though so I hope the others will make
> suggestions if there are other solutions :).

My first concern is that this only partially solves the quoted problem.

Consider the QEMU RBD module which is

   /usr/lib64/qemu/block-rbd.so

This links to

   /usr/lib64/librbd.so.1

On host OS upgrade, just before uninstalling old QEMU we copy RBD
module into:

   /var/run/qemu-$VER/block-rbd.so

....but the host OS upgrade also upgrades RBD itself to a new
major version which ships

   /usr/lib64/librbd.so.2 
 
We've got /var/run/qemu-$VER/block-rbd.so saved, but we didn't
transitively save all the things that this linked to, so there's
no guarantee it will still be possible to load it.

IOW, this approach of saving QEMU block modules to a scratch dir
works, *provided* everything else that this QEMU block module needs
still exists in a compatible form.

Some distros may have a policy that no incompatible soname bumps
are permitted in updates, but that's not universal.

Hi Daniel,
Yeah for "us" being Ubuntu that would certainly be true.
An upgrade of librbd.so.1 to librbd.so.2 would only happen at a major upgrade.
For example in Ubuntu terms e.g. 18.04 to 20.04. Those upgrades usually
come with a requirement to reboot anyway - I'm not trying to solve these.

The much more common and frequent small updates to qemu with individual
fixes are what triggers this bug and what my proposal would solve. 

On the other hand this is not a big amount of new code, so is not
a huge maint problem even if only a few people ever use it. I would
be a bit more comfortable if this search path addition was perhaps
enabled by an opt-in configure argument, rather than being always
present.

I already suggested opt-in configure before.
I'd keep this just enable/disable unless we really can't agree on a path.
That should be no big issue to add this in v2.

I'm also uneasy about the idea of using a search path ordering for
doing this.  This means that an old running QEMU is always going
to first try to load the block-rbd.so from the new QEMU, expect
to see that fail, and only then fallback to load the block-rbd.so
that actually matches its build.

IIRC, we have an embedded build-id in the modules that should
guarantee that the first load attempt always fails, but I'm still 
uneasy about that at a conceptual level.
 
Yes that build-id is what prevents it loading the modules of the new package.

The intention was to keep this change small and not a burden for anyone.
I'm not yet having an idea for a conceptual change that would avoid to rely
on the build-id AND at the same time not make this a huge patch with
probably many new issues we'll find out only much later.

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



--
Christian Ehrhardt
Staff Engineer, Ubuntu Server
Canonical Ltd

reply via email to

[Prev in Thread] Current Thread [Next in Thread]