From nobody Sun Feb 8 13:48:20 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=1642011094; cv=none; d=zohomail.com; s=zohoarc; b=cK6anCjr8p51SR3/QUyWgkhx0Xz6JFMcyf91tM/LKopbfqjO57n4MoMr0U2Mzv/KauR9f+Wmqy3BZUU+2wRFs+WBBrKyvH00AXxTpWGG/q5hfrcoruczl6mfMHn0F5+p50rIhvDiJWYXKWx7Ook2K6XFxwTOhG6T7ZSGvYxDsR0= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1642011094; 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=EyhqNP4kz/9ot7zQCTYTWgXL3HYwEy4lt0l2cQE5LYA=; b=VTrwW7cwWzeNs04wIxft2KWqn4gtFoVw4l4TU7jMIGPZs7bdqAPcBUhWaxMZPPh7lIFqAWvXwgEIWuE9709rIfeLCYSFCilf4OooyN/TaPMPtu1RsA51N7FlOEnZHul8gqoV3ZlmAa80J/hgpPhbHtE+2RbdKYOR05P5p/qgK/M= 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 1642011094322880.0426022196805; Wed, 12 Jan 2022 10:11:34 -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-650-L8gsn0csM1q_OnnQL0ZR3Q-1; Wed, 12 Jan 2022 13:11:31 -0500 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id ACB4183DD30; Wed, 12 Jan 2022 18:11:23 +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 828081091EDA; Wed, 12 Jan 2022 18:11:23 +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 3E917180BADA; Wed, 12 Jan 2022 18:11:23 +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 20CIAe3E003107 for ; Wed, 12 Jan 2022 13:10:40 -0500 Received: by smtp.corp.redhat.com (Postfix) id E93BF8CB1D; Wed, 12 Jan 2022 18:10:40 +0000 (UTC) Received: from speedmetal.lan (unknown [10.40.208.34]) by smtp.corp.redhat.com (Postfix) with ESMTP id 4750A8AD18 for ; Wed, 12 Jan 2022 18:10:40 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1642011093; 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=EyhqNP4kz/9ot7zQCTYTWgXL3HYwEy4lt0l2cQE5LYA=; b=AAxElXZSf7RKVShWKCKZbzH35IixBG2QW8kQjMFFQ4s3hFqB7JiZHx5XKaWC3BTaY5PUnt +w19Vy1QPhoqDEGaCcsSspLn6X9vVUDM5TgITiXwC0aOfH85+UtqWJmOMM/RudO0zogFuT d3QH6xKztqSGK0A6pAi3rRsE9sANAxA= X-MC-Unique: L8gsn0csM1q_OnnQL0ZR3Q-1 From: Peter Krempa To: libvir-list@redhat.com Subject: [PATCH 11/17] virDomainSnapshotRedefineValidate: Don't modify the snapshot definition Date: Wed, 12 Jan 2022 19:10:11 +0100 Message-Id: <114eac9bd36fb3c985df70e22d33567d6e8bdec2.1642010887.git.pkrempa@redhat.com> 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.84 on 10.5.11.22 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: 1642011096152100002 Content-Type: text/plain; charset="utf-8" It is not expected that a function with 'Validate' in the name actually modifies the validated object, even worse when it even modifies another object and the ultimatively worst bit is that it doesn't undo the mess if the validation fails midway. Move the stealing of the domain definition from the definition of a snapshot being redefined into the caller along with the call to virDomainSnapshotAlignDisks. Signed-off-by: Peter Krempa --- src/conf/snapshot_conf.c | 56 +++++++++++++++++----------------------- 1 file changed, 24 insertions(+), 32 deletions(-) diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index 4d2cfae128..499fc5ad97 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -463,9 +463,7 @@ virDomainSnapshotDefParseString(const char *xmlStr, } -/* Perform sanity checking on a redefined snapshot definition. If - * @other is non-NULL, this may include swapping def->parent.dom from other - * into def. */ +/* Perform sanity checking on a redefined snapshot definition. */ static int virDomainSnapshotRedefineValidate(virDomainSnapshotDef *def, const unsigned char *domain_uuid, @@ -473,10 +471,6 @@ virDomainSnapshotRedefineValidate(virDomainSnapshotDef= *def, virDomainXMLOption *xmlopt, unsigned int flags) { - virDomainSnapshotLocation align_location =3D VIR_DOMAIN_SNAPSHOT_LOCAT= ION_INTERNAL; - bool external =3D def->state =3D=3D VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT = || - virDomainSnapshotDefIsExternal(def); - if ((flags & VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY) && def->state !=3D VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT) { virReportError(VIR_ERR_INVALID_ARG, @@ -524,22 +518,10 @@ virDomainSnapshotRedefineValidate(virDomainSnapshotDe= f *def, if (!virDomainDefCheckABIStability(otherdef->parent.dom, def->parent.dom, xmlopt= )) return -1; - } else { - /* Transfer the domain def */ - def->parent.dom =3D g_steal_pointer(&otherdef->parent.dom); } } } - if (def->parent.dom) { - if (external) - align_location =3D VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL; - - if (virDomainSnapshotAlignDisks(def, NULL, align_location, true) <= 0) - return -1; - } - - return 0; } @@ -982,28 +964,38 @@ virDomainSnapshotRedefinePrep(virDomainObj *vm, { virDomainSnapshotDef *snapdef =3D *defptr; virDomainMomentObj *other; - virDomainSnapshotDef *otherdef =3D NULL; - bool check_if_stolen; + virDomainSnapshotDef *otherSnapDef =3D NULL; + virDomainDef *otherDomDef =3D NULL; + virDomainSnapshotLocation align_location =3D VIR_DOMAIN_SNAPSHOT_LOCAT= ION_INTERNAL; if (virDomainSnapshotCheckCycles(vm->snapshots, snapdef, vm->def->name= ) < 0) return -1; - other =3D virDomainSnapshotFindByName(vm->snapshots, snapdef->parent.n= ame); - if (other) - otherdef =3D virDomainSnapshotObjGetDef(other); - check_if_stolen =3D other && otherdef->parent.dom; - if (virDomainSnapshotRedefineValidate(snapdef, vm->def->uuid, other, x= mlopt, - flags) < 0) { - /* revert any stealing of the snapshot domain definition */ - if (check_if_stolen && snapdef->parent.dom && !otherdef->parent.do= m) - otherdef->parent.dom =3D g_steal_pointer(&snapdef->parent.dom); - return -1; + if ((other =3D virDomainSnapshotFindByName(vm->snapshots, snapdef->par= ent.name))) { + otherSnapDef =3D virDomainSnapshotObjGetDef(other); + otherDomDef =3D otherSnapDef->parent.dom; } + + if (virDomainSnapshotRedefineValidate(snapdef, vm->def->uuid, other, x= mlopt, flags) < 0) + return -1; + + if (snapdef->state =3D=3D VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT || + virDomainSnapshotDefIsExternal(snapdef)) + align_location =3D VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL; + + 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(otherdef); + virObjectUnref(otherSnapDef); other->def =3D &(*defptr)->parent; *defptr =3D NULL; *snap =3D other; --=20 2.31.1