[libvirt] [PATCH v2 12/12] conf: Check for hostdev conflicts when assign default disk address

[Ján Tomko]

On Wed, Jul 22, 2015 at 10:54:34AM -0400, John Ferlan wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1210587  (completed)
> 
> When generating the default drive address for a SCSI <disk> device,
> check the generated address to ensure it doesn't conflict with a SCSI
> <hostdev> address. The <disk> address generation algorithm uses the
> <target> "dev" name in order to determine which controller and unit
> in order to place the device. Since a SCSI <hostdev> device doesn't
> require a target device name, its placement on the guest SCSI address
> "could" conflict.  For instance, if a SCSI <hostdev> exists at
> controller=0 unit=0 and an attempt to hotplug 'sda' into the guest
> made, there would be a conflict if the <hostdev> is already using
> /dev/sda.
> 
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
>  src/conf/domain_conf.c  | 16 ++++++++++++++--
>  src/conf/domain_conf.h  |  3 ++-
>  src/qemu/qemu_command.c |  4 ++--
>  src/vmx/vmx.c           | 22 ++++++++++++----------
>  src/vmx/vmx.h           |  3 ++-
>  5 files changed, 32 insertions(+), 16 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 39a4cf8..0dac60c 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -4079,7 +4079,7 @@ virDomainDeviceDefPostParseInternal(virDomainDeviceDefPtr dev,
>          }
>  
>          if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
> -            virDomainDiskDefAssignAddress(xmlopt, disk) < 0)
> +            virDomainDiskDefAssignAddress(xmlopt, disk, def) < 0)
>              return -1;
>      }
>  
> @@ -5860,7 +5860,8 @@ virDomainDiskDiffersSourceOnly(virDomainDiskDefPtr disk,
>  
>  int
>  virDomainDiskDefAssignAddress(virDomainXMLOptionPtr xmlopt,
> -                              virDomainDiskDefPtr def)
> +                              virDomainDiskDefPtr def,
> +                              const virDomainDef *vmdef)

This function does not really do any 'real' assignment, it just
translates the target name to a fixed address.
Unlike PCI address assignment, where we find unoccupied slots based on
the domain defition, the domain definition is not really needed here.

Moving the address check conflict to virDomainDefPostParse, after
the addresses have been filled out by virDomainDeviceDefPostParse,
would remove the need to change all the function prototypes and it would
be a good place to check for address conflicts between disks too - after
this series, two drives with the same addresses are allowed, as long as
they have different target names.

Jan

>  {
>      int idx = virDiskNameToIndex(def->dst);
>      if (idx < 0) {
> @@ -5896,6 +5897,17 @@ virDomainDiskDefAssignAddress(virDomainXMLOptionPtr xmlopt,
>              unit = idx % 7;
>          }
>  
> +        if (virDomainDriveAddressIsUsedByHostdev(vmdef,
> +                                                 VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI,
> +                                                 controller, 0, 0, unit)) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                           _("using disk target name '%s' conflicts with "
> +                             "SCSI host device address controller='%u' "
> +                             "bus='%u' target='%u' unit='%u"),
> +                           def->dst, controller, 0, 0, unit);
> +            return -1;
> +        }
> +
>          def->info.addr.drive.controller = controller;
>          def->info.addr.drive.bus = 0;
>          def->info.addr.drive.target = 0;
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150803/a7407cfd/attachment-0001.sig>