[libvirt] [PATCH 1/4] qemu: Add support for generic PCIe Root Ports

[Andrea Bolognani]

On Wed, 2017-03-15 at 15:18 -0400, Laine Stump wrote:
> > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1408808
> 
> Yes, but kind of no. It makes it available, but still difficult and
> cumbersome to use. I would say it *partially* resolves that BZ, with
> full resolution coming from the followup patches that make it the
> default. (I'm only pointing this out because we wouldn't want some
> distro maintainer to be looking through patches for backport and
> erroneously believe, based on the commit log, that this was the only
> patch they needed).

I was debating about this myself.

Strictly speaking, the bug report is about adding support
for generic PCIe Root Ports, which this patch does.

That said, I see your point, and since I don't have any
specific objection to moving the Resolves: to the next
patch I'll do just that.

[...]
> BTW, although you added an entry to your new-fangled "news" file in
> patch 4, you never added new info to formatdomain.html.in - at least
> that should be included in this patch (with a small addition to its text
> when you change the default for aarch64).

The existing documentation is fairly vague about PCI
controller models:

  PCI controllers also have an optional subelement <model>
  with an attribute name. The name attribute holds the
  name of the specific device that qemu is emulating (e.g.
  "i82801b11-bridge") rather than simply the class of device
  ("dmi-to-pci-bridge", "pci-bridge"), which is set in the
  controller element's model attribute. In almost all cases,
  you should not manually add a <model> subelement to a
  controller, nor should you modify one that is automatically
  generated by libvirt. Since 1.2.19 (QEMU only).

Nowhere are the valid models for each PCI controllers listed,
and that's fine in my book: the documentation explicitly
tells the user that they should let libvirt do its thing in
basically all cases.

Do you really think we should make that more explicit?

[...]
> > @@ -338,7 +338,9 @@ VIR_ENUM_IMPL(virDomainControllerPCIModelName,
> >                "x3130-upstream",
> >                "xio3130-downstream",
> >                "pxb",
> > -              "pxb-pcie")
> > +              "pxb-pcie",
> > +              "pcie-root-port",
> 
> Sigh. As this becomes the norm, it's going to make libvirt config look
> redundant (and is also likely to confuse people about which attribute to
> change if they want to use iohh3420 instead of the generic one), but
> there's nothing that can be done about it. (I agree that this was the
> best choice for device name in qemu btw).

You just have to change the model, as opposed to, you
know... The model. What's so confusing about that? :D

-- 
Andrea Bolognani / Red Hat / Virtualization