5:47 a.m.
Reject duplicate drive addresses at domain definition. Hot-plug for
disks and hostdevs is still to fix and this is why the old version of
drive address checking is still there. Additionally, it isn't that
easy to be sure that the changes won't break other drivers as these
are common code changes.
Marc Hartmayer (5):
conf: simplify functions virDomainSCSIDriveAddressIsUsedBy*()
conf: virDomainDriveAddressIsUsedByDisk: Rename type to bus_type
tests: don't use duplicate disk addresses
conf: add global check for duplicate drive addresses
tests: add test cases for address conflicts
src/conf/domain_conf.c | 184 +++++++++++++++++----
.../qemuxml2argv-disk-drive-address-conflict.xml | 27 +++
...xml2argv-disk-hostdev-scsi-address-conflict.xml | 30 ++++
...emuxml2argv-hostdevs-drive-address-conflict.xml | 33 ++++
.../qemuxml2argv-seclabel-dynamic-override.args | 4 +-
.../qemuxml2argv-seclabel-dynamic-override.xml | 2 +-
tests/qemuxml2argvtest.c | 8 +
.../qemuxml2xmlout-seclabel-dynamic-override.xml | 2 +-
8 files changed, 251 insertions(+), 39 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-address-conflict.xml
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-disk-hostdev-scsi-address-conflict.xml
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-hostdevs-drive-address-conflict.xml
--
2.5.5
5:47 a.m.
New subject: [libvirt] [PATCH 1/5] conf: simplify functions virDomainSCSIDriveAddressIsUsedBy*()
Pass the virDomainDeviceDriveAddress as a struct instead of individual
arguments. Reworked the function descriptions.
Signed-off-by: Marc Hartmayer <mhartmay(a)linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.vnet.ibm.com>
---
src/conf/domain_conf.c | 76 ++++++++++++++++++++++++++++----------------------
1 file changed, 43 insertions(+), 33 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b001efc..6836d4e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4016,16 +4016,19 @@ virDomainDefPostParseGraphics(virDomainDef *def)
}
-/* Check if a drive type address $controller:$bus:$target:$unit is already
- * taken by a disk or not.
+/**
+ * virDomainDriveAddressIsUsedByDisk:
+ * @def: domain definition containing the disks to check
+ * @type: bus type
+ * @addr: address to check for duplicates
+ *
+ * Return true if any disk is already using the given address on the
+ * given bus, false otherwise.
*/
static bool
virDomainDriveAddressIsUsedByDisk(const virDomainDef *def,
virDomainDiskBus type,
- unsigned int controller,
- unsigned int bus,
- unsigned int target,
- unsigned int unit)
+ const virDomainDeviceDriveAddress *addr)
{
virDomainDiskDefPtr disk;
size_t i;
@@ -4037,10 +4040,10 @@ virDomainDriveAddressIsUsedByDisk(const virDomainDef *def,
disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE)
continue;
- if (disk->info.addr.drive.controller == controller &&
- disk->info.addr.drive.unit == unit &&
- disk->info.addr.drive.bus == bus &&
- disk->info.addr.drive.target == target)
+ if (disk->info.addr.drive.controller == addr->controller &&
+ disk->info.addr.drive.unit == addr->unit &&
+ disk->info.addr.drive.bus == addr->bus &&
+ disk->info.addr.drive.target == addr->target)
return true;
}
@@ -4048,16 +4051,19 @@ virDomainDriveAddressIsUsedByDisk(const virDomainDef *def,
}
-/* Check if a drive type address $controller:$target:$bus:$unit is already
- * taken by a host device or not.
+/**
+ * virDomainDriveAddressIsUsedByHostdev:
+ * @def: domain definition containing the hostdevs to check
+ * @type: bus type
+ * @addr: address to check for duplicates
+ *
+ * Return true if any hostdev is already using the given address on the
+ * given bus, false otherwise.
*/
static bool
virDomainDriveAddressIsUsedByHostdev(const virDomainDef *def,
virDomainHostdevSubsysType type,
- unsigned int controller,
- unsigned int bus,
- unsigned int target,
- unsigned int unit)
+ const virDomainDeviceDriveAddress *addr)
{
virDomainHostdevDefPtr hostdev;
size_t i;
@@ -4069,10 +4075,10 @@ virDomainDriveAddressIsUsedByHostdev(const virDomainDef *def,
hostdev->info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE)
continue;
- if (hostdev->info->addr.drive.controller == controller &&
- hostdev->info->addr.drive.unit == unit &&
- hostdev->info->addr.drive.bus == bus &&
- hostdev->info->addr.drive.target == target)
+ if (hostdev->info->addr.drive.controller == addr->controller &&
+ hostdev->info->addr.drive.unit == addr->unit &&
+ hostdev->info->addr.drive.bus == addr->bus &&
+ hostdev->info->addr.drive.target == addr->target)
return true;
}
@@ -4080,24 +4086,29 @@ virDomainDriveAddressIsUsedByHostdev(const virDomainDef *def,
}
+/**
+ * virDomainSCSIDriveAddressIsUsed:
+ * @def: domain definition to check against
+ * @addr: address to check for duplicates
+ *
+ * Return true if the SCSI drive address is already in use, false
+ * otherwise.
+ */
static bool
virDomainSCSIDriveAddressIsUsed(const virDomainDef *def,
- unsigned int controller,
- unsigned int bus,
- unsigned int target,
- unsigned int unit)
+ const virDomainDeviceDriveAddress *addr)
{
/* In current implementation, the maximum unit number of a controller
* is either 16 or 7 (narrow SCSI bus), and if the maximum unit number
* is 16, the controller itself is on unit 7 */
- if (unit == 7)
+ if (addr->unit == 7)
return true;
if (virDomainDriveAddressIsUsedByDisk(def, VIR_DOMAIN_DISK_BUS_SCSI,
- controller, bus, target, unit) ||
+ addr) ||
virDomainDriveAddressIsUsedByHostdev(def,
VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI,
- controller, bus, target, unit))
+ addr))
return true;
return false;
@@ -4114,7 +4125,8 @@ virDomainControllerSCSINextUnit(const virDomainDef *def,
for (i = 0; i < max_unit; i++) {
/* Default to assigning addresses using bus = target = 0 */
- if (!virDomainSCSIDriveAddressIsUsed(def, controller, 0, 0, i))
+ const virDomainDeviceDriveAddress addr = {controller, 0, 0, i};
+ if (!virDomainSCSIDriveAddressIsUsed(def, &addr))
return i;
}
@@ -4270,10 +4282,7 @@ virDomainDeviceDefPostParseInternal(virDomainDeviceDefPtr dev,
virDomainDeviceDriveAddressPtr addr = &hdev->info->addr.drive;
if (virDomainDriveAddressIsUsedByDisk(def,
VIR_DOMAIN_DISK_BUS_SCSI,
- addr->controller,
- addr->bus,
- addr->target,
- addr->unit)) {
+ addr)) {
virReportError(VIR_ERR_XML_ERROR,
_("SCSI host address controller='%u'
"
"bus='%u' target='%u'
unit='%u' in "
@@ -6559,9 +6568,10 @@ virDomainDiskDefAssignAddress(virDomainXMLOptionPtr xmlopt,
unit = idx % 7;
}
+ const virDomainDeviceDriveAddress addr = {controller, 0, 0, unit};
if (virDomainDriveAddressIsUsedByHostdev(vmdef,
VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI,
- controller, 0, 0, unit)) {
+ &addr)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("using disk target name '%s' conflicts with
"
"SCSI host device address controller='%u'
"
--
2.5.5
5:47 a.m.
New subject: [libvirt] [PATCH 2/5] conf: virDomainDriveAddressIsUsedByDisk: Rename type to bus_type
Comparing the parameter 'type' against the member 'bus' instead of
against the member 'type' is quite confusing. Rename the parameter
'type' to 'bus_type' to clarify its meaning.
Signed-off-by: Marc Hartmayer <mhartmay(a)linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.vnet.ibm.com>
---
src/conf/domain_conf.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6836d4e..cd30728 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4019,7 +4019,7 @@ virDomainDefPostParseGraphics(virDomainDef *def)
/**
* virDomainDriveAddressIsUsedByDisk:
* @def: domain definition containing the disks to check
- * @type: bus type
+ * @bus_type: bus type
* @addr: address to check for duplicates
*
* Return true if any disk is already using the given address on the
@@ -4027,7 +4027,7 @@ virDomainDefPostParseGraphics(virDomainDef *def)
*/
static bool
virDomainDriveAddressIsUsedByDisk(const virDomainDef *def,
- virDomainDiskBus type,
+ virDomainDiskBus bus_type,
const virDomainDeviceDriveAddress *addr)
{
virDomainDiskDefPtr disk;
@@ -4036,7 +4036,7 @@ virDomainDriveAddressIsUsedByDisk(const virDomainDef *def,
for (i = 0; i < def->ndisks; i++) {
disk = def->disks[i];
- if (disk->bus != type ||
+ if (disk->bus != bus_type ||
disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE)
continue;
--
2.5.5
5:47 a.m.
New subject: [libvirt] [PATCH 3/5] tests: don't use duplicate disk addresses
Don't use duplicate disk addresses in test cases unless it's useful. At
least the test case will break once we have a check for uniqueness of
addresses at time of domain definition.
Signed-off-by: Marc Hartmayer <mhartmay(a)linux.vnet.ibm.com>
---
tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-override.args | 4 ++--
tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-override.xml | 2 +-
tests/qemuxml2xmloutdata/qemuxml2xmlout-seclabel-dynamic-override.xml | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-override.args
b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-override.args
index 6cf8cd8..02fa000 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-override.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-override.args
@@ -19,6 +19,6 @@ QEMU_AUDIO_DRV=none \
-usb \
-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \
-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
--drive file=/dev/HostVG/QEMUGuest2,format=raw,if=none,id=drive-ide0-0-0 \
--device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
+-drive file=/dev/HostVG/QEMUGuest2,format=raw,if=none,id=drive-ide0-0-1 \
+-device ide-drive,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1 \
-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-override.xml
b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-override.xml
index 448ff3a..71d3a5f 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-override.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-override.xml
@@ -28,7 +28,7 @@
</seclabel>
</source>
<target dev='hdb' bus='ide'/>
- <address type='drive' controller='0' bus='0'
target='0' unit='0'/>
+ <address type='drive' controller='0' bus='0'
target='0' unit='1'/>
</disk>
<controller type='usb' index='0'/>
<controller type='ide' index='0'/>
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-seclabel-dynamic-override.xml
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-seclabel-dynamic-override.xml
index f84b17c..2b753a5 100644
--- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-seclabel-dynamic-override.xml
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-seclabel-dynamic-override.xml
@@ -28,7 +28,7 @@
</seclabel>
</source>
<target dev='hdb' bus='ide'/>
- <address type='drive' controller='0' bus='0'
target='0' unit='0'/>
+ <address type='drive' controller='0' bus='0'
target='0' unit='1'/>
</disk>
<controller type='usb' index='0'>
<address type='pci' domain='0x0000' bus='0x00'
slot='0x01' function='0x2'/>
--
2.5.5
5:47 a.m.
New subject: [libvirt] [PATCH 4/5] conf: add global check for duplicate drive addresses
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(a)linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.vnet.ibm.com>
---
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;
+
+ 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.
+ */
+ }
+
+ for (i = 0; i < def->nhostdevs; i++) {
+ virDomainHostdevDefPtr hdev_i = def->hostdevs[i];
+ virDomainDeviceInfoPtr hdev_info_i = hdev_i->info;
+ virDomainDeviceDriveAddressPtr hdev_addr_i;
+ if (!virHostdevIsSCSIDevice(hdev_i))
+ continue;
+
+ if (hdev_i->info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE)
+ continue;
+
+ hdev_addr_i = &hdev_info_i->addr.drive;
+ for (j = i + 1; j < def->nhostdevs; j++) {
+ virDomainHostdevDefPtr hdev_j = def->hostdevs[j];
+ virDomainDeviceInfoPtr hdev_info_j = hdev_j->info;
+ if (!virHostdevIsSCSIDevice(hdev_j))
+ continue;
+
+ /* Address type check for hdev_j will be done implicitly
+ * in virDomainDeviceInfoAddressIsEqual() */
+
+ if (virDomainDeviceInfoAddressIsEqual(hdev_info_i, hdev_info_j)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("SCSI host address controller='%u' "
+ "bus='%u' target='%u'
unit='%u' in "
+ "use by another SCSI host device"),
+ hdev_addr_i->bus,
+ hdev_addr_i->controller,
+ hdev_addr_i->target,
+ hdev_addr_i->unit);
+ return -1;
+ }
+ }
+
+ if (virDomainDriveAddressIsUsedByDisk(def, VIR_DOMAIN_DISK_BUS_SCSI,
+ hdev_addr_i)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("SCSI host address controller='%u' "
+ "bus='%u' target='%u' unit='%u'
in "
+ "use by another SCSI disk"),
+ hdev_addr_i->bus,
+ hdev_addr_i->controller,
+ hdev_addr_i->target,
+ hdev_addr_i->unit);
+ return -1;
+ }
+ }
+
+ return 0;
+}
+
static int
virDomainDefValidateInternal(const virDomainDef *def)
@@ -4848,6 +4949,9 @@ virDomainDefValidateInternal(const virDomainDef *def)
if (virDomainDefCheckDuplicateDiskInfo(def) < 0)
return -1;
+ if (virDomainDefCheckDuplicateDriveAddresses(def) < 0)
+ return -1;
+
if (virDomainDefGetVcpusTopology(def, NULL) < 0)
return -1;
--
2.5.5
10:28 a.m.
New subject: [libvirt] [PATCH 4/5] conf: add global check for duplicate drive addresses
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(a)linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.vnet.ibm.com>
---
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
5:47 a.m.
New subject: [libvirt] [PATCH 5/5] tests: add test cases for address conflicts
Add test cases for address conflicts between disks and hostdevs that are
using drive addresses.
Signed-off-by: Marc Hartmayer <mhartmay(a)linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.vnet.ibm.com>
---
.../qemuxml2argv-disk-drive-address-conflict.xml | 27 ++++++++++++++++++
...xml2argv-disk-hostdev-scsi-address-conflict.xml | 30 ++++++++++++++++++++
...emuxml2argv-hostdevs-drive-address-conflict.xml | 33 ++++++++++++++++++++++
tests/qemuxml2argvtest.c | 8 ++++++
4 files changed, 98 insertions(+)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-address-conflict.xml
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-disk-hostdev-scsi-address-conflict.xml
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-hostdevs-drive-address-conflict.xml
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-address-conflict.xml
b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-address-conflict.xml
new file mode 100644
index 0000000..83426ab
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-address-conflict.xml
@@ -0,0 +1,27 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219136</memory>
+ <currentMemory unit='KiB'>219136</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu</emulator>
+ <disk type='block' device='disk'>
+ <source dev='/dev/HostVG/QEMUGuest1'/>
+ <target dev='sda' bus='sata'/>
+ </disk>
+ <disk type='block' device='disk'>
+ <source dev='/dev/HostVG/QEMUGuest2'/>
+ <target dev='sdb' bus='sata'/>
+ <address type='drive' controller='0' bus='0'
target='0' unit='0'/>
+ </disk>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-hostdev-scsi-address-conflict.xml
b/tests/qemuxml2argvdata/qemuxml2argv-disk-hostdev-scsi-address-conflict.xml
new file mode 100644
index 0000000..b38ad95
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-hostdev-scsi-address-conflict.xml
@@ -0,0 +1,30 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219136</memory>
+ <currentMemory unit='KiB'>219136</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu</emulator>
+ <disk type='block' device='disk'>
+ <source dev='/dev/HostVG/QEMUGuest1'/>
+ <target dev='sda' bus='scsi'/>
+ </disk>
+ <hostdev mode='subsystem' type='scsi' managed='no'>
+ <source>
+ <adapter name='scsi_host2'/>
+ <address bus='0' target='0' unit='1'/>
+ </source>
+ <address type='drive' controller='0' bus='0'
target='0' unit='0'/>
+ </hostdev>
+ <controller type='scsi' index='0'
model='virtio-scsi'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdevs-drive-address-conflict.xml
b/tests/qemuxml2argvdata/qemuxml2argv-hostdevs-drive-address-conflict.xml
new file mode 100644
index 0000000..00ac498
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdevs-drive-address-conflict.xml
@@ -0,0 +1,33 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219136</memory>
+ <currentMemory unit='KiB'>219136</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu</emulator>
+ <hostdev mode='subsystem' type='scsi' managed='no'>
+ <source>
+ <adapter name='scsi_host2'/>
+ <address bus='0' target='0' unit='1'/>
+ </source>
+ <address type='drive' controller='0' bus='0'
target='0' unit='0'/>
+ </hostdev>
+ <hostdev mode='subsystem' type='scsi' managed='no'>
+ <source>
+ <adapter name='scsi_host2'/>
+ <address bus='0' target='0' unit='2'/>
+ </source>
+ <address type='drive' controller='0' bus='0'
target='0' unit='0'/>
+ </hostdev>
+ <controller type='scsi' index='0'
model='virtio-scsi'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index d12077c..cd2c4a1 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -975,6 +975,14 @@ mymain(void)
DO_TEST_PARSE_ERROR("disk-same-targets",
QEMU_CAPS_SCSI_LSI,
QEMU_CAPS_DEVICE_USB_STORAGE, QEMU_CAPS_NODEFCONFIG);
+ DO_TEST_PARSE_ERROR("disk-drive-address-conflict",
+ QEMU_CAPS_ICH9_AHCI);
+ DO_TEST_PARSE_ERROR("disk-hostdev-scsi-address-conflict",
+ QEMU_CAPS_VIRTIO_SCSI,
+ QEMU_CAPS_DEVICE_SCSI_GENERIC);
+ DO_TEST_PARSE_ERROR("hostdevs-drive-address-conflict",
+ QEMU_CAPS_VIRTIO_SCSI,
+ QEMU_CAPS_DEVICE_SCSI_GENERIC);
DO_TEST("event_idx",
QEMU_CAPS_VIRTIO_BLK_EVENT_IDX,
QEMU_CAPS_VIRTIO_NET_EVENT_IDX,
--
2.5.5
10:28 a.m.
On 30.11.2016 12:47, Marc Hartmayer wrote:
Reject duplicate drive addresses at domain definition. Hot-plug for
disks and hostdevs is still to fix and this is why the old version of
drive address checking is still there. Additionally, it isn't that
easy to be sure that the changes won't break other drivers as these
are common code changes.
Marc Hartmayer (5):
conf: simplify functions virDomainSCSIDriveAddressIsUsedBy*()
conf: virDomainDriveAddressIsUsedByDisk: Rename type to bus_type
tests: don't use duplicate disk addresses
conf: add global check for duplicate drive addresses
tests: add test cases for address conflicts
src/conf/domain_conf.c | 184 +++++++++++++++++----
.../qemuxml2argv-disk-drive-address-conflict.xml | 27 +++
...xml2argv-disk-hostdev-scsi-address-conflict.xml | 30 ++++
...emuxml2argv-hostdevs-drive-address-conflict.xml | 33 ++++
.../qemuxml2argv-seclabel-dynamic-override.args | 4 +-
.../qemuxml2argv-seclabel-dynamic-override.xml | 2 +-
tests/qemuxml2argvtest.c | 8 +
.../qemuxml2xmlout-seclabel-dynamic-override.xml | 2 +-
8 files changed, 251 insertions(+), 39 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-address-conflict.xml
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-disk-hostdev-scsi-address-conflict.xml
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-hostdevs-drive-address-conflict.xml
ACK series. We are currently in freeze so I will push this after the
release. Technically this could go in as it is a bug fix, but rather a
big one so I'd rather push it after the release, if you don't mind.
Michal
4:12 a.m.
On 02.12.2016 17:28, Michal Privoznik wrote:
On 30.11.2016 12:47, Marc Hartmayer wrote:
> Reject duplicate drive addresses at domain definition. Hot-plug for
> disks and hostdevs is still to fix and this is why the old version of
> drive address checking is still there. Additionally, it isn't that
> easy to be sure that the changes won't break other drivers as these
> are common code changes.
>
> Marc Hartmayer (5):
> conf: simplify functions virDomainSCSIDriveAddressIsUsedBy*()
> conf: virDomainDriveAddressIsUsedByDisk: Rename type to bus_type
> tests: don't use duplicate disk addresses
> conf: add global check for duplicate drive addresses
> tests: add test cases for address conflicts
>
> src/conf/domain_conf.c | 184 +++++++++++++++++----
> .../qemuxml2argv-disk-drive-address-conflict.xml | 27 +++
> ...xml2argv-disk-hostdev-scsi-address-conflict.xml | 30 ++++
> ...emuxml2argv-hostdevs-drive-address-conflict.xml | 33 ++++
> .../qemuxml2argv-seclabel-dynamic-override.args | 4 +-
> .../qemuxml2argv-seclabel-dynamic-override.xml | 2 +-
> tests/qemuxml2argvtest.c | 8 +
> .../qemuxml2xmlout-seclabel-dynamic-override.xml | 2 +-
> 8 files changed, 251 insertions(+), 39 deletions(-)
> create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-address-conflict.xml
> create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-disk-hostdev-scsi-address-conflict.xml
> create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-hostdevs-drive-address-conflict.xml
>
ACK series. We are currently in freeze so I will push this after the
release. Technically this could go in as it is a bug fix, but rather a
big one so I'd rather push it after the release, if you don't mind.
I've just pushed these. Thank you for the contribution.
Michal
5:37 a.m.
On Mon, Dec 05, 2016 at 11:12 AM +0100, Michal Privoznik <mprivozn(a)redhat.com>
wrote:
> ACK series. We are currently in freeze so I will push this after
the
> release. Technically this could go in as it is a bug fix, but rather a
> big one so I'd rather push it after the release, if you don't mind.
I've just pushed these. Thank you for the contribution.
Michal
Thanks.
Marc
--
Beste Grüße / Kind regards
Marc Hartmayer
IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
2939
days inactive
2944
days old
9 comments
2 participants
participants (2)
-
Marc Hartmayer -
Michal Privoznik