From nobody Mon Feb 9 00:31:10 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 1550952049771356.098433201229; Sat, 23 Feb 2019 12:00:49 -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 A88B781F0D; Sat, 23 Feb 2019 20:00: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 75BB9608C3; Sat, 23 Feb 2019 20:00:45 +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 66C6A3FB11; Sat, 23 Feb 2019 20:00:40 +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 x1NK0dK9024942 for ; Sat, 23 Feb 2019 15:00:39 -0500 Received: by smtp.corp.redhat.com (Postfix) id 292806013A; Sat, 23 Feb 2019 20:00:39 +0000 (UTC) Received: from blue.redhat.com (ovpn-116-222.phx2.redhat.com [10.3.116.222]) by smtp.corp.redhat.com (Postfix) with ESMTP id EB872600CD for ; Sat, 23 Feb 2019 20:00:38 +0000 (UTC) From: Eric Blake To: libvir-list@redhat.com Date: Sat, 23 Feb 2019 14:00:35 -0600 Message-Id: <20190223200036.11626-2-eblake@redhat.com> In-Reply-To: <20190223200036.11626-1-eblake@redhat.com> References: <20190223200036.11626-1-eblake@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH 1/2] snapshot: Permit redefine of offline external snapshot 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.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Sat, 23 Feb 2019 20:00:47 +0000 (UTC) Content-Type: text/plain; charset="utf-8" Due to historical back-compat, 'virsh snapshot-create-as' favors internal snapshots (but can't be used on domains with raw storage), while 'virsh snapshot-create-as --disk-only) favors external snapshots. What's more, snapshots created with --disk-only while the domain was running are marked as snapshot state 'disk-snapshot', while snapshots created while the domain was offline are marked as snapshot state 'shutdown' (a 'disk-snapshot' image might not be quiescent, while a 'shutdown' snapshot always is). But this leads to some interesting problems: if we create a --disk-only snapshot of an offline guest, and then immediately try to 'virsh snapshot-create --redefine' using the resulting XML to overwrite the existing snapashot in place, things silently succeed, but 'virsh snapshot-create --redefine --disk-only' fails because the snapshot state is not 'disk-only'. Worse, if we delete the snapshot metadata first and then try to recreate things, omitting --disk-only fails because the verification code wants to force the default of an internal snapshot (which doesn't work with raw disks), and using --disk-only fails because the verification code doesn't see the 'disk-only' state in the snapshot xml - making it impossible to recreate the snapshot metadata. Ideally, the presence or absence of the --disk-only flag, and the presence or absence of an existing snapshot being overwritten, shouldn't matter; if the XML is valid for one situation, it should always be valid to redefine the metadata for that snapshot. Fix things by uniformly declaring that a redefined snapshot in the 'shutdown' state can permit a disk-only external snapshot (we got it right in only one out of three places in the function). See also https://bugzilla.redhat.com/1680304; this fixes the domain-agnostic problems mentioned there, but another patch is needed to fix further oddities with the qemu driver. I did not check for sure, but this has probably been broken even prior to when commit 670e86bf (1.1.4) factored redefine prep out of qemu code into the common snapshot_conf code. Signed-off-by: Eric Blake Reviewed-by: John Ferlan --- src/conf/snapshot_conf.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index 489c25f511..3372c4df86 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -1225,6 +1225,8 @@ virDomainSnapshotRedefinePrep(virDomainPtr domain, int align_location =3D VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL; bool align_match =3D true; virDomainSnapshotObjPtr other; + bool offline =3D def->state =3D=3D VIR_DOMAIN_DISK_SNAPSHOT || + virDomainSnapshotDefIsExternal(def); /* Prevent circular chains */ if (def->parent) { @@ -1259,14 +1261,12 @@ virDomainSnapshotRedefinePrep(virDomainPtr domain, } /* Check that any replacement is compatible */ - if ((flags & VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY) && - def->state !=3D VIR_DOMAIN_DISK_SNAPSHOT) { + if ((flags & VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY) && !offline) { virReportError(VIR_ERR_INVALID_ARG, _("disk-only flag for snapshot %s requires " "disk-snapshot state"), def->name); goto cleanup; - } if (def->dom && @@ -1315,8 +1315,7 @@ virDomainSnapshotRedefinePrep(virDomainPtr domain, } if (def->dom) { - if (def->state =3D=3D VIR_DOMAIN_DISK_SNAPSHOT || - virDomainSnapshotDefIsExternal(def)) { + if (offline) { align_location =3D VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL; align_match =3D false; } @@ -1346,7 +1345,7 @@ virDomainSnapshotRedefinePrep(virDomainPtr domain, *snap =3D other; } else { if (def->dom) { - if (def->state =3D=3D VIR_DOMAIN_DISK_SNAPSHOT || + if (offline || def->memory =3D=3D VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) { align_location =3D VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL; align_match =3D false; --=20 2.20.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list