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@xxxxxxxxxx> --- src/conf/domain_conf.c | 40 +++++++++++++++++++++++++++++----------- 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, 47 insertions(+), 25 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6259d4a..0999e86 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5672,7 +5672,8 @@ virDomainDiskFindByBusAndDst(virDomainDefPtr def, int virDomainDiskDefAssignAddress(virDomainXMLOptionPtr xmlopt, - virDomainDiskDefPtr def) + virDomainDiskDefPtr def, + const virDomainDef *vmdef) { int idx = virDiskNameToIndex(def->dst); if (idx < 0) { @@ -5683,7 +5684,10 @@ virDomainDiskDefAssignAddress(virDomainXMLOptionPtr xmlopt, } switch (def->bus) { - case VIR_DOMAIN_DISK_BUS_SCSI: + case VIR_DOMAIN_DISK_BUS_SCSI: { + unsigned int controller; + unsigned int unit; + def->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE; if (xmlopt->config.hasWideSCSIBus) { @@ -5692,22 +5696,36 @@ virDomainDiskDefAssignAddress(virDomainXMLOptionPtr xmlopt, * Unit 7 is the SCSI controller itself. Therefore unit 7 * cannot be assigned to disks and is skipped. */ - def->info.addr.drive.controller = idx / 15; - def->info.addr.drive.bus = 0; - def->info.addr.drive.unit = idx % 15; + controller = idx / 15; + unit = idx % 15; /* Skip the SCSI controller at unit 7 */ - if (def->info.addr.drive.unit >= 7) - ++def->info.addr.drive.unit; + if (unit >= 7) + ++unit; } else { /* For a narrow SCSI bus we define the default mapping to be * 7 units per bus, 1 bus per controller, many controllers */ - def->info.addr.drive.controller = idx / 7; - def->info.addr.drive.bus = 0; - def->info.addr.drive.unit = idx % 7; + controller = idx / 7; + unit = idx % 7; + } + + if (virDomainDriveAddressIsUsedByHostdev(vmdef, + VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI, + controller, 0, 0, unit)) { + virReportError(VIR_ERR_XML_ERROR, + _("address generated using disk target name '%s' " + "may conflict 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; + def->info.addr.drive.unit = unit; break; + } case VIR_DOMAIN_DISK_BUS_IDE: /* For IDE we define the default mapping to be 2 units @@ -7261,7 +7279,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, if (!(flags & VIR_DOMAIN_DEF_PARSE_DISK_SOURCE)) { if (def->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE - && virDomainDiskDefAssignAddress(xmlopt, def) < 0) + && virDomainDiskDefAssignAddress(xmlopt, def, vmdef) < 0) goto error; if (virDomainDiskBackingStoreParse(ctxt, def->src) < 0) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index c96a6e4..5b1c838 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2717,7 +2717,8 @@ int virDomainDiskInsert(virDomainDefPtr def, void virDomainDiskInsertPreAlloced(virDomainDefPtr def, virDomainDiskDefPtr disk); int virDomainDiskDefAssignAddress(virDomainXMLOptionPtr xmlopt, - virDomainDiskDefPtr def); + virDomainDiskDefPtr def, + const virDomainDef *vmdef); virDomainDiskDefPtr virDomainDiskRemove(virDomainDefPtr def, size_t i); diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 5444638..11e8e16 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -11784,7 +11784,7 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt, else def->dst[2] = 'a' + idx; - if (virDomainDiskDefAssignAddress(xmlopt, def) < 0) { + if (virDomainDiskDefAssignAddress(xmlopt, def, dom) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("invalid device name '%s'"), def->dst); virDomainDiskDefFree(def); @@ -12927,7 +12927,7 @@ qemuParseCommandLine(virCapsPtr qemuCaps, } } - if (virDomainDiskDefAssignAddress(xmlopt, disk) < 0) { + if (virDomainDiskDefAssignAddress(xmlopt, disk, def) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Cannot assign address for device name '%s'"), disk->dst); diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index aede2ad..d031398 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -960,7 +960,9 @@ virVMXFloppyDiskNameToUnit(const char *name, int *unit) static int -virVMXVerifyDiskAddress(virDomainXMLOptionPtr xmlopt, virDomainDiskDefPtr disk) +virVMXVerifyDiskAddress(virDomainXMLOptionPtr xmlopt, + virDomainDiskDefPtr disk, + virDomainDefPtr vmdef) { virDomainDiskDef def; virDomainDeviceDriveAddressPtr drive; @@ -979,7 +981,7 @@ virVMXVerifyDiskAddress(virDomainXMLOptionPtr xmlopt, virDomainDiskDefPtr disk) def.dst = disk->dst; def.bus = disk->bus; - if (virDomainDiskDefAssignAddress(xmlopt, &def) < 0) { + if (virDomainDiskDefAssignAddress(xmlopt, &def, vmdef) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not verify disk address")); return -1; @@ -1588,7 +1590,7 @@ virVMXParseConfig(virVMXContext *ctx, if (virVMXParseDisk(ctx, xmlopt, conf, VIR_DOMAIN_DISK_DEVICE_DISK, VIR_DOMAIN_DISK_BUS_SCSI, controller, unit, - &def->disks[def->ndisks]) < 0) { + &def->disks[def->ndisks], def) < 0) { goto cleanup; } @@ -1599,7 +1601,7 @@ virVMXParseConfig(virVMXContext *ctx, if (virVMXParseDisk(ctx, xmlopt, conf, VIR_DOMAIN_DISK_DEVICE_CDROM, VIR_DOMAIN_DISK_BUS_SCSI, controller, unit, - &def->disks[def->ndisks]) < 0) { + &def->disks[def->ndisks], def) < 0) { goto cleanup; } @@ -1613,7 +1615,7 @@ virVMXParseConfig(virVMXContext *ctx, for (unit = 0; unit < 2; ++unit) { if (virVMXParseDisk(ctx, xmlopt, conf, VIR_DOMAIN_DISK_DEVICE_DISK, VIR_DOMAIN_DISK_BUS_IDE, bus, unit, - &def->disks[def->ndisks]) < 0) { + &def->disks[def->ndisks], def) < 0) { goto cleanup; } @@ -1624,7 +1626,7 @@ virVMXParseConfig(virVMXContext *ctx, if (virVMXParseDisk(ctx, xmlopt, conf, VIR_DOMAIN_DISK_DEVICE_CDROM, VIR_DOMAIN_DISK_BUS_IDE, bus, unit, - &def->disks[def->ndisks]) < 0) { + &def->disks[def->ndisks], def) < 0) { goto cleanup; } @@ -1637,7 +1639,7 @@ virVMXParseConfig(virVMXContext *ctx, for (unit = 0; unit < 2; ++unit) { if (virVMXParseDisk(ctx, xmlopt, conf, VIR_DOMAIN_DISK_DEVICE_FLOPPY, VIR_DOMAIN_DISK_BUS_FDC, 0, unit, - &def->disks[def->ndisks]) < 0) { + &def->disks[def->ndisks], def) < 0) { goto cleanup; } @@ -1925,7 +1927,7 @@ virVMXParseSCSIController(virConfPtr conf, int controller, bool *present, int virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr conf, int device, int busType, int controllerOrBus, int unit, - virDomainDiskDefPtr *def) + virDomainDiskDefPtr *def, virDomainDefPtr vmdef) { /* * device = {VIR_DOMAIN_DISK_DEVICE_DISK, @@ -2280,7 +2282,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con goto cleanup; } - if (virDomainDiskDefAssignAddress(xmlopt, *def) < 0) { + if (virDomainDiskDefAssignAddress(xmlopt, *def, vmdef) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not assign address to disk '%s'"), virDomainDiskGetSource(*def)); @@ -3189,7 +3191,7 @@ virVMXFormatConfig(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virDomainDe /* def:disks */ for (i = 0; i < def->ndisks; ++i) { - if (virVMXVerifyDiskAddress(xmlopt, def->disks[i]) < 0 || + if (virVMXVerifyDiskAddress(xmlopt, def->disks[i], def) < 0 || virVMXHandleLegacySCSIDiskDriverName(def, def->disks[i]) < 0) { goto cleanup; } diff --git a/src/vmx/vmx.h b/src/vmx/vmx.h index 6a68c8b..e986124 100644 --- a/src/vmx/vmx.h +++ b/src/vmx/vmx.h @@ -89,7 +89,8 @@ int virVMXParseSCSIController(virConfPtr conf, int controller, bool *present, int virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr conf, int device, int busType, - int controllerOrBus, int unit, virDomainDiskDefPtr *def); + int controllerOrBus, int unit, virDomainDiskDefPtr *def, + virDomainDefPtr vmdef); int virVMXParseFileSystem(virConfPtr conf, int number, virDomainFSDefPtr *def); -- 2.1.0 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list