From nobody Sun Feb 8 18:30:18 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=pass(p=none dis=none) header.from=redhat.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1551756912986160.8838472315108; Mon, 4 Mar 2019 19:35:12 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 3BD97307EA9F; Tue, 5 Mar 2019 03:35:11 +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 102255D732; Tue, 5 Mar 2019 03:35:11 +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 B2CA13FAF4; Tue, 5 Mar 2019 03:35:10 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id x253Yuuw004185 for ; Mon, 4 Mar 2019 22:34:56 -0500 Received: by smtp.corp.redhat.com (Postfix) id B828B60A9A; Tue, 5 Mar 2019 03:34:56 +0000 (UTC) Received: from blue.redhat.com (ovpn-118-35.phx2.redhat.com [10.3.118.35]) by smtp.corp.redhat.com (Postfix) with ESMTP id 66F1E60C7F; Tue, 5 Mar 2019 03:34:56 +0000 (UTC) From: Eric Blake To: libvir-list@redhat.com Date: Mon, 4 Mar 2019 21:34:37 -0600 Message-Id: <20190305033445.17140-11-eblake@redhat.com> In-Reply-To: <20190305033445.17140-1-eblake@redhat.com> References: <20190305033445.17140-1-eblake@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-loop: libvir-list@redhat.com Cc: nsoffer@redhat.com Subject: [libvirt] [PATCH v3 10/18] snapshot: Split out virDomainSnapshotRedefineValidate helper 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: , 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.15 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.44]); Tue, 05 Mar 2019 03:35:11 +0000 (UTC) Content-Type: text/plain; charset="utf-8" Pull out the portion of virDomainSnapshotRefinePrep() that deals with definition sanity into a separate helper routine that can be reused with bulk redefine, leaving behind only the code specific to loop checking and in-place updates that are only needed in single-definition handling. Signed-off-by: Eric Blake Reviewed-by: John Ferlan --- src/conf/snapshot_conf.c | 186 ++++++++++++++++++++------------------- 1 file changed, 96 insertions(+), 90 deletions(-) diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index 386ec82d15..a5b05eadf4 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -426,6 +426,87 @@ virDomainSnapshotDefParseString(const char *xmlStr, } +/* Perform sanity checking on a redefined snapshot definition. If + * @other is non-NULL, this may include swapping def->dom from other + * into def. */ +static int +virDomainSnapshotRedefineValidate(virDomainSnapshotDefPtr def, + const unsigned char *domain_uuid, + virDomainSnapshotObjPtr other, + virDomainXMLOptionPtr xmlopt, + unsigned int flags) +{ + int align_location =3D VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL; + bool align_match =3D true; + bool external =3D def->state =3D=3D VIR_SNAP_STATE_DISK_SNAPSHOT || + virDomainSnapshotDefIsExternal(def); + + if ((flags & VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY) && !external) { + virReportError(VIR_ERR_INVALID_ARG, + _("disk-only flag for snapshot %s requires " + "disk-snapshot state"), + def->name); + return -1; + } + if (def->dom && memcmp(def->dom->uuid, domain_uuid, VIR_UUID_BUFLEN)) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + + virUUIDFormat(domain_uuid, uuidstr); + virReportError(VIR_ERR_INVALID_ARG, + _("definition for snapshot %s must use uuid %s"), + def->name, uuidstr); + return -1; + } + + if (other) { + if ((other->def->state =3D=3D VIR_SNAP_STATE_RUNNING || + other->def->state =3D=3D VIR_SNAP_STATE_PAUSED) !=3D + (def->state =3D=3D VIR_SNAP_STATE_RUNNING || + def->state =3D=3D VIR_SNAP_STATE_PAUSED)) { + virReportError(VIR_ERR_INVALID_ARG, + _("cannot change between online and offline " + "snapshot state in snapshot %s"), + def->name); + return -1; + } + + if ((other->def->state =3D=3D VIR_SNAP_STATE_DISK_SNAPSHOT) !=3D + (def->state =3D=3D VIR_SNAP_STATE_DISK_SNAPSHOT)) { + virReportError(VIR_ERR_INVALID_ARG, + _("cannot change between disk only and " + "full system in snapshot %s"), + def->name); + return -1; + } + + if (other->def->dom) { + if (def->dom) { + if (!virDomainDefCheckABIStability(other->def->dom, + def->dom, xmlopt)) + return -1; + } else { + /* Transfer the domain def */ + def->dom =3D other->def->dom; + other->def->dom =3D NULL; + } + } + } + + if (def->dom) { + if (external) { + align_location =3D VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL; + align_match =3D false; + } + if (virDomainSnapshotAlignDisks(def, align_location, + align_match) < 0) + return -1; + } + + + return 0; +} + + /** * virDomainSnapshotDefAssignExternalNames: * @def: snapshot def object @@ -1325,12 +1406,8 @@ virDomainSnapshotRedefinePrep(virDomainPtr domain, unsigned int flags) { virDomainSnapshotDefPtr def =3D *defptr; - int ret =3D -1; - int align_location =3D VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL; - bool align_match =3D true; virDomainSnapshotObjPtr other; - bool external =3D def->state =3D=3D VIR_SNAP_STATE_DISK_SNAPSHOT || - virDomainSnapshotDefIsExternal(def); + bool check_stolen; /* Prevent circular chains */ if (def->parent) { @@ -1338,21 +1415,21 @@ virDomainSnapshotRedefinePrep(virDomainPtr domain, virReportError(VIR_ERR_INVALID_ARG, _("cannot set snapshot %s as its own parent"), def->name); - goto cleanup; + return -1; } other =3D virDomainSnapshotFindByName(vm->snapshots, def->parent); if (!other) { virReportError(VIR_ERR_INVALID_ARG, _("parent %s for snapshot %s not found"), def->parent, def->name); - goto cleanup; + return -1; } while (other->def->parent) { if (STREQ(other->def->parent, def->name)) { virReportError(VIR_ERR_INVALID_ARG, _("parent %s would create cycle to %s"), other->def->name, def->name); - goto cleanup; + return -1; } other =3D virDomainSnapshotFindByName(vm->snapshots, other->def->parent); @@ -1364,77 +1441,18 @@ virDomainSnapshotRedefinePrep(virDomainPtr domain, } } - /* Check that any replacement is compatible */ - if ((flags & VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY) && !external) { - virReportError(VIR_ERR_INVALID_ARG, - _("disk-only flag for snapshot %s requires " - "disk-snapshot state"), - def->name); - goto cleanup; - } - - if (def->dom && - memcmp(def->dom->uuid, domain->uuid, VIR_UUID_BUFLEN)) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - - virUUIDFormat(domain->uuid, uuidstr); - virReportError(VIR_ERR_INVALID_ARG, - _("definition for snapshot %s must use uuid %s"), - def->name, uuidstr); - goto cleanup; - } - other =3D virDomainSnapshotFindByName(vm->snapshots, def->name); + check_stolen =3D other && other->def->dom; + if (virDomainSnapshotRedefineValidate(def, domain->uuid, other, xmlopt, + flags) < 0) { + /* revert stealing of the snapshot domain definition */ + if (check_stolen && def->dom && !other->def->dom) { + other->def->dom =3D def->dom; + def->dom =3D NULL; + } + return -1; + } if (other) { - if ((other->def->state =3D=3D VIR_SNAP_STATE_RUNNING || - other->def->state =3D=3D VIR_SNAP_STATE_PAUSED) !=3D - (def->state =3D=3D VIR_SNAP_STATE_RUNNING || - def->state =3D=3D VIR_SNAP_STATE_PAUSED)) { - virReportError(VIR_ERR_INVALID_ARG, - _("cannot change between online and offline " - "snapshot state in snapshot %s"), - def->name); - goto cleanup; - } - - if ((other->def->state =3D=3D VIR_SNAP_STATE_DISK_SNAPSHOT) !=3D - (def->state =3D=3D VIR_SNAP_STATE_DISK_SNAPSHOT)) { - virReportError(VIR_ERR_INVALID_ARG, - _("cannot change between disk only and " - "full system in snapshot %s"), - def->name); - goto cleanup; - } - - if (other->def->dom) { - if (def->dom) { - if (!virDomainDefCheckABIStability(other->def->dom, - def->dom, xmlopt)) - goto cleanup; - } else { - /* Transfer the domain def */ - def->dom =3D other->def->dom; - other->def->dom =3D NULL; - } - } - - if (def->dom) { - if (external) { - align_location =3D VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL; - align_match =3D false; - } - - if (virDomainSnapshotAlignDisks(def, align_location, - align_match) < 0) { - /* revert stealing of the snapshot domain definition */ - if (def->dom && !other->def->dom) { - other->def->dom =3D def->dom; - def->dom =3D NULL; - } - goto cleanup; - } - } - if (other =3D=3D vm->current_snapshot) { *update_current =3D true; vm->current_snapshot =3D NULL; @@ -1447,19 +1465,7 @@ virDomainSnapshotRedefinePrep(virDomainPtr domain, other->def =3D def; *defptr =3D NULL; *snap =3D other; - } else { - if (def->dom) { - if (external) { - align_location =3D VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL; - align_match =3D false; - } - if (virDomainSnapshotAlignDisks(def, align_location, - align_match) < 0) - goto cleanup; - } } - ret =3D 0; - cleanup: - return ret; + return 0; } --=20 2.20.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list