On Mon, Nov 25, 2019 at 02:07:13PM +0100, Peter Krempa wrote:
On Mon, Nov 25, 2019 at 12:59:00 +0000, Daniel Berrange wrote:
> On Mon, Nov 25, 2019 at 01:53:47PM +0100, Peter Krempa wrote:
> > The redetection was originally added in 43c01d3838 as a way to recover
> > from libvirtd upgrade from the time when we didn't persist the qemu
> > capabilities in the status XML. Also this the oldest supported qemu by
> > more than two years.
> >
> > Even if somebody would have a running VM running at least qemu 1.5 with
> > such an old libvirt we certainly wouldn't do the right thing by
> > redetecting the capabilities and then trying to communicate with qemu.
> >
> > For now it will be the best to just stop considering this scenario any
> > more.
> >
> > Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
> > ---
> > src/qemu/qemu_process.c | 7 -------
> > 1 file changed, 7 deletions(-)
> >
> > diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> > index 1b88c471f4..a76a8da841 100644
> > --- a/src/qemu/qemu_process.c
> > +++ b/src/qemu/qemu_process.c
> > @@ -8085,13 +8085,6 @@ qemuProcessReconnect(void *opaque)
> > goto error;
> > }
> >
> > - /* If upgrading from old libvirtd we won't have found any
> > - * caps in the domain status, so re-query them
> > - */
> > - if (!priv->qemuCaps &&
> > - (qemuDomainUpdateQEMUCaps(obj, driver->qemuCapsCache) < 0))
> > - goto error;
>
> Shouldn't we be making missing qemuCaps into a fatal error
> when loading that VM.
I thought about it for a bit. If we make it an error the VM will vanish
after upgrade (and continue running). If we don't make an error it will
not vanish and you'll be able to at least destroy it.
Given that such situation is super-unlikely, I opted to remove it.
Nobody will probably run into this situation and if yes then the VM will
not vanish at least.
I can add an error though, but to me it seems it's not worth.
My concern is that pretty much all our later code will assume that
priv->qemuCaps is non-NULL. So by ignoring the error, the VM might
not vanish initially, but libvirtd may well crash shortly after.
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 :|