On 30.11.2016 12:47, Marc Hartmayer wrote:
> Add a global check for duplicate drive addresses. This will fix the
> problem of duplicate disk and hostdev drive addresses.
>
> Example for duplicate drive addresses:
> <disk>
> ...
> <target name='sda'/>
> </disk>
> <disk>
> ...
> <target name='sdb'/>
> <address type='drive' controller=0 bus=0 target=0 unit=0/>
> </disk>
>
> Another example:
> <hostdev mode='subsystem' type='scsi' managed='no'>
> <source>
> ...
> </source>
> <address type='drive' controller='0' bus='0' target='0' unit='0'/>
> </hostdev>
> <hostdev mode='subsystem' type='scsi' managed='no'>
> <source>
> ...
> </source>
> <address type='drive' controller='0' bus='0' target='0' unit='0'/>
> </hostdev>
>
> Unfortunately the fixes (1b08cc170a84077afd4d15f4639a9a2cf398e9a2,
> 8d46386bfe01b84982e25e915ad9cfbae5cf4cb1) weren't enough to catch these
> cases and it isn't possible to add additional checks in
> virDomainDeviceDefPostParseInternal() for SCSI hostdevs or
> virDomainDiskDefAssignAddress() for SCSI/IDE/FDC/SATA disks without
> adding another parse flag (virDomainDefParseFlags) to disable this
> validation while updating or detaching a disk or hostdev.
>
> Signed-off-by: Marc Hartmayer <mhartmay@xxxxxxxxxxxxxxxxxx>
> Reviewed-by: Boris Fiuczynski <fiuczy@xxxxxxxxxxxxxxxxxx>
> ---
> src/conf/domain_conf.c | 104 +++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 104 insertions(+)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index cd30728..cb47980 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -4841,6 +4841,107 @@ virDomainDefCheckDuplicateDiskInfo(const virDomainDef *def)
> return 0;
> }
>
> +/**
> + * virDomainDefCheckDuplicateDriveAddresses:
> + * @def: domain definition to check against
> + *
> + * This function checks @def for duplicate drive addresses. Drive
> + * addresses are only in use for disks and hostdevs at the moment.
> + *
> + * Returns 0 in case of there are no duplicate drive addresses, -1
> + * otherwise.
> + */
> +static int
> +virDomainDefCheckDuplicateDriveAddresses(const virDomainDef *def)
> +{
> + size_t i;
> + size_t j;
> +
> + for (i = 0; i < def->ndisks; i++) {
> + virDomainDiskDefPtr disk_i = def->disks[i];
> + virDomainDeviceInfoPtr disk_info_i = &disk_i->info;
> + if (disk_info_i->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE)
> + continue;
We like to have an empty line between variable declarations and the
actual code. Just like you did at the beginning of this function. But
that's something I can fix before pushing.
> +
> + for (j = i + 1; j < def->ndisks; j++) {
> + virDomainDiskDefPtr disk_j = def->disks[j];
> + virDomainDeviceInfoPtr disk_info_j = &disk_j->info;
> + if (disk_i->bus != disk_j->bus)
> + continue;
> +
> + if (disk_info_j->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE)
> + continue;
> +
> + if (virDomainDeviceInfoAddressIsEqual(disk_info_i, disk_info_j)) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("Found duplicate drive address for disk with "
> + "target name '%s' controller='%u' bus='%u' "
> + "target='%u' unit='%u'"),
> + disk_i->dst,
> + disk_info_i->addr.drive.controller,
> + disk_info_i->addr.drive.bus,
> + disk_info_i->addr.drive.target,
> + disk_info_i->addr.drive.unit);
> + return -1;
> + }
> + }
> +
> + /* Note: There is no need to check for conflicts with SCSI
> + * hostdevs above, because conflicts with hostdevs are checked
> + * in the next loop.
> + */
> + }
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
