[libvirt] [PATCH v4 03/14] conf: Introduce virDomainHostdevDefPostParse

[Laine Stump]

On 03/22/2017 11:27 AM, Erik Skultety wrote:
> Just to make the code a bit cleaner, move hostdev specific post parse
> code to its own function just in case it grows in the future.
> 
> Signed-off-by: Erik Skultety <eskultet at redhat.com>
> ---
>  src/conf/domain_conf.c | 75 +++++++++++++++++++++++++++++++-------------------
>  1 file changed, 47 insertions(+), 28 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 568bf6722e..a5ab42297d 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -4219,6 +4219,50 @@ virDomainHostdevAssignAddress(virDomainXMLOptionPtr xmlopt,
>  
>  
>  static int
> +virDomainHostdevDefPostParse(virDomainHostdevDefPtr dev,
> +                             const virDomainDef *def,
> +                             virDomainXMLOptionPtr xmlopt)
> +{
> +    if (dev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS)
> +        return 0;
> +
> +    switch (dev->source.subsys.type) {

I had to actually go look at the definition of virHostdevIsSCSIDevice()
to see that the above lines do exactly duplicate the original
functionality. And I like it the way you've done it (putting in a
switch() on the type rather than hiding the check in a utility function)
better - it will make more sense when it's expanded.


ACK.

> +    case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI:
> +        if (dev->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
> +            virDomainHostdevAssignAddress(xmlopt, def, dev) < 0) {
> +            virReportError(VIR_ERR_XML_ERROR, "%s",
> +                           _("Cannot assign SCSI host device address"));
> +            return -1;
> +        } else {
> +            /* Ensure provided address doesn't conflict with existing
> +             * scsi disk drive address
> +             */
> +            virDomainDeviceDriveAddressPtr addr = &dev->info->addr.drive;
> +            if (virDomainDriveAddressIsUsedByDisk(def,
> +                                                  VIR_DOMAIN_DISK_BUS_SCSI,
> +                                                  addr)) {
> +                virReportError(VIR_ERR_XML_ERROR,
> +                               _("SCSI host address controller='%u' "
> +                                 "bus='%u' target='%u' unit='%u' in "
> +                                 "use by a SCSI disk"),
> +                               addr->controller, addr->bus,
> +                               addr->target, addr->unit);
> +                return -1;
> +            }
> +        }
> +        break;
> +    case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB:
> +    case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI:
> +    case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST:
> +    case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
> +        break;
> +    }
> +
> +    return 0;
> +}
> +
> +
> +static int
>  virDomainDeviceDefPostParseInternal(virDomainDeviceDefPtr dev,
>                                      const virDomainDef *def,
>                                      virCapsPtr caps ATTRIBUTE_UNUSED,
> @@ -4299,34 +4343,9 @@ virDomainDeviceDefPostParseInternal(virDomainDeviceDefPtr dev,
>          video->vram = VIR_ROUND_UP_POWER_OF_TWO(video->vram);
>      }
>  
> -    if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) {
> -        virDomainHostdevDefPtr hdev = dev->data.hostdev;
> -
> -        if (virHostdevIsSCSIDevice(hdev)) {
> -            if (hdev->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
> -                virDomainHostdevAssignAddress(xmlopt, def, hdev) < 0) {
> -                virReportError(VIR_ERR_XML_ERROR, "%s",
> -                               _("Cannot assign SCSI host device address"));
> -                return -1;
> -            } else {
> -                /* Ensure provided address doesn't conflict with existing
> -                 * scsi disk drive address
> -                 */
> -                virDomainDeviceDriveAddressPtr addr = &hdev->info->addr.drive;
> -                if (virDomainDriveAddressIsUsedByDisk(def,
> -                                                      VIR_DOMAIN_DISK_BUS_SCSI,
> -                                                      addr)) {
> -                    virReportError(VIR_ERR_XML_ERROR,
> -                                   _("SCSI host address controller='%u' "
> -                                     "bus='%u' target='%u' unit='%u' in "
> -                                     "use by a SCSI disk"),
> -                                   addr->controller, addr->bus,
> -                                   addr->target, addr->unit);
> -                    return -1;
> -                }
> -            }
> -        }
> -    }
> +    if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV &&
> +        virDomainHostdevDefPostParse(dev->data.hostdev, def, xmlopt) < 0)
> +        return -1;
>  
>      if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER) {
>          virDomainControllerDefPtr cdev = dev->data.controller;
>