Re: [libvirt] [PATCH 1/4] qemu: Add support for generic PCIe Root Ports
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