Re: [libvirt] [PATCH v2 08/15] qemu: Validate PCI controllers (index)

> +qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *controller, > + const virDomainDef *def) > { > + const virDomainPCIControllerOpts *pciopts = &controller->opts.pciopts; > + const char *model = virDomainControllerModelPCITypeToString(controller->model); > + const char *modelName = virDomainControllerPCIModelNameTypeToString(pciopts->modelName); > + > + if (!model) { > + virReportError(VIR_ERR_INTERNAL_ERROR, > + _("Unknown virDomainControllerModelPCI value: %d"), Including type names in error messages is not too nice as a user facing error - better to use plain english "Unexpected PCI controller model")

> + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT: > + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT: > + /* pci-root controllers for pSeries guests can have any index, but > + * all other pci-root and pcie-root controller must have index 0 */ > + if (controller->idx != 0 && > + !(qemuDomainIsPSeries(def) && > + controller->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT)) { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > + _("Index for '%s' controller must be 0"), > + model); > + return -1; > + } > + break; > + > + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST: Any time there is a _LAST element we should have an error report too virReportError(VIR_ERR_INTERNAL_ERROR, _("Unexpected controller model %d"), controller->model); return -1; It should also have an 'default:' next to the LAST. Both are things that should never occur unless we've screwed up code elsewherein libvirt, but we want to be robust about catching such screw ups. This is particularly important for controller models, as we have many different controller type specific enums all stored in the same struct field