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@xxxxxxxxxx> > > --- > > 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@xxxxxxxxxx> 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...
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
