[libvirt] [PATCH] qemu: Prevent isolation group-related guest disappearance
Peter Krempa
pkrempa at redhat.com
Fri Aug 25 10:12:05 UTC 2017
On Fri, Aug 25, 2017 at 11:41:22 +0200, Martin Kletzander wrote:
> On Thu, Aug 24, 2017 at 05:12:20PM +0200, Andrea Bolognani wrote:
> > We can't retrieve the isolation group of a device that's
> > not present in the system. However, it's very common for
> > VFs to be created late in the boot, so they might not be
> > present yet when libvirtd starts, which would cause the
> > guests using them to disappear.
> >
> > If a PCI address has already been set for the host device
> > in question, we can assume that it either existed at some
> > point in the past or the user is assigning addresses
> > manually: in both cases, we should not error out and just
> > ignore the (temporary) failure.
> >
> > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1484254
> >
> > Signed-off-by: Andrea Bolognani <abologna at redhat.com>
> > ---
> > src/qemu/qemu_domain_address.c | 12 ++++++++++++
> > 1 file changed, 12 insertions(+)
> >
>
> Not an expert on this topic, but I did my due diligence and it makes
> sense to me, so
>
> Reviewed-by: Martin Kletzander <mkletzan at redhat.com>
Disclamer: I'm not objecting to the ACK, it's clearly better than it
was.
>
> if that's enough for you.
>
> > diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
> > index 16bf0cdf9..7f12f186b 100644
> > --- a/src/qemu/qemu_domain_address.c
> > +++ b/src/qemu/qemu_domain_address.c
> > @@ -1012,6 +1012,18 @@ qemuDomainFillDeviceIsolationGroup(virDomainDefPtr def,
> > tmp = virPCIDeviceAddressGetIOMMUGroupNum(hostAddr);
> >
> > if (tmp < 0) {
> > + /* If there's already a PCI address assigned to the device
> > + * we move on instead of erroring out.
> > + *
> > + * This means we still don't allow non-existing host
> > + * devices to be assigned to guests; however, if the host
> > + * device existed at some point in the past but no longer
> > + * does, which can happen very easily when dealing with VFs,
> > + * we prevent the guest from disappearing and give the user
> > + * an opportunity to edit its configuration */
> > + if (virDeviceInfoPCIAddressPresent(info))
> > + goto skip;
> > +
> > virReportError(VIR_ERR_INTERNAL_ERROR,
> > _("Can't look up isolation group for host device "
> > "%04x:%02x:%02x.%x"),
Why on earth is this in the domain address callback? That means that
it's filled on parse. That's silly for a thing like the isolation group.
The isolation group is necessary only when you are going to start the VM
so only then it should be determined. That would prevent a bug like this
altogether.
Also there's PCI hotplug...
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170825/45e122e8/attachment-0001.sig>
More information about the libvir-list
mailing list