On Fri, 2018-03-02 at 18:44 -0500, Laine Stump wrote: [...]
+static int +virDomainControllerPCIModelNameToQEMUCaps(int modelName) +{ + switch ((virDomainControllerPCIModelName) modelName) { + case VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCI_BRIDGE: + return QEMU_CAPS_DEVICE_PCI_BRIDGE; + case VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_I82801B11_BRIDGE: + return QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE; + case VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_IOH3420: + return QEMU_CAPS_DEVICE_IOH3420; + case VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_X3130_UPSTREAM: + return QEMU_CAPS_DEVICE_X3130_UPSTREAM; + case VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_XIO3130_DOWNSTREAM: + return QEMU_CAPS_DEVICE_XIO3130_DOWNSTREAM; + case VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PXB: + return QEMU_CAPS_DEVICE_PXB; + case VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PXB_PCIE: + return QEMU_CAPS_DEVICE_PXB_PCIE; + case VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCIE_ROOT_PORT: + return QEMU_CAPS_DEVICE_PCIE_ROOT_PORT; + case VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_SPAPR_PCI_HOST_BRIDGE: + return QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE; + case VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE: + return 0; + case VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_LAST: + default: + return 1;
What's the deal with "1"?
Should have been -1, of course :) [...]
+ /* The default PHB for pSeries guests doesn't need any QEMU capability + * to be used, so we should skip the capabilities check altogether */
... and the check would fail if we didn't skip?
Correct: virDomainControllerPCIModelNameToQEMUCaps() would return QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE, and the check below would fail. I'm actually fairly confident we could drop that hunk with no ill effect, because the spapr-pci-host-bridge must have existed even before we started checking for it, even if it might possibly have been, at some point, not user-instantiable; however, the capabilities data for ppc64 in our test suite doesn't go far back enough to prove this is the case, so special-casing it seems like the safest option. I'll try to see if I can build QEMU 1.2.0 (that's the earliest we claim support for, right?) on ppc64 with a reasonable amount of work, and if my hunch is correct I'll post a follow-up cleanup patch that gets rid of it. -- Andrea Bolognani / Red Hat / Virtualization