[libvirt] [PATCH v7 02/11] qemu: Validate PCI controller options (modelName)

[Laine Stump]

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 at redhat.com>
> ---
>  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 at laine.org>