Re: [libvirt] [PATCH v5 11/12] qemu: Validate PCI controllers (QEMU capabilities)
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