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

[Daniel P. Berrangé]

On Fri, Feb 16, 2018 at 05:28:05PM +0100, Andrea Bolognani wrote:
> Signed-off-by: Andrea Bolognani <abologna at redhat.com>
> ---
>  src/qemu/qemu_domain.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 54 insertions(+), 2 deletions(-)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 9dc3d5597..e4e67c585 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -4243,9 +4243,61 @@ qemuDomainDeviceDefValidateControllerSCSI(const virDomainControllerDef *controll
>  
>  
>  static int
> -qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *controller ATTRIBUTE_UNUSED,
> -                                         const virDomainDef *def ATTRIBUTE_UNUSED)
> +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")

> +                       controller->model);
> +        return -1;
> +    }
> +    if (!modelName) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("Unknown virDomainControllerPCIModelName value: %d"),
> +                       pciopts->modelName);

Likewise here.

> +        return -1;
> +    }
> +
> +    /* index */
> +    switch ((virDomainControllerModelPCI) controller->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:
> +        if (controller->idx == 0) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                           _("Index for '%s' controller must be > 0"),
> +                           model);
> +            return -1;
> +        }
> +        break;
> +
> +    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

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|