From nobody Mon Feb 9 13:58:54 2026 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of redhat.com designates 170.10.133.124 as permitted sender) client-ip=170.10.133.124; envelope-from=libvir-list-bounces@redhat.com; helo=us-smtp-delivery-124.mimecast.com; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1642011110; cv=none; d=zohomail.com; s=zohoarc; b=JIOhs8uWRDqf4AyjSEW6Xs0gXvtuMq5tdIB5qnWjQkbxaG15mdhFd7H0IJO+iKfCQpr+zKqXapL5IJl7Kh1dAQz37LkdaOQO+7nk2CRGud0h2JChmCrUGzRCF67mev0j1Ks4+CqBSLLmzkyy2Z8MGulA+8Z1i7gDe/rfFmzD3Fk= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1642011110; h=Content-Type:Content-Transfer-Encoding:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:To; bh=9nH2PxvOrqIcUM2j1qfnN4PIprjY4zsKdZ8Uj29GB3Q=; b=cRwaD/bxU5dHHrC9Bob71MH96dj3s3K8YXkQCNpzZXIjq4kDgxrTKBV3vJ6x0wVtFOhUFI7qxZsQGllnceePhQcoNzay0X12nQ3WgLb1LKabNCS20P6FDDoA+luHBpjT2tkOxjj7yy9nHL49OwKoEvOAbCwSUdDj1/A29qrxxmA= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass header.from= (p=none dis=none) Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mx.zohomail.com with SMTPS id 1642011110640968.2474954619743; Wed, 12 Jan 2022 10:11:50 -0800 (PST) Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-628-M97KCdMrOeSS8GEJ6A_Dzw-1; Wed, 12 Jan 2022 13:11:46 -0500 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 mimecast-mx01.redhat.com (Postfix) with ESMTPS id D614C100C686; Wed, 12 Jan 2022 18:11:40 +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 B386577464; Wed, 12 Jan 2022 18:11:40 +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 7EAD44A707; Wed, 12 Jan 2022 18:11:40 +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 20CIArAx003196 for ; Wed, 12 Jan 2022 13:10:53 -0500 Received: by smtp.corp.redhat.com (Postfix) id 173838AC23; Wed, 12 Jan 2022 18:10:53 +0000 (UTC) Received: from speedmetal.lan (unknown [10.40.208.34]) by smtp.corp.redhat.com (Postfix) with ESMTP id 777538AC21 for ; Wed, 12 Jan 2022 18:10:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1642011109; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to: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=9nH2PxvOrqIcUM2j1qfnN4PIprjY4zsKdZ8Uj29GB3Q=; b=Mail0ITKI3PuNiVZrYiVSJNxURC63Qcw2QmoAobijIOcczWOKYg7cyLbv+2KABdBV+m2/j 8k+X53+cGBNK4rcblvYSMzZOkoiRD4bRLXmGZdTryr/HgM8xFGOmMh831Masj0eeul1rHy LMwCmaoss678JJqMZSYY5JkN1IVdJAY= X-MC-Unique: M97KCdMrOeSS8GEJ6A_Dzw-1 From: Peter Krempa To: libvir-list@redhat.com Subject: [PATCH 17/17] virDomainSnapshotRedefinePrep: Don't do partial redefine Date: Wed, 12 Jan 2022 19:10:17 +0100 Message-Id: In-Reply-To: References: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-loop: libvir-list@redhat.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.15 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=libvir-list-bounces@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: quoted-printable X-ZohoMail-DKIM: pass (identity @redhat.com) X-ZM-MESSAGEID: 1642011111468100003 Content-Type: text/plain; charset="utf-8" 'virDomainSnapshotRedefinePrep' does everything needed for a redefine when the snapshot exists but not when we are defining metadata for a new snapshot. This gives us weird semantics. Extract the code for replacing the definition of an existing snapshot into a new helper 'virDomainSnapshotReplaceDef' and refactor all callers. Signed-off-by: Peter Krempa --- src/conf/snapshot_conf.c | 20 +++----------------- src/conf/snapshot_conf.h | 2 +- src/conf/virdomainsnapshotobjlist.c | 19 +++++++++++++++++++ src/conf/virdomainsnapshotobjlist.h | 3 +++ src/libvirt_private.syms | 1 + src/qemu/qemu_snapshot.c | 9 +++++---- src/test/test_driver.c | 6 ++++-- 7 files changed, 36 insertions(+), 24 deletions(-) diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index 499fc5ad97..2d4c7190ba 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -957,12 +957,11 @@ virDomainSnapshotIsExternal(virDomainMomentObj *snap) int virDomainSnapshotRedefinePrep(virDomainObj *vm, - virDomainSnapshotDef **defptr, + virDomainSnapshotDef *snapdef, virDomainMomentObj **snap, virDomainXMLOption *xmlopt, unsigned int flags) { - virDomainSnapshotDef *snapdef =3D *defptr; virDomainMomentObj *other; virDomainSnapshotDef *otherSnapDef =3D NULL; virDomainDef *otherDomDef =3D NULL; @@ -976,6 +975,8 @@ virDomainSnapshotRedefinePrep(virDomainObj *vm, otherDomDef =3D otherSnapDef->parent.dom; } + *snap =3D other; + if (virDomainSnapshotRedefineValidate(snapdef, vm->def->uuid, other, x= mlopt, flags) < 0) return -1; @@ -986,20 +987,5 @@ virDomainSnapshotRedefinePrep(virDomainObj *vm, if (virDomainSnapshotAlignDisks(snapdef, otherDomDef, align_location, = true) < 0) return -1; - if (other) { - /* steal the domain definition if redefining an existing snapshot = which - * with a snapshot definition lacking the domain definition */ - if (!snapdef->parent.dom) - snapdef->parent.dom =3D g_steal_pointer(&otherSnapDef->parent.= dom); - - /* Drop and rebuild the parent relationship, but keep all - * child relations by reusing snap. */ - virDomainMomentDropParent(other); - virObjectUnref(otherSnapDef); - other->def =3D &(*defptr)->parent; - *defptr =3D NULL; - *snap =3D other; - } - return 0; } diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h index b04163efae..c8997c710c 100644 --- a/src/conf/snapshot_conf.h +++ b/src/conf/snapshot_conf.h @@ -134,7 +134,7 @@ bool virDomainSnapshotDefIsExternal(virDomainSnapshotDe= f *def); bool virDomainSnapshotIsExternal(virDomainMomentObj *snap); int virDomainSnapshotRedefinePrep(virDomainObj *vm, - virDomainSnapshotDef **def, + virDomainSnapshotDef *snapdef, virDomainMomentObj **snap, virDomainXMLOption *xmlopt, unsigned int flags); diff --git a/src/conf/virdomainsnapshotobjlist.c b/src/conf/virdomainsnapsh= otobjlist.c index 6b074d5994..2520a4bca4 100644 --- a/src/conf/virdomainsnapshotobjlist.c +++ b/src/conf/virdomainsnapshotobjlist.c @@ -54,6 +54,25 @@ virDomainSnapshotAssignDef(virDomainSnapshotObjList *sna= pshots, } +void +virDomainSnapshotReplaceDef(virDomainMomentObj *snap, + virDomainSnapshotDef **snapdefptr) +{ + virDomainSnapshotDef *snapdef =3D *snapdefptr; + g_autoptr(virDomainSnapshotDef) origsnapdef =3D virDomainSnapshotObjGe= tDef(snap); + + /* steal the domain definition if redefining an existing snapshot which + * with a snapshot definition lacking the domain definition */ + if (!snapdef->parent.dom) + snapdef->parent.dom =3D g_steal_pointer(&origsnapdef->parent.dom); + + /* Drop and rebuild the parent relationship, but keep all child relati= ons by reusing snap. */ + virDomainMomentDropParent(snap); + snap->def =3D &snapdef->parent; + *snapdefptr =3D NULL; +} + + static bool virDomainSnapshotFilter(virDomainMomentObj *obj, unsigned int flags) diff --git a/src/conf/virdomainsnapshotobjlist.h b/src/conf/virdomainsnapsh= otobjlist.h index bdbc17f6d5..ce9d77e10b 100644 --- a/src/conf/virdomainsnapshotobjlist.h +++ b/src/conf/virdomainsnapshotobjlist.h @@ -32,6 +32,9 @@ void virDomainSnapshotObjListFree(virDomainSnapshotObjLis= t *snapshots); virDomainMomentObj *virDomainSnapshotAssignDef(virDomainSnapshotObjList *s= napshots, virDomainSnapshotDef **snap= defptr); +void virDomainSnapshotReplaceDef(virDomainMomentObj *snap, + virDomainSnapshotDef **snapdefptr); + int virDomainSnapshotObjListGetNames(virDomainSnapshotObjList *snapshots, virDomainMomentObj *from, char **const names, int maxnames, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 5b76e66e61..f75dea36c4 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1212,6 +1212,7 @@ virDomainSnapshotObjListNew; virDomainSnapshotObjListNum; virDomainSnapshotObjListRemove; virDomainSnapshotObjListRemoveAll; +virDomainSnapshotReplaceDef; virDomainSnapshotSetCurrent; virDomainSnapshotUpdateRelations; diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index 3e35ff5463..9cf185026c 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -1715,15 +1715,16 @@ qemuSnapshotRedefine(virDomainObj *vm, virDomainSnapshotPtr ret =3D NULL; g_autoptr(virDomainSnapshotDef) snapdef =3D virObjectRef(snapdeftmp); - if (virDomainSnapshotRedefinePrep(vm, &snapdef, &snap, - driver->xmlopt, - flags) < 0) + if (virDomainSnapshotRedefinePrep(vm, snapdef, &snap, driver->xmlopt, = flags) < 0) return NULL; - if (!snap) { + if (snap) { + virDomainSnapshotReplaceDef(snap, &snapdef); + } else { if (!(snap =3D virDomainSnapshotAssignDef(vm->snapshots, &snapdef)= )) return NULL; } + /* XXX Should we validate that the redefined snapshot even * makes sense, such as checking that qemu-img recognizes the * snapshot name in at least one of the domain's disks? */ diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 14617d4f0d..1504334c30 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -8749,10 +8749,12 @@ testDomainSnapshotRedefine(virDomainObj *vm, virDomainMomentObj *snap =3D NULL; g_autoptr(virDomainSnapshotDef) snapdef =3D virObjectRef(snapdeftmp); - if (virDomainSnapshotRedefinePrep(vm, &snapdef, &snap, xmlopt, flags) = < 0) + if (virDomainSnapshotRedefinePrep(vm, snapdef, &snap, xmlopt, flags) <= 0) return NULL; - if (!snap) { + if (snap) { + virDomainSnapshotReplaceDef(snap, &snapdef); + } else { if (!(snap =3D virDomainSnapshotAssignDef(vm->snapshots, &snapdef)= )) return NULL; } --=20 2.31.1