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 <[email protected]> > > --- > > 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 <[email protected]>
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...
signature.asc
Description: PGP signature
-- libvir-list mailing list [email protected] https://www.redhat.com/mailman/listinfo/libvir-list
