Signed-off-by: Andrea Bolognani <abologna@redhat.com>
---
src/qemu/qemu_domain.c | 190 ++++++++++++++++++++-----------------------------
1 file changed, 77 insertions(+), 113 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index f54e7b87ae..e0ab43e139 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4270,11 +4270,8 @@ qemuDomainDeviceDefValidateControllerSCSI(const virDomainControllerDef *controll
static int
qemuDomainDeviceDefValidateControllerPCIOld(const virDomainControllerDef *controller,
const virDomainDef *def,
- virQEMUCapsPtr qemuCaps)
+ virQEMUCapsPtr qemuCaps ATTRIBUTE_UNUSED)
{
- virDomainControllerModelPCI model = controller->model;
- const virDomainPCIControllerOpts *pciopts;
-
/* skip pcie-root */
if (controller->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT)
return 0;
@@ -4285,119 +4282,50 @@ qemuDomainDeviceDefValidateControllerPCIOld(const virDomainControllerDef *contro
controller->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT)
return 0;
- pciopts = &controller->opts.pciopts;
-
- /* Second pass - now the model specific checks */
- switch (model) {
- case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE:
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PCI_BRIDGE)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("the pci-bridge controller is not supported "
- "in this QEMU binary"));
- return -1;
- }
-
- break;
-
- case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS:
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PXB)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("the pxb controller is not supported in this "
- "QEMU binary"));
- return -1;
- }
-
- break;
-
- case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE:
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("the dmi-to-pci-bridge (i82801b11-bridge) "
- "controller is not supported in this QEMU binary"));
- return -1;
- }
-
- break;
-
- case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT:
- if ((pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_IOH3420) &&
- !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_IOH3420)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("the pcie-root-port (ioh3420) controller "
- "is not supported in this QEMU binary"));
- return -1;
- }
-
- if ((pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCIE_ROOT_PORT) &&
- !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PCIE_ROOT_PORT)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("the pcie-root-port (pcie-root-port) controller "
- "is not supported in this QEMU binary"));
- return -1;
- }
-
- break;
-
- case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT:
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_X3130_UPSTREAM)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("the pcie-switch-upstream-port (x3130-upstream) "
- "controller is not supported in this QEMU binary"));
- return -1;
- }
-
- break;
-
- case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT:
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_XIO3130_DOWNSTREAM)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("The pcie-switch-downstream-port "
- "(xio3130-downstream) controller is not "
- "supported in this QEMU binary"));
- return -1;
- }
-
- break;
-
- case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS:
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PXB_PCIE)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("the pxb-pcie controller is not supported "
- "in this QEMU binary"));
- return -1;
- }
-
- break;
-
- case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
- /* Skip the implicit one */
- if (pciopts->targetIndex == 0)
- return 0;
-
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("the spapr-pci-host-bridge controller is not "
- "supported in this QEMU binary"));
- return -1;
- }
-
- if (pciopts->numaNode != -1 &&
- !virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPAPR_PCI_HOST_BRIDGE_NUMA_NODE)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("the spapr-pci-host-bridge controller doesn't "
- "support numa_node in this QEMU binary"));
- return -1;
- }
+ return 0;
+}
- break;
- case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
- case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
- case VIR_DOMAIN_CONTROLLER_MODEL_PCI_DEFAULT:
- break;
+/**
+ * virDomainControllerPCIModelNameToQEMUCaps:
+ * @modelName: model name
+ *
+ * Maps model names for PCI controllers (virDomainControllerPCIModelName)
+ * to the QEMU capabilities required to use them (virQEMUCapsFlags).
+ *
+ * Returns: the QEMU capability itself (>0) on success; 0 if no QEMU
+ * capability is needed; <0 on error.
+ */
+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;
}
- return 0;
+ return -1;
}
@@ -4410,6 +4338,7 @@ qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *cont,
const virDomainPCIControllerOpts *pciopts = &cont->opts.pciopts;
const char *model = virDomainControllerModelPCITypeToString(cont->model);
const char *modelName = virDomainControllerPCIModelNameTypeToString(pciopts->modelName);
+ int cap = virDomainControllerPCIModelNameToQEMUCaps(pciopts->modelName);
if (!model) {
virReportEnumRangeError(virDomainControllerModelPCI, cont->model);
@@ -4859,6 +4788,41 @@ qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *cont,
virReportEnumRangeError(virDomainControllerModelPCI, cont->model);
}
+ /* The default PHB for pSeries guests doesn't need any QEMU capability
+ * to be used, so we should skip the capabilities check altogether */
+ if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT &&
+ pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_SPAPR_PCI_HOST_BRIDGE &&
+ pciopts->targetIndex == 0) {
+ goto done;
+ }
+
+ /* QEMU device availability */
+ if (cap < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Unknown QEMU device for '%s' controller"),
+ modelName);
+ return -1;
+ }
+ if (cap > 0 && !virQEMUCapsGet(qemuCaps, cap)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("The '%s' device is not supported by this QEMU binary"),
+ modelName);
+ return -1;
+ }
+
+ /* PHBs didn't support numaNode from the very beginning, so an extra
+ * capability check is required */
+ if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT &&
+ pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_SPAPR_PCI_HOST_BRIDGE &&
+ pciopts->numaNode != -1 &&
+ !virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPAPR_PCI_HOST_BRIDGE_NUMA_NODE)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Option '%s' is not supported by '%s' device with this QEMU binary"),
+ "numaNode", modelName);
+ return -1;
+ }
+
+ done:
return qemuDomainDeviceDefValidateControllerPCIOld(cont, def, qemuCaps);
}
--
2.14.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 03/02/2018 10:13 AM, Andrea Bolognani wrote: > Signed-off-by: Andrea Bolognani <abologna@redhat.com> > --- > src/qemu/qemu_domain.c | 190 ++++++++++++++++++++----------------------------- > 1 file changed, 77 insertions(+), 113 deletions(-) > > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c > index f54e7b87ae..e0ab43e139 100644 > --- a/src/qemu/qemu_domain.c > +++ b/src/qemu/qemu_domain.c > @@ -4270,11 +4270,8 @@ qemuDomainDeviceDefValidateControllerSCSI(const virDomainControllerDef *controll > static int > qemuDomainDeviceDefValidateControllerPCIOld(const virDomainControllerDef *controller, > const virDomainDef *def, > - virQEMUCapsPtr qemuCaps) > + virQEMUCapsPtr qemuCaps ATTRIBUTE_UNUSED) > { > - virDomainControllerModelPCI model = controller->model; > - const virDomainPCIControllerOpts *pciopts; > - > /* skip pcie-root */ > if (controller->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) > return 0; > @@ -4285,119 +4282,50 @@ qemuDomainDeviceDefValidateControllerPCIOld(const virDomainControllerDef *contro > controller->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) > return 0; > > - pciopts = &controller->opts.pciopts; > - > - /* Second pass - now the model specific checks */ > - switch (model) { > - case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE: > - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PCI_BRIDGE)) { > - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > - _("the pci-bridge controller is not supported " > - "in this QEMU binary")); > - return -1; > - } > - > - break; > - > - case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS: > - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PXB)) { > - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > - _("the pxb controller is not supported in this " > - "QEMU binary")); > - return -1; > - } > - > - break; > - > - case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE: > - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE)) { > - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > - _("the dmi-to-pci-bridge (i82801b11-bridge) " > - "controller is not supported in this QEMU binary")); > - return -1; > - } > - > - break; > - > - case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT: > - if ((pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_IOH3420) && > - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_IOH3420)) { > - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > - _("the pcie-root-port (ioh3420) controller " > - "is not supported in this QEMU binary")); > - return -1; > - } > - > - if ((pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCIE_ROOT_PORT) && > - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PCIE_ROOT_PORT)) { > - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > - _("the pcie-root-port (pcie-root-port) controller " > - "is not supported in this QEMU binary")); > - return -1; > - } > - > - break; > - > - case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT: > - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_X3130_UPSTREAM)) { > - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > - _("the pcie-switch-upstream-port (x3130-upstream) " > - "controller is not supported in this QEMU binary")); > - return -1; > - } > - > - break; > - > - case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT: > - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_XIO3130_DOWNSTREAM)) { > - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > - _("The pcie-switch-downstream-port " > - "(xio3130-downstream) controller is not " > - "supported in this QEMU binary")); > - return -1; > - } > - > - break; > - > - case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS: > - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PXB_PCIE)) { > - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > - _("the pxb-pcie controller is not supported " > - "in this QEMU binary")); > - return -1; > - } > - > - break; > - > - case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT: > - /* Skip the implicit one */ > - if (pciopts->targetIndex == 0) > - return 0; > - > - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE)) { > - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > - _("the spapr-pci-host-bridge controller is not " > - "supported in this QEMU binary")); > - return -1; > - } > - > - if (pciopts->numaNode != -1 && > - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPAPR_PCI_HOST_BRIDGE_NUMA_NODE)) { > - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > - _("the spapr-pci-host-bridge controller doesn't " > - "support numa_node in this QEMU binary")); > - return -1; > - } > + return 0; > +} > > - break; > > - case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT: > - case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST: > - case VIR_DOMAIN_CONTROLLER_MODEL_PCI_DEFAULT: > - break; > +/** > + * virDomainControllerPCIModelNameToQEMUCaps: > + * @modelName: model name > + * > + * Maps model names for PCI controllers (virDomainControllerPCIModelName) > + * to the QEMU capabilities required to use them (virQEMUCapsFlags). > + * > + * Returns: the QEMU capability itself (>0) on success; 0 if no QEMU > + * capability is needed; <0 on error. > + */ > +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"? > } > > - return 0; > + return -1; > } > > > @@ -4410,6 +4338,7 @@ qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *cont, > const virDomainPCIControllerOpts *pciopts = &cont->opts.pciopts; > const char *model = virDomainControllerModelPCITypeToString(cont->model); > const char *modelName = virDomainControllerPCIModelNameTypeToString(pciopts->modelName); > + int cap = virDomainControllerPCIModelNameToQEMUCaps(pciopts->modelName); > > if (!model) { > virReportEnumRangeError(virDomainControllerModelPCI, cont->model); > @@ -4859,6 +4788,41 @@ qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *cont, > virReportEnumRangeError(virDomainControllerModelPCI, cont->model); > } > > + /* 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? If so, then okay. Depending on the answers to the two questions above: Conditional-Reviewed-by: Laine Stump <laine@laine.org> :-) > + if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT && > + pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_SPAPR_PCI_HOST_BRIDGE && > + pciopts->targetIndex == 0) { > + goto done; > + } > + > + /* QEMU device availability */ > + if (cap < 0) { > + virReportError(VIR_ERR_INTERNAL_ERROR, > + _("Unknown QEMU device for '%s' controller"), > + modelName); > + return -1; > + } > + if (cap > 0 && !virQEMUCapsGet(qemuCaps, cap)) { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > + _("The '%s' device is not supported by this QEMU binary"), > + modelName); > + return -1; > + } > + > + /* PHBs didn't support numaNode from the very beginning, so an extra > + * capability check is required */ > + if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT && > + pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_SPAPR_PCI_HOST_BRIDGE && > + pciopts->numaNode != -1 && > + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPAPR_PCI_HOST_BRIDGE_NUMA_NODE)) { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > + _("Option '%s' is not supported by '%s' device with this QEMU binary"), > + "numaNode", modelName); > + return -1; > + } > + > + done: > return qemuDomainDeviceDefValidateControllerPCIOld(cont, def, qemuCaps); > } > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, 2018-03-05 at 16:58 +0100, Andrea Bolognani wrote: > 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. It's actually 0.12, not 1.2.0. Whoops. According to my research, the pSeries machine type was introduced in 0.15 but didn't have PCI support until 1.0, by which point the spapr-pci-host-bridge device had been introduced too. tl;dr the hunk in question can safely be dropped. I'll respin. -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2025 Red Hat, Inc.