On 03/08/2018 08:34 AM, Andrea Bolognani wrote: > https://bugzilla.redhat.com/show_bug.cgi?id=1483816 > > Signed-off-by: Andrea Bolognani <abologna@xxxxxxxxxx> > --- > src/qemu/qemu_domain.c | 226 +++++++++++++++++++++++++++++++------------------ > 1 file changed, 142 insertions(+), 84 deletions(-) > > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c > index f1139cbac3..d8669ee9b3 100644 > --- a/src/qemu/qemu_domain.c > +++ b/src/qemu/qemu_domain.c > @@ -4274,7 +4274,6 @@ qemuDomainDeviceDefValidateControllerPCIOld(const virDomainControllerDef *contro > { > virDomainControllerModelPCI model = controller->model; > const virDomainPCIControllerOpts *pciopts; > [...] > > > +#define virReportControllerMissingOption(cont, model, modelName, option) \ > + virReportError(VIR_ERR_INTERNAL_ERROR, \ > + _("Required option '%s' is not set for PCI controller " \ > + "with index '%d', model '%s' and modelName '%s'"), \ > + (option), (cont->idx), (model), (modelName)); > +#define virReportControllerInvalidOption(cont, model, modelName, option) \ > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, \ > + _("Option '%s' is not valid for PCI controller " \ > + "with index '%d', model '%s' and modelName '%s'"), \ > + (option), (cont->idx), (model), (modelName)); > +#define virReportControllerInvalidValue(cont, model, modelName, option) \ > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, \ > + _("Option '%s' has invalid value for PCI controller " \ > + "with index '%d', model '%s' and modelName '%s'"), \ > + (option), (cont->idx), (model), (modelName)); > + > + > static int > qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *cont, > const virDomainDef *def, > @@ -4566,10 +4499,135 @@ qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *cont, > return -1; > } > > + /* modelName */ > + switch ((virDomainControllerModelPCI) cont->model) { > + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE: > + case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE: > + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT: > + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT: > + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT: > + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS: > + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS: > + /* modelName should have been set automatically */ > + if (pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) { > + virReportControllerMissingOption(cont, model, modelName, "modelName"); "Required option 'modelName' is not set for PCI controller with index '1', model 'pcie-root-port', modelName 'none'." Yep, looks good to me. > + return -1; > + } > + break; > + > + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT: > + /* modelName must be set for pSeries guests, but it's an error > + * for it to be set for any other guest */ > + if (qemuDomainIsPSeries(def)) { > + if (pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) { > + virReportControllerMissingOption(cont, model, modelName, "modelName"); > + return -1; > + } > + } else { > + if (pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) { > + virReportControllerInvalidOption(cont, model, modelName, "modelName"); > + return -1; ...and since we've already errored out on modelName values outside the accepted range, this is okay too. > [...] Reviewed-by: Laine Stump <laine@xxxxxxxxx> -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
