[libvirt] [PATCH 2/9] domain_conf: Validate redirdev after parsing

[Peter Krempa]

On Thu, Jun 09, 2016 at 17:02:37 +0200, Michal Privoznik wrote:
> There's currently just one limitation: redirdevs that want to go
> on USB bus require a USB controller, surprisingly.
> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/conf/domain_conf.c | 33 +++++++++++++++++++++++++++------
>  1 file changed, 27 insertions(+), 6 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 85f6e31..8f5935c 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -4567,14 +4567,39 @@ virDomainDiskDefValidate(const virDomainDiskDef *disk)
>      return 0;
>  }
>  
> +static int
> +virDomainRedirdevDefValidate(const virDomainDef *def,
> +                             const virDomainRedirdevDef *redirdev)
> +{
> +    size_t i;
> +
> +    if (redirdev->bus != VIR_DOMAIN_REDIRDEV_BUS_USB)

This condition makes it harder to add checks for non-usb redirdevs
later.

> +        return 0;
> +
> +    for (i = 0; i < def->ncontrollers; i++) {
> +        virDomainControllerDefPtr controller = def->controllers[i];
> +
> +        if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_USB &&
> +            controller->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE) {

We already have a helper to do this called 'virDomainDefHasUSB'


> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                           _("Can't add redirected USB device: "
> +                             "USB is disabled for this domain"));
> +        }
> +    }
> +
> +    return 0;
> +}
> +
>  
>  static int
>  virDomainDeviceDefValidateInternal(const virDomainDeviceDef *dev,
> -                                   const virDomainDef *def ATTRIBUTE_UNUSED)
> +                                   const virDomainDef *def)
>  {
>      switch ((virDomainDeviceType) dev->type) {
>      case VIR_DOMAIN_DEVICE_DISK:
>          return virDomainDiskDefValidate(dev->data.disk);
> +    case VIR_DOMAIN_DEVICE_REDIRDEV:
> +        return virDomainRedirdevDefValidate(def, dev->data.redirdev);
>      case VIR_DOMAIN_DEVICE_LEASE:
>      case VIR_DOMAIN_DEVICE_FS:
>      case VIR_DOMAIN_DEVICE_NET:
> @@ -4586,7 +4611,6 @@ virDomainDeviceDefValidateInternal(const virDomainDeviceDef *dev,
>      case VIR_DOMAIN_DEVICE_CONTROLLER:
>      case VIR_DOMAIN_DEVICE_GRAPHICS:
>      case VIR_DOMAIN_DEVICE_HUB:
> -    case VIR_DOMAIN_DEVICE_REDIRDEV:
>      case VIR_DOMAIN_DEVICE_SMARTCARD:
>      case VIR_DOMAIN_DEVICE_CHR:
>      case VIR_DOMAIN_DEVICE_MEMBALLOON:
> @@ -17060,10 +17084,7 @@ virDomainDefParseXML(xmlDocPtr xml,
>          if (!redirdev)
>              goto error;
>  
> -        if (redirdev->bus == VIR_DOMAIN_REDIRDEV_BUS_USB && usb_none) {
> -             virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -                            _("Can't add redirected USB device: "
> -                              "USB is disabled for this domain"));
> +        if (virDomainRedirdevDefValidate(def, redirdev) < 0) {

Ideally this would be removed altogether. Such checks shouldn't be in
the parser.

>              virDomainRedirdevDefFree(redirdev);
>              goto error;
>          }