From nobody Sat Feb 7 06:20:36 2026 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=fail(p=none dis=none) header.from=virtuozzo.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 154590615374592.59796579442241; Thu, 27 Dec 2018 02:22:33 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 12BBC41A53; Thu, 27 Dec 2018 10:22:32 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id D0A46608EC; Thu, 27 Dec 2018 10:22:31 +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 8268018433AF; Thu, 27 Dec 2018 10:22:31 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id wBRALm19028164 for ; Thu, 27 Dec 2018 05:21:48 -0500 Received: by smtp.corp.redhat.com (Postfix) id 0F96A5C3FA; Thu, 27 Dec 2018 10:21:48 +0000 (UTC) Received: from mx1.redhat.com (ext-mx10.extmail.prod.ext.phx2.redhat.com [10.5.110.39]) by smtp.corp.redhat.com (Postfix) with ESMTPS id F28745C23A; Thu, 27 Dec 2018 10:21:45 +0000 (UTC) Received: from relay.sw.ru (relay.sw.ru [185.231.240.75]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 1A4E4A24EB; Thu, 27 Dec 2018 10:21:43 +0000 (UTC) Received: from [10.94.3.220] (helo=dim-vz7.qa.sw.ru) by relay.sw.ru with esmtp (Exim 4.91) (envelope-from ) id 1gcSnN-0003jg-D4; Thu, 27 Dec 2018 13:21:41 +0300 From: Nikolay Shirokovskiy To: libvir-list@redhat.com Date: Thu, 27 Dec 2018 13:20:51 +0300 Message-Id: <1545906052-101494-10-git-send-email-nshirokovskiy@virtuozzo.com> In-Reply-To: <1545906052-101494-1-git-send-email-nshirokovskiy@virtuozzo.com> References: <1545906052-101494-1-git-send-email-nshirokovskiy@virtuozzo.com> X-Greylist: Sender passed SPF test, ACL 238 matched, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Thu, 27 Dec 2018 10:21:43 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Thu, 27 Dec 2018 10:21:43 +0000 (UTC) for IP:'185.231.240.75' DOMAIN:'relay.sw.ru' HELO:'relay.sw.ru' FROM:'nshirokovskiy@virtuozzo.com' RCPT:'' X-RedHat-Spam-Score: -0.001 (SPF_PASS) 185.231.240.75 relay.sw.ru 185.231.240.75 relay.sw.ru X-Scanned-By: MIMEDefang 2.78 on 10.5.110.39 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH 09/10] qemu: snapshot: align disks consistently 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: , MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Thu, 27 Dec 2018 10:22:32 +0000 (UTC) Content-Type: text/plain; charset="utf-8" There is inconsitency in how we set disk snapshot attribute for missing disk and for disk explicitly present in snapshot definition in virDomainSnapshot= AlignDisks. First for explicit disk we do not check if disk source is present. After the previous patch this will cause snapshot failures so we'd better disable snapshotting of such disk at this place. (For the record: we could have failures before previous patch too, it just does not makes sense to snapshot disk without source). Second for missing disks with empty source disabling snapshot take preceden= ce over user settings. This does not feel right. It is better report to user t= hat option he wanted in not supported/possible rather then ignoring it. This can break things a bit. Hopefully nobody really uses domain disk snapshot setting. Next let's remove setting disk snapshot on parse stage and move it altogeth= er to one place - disk align function. Other hypervisors does not check this attribute in domain disk config so we are free here. Now we can place logic for setting disk snapshot value in one function - virDomainSnapshotSetDiskSnapshot. Signed-off-by: Nikolay Shirokovskiy --- src/conf/domain_conf.c | 6 +----- src/conf/snapshot_conf.c | 38 +++++++++++++++++++++++--------------- 2 files changed, 24 insertions(+), 20 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d8dfd16..0fbd739 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9759,8 +9759,6 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, snapshot); goto error; } - } else if (def->src->readonly) { - def->snapshot =3D VIR_DOMAIN_SNAPSHOT_LOCATION_NONE; } =20 if (rawio) { @@ -24285,9 +24283,7 @@ virDomainDiskDefFormat(virBufferPtr buf, if (def->sgio) virBufferAsprintf(buf, " sgio=3D'%s'", sgio); =20 - if (def->snapshot && - !(def->snapshot =3D=3D VIR_DOMAIN_SNAPSHOT_LOCATION_NONE && - def->src->readonly)) + if (def->snapshot) virBufferAsprintf(buf, " snapshot=3D'%s'", virDomainSnapshotLocationTypeToString(def->snaps= hot)); virBufferAddLit(buf, ">\n"); diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index 002cd45..172dff8 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -521,6 +521,25 @@ virDomainSnapshotCompareDiskIndex(const void *a, const= void *b) return diska->idx - diskb->idx; } =20 + +static void +virDomainSnapshotSetDiskSnapshot(virDomainSnapshotDiskDefPtr disk, + virDomainDiskDefPtr dom_disk, + int default_snapshot) +{ + if (disk->snapshot) + return; + + if (dom_disk->snapshot) + disk->snapshot =3D dom_disk->snapshot; + else if (dom_disk->src->readonly || + virStorageSourceIsEmpty(dom_disk->src)) + disk->snapshot =3D VIR_DOMAIN_SNAPSHOT_LOCATION_NONE; + else + disk->snapshot =3D default_snapshot; +} + + /* Align def->disks to def->domain. Sort the list of def->disks, * filling in any missing disks or snapshot state defaults given by * the domain, with a fallback to a passed in default. Convert paths @@ -562,7 +581,6 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def, virDomainSnapshotDiskDefPtr disk =3D &def->disks[i]; virDomainDiskDefPtr dom_disk; int idx =3D virDomainDiskIndexByName(def->dom, disk->name, false); - int disk_snapshot; =20 if (idx < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -580,13 +598,8 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr de= f, disk->idx =3D idx; dom_disk =3D def->dom->disks[idx]; =20 - disk_snapshot =3D dom_disk->snapshot; - if (!disk->snapshot) { - if (disk_snapshot) - disk->snapshot =3D disk_snapshot; - else - disk->snapshot =3D default_snapshot; - } + virDomainSnapshotSetDiskSnapshot(disk, dom_disk, default_snapshot); + if (STRNEQ(disk->name, dom_disk->dst)) { VIR_FREE(disk->name); if (VIR_STRDUP(disk->name, dom_disk->dst) < 0) @@ -612,15 +625,10 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr d= ef, goto cleanup; disk->idx =3D i; =20 - /* Don't snapshot empty drives */ - if (virStorageSourceIsEmpty(def->dom->disks[i]->src)) - disk->snapshot =3D VIR_DOMAIN_SNAPSHOT_LOCATION_NONE; - else - disk->snapshot =3D def->dom->disks[i]->snapshot; + virDomainSnapshotSetDiskSnapshot(disk, def->dom->disks[i], + default_snapshot); =20 disk->src->type =3D VIR_STORAGE_TYPE_FILE; - if (!disk->snapshot) - disk->snapshot =3D default_snapshot; } =20 qsort(&def->disks[0], def->ndisks, sizeof(def->disks[0]), --=20 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list