[libvirt] [PATCH] qemu: Don't special case mdevs when assigning PCI addresses

[Erik Skultety]

On Fri, Jun 02, 2017 at 08:20:12AM +0200, Andrea Bolognani wrote:
> On Thu, 2017-06-01 at 21:24 -0400, Laine Stump wrote:
> > > @@ -645,9 +645,6 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev,
> > >              return pcieFlags;
> > >          }
> > >
> > > -        if (hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV)
> > > -            return pcieFlags;
> > > -
> > >          if (!(pciDev = virPCIDeviceNew(hostAddr->domain,
> > >                                         hostAddr->bus,
> > >                                         hostAddr->slot,
> >
> > This can't work. From the host's point of view, there is no PCI device
> > whose config space can be read, and from the code's point of view,
> > hostAddr->domain|bus|slot are invalid (they are in
> > hostdev->source.subsys.u.pci.addr, which is only valid when
> > (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI).

In fact it does work, because we return pcieFlags on error which might only be
a problem if the backing device itself would be a legacy PCI.

>
> Surely you meant to use '==' here?
>
> > Is there maybe something standard in the mdev's sysfs entry that could
> > be used to determine PCI vs PCIe? I think back when we were discussing
> > the implementation of this, Alex had said there wasn't.

No, there's not without looking at the parent PCI device. IIRC Alex said we
would have to go to the parent device's sysfs, read the config space and
determine it from there.

>
> If that's the case, then there should be a comment explaining
> why mdevs are treated this way. It's not at all obvious to a
> reader who's not familiar with them :)

You're absolutely right, I have to admit, I don't remember the exact reason why
I added that check, whether it was just to follow a private discussion about
this or I actually checked the log for errors and figured this myself. Feel
free to send another patch adding the comment.

Anyhow, thank you Laine for providing feedback, since when Andrea described the
issue to me, it made sense and looked to me like a feasible thing to do the
whole time and I couldn't come up with an argument why it wasn't OK, so,
appreciated.

 Erik
>
> --
> Andrea Bolognani / Red Hat / Virtualization