From nobody Sun Feb 8 22:58:17 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 ARC-Seal: i=1; a=rsa-sha256; t=1563947796; cv=none; d=zoho.com; s=zohoarc; b=HrSzyFPuCHI3O5AwwlRVgyFuy5jrtwd1TXGyifCcfkYhPm/sQkTv7LfMmaBI/HB8gTpPifucUQ6JhcmomJLOjek0SwNi3dXC4umBp1rIGQBRXdsv4F+UZCIIqdOOXyiBtmOmm1LfihF3t+yctvZf5j/RCKVcI1p9tWVjOMmsTvE= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zoho.com; s=zohoarc; t=1563947796; h=Content-Type:Content-Transfer-Encoding:Cc: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:ARC-Authentication-Results; bh=m29X5b+VS2Rs8FsJHnE5f2UZIdzjctwdDJGDUEwKaGI=; b=G+Xl38CL5kUmb1ov3OZ+bQFGKVmrH7LrMM5VW5y8zRkqGsisUGsUsjKQyNnl9T1B9Ta24NwLeYikXFB3XwVL+2xg9CLyIEUiZ4T5cOj1N2Bhfr4uRvjIKMtyQUVz+ihyQsONDgkV7K9TaDRkCJAldG+mb8E+L1ZEhK6WnG6rAQQ= ARC-Authentication-Results: i=1; mx.zoho.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 header.from= (p=none dis=none) header.from= Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1563947796384860.3027899499485; Tue, 23 Jul 2019 22:56:36 -0700 (PDT) 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 D821A81DF2; Wed, 24 Jul 2019 05:56:34 +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 B5B7717C41; Wed, 24 Jul 2019 05:56:34 +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 7D9B53CB9; Wed, 24 Jul 2019 05:56:34 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id x6O5uDKj029087 for ; Wed, 24 Jul 2019 01:56:13 -0400 Received: by smtp.corp.redhat.com (Postfix) id 4A47760C05; Wed, 24 Jul 2019 05:56:13 +0000 (UTC) Received: from blue.redhat.com (ovpn-116-93.phx2.redhat.com [10.3.116.93]) by smtp.corp.redhat.com (Postfix) with ESMTP id CE8D660BEC; Wed, 24 Jul 2019 05:56:12 +0000 (UTC) From: Eric Blake To: libvir-list@redhat.com Date: Wed, 24 Jul 2019 00:55:51 -0500 Message-Id: <20190724055609.30691-2-eblake@redhat.com> In-Reply-To: <20190724055609.30691-1-eblake@redhat.com> References: <20190724055609.30691-1-eblake@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-loop: libvir-list@redhat.com Cc: nsoffer@redhat.com, eshenitz@redhat.com, pkrempa@redhat.com Subject: [libvirt] [PATCH v10 01/19] snapshot: Don't leak moment obj list metaroot to callers 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.25]); Wed, 24 Jul 2019 05:56:35 +0000 (UTC) Content-Type: text/plain; charset="utf-8" virDomainSnapshotFindByName(list, NULL) should return NULL, rather than the internal-use-only metaroot. Most existing callers pass in a non-NULL name; the few that don't are immediately calling virDomainMomentSetParent (which indeed needs the metaroot if the parent name is NULL); but as the leak is ugly, it is worth instead making virDomainMomentSetParent static and adding a new function for resolving the link of a brand new moment within its list (and where we are either requesting NULL for a new root moment, or requesting the name of the current moment, and therefore the new function does not need to return failure, but merely warn if future code breaks our assumptions). Missed when commit 02c4e24d refactored things to attempt to remove direct metaroot manipulations out of the qemu and test drivers into internal-only details, and made more obvious when commit dc8d3dc6 factored it out into a separate file. Signed-off-by: Eric Blake Reviewed-by: Daniel P. Berrang=C3=A9 --- src/conf/virdomainmomentobjlist.h | 4 ++-- src/conf/virdomainsnapshotobjlist.h | 2 ++ src/conf/virdomainmomentobjlist.c | 34 +++++++++++++++++++++++++---- src/conf/virdomainsnapshotobjlist.c | 9 ++++++++ src/libvirt_private.syms | 2 +- src/qemu/qemu_driver.c | 5 +---- src/test/test_driver.c | 5 +---- 7 files changed, 46 insertions(+), 15 deletions(-) diff --git a/src/conf/virdomainmomentobjlist.h b/src/conf/virdomainmomentob= jlist.h index 4067e928f4..897d8d54d1 100644 --- a/src/conf/virdomainmomentobjlist.h +++ b/src/conf/virdomainmomentobjlist.h @@ -60,8 +60,8 @@ void virDomainMomentDropParent(virDomainMomentObjPtr mome= nt); void virDomainMomentDropChildren(virDomainMomentObjPtr moment); void virDomainMomentMoveChildren(virDomainMomentObjPtr from, virDomainMomentObjPtr to); -void virDomainMomentSetParent(virDomainMomentObjPtr moment, - virDomainMomentObjPtr parent); +void virDomainMomentLinkParent(virDomainMomentObjListPtr moments, + virDomainMomentObjPtr moment); virDomainMomentObjListPtr virDomainMomentObjListNew(void); void virDomainMomentObjListFree(virDomainMomentObjListPtr moments); diff --git a/src/conf/virdomainsnapshotobjlist.h b/src/conf/virdomainsnapsh= otobjlist.h index fed8d22bc8..b8c80b39ed 100644 --- a/src/conf/virdomainsnapshotobjlist.h +++ b/src/conf/virdomainsnapshotobjlist.h @@ -51,6 +51,8 @@ void virDomainSnapshotObjListRemoveAll(virDomainSnapshotO= bjListPtr snapshots); int virDomainSnapshotForEach(virDomainSnapshotObjListPtr snapshots, virHashIterator iter, void *data); +void virDomainSnapshotLinkParent(virDomainSnapshotObjListPtr snapshots, + virDomainMomentObjPtr moment); int virDomainSnapshotUpdateRelations(virDomainSnapshotObjListPtr snapshots= ); int virDomainSnapshotCheckCycles(virDomainSnapshotObjListPtr snapshots, virDomainSnapshotDefPtr def, diff --git a/src/conf/virdomainmomentobjlist.c b/src/conf/virdomainmomentob= jlist.c index 7cb1745525..5c147bdbbf 100644 --- a/src/conf/virdomainmomentobjlist.c +++ b/src/conf/virdomainmomentobjlist.c @@ -151,7 +151,7 @@ virDomainMomentDropChildren(virDomainMomentObjPtr momen= t) /* Add @moment to @parent's list of children. */ -void +static void virDomainMomentSetParent(virDomainMomentObjPtr moment, virDomainMomentObjPtr parent) { @@ -162,6 +162,27 @@ virDomainMomentSetParent(virDomainMomentObjPtr moment, } +/* Add @moment to the appropriate parent's list of children. The + * caller must ensure that moment->def->parent_name is either NULL + * (for a new root) or set to an existing moment already in the + * list. */ +void +virDomainMomentLinkParent(virDomainMomentObjListPtr moments, + virDomainMomentObjPtr moment) +{ + virDomainMomentObjPtr parent; + + parent =3D virDomainMomentFindByName(moments, moment->def->parent_name= ); + if (!parent) { + parent =3D &moments->metaroot; + if (moment->def->parent_name) + VIR_WARN("moment %s lacks parent %s", moment->def->name, + moment->def->parent_name); + } + virDomainMomentSetParent(moment, parent); +} + + /* Take all children of @from and convert them into children of @to. */ void virDomainMomentMoveChildren(virDomainMomentObjPtr from, @@ -390,7 +411,9 @@ virDomainMomentObjPtr virDomainMomentFindByName(virDomainMomentObjListPtr moments, const char *name) { - return name ? virHashLookup(moments->objs, name) : &moments->metaroot; + if (name) + return virHashLookup(moments->objs, name); + return NULL; } @@ -484,9 +507,12 @@ virDomainMomentSetRelations(void *payload, parent =3D virDomainMomentFindByName(curr->moments, obj->def->parent_n= ame); if (!parent) { - curr->err =3D -1; parent =3D &curr->moments->metaroot; - VIR_WARN("moment %s lacks parent", obj->def->name); + if (obj->def->parent_name) { + curr->err =3D -1; + VIR_WARN("moment %s lacks parent %s", obj->def->name, + obj->def->parent_name); + } } else { tmp =3D parent; while (tmp && tmp->def) { diff --git a/src/conf/virdomainsnapshotobjlist.c b/src/conf/virdomainsnapsh= otobjlist.c index 99bc4bb0c5..95622f0ba7 100644 --- a/src/conf/virdomainsnapshotobjlist.c +++ b/src/conf/virdomainsnapshotobjlist.c @@ -223,6 +223,15 @@ virDomainSnapshotForEach(virDomainSnapshotObjListPtr s= napshots, } +/* Populate parent link of a given snapshot. */ +void +virDomainSnapshotLinkParent(virDomainSnapshotObjListPtr snapshots, + virDomainMomentObjPtr snap) +{ + return virDomainMomentLinkParent(snapshots->base, snap); +} + + /* Populate parent link and child count of all snapshots, with all * assigned defs having relations starting as 0/NULL. Return 0 on * success, -1 if a parent is missing or if a circular relationship diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index dff97bd82a..ff5a77b0e2 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -978,7 +978,6 @@ virDomainMomentDropParent; virDomainMomentForEachChild; virDomainMomentForEachDescendant; virDomainMomentMoveChildren; -virDomainMomentSetParent; # conf/virdomainobjlist.h @@ -1007,6 +1006,7 @@ virDomainSnapshotFindByName; virDomainSnapshotForEach; virDomainSnapshotGetCurrent; virDomainSnapshotGetCurrentName; +virDomainSnapshotLinkParent; virDomainSnapshotObjListFree; virDomainSnapshotObjListGetNames; virDomainSnapshotObjListNew; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 482f915b67..065e0a1bd8 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15540,7 +15540,6 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, bool update_current =3D true; bool redefine =3D flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE; unsigned int parse_flags =3D VIR_DOMAIN_SNAPSHOT_PARSE_DISKS; - virDomainMomentObjPtr other =3D NULL; int align_location =3D VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL; bool align_match =3D true; virQEMUDriverConfigPtr cfg =3D NULL; @@ -15807,9 +15806,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, snap->def->name); virDomainSnapshotObjListRemove(vm->snapshots, snap); } else { - other =3D virDomainSnapshotFindByName(vm->snapshots, - snap->def->parent_name); - virDomainMomentSetParent(snap, other); + virDomainSnapshotLinkParent(vm->snapshots, snap); } } else if (snap) { virDomainSnapshotObjListRemove(vm->snapshots, snap); diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 2e33a9dd55..d96dd638e7 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -7361,12 +7361,9 @@ testDomainSnapshotCreateXML(virDomainPtr domain, cleanup: if (vm) { if (snapshot) { - virDomainMomentObjPtr other; if (update_current) virDomainSnapshotSetCurrent(vm->snapshots, snap); - other =3D virDomainSnapshotFindByName(vm->snapshots, - snap->def->parent_name); - virDomainMomentSetParent(snap, other); + virDomainSnapshotLinkParent(vm->snapshots, snap); } virDomainObjEndAPI(&vm); } --=20 2.20.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list