From nobody Sun Feb 8 22:17:47 2026 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of redhat.com designates 205.139.110.61 as permitted sender) client-ip=205.139.110.61; envelope-from=libvir-list-bounces@redhat.com; helo=us-smtp-delivery-1.mimecast.com; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 205.139.110.61 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass(p=none dis=none) header.from=redhat.com Return-Path: Received: from us-smtp-delivery-1.mimecast.com (us-smtp-1.mimecast.com [205.139.110.61]) by mx.zohomail.com with SMTPS id 1580284555767630.9362141905006; Tue, 28 Jan 2020 23:55:55 -0800 (PST) Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-152-99xNfDtpPZW0fVDZlztHTA-1; Wed, 29 Jan 2020 02:55:51 -0500 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id A8DF78010DB; Wed, 29 Jan 2020 07:55:46 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 8375C5C298; Wed, 29 Jan 2020 07:55:46 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 0CD3685780; Wed, 29 Jan 2020 07:55:46 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id 00T7sc84024242 for ; Wed, 29 Jan 2020 02:54:38 -0500 Received: by smtp.corp.redhat.com (Postfix) id 951BE8578F; Wed, 29 Jan 2020 07:54:38 +0000 (UTC) Received: from moe.redhat.com (unknown [10.43.2.30]) by smtp.corp.redhat.com (Postfix) with ESMTP id EA77384BB8; Wed, 29 Jan 2020 07:54:36 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1580284554; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:list-id:list-help: list-unsubscribe:list-subscribe:list-post; bh=ijPJqPMKqEiQPYBIRG2TymnOd5ywJM/pnwOzK22R378=; b=AKqolfjFbo5/RFQjfWD/DuOCCWLoduapSddZGRZtQqNaWZLPqwfB16M7eaNSGPJbgwbXA4 QRnRvn/gPYagR+2QCrPCXNi52k0EQnhWVBpW99ck2QoqdjlxOR3Cw8jpvBoEqj+4o17krJ saMmjuDZ+GPLDsn6mi6VVmVuEUcwO5Y= From: Michal Privoznik To: libvir-list@redhat.com Subject: [PATCH v2 4/7] qemu: check iotune params same for all disk in group Date: Wed, 29 Jan 2020 08:54:15 +0100 Message-Id: In-Reply-To: References: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-loop: libvir-list@redhat.com Cc: nshirokovskiy@virtuozzo.com X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-MC-Unique: 99xNfDtpPZW0fVDZlztHTA-1 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: quoted-printable X-ZohoMail-DKIM: pass (identity @redhat.com) Content-Type: text/plain; charset="utf-8" From: Nikolay Shirokovskiy 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 Signed-off-by: Michal Privoznik --- src/conf/domain_conf.c | 26 +++++++++++++++++++++++++ src/conf/domain_conf.h | 5 +++++ src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 42 +++++++++++++++++++++++++++++++++------- src/qemu/qemu_command.h | 3 +++ src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_hotplug.c | 2 +- tests/qemublocktest.c | 8 ++++++-- 8 files changed, 78 insertions(+), 11 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 0eeffb6ed0..89c9ed2834 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -31785,3 +31785,29 @@ virDomainBlockIoTuneInfoHasAny(const virDomainBloc= kIoTuneInfo *iotune) virDomainBlockIoTuneInfoHasMax(iotune) || virDomainBlockIoTuneInfoHasMaxLength(iotune); } + + +bool +virDomainBlockIoTuneInfoEqual(const virDomainBlockIoTuneInfo *a, + const virDomainBlockIoTuneInfo *b) +{ + return a->total_bytes_sec =3D=3D b->total_bytes_sec && + a->read_bytes_sec =3D=3D b->read_bytes_sec && + a->write_bytes_sec =3D=3D b->write_bytes_sec && + a->total_iops_sec =3D=3D b->total_iops_sec && + a->read_iops_sec =3D=3D b->read_iops_sec && + a->write_iops_sec =3D=3D b->write_iops_sec && + a->total_bytes_sec_max =3D=3D b->total_bytes_sec_max && + a->read_bytes_sec_max =3D=3D b->read_bytes_sec_max && + a->write_bytes_sec_max =3D=3D b->write_bytes_sec_max && + a->total_iops_sec_max =3D=3D b->total_iops_sec_max && + a->read_iops_sec_max =3D=3D b->read_iops_sec_max && + a->write_iops_sec_max =3D=3D b->write_iops_sec_max && + a->size_iops_sec =3D=3D b->size_iops_sec && + a->total_bytes_sec_max_length =3D=3D b->total_bytes_sec_max_length= && + a->read_bytes_sec_max_length =3D=3D b->read_bytes_sec_max_length && + a->write_bytes_sec_max_length =3D=3D b->write_bytes_sec_max_length= && + a->total_iops_sec_max_length =3D=3D b->total_iops_sec_max_length && + a->read_iops_sec_max_length =3D=3D b->read_iops_sec_max_length && + a->write_iops_sec_max_length =3D=3D b->write_iops_sec_max_length; +} diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index a037d9c2f2..5c3156a0aa 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -489,6 +489,7 @@ struct _virDomainBlockIoTuneInfo { unsigned long long total_iops_sec_max_length; unsigned long long read_iops_sec_max_length; unsigned long long write_iops_sec_max_length; + /* Don't forget to update virDomainBlockIoTuneInfoEqual. */ }; =20 =20 @@ -3710,3 +3711,7 @@ virDomainBlockIoTuneInfoHasMaxLength(const virDomainB= lockIoTuneInfo *iotune); =20 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 eea90ce0dc..5e9eb34459 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -226,6 +226,7 @@ virDomainActualNetDefFree; virDomainActualNetDefValidate; virDomainBlockedReasonTypeFromString; virDomainBlockedReasonTypeToString; +virDomainBlockIoTuneInfoEqual; virDomainBlockIoTuneInfoHasAny; virDomainBlockIoTuneInfoHasBasic; virDomainBlockIoTuneInfoHasMax; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index f4b7251aab..1668c85666 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1155,6 +1155,7 @@ qemuDiskConfigBlkdeviotuneEnabled(virDomainDiskDefPtr= disk) */ static int qemuCheckDiskConfigBlkdeviotune(virDomainDiskDefPtr disk, + const virDomainDef *def, virQEMUCapsPtr qemuCaps) { /* group_name by itself is ignored by qemu */ @@ -1166,6 +1167,28 @@ qemuCheckDiskConfigBlkdeviotune(virDomainDiskDefPtr = disk, return -1; } =20 + /* checking def here is only for calling from tests */ + if (disk->blkdeviotune.group_name) { + size_t i; + + for (i =3D 0; i < def->ndisks; i++) { + virDomainDiskDefPtr d =3D def->disks[i]; + + if (STREQ(d->dst, disk->dst) || + STRNEQ_NULLABLE(d->blkdeviotune.group_name, + disk->blkdeviotune.group_name)) + continue; + + if (!virDomainBlockIoTuneInfoEqual(&d->blkdeviotune, + &disk->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 || @@ -1228,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; =20 if (disk->wwn) { @@ -1728,6 +1752,7 @@ qemuBuildDiskFrontendAttributes(virDomainDiskDefPtr d= isk, =20 static char * qemuBuildDriveStr(virDomainDiskDefPtr disk, + const virDomainDef *def, virQEMUCapsPtr qemuCaps) { g_auto(virBuffer) opt =3D VIR_BUFFER_INITIALIZER; @@ -1754,7 +1779,7 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, } =20 /* if we are using -device this will be checked elsewhere */ - if (qemuCheckDiskConfig(disk, qemuCaps) < 0) + if (qemuCheckDiskConfig(disk, def, qemuCaps) < 0) return NULL; =20 virBufferAsprintf(&opt, "if=3D%s", @@ -1896,7 +1921,7 @@ qemuBuildDiskDeviceStr(const virDomainDef *def, g_autofree char *scsiVPDDeviceId =3D NULL; int controllerModel; =20 - if (qemuCheckDiskConfig(disk, qemuCaps) < 0) + if (qemuCheckDiskConfig(disk, def, qemuCaps) < 0) return NULL; =20 if (!qemuDomainCheckCCWS390AddressSupport(def, &disk->info, qemuCaps, = disk->dst)) @@ -2401,6 +2426,7 @@ qemuBuildBlockStorageSourceAttachDataCommandline(virC= ommandPtr cmd, static int qemuBuildDiskSourceCommandLine(virCommandPtr cmd, virDomainDiskDefPtr disk, + const virDomainDef *def, virQEMUCapsPtr qemuCaps) { g_autoptr(qemuBlockStorageSourceChainData) data =3D NULL; @@ -2420,7 +2446,7 @@ qemuBuildDiskSourceCommandLine(virCommandPtr cmd, !(copyOnReadProps =3D qemuBlockStorageGetCopyOnReadProps(disk)= )) return -1; } else { - if (!(data =3D qemuBuildStorageSourceChainAttachPrepareDrive(disk,= qemuCaps))) + if (!(data =3D qemuBuildStorageSourceChainAttachPrepareDrive(disk,= def, qemuCaps))) return -1; } =20 @@ -2450,7 +2476,7 @@ qemuBuildDiskCommandLine(virCommandPtr cmd, { g_autofree char *optstr =3D NULL; =20 - if (qemuBuildDiskSourceCommandLine(cmd, disk, qemuCaps) < 0) + if (qemuBuildDiskSourceCommandLine(cmd, disk, def, qemuCaps) < 0) return -1; =20 if (!qemuDiskBusNeedsDriveArg(disk->bus)) { @@ -10164,6 +10190,7 @@ qemuBuildHotpluggableCPUProps(const virDomainVcpuDe= f *vcpu) */ qemuBlockStorageSourceAttachDataPtr qemuBuildStorageSourceAttachPrepareDrive(virDomainDiskDefPtr disk, + const virDomainDef *def, virQEMUCapsPtr qemuCaps) { g_autoptr(qemuBlockStorageSourceAttachData) data =3D NULL; @@ -10171,7 +10198,7 @@ qemuBuildStorageSourceAttachPrepareDrive(virDomainD= iskDefPtr disk, if (VIR_ALLOC(data) < 0) return NULL; =20 - if (!(data->driveCmd =3D qemuBuildDriveStr(disk, qemuCaps)) || + if (!(data->driveCmd =3D qemuBuildDriveStr(disk, def, qemuCaps)) || !(data->driveAlias =3D qemuAliasDiskDriveFromDisk(disk))) return NULL; =20 @@ -10229,6 +10256,7 @@ qemuBuildStorageSourceAttachPrepareCommon(virStorag= eSourcePtr src, */ qemuBlockStorageSourceChainDataPtr qemuBuildStorageSourceChainAttachPrepareDrive(virDomainDiskDefPtr disk, + const virDomainDef *def, virQEMUCapsPtr qemuCaps) { g_autoptr(qemuBlockStorageSourceAttachData) elem =3D NULL; @@ -10237,7 +10265,7 @@ qemuBuildStorageSourceChainAttachPrepareDrive(virDo= mainDiskDefPtr disk, if (VIR_ALLOC(data) < 0) return NULL; =20 - if (!(elem =3D qemuBuildStorageSourceAttachPrepareDrive(disk, qemuCaps= ))) + if (!(elem =3D qemuBuildStorageSourceAttachPrepareDrive(disk, def, qem= uCaps))) return NULL; =20 if (qemuBuildStorageSourceAttachPrepareCommon(disk->src, elem, qemuCap= s) < 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); =20 qemuBlockStorageSourceAttachDataPtr qemuBuildStorageSourceAttachPrepareDrive(virDomainDiskDefPtr disk, + const virDomainDef *def, virQEMUCapsPtr qemuCaps); int qemuBuildStorageSourceAttachPrepareCommon(virStorageSourcePtr src, @@ -114,6 +115,7 @@ qemuBuildStorageSourceAttachPrepareCommon(virStorageSou= rcePtr src, =20 qemuBlockStorageSourceChainDataPtr qemuBuildStorageSourceChainAttachPrepareDrive(virDomainDiskDefPtr disk, + const virDomainDef *def, virQEMUCapsPtr qemuCaps); =20 =20 @@ -199,6 +201,7 @@ bool qemuDiskConfigBlkdeviotuneEnabled(virDomainDiskDefPtr disk); =20 int qemuCheckDiskConfig(virDomainDiskDefPtr disk, + const virDomainDef *def, virQEMUCapsPtr qemuCaps); =20 bool diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index dfc7111937..255b77cbb4 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8136,7 +8136,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 31d455505b..83cc5468c2 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 =3D qemuBuildStorageSourceChainAttachPrepareDrive(disk, + if (!(data =3D qemuBuildStorageSourceChainAttachPrepareDrive(disk,= vm->def, priv->q= emuCaps))) goto cleanup; } diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index 7ff6a6b17b..3076dc9645 100644 --- a/tests/qemublocktest.c +++ b/tests/qemublocktest.c @@ -185,6 +185,7 @@ static int testQemuDiskXMLToProps(const void *opaque) { struct testQemuDiskXMLToJSONData *data =3D (void *) opaque; + g_autoptr(virDomainDef) vmdef =3D NULL; virDomainDiskDefPtr disk =3D NULL; virStorageSourcePtr n; virJSONValuePtr formatProps =3D NULL; @@ -204,7 +205,11 @@ testQemuDiskXMLToProps(const void *opaque) VIR_DOMAIN_DEF_PARSE_STATUS))) goto cleanup; =20 - if (qemuCheckDiskConfig(disk, data->qemuCaps) < 0 || + if (!(vmdef =3D virDomainDefNew()) || + virDomainDiskInsert(vmdef, disk) < 0) + goto cleanup; + + if (qemuCheckDiskConfig(disk, vmdef, data->qemuCaps) < 0 || qemuDomainDeviceDefValidateDisk(disk, data->qemuCaps) < 0) { VIR_TEST_VERBOSE("invalid configuration for disk"); goto cleanup; @@ -242,7 +247,6 @@ testQemuDiskXMLToProps(const void *opaque) cleanup: virJSONValueFree(formatProps); virJSONValueFree(storageProps); - virDomainDiskDefFree(disk); VIR_FREE(xmlpath); VIR_FREE(xmlstr); return ret; --=20 2.24.1