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(a)redhat.com>
---
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