Currently it is possible to start a domain which have disks
in same iotune group and at the same time having different iotune
params. Both params set are passed to qemu in command line and the one
that is passed later down command line is get actually set.
Let's prohibit such configurations.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@xxxxxxxxxxxxx>
---
src/conf/domain_conf.c | 29 ++++++++++++++++++++++++++++
src/conf/domain_conf.h | 4 ++++
src/libvirt_private.syms | 1 +
src/qemu/qemu_command.c | 41 +++++++++++++++++++++++++++++++++-------
src/qemu/qemu_command.h | 3 +++
src/qemu/qemu_driver.c | 2 +-
src/qemu/qemu_hotplug.c | 2 +-
tests/qemublocktest.c | 2 +-
8 files changed, 74 insertions(+), 10 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f75841c60f..45002782fc 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -31696,3 +31696,32 @@ virDomainBlockIoTuneInfoHasAny(const virDomainBlockIoTuneInfo *iotune)
virDomainBlockIoTuneInfoHasMax(iotune) ||
virDomainBlockIoTuneInfoHasMaxLength(iotune);
}
+
+
+bool
+virDomainBlockIoTuneInfoEqual(const virDomainBlockIoTuneInfo *a,
+ const virDomainBlockIoTuneInfo *b)
+{
+ if (a->total_bytes_sec == b->total_bytes_sec &&
+ a->read_bytes_sec == b->read_bytes_sec &&
+ a->write_bytes_sec == b->write_bytes_sec &&
+ a->total_iops_sec == b->total_iops_sec &&
+ a->read_iops_sec == b->read_iops_sec &&
+ a->write_iops_sec == b->write_iops_sec &&
+ a->total_bytes_sec_max == b->total_bytes_sec_max &&
+ a->read_bytes_sec_max == b->read_bytes_sec_max &&
+ a->write_bytes_sec_max == b->write_bytes_sec_max &&
+ a->total_iops_sec_max == b->total_iops_sec_max &&
+ a->read_iops_sec_max == b->read_iops_sec_max &&
+ a->write_iops_sec_max == b->write_iops_sec_max &&
+ a->size_iops_sec == b->size_iops_sec &&
+ a->total_bytes_sec_max_length == b->total_bytes_sec_max_length &&
+ a->read_bytes_sec_max_length == b->read_bytes_sec_max_length &&
+ a->write_bytes_sec_max_length == b->write_bytes_sec_max_length &&
+ a->total_iops_sec_max_length == b->total_iops_sec_max_length &&
+ a->read_iops_sec_max_length == b->read_iops_sec_max_length &&
+ a->write_iops_sec_max_length == b->write_iops_sec_max_length)
+ return true;
+
+ return false;
+}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 8b373184ca..33d7ab97bd 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -3696,3 +3696,7 @@ virDomainBlockIoTuneInfoHasMaxLength(const virDomainBlockIoTuneInfo *iotune);
bool
virDomainBlockIoTuneInfoHasAny(const virDomainBlockIoTuneInfo *iotune);
+
+bool
+virDomainBlockIoTuneInfoEqual(const virDomainBlockIoTuneInfo *a,
+ const virDomainBlockIoTuneInfo *b);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index a8dc2c30ea..fddf715220 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -227,6 +227,7 @@ virDomainActualNetDefFree;
virDomainActualNetDefValidate;
virDomainBlockedReasonTypeFromString;
virDomainBlockedReasonTypeToString;
+virDomainBlockIoTuneInfoEqual;
virDomainBlockIoTuneInfoHasAny;
virDomainBlockIoTuneInfoHasBasic;
virDomainBlockIoTuneInfoHasMax;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 29928ac1c8..94a49b7e4f 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1156,6 +1156,7 @@ qemuDiskConfigBlkdeviotuneEnabled(virDomainDiskDefPtr disk)
*/
static int
qemuCheckDiskConfigBlkdeviotune(virDomainDiskDefPtr disk,
+ const virDomainDef *def,
virQEMUCapsPtr qemuCaps)
{
/* group_name by itself is ignored by qemu */
@@ -1167,6 +1168,27 @@ qemuCheckDiskConfigBlkdeviotune(virDomainDiskDefPtr disk,
return -1;
}
+ /* checking def here is only for calling from tests */
+ if (disk->blkdeviotune.group_name && def) {
+ size_t i;
+
+ for (i = 0; i < def->ndisks; i++) {
+ virDomainDiskDefPtr d = def->disks[i];
+
+ if (!d->blkdeviotune.group_name ||
+ STRNEQ(disk->blkdeviotune.group_name, d->blkdeviotune.group_name))
+ continue;
+
+ if (!virDomainBlockIoTuneInfoEqual(&disk->blkdeviotune,
+ &d->blkdeviotune)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("different iotunes for disks %s and %s"),
+ disk->dst, d->dst);
+ return -1;
+ }
+ }
+ }
+
if (disk->blkdeviotune.total_bytes_sec > QEMU_BLOCK_IOTUNE_MAX ||
disk->blkdeviotune.read_bytes_sec > QEMU_BLOCK_IOTUNE_MAX ||
disk->blkdeviotune.write_bytes_sec > QEMU_BLOCK_IOTUNE_MAX ||
@@ -1229,9 +1251,10 @@ qemuCheckDiskConfigBlkdeviotune(virDomainDiskDefPtr disk,
*/
int
qemuCheckDiskConfig(virDomainDiskDefPtr disk,
+ const virDomainDef *def,
virQEMUCapsPtr qemuCaps)
{
- if (qemuCheckDiskConfigBlkdeviotune(disk, qemuCaps) < 0)
+ if (qemuCheckDiskConfigBlkdeviotune(disk, def, qemuCaps) < 0)
return -1;
if (disk->wwn) {
@@ -1729,6 +1752,7 @@ qemuBuildDiskFrontendAttributes(virDomainDiskDefPtr disk,
static char *
qemuBuildDriveStr(virDomainDiskDefPtr disk,
+ const virDomainDef *def,
virQEMUCapsPtr qemuCaps)
{
g_auto(virBuffer) opt = VIR_BUFFER_INITIALIZER;
@@ -1755,7 +1779,7 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk,
}
/* if we are using -device this will be checked elsewhere */
- if (qemuCheckDiskConfig(disk, qemuCaps) < 0)
+ if (qemuCheckDiskConfig(disk, def, qemuCaps) < 0)
goto error;
virBufferAsprintf(&opt, "if=%s",
@@ -1900,7 +1924,7 @@ qemuBuildDiskDeviceStr(const virDomainDef *def,
g_autofree char *scsiVPDDeviceId = NULL;
int controllerModel;
- if (qemuCheckDiskConfig(disk, qemuCaps) < 0)
+ if (qemuCheckDiskConfig(disk, def, qemuCaps) < 0)
goto error;
if (!qemuDomainCheckCCWS390AddressSupport(def, &disk->info, qemuCaps, disk->dst))
@@ -2408,6 +2432,7 @@ qemuBuildBlockStorageSourceAttachDataCommandline(virCommandPtr cmd,
static int
qemuBuildDiskSourceCommandLine(virCommandPtr cmd,
virDomainDiskDefPtr disk,
+ const virDomainDef *def,
virQEMUCapsPtr qemuCaps)
{
g_autoptr(qemuBlockStorageSourceChainData) data = NULL;
@@ -2427,7 +2452,7 @@ qemuBuildDiskSourceCommandLine(virCommandPtr cmd,
!(copyOnReadProps = qemuBlockStorageGetCopyOnReadProps(disk)))
return -1;
} else {
- if (!(data = qemuBuildStorageSourceChainAttachPrepareDrive(disk, qemuCaps)))
+ if (!(data = qemuBuildStorageSourceChainAttachPrepareDrive(disk, def, qemuCaps)))
return -1;
}
@@ -2457,7 +2482,7 @@ qemuBuildDiskCommandLine(virCommandPtr cmd,
{
g_autofree char *optstr = NULL;
- if (qemuBuildDiskSourceCommandLine(cmd, disk, qemuCaps) < 0)
+ if (qemuBuildDiskSourceCommandLine(cmd, disk, def, qemuCaps) < 0)
return -1;
if (!qemuDiskBusNeedsDriveArg(disk->bus)) {
@@ -10138,6 +10163,7 @@ qemuBuildHotpluggableCPUProps(const virDomainVcpuDef *vcpu)
*/
qemuBlockStorageSourceAttachDataPtr
qemuBuildStorageSourceAttachPrepareDrive(virDomainDiskDefPtr disk,
+ const virDomainDef *def,
virQEMUCapsPtr qemuCaps)
{
g_autoptr(qemuBlockStorageSourceAttachData) data = NULL;
@@ -10145,7 +10171,7 @@ qemuBuildStorageSourceAttachPrepareDrive(virDomainDiskDefPtr disk,
if (VIR_ALLOC(data) < 0)
return NULL;
- if (!(data->driveCmd = qemuBuildDriveStr(disk, qemuCaps)) ||
+ if (!(data->driveCmd = qemuBuildDriveStr(disk, def, qemuCaps)) ||
!(data->driveAlias = qemuAliasDiskDriveFromDisk(disk)))
return NULL;
@@ -10203,6 +10229,7 @@ qemuBuildStorageSourceAttachPrepareCommon(virStorageSourcePtr src,
*/
qemuBlockStorageSourceChainDataPtr
qemuBuildStorageSourceChainAttachPrepareDrive(virDomainDiskDefPtr disk,
+ const virDomainDef *def,
virQEMUCapsPtr qemuCaps)
{
g_autoptr(qemuBlockStorageSourceAttachData) elem = NULL;
@@ -10211,7 +10238,7 @@ qemuBuildStorageSourceChainAttachPrepareDrive(virDomainDiskDefPtr disk,
if (VIR_ALLOC(data) < 0)
return NULL;
- if (!(elem = qemuBuildStorageSourceAttachPrepareDrive(disk, qemuCaps)))
+ if (!(elem = qemuBuildStorageSourceAttachPrepareDrive(disk, def, qemuCaps)))
return NULL;
if (qemuBuildStorageSourceAttachPrepareCommon(disk->src, elem, qemuCaps) < 0)
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 786991fd3d..d7fdba9942 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -105,6 +105,7 @@ bool qemuDiskBusNeedsDriveArg(int bus);
qemuBlockStorageSourceAttachDataPtr
qemuBuildStorageSourceAttachPrepareDrive(virDomainDiskDefPtr disk,
+ const virDomainDef *def,
virQEMUCapsPtr qemuCaps);
int
qemuBuildStorageSourceAttachPrepareCommon(virStorageSourcePtr src,
@@ -114,6 +115,7 @@ qemuBuildStorageSourceAttachPrepareCommon(virStorageSourcePtr src,
qemuBlockStorageSourceChainDataPtr
qemuBuildStorageSourceChainAttachPrepareDrive(virDomainDiskDefPtr disk,
+ const virDomainDef *def,
virQEMUCapsPtr qemuCaps);
@@ -199,6 +201,7 @@ bool
qemuDiskConfigBlkdeviotuneEnabled(virDomainDiskDefPtr disk);
int qemuCheckDiskConfig(virDomainDiskDefPtr disk,
+ const virDomainDef *def,
virQEMUCapsPtr qemuCaps);
bool
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ec8faf384c..c41179edb1 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8126,7 +8126,7 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef,
}
if (virDomainDiskTranslateSourcePool(disk) < 0)
return -1;
- if (qemuCheckDiskConfig(disk, NULL) < 0)
+ if (qemuCheckDiskConfig(disk, vmdef, NULL) < 0)
return -1;
if (qemuCheckDiskConfigAgainstDomain(vmdef, disk) < 0)
return -1;
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index f2a9386433..6a95fd49ac 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -700,7 +700,7 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
priv->qemuCaps)))
goto cleanup;
} else {
- if (!(data = qemuBuildStorageSourceChainAttachPrepareDrive(disk,
+ if (!(data = qemuBuildStorageSourceChainAttachPrepareDrive(disk, vm->def,
priv->qemuCaps)))
goto cleanup;
}
diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index 3e9edb85f0..670e5bfab1 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -204,7 +204,7 @@ testQemuDiskXMLToProps(const void *opaque)
VIR_DOMAIN_DEF_PARSE_STATUS)))
goto cleanup;
- if (qemuCheckDiskConfig(disk, data->qemuCaps) < 0 ||
+ if (qemuCheckDiskConfig(disk, NULL, data->qemuCaps) < 0 ||
qemuDomainDeviceDefValidateDisk(disk, data->qemuCaps) < 0) {
VIR_TEST_VERBOSE("invalid configuration for disk");
goto cleanup;
--
2.23.0
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
