[libvirt] [PATCH 1/2] conf: Detect misconfiguration between disk bus and disk address

[Peter Krempa]

On Tue, Nov 15, 2016 at 19:40:58 +0100, Marc Hartmayer wrote:
> This patch detects a misconfiguration between the disk bus type and disk
> address type for controller based disk buses (SATA, SCSI, FDC and
> IDE). The addresses of these bus types are all managed in common code so
> it's possible to decide in common code whether the disk address and bus
> type are compatible or not.
> 
> Signed-off-by: Marc Hartmayer <mhartmay at linux.vnet.ibm.com>
> Reviewed-by: Bjoern Walk <bwalk at linux.vnet.ibm.com>
> ---
>  src/conf/domain_conf.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 46 insertions(+)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 6e008e2..0cdcc9f 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -4664,6 +4664,38 @@ virDomainDefPostParse(virDomainDefPtr def,
>  }
>  
>  
> +/**
> + * virDomainDiskAddressDiskBusCompatibility:
> + * @bus: disk bus type
> + * @addressType: disk address type
> + *
> + * Check if the specified disk address type @addressType is compatible
> + * with the specified disk bus type @bus. This function checks
> + * compatibility with the bus types SATA, SCSI, FDC, and IDE only,
> + * because only these are handled in common code.
> + *
> + * Returns true if compatible or can't be decided in common code,
> + *         false if known to be not compatible.
> + */
> +static bool
> +virDomainDiskAddressDiskBusCompatibility(virDomainDiskBus bus,
> +                                         virDomainDeviceAddressType addressType)
> +{
> +    if (addressType == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
> +        return true;
> +
> +    switch (bus) {
> +    case VIR_DOMAIN_DISK_BUS_SATA:
> +    case VIR_DOMAIN_DISK_BUS_SCSI:
> +    case VIR_DOMAIN_DISK_BUS_FDC:
> +    case VIR_DOMAIN_DISK_BUS_IDE:
> +        return addressType == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE;
> +    default:

We tend to use a full enumeration of the types rather than the default
case along with a typecast of the switched variable to the correct type
so that the compiler checks if a new enum value is added.

> +        return true;
> +    }
> +}
> +
> +
>  static int
>  virDomainDiskDefValidate(const virDomainDiskDef *disk)
>  {
> @@ -4681,6 +4713,20 @@ virDomainDiskDefValidate(const virDomainDiskDef *disk)
>          }
>      }
>  
> +    /* Reject disks with a bus type that is not compatible with the
> +     * given address type. The function considers only buses that are
> +     * handled in common code. For other bus types it's not possible
> +     * to decide compatibility in common code.
> +     */

This comment is kind of redundant with the comment of the function.

> +    if (!virDomainDiskAddressDiskBusCompatibility(disk->bus, disk->info.type)) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                       _("Invalid address type '%s' for the disk '%s' with the bus type '%s'"),
> +                       virDomainDeviceAddressTypeToString(disk->info.type),
> +                       disk->dst,
> +                       virDomainDiskBusTypeToString(disk->bus));
> +        return -1;
> +    }
> +
>      return 0;
>  }

Peter
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20161116/5040af5e/attachment-0001.sig>