[libvirt] [PATCH v5 11/12] qemu: Validate PCI controllers (QEMU capabilities)

[Andrea Bolognani]

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