From nobody Mon Feb 9 15:09:45 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 155306050587312.1507606584114; Tue, 19 Mar 2019 22:41:45 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id E3417C05D3E5; Wed, 20 Mar 2019 05:41:43 +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 B6F6418232; Wed, 20 Mar 2019 05:41:43 +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 7CA1E3FA4B; Wed, 20 Mar 2019 05:41:43 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id x2K5fS2P017240 for ; Wed, 20 Mar 2019 01:41:28 -0400 Received: by smtp.corp.redhat.com (Postfix) id 5A85717F8D; Wed, 20 Mar 2019 05:41:28 +0000 (UTC) Received: from blue.redhat.com (ovpn-116-65.phx2.redhat.com [10.3.116.65]) by smtp.corp.redhat.com (Postfix) with ESMTP id D2A651A913; Wed, 20 Mar 2019 05:41:25 +0000 (UTC) From: Eric Blake To: libvir-list@redhat.com Date: Wed, 20 Mar 2019 00:40:56 -0500 Message-Id: <20190320054105.17689-8-eblake@redhat.com> In-Reply-To: <20190320054105.17689-1-eblake@redhat.com> References: <20190320054105.17689-1-eblake@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-loop: libvir-list@redhat.com Cc: jtomko@redhat.com Subject: [libvirt] [PATCH 07/16] snapshot: Add accessors for updating snapshot list relations 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.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Wed, 20 Mar 2019 05:41:44 +0000 (UTC) Content-Type: text/plain; charset="utf-8" Rather than allowing a leaky abstraction where multiple drivers have to open-code operations that update the relations in a virDomainSnapshotObjList, it is better to add accessor functions so that updates to relations are maintained closer to the internals. The goal here is to avoid access to, nchildren, first_child, or sibling outside of the lowest level functions, making it easier to refactor later on. While many of the conversions are fairly straightforward, the MoveChildren refactoring can be a bit interesting to follow. Signed-off-by: Eric Blake Reviewed-by: John Ferlan --- src/conf/virdomainsnapshotobj.h | 5 ++++ src/conf/virdomainsnapshotobjlist.h | 2 ++ src/conf/virdomainsnapshotobj.c | 42 +++++++++++++++++++++++++++++ src/conf/virdomainsnapshotobjlist.c | 42 ++++++++++++++++++----------- src/libvirt_private.syms | 3 +++ src/qemu/qemu_driver.c | 19 +++---------- src/test/test_driver.c | 18 +++---------- 7 files changed, 85 insertions(+), 46 deletions(-) diff --git a/src/conf/virdomainsnapshotobj.h b/src/conf/virdomainsnapshotob= j.h index 957f1b2ea8..0981ea4c25 100644 --- a/src/conf/virdomainsnapshotobj.h +++ b/src/conf/virdomainsnapshotobj.h @@ -46,5 +46,10 @@ int virDomainSnapshotForEachDescendant(virDomainSnapshot= ObjPtr snapshot, virHashIterator iter, void *data); void virDomainSnapshotDropParent(virDomainSnapshotObjPtr snapshot); +void virDomainSnapshotDropChildren(virDomainSnapshotObjPtr snapshot); +void virDomainSnapshotMoveChildren(virDomainSnapshotObjPtr from, + virDomainSnapshotObjPtr to); +void virDomainSnapshotSetParent(virDomainSnapshotObjPtr snapshot, + virDomainSnapshotObjPtr parent); #endif /* LIBVIRT_VIRDOMAINSNAPSHOTOBJ_H */ diff --git a/src/conf/virdomainsnapshotobjlist.h b/src/conf/virdomainsnapsh= otobjlist.h index e210849441..c13a0b4026 100644 --- a/src/conf/virdomainsnapshotobjlist.h +++ b/src/conf/virdomainsnapshotobjlist.h @@ -55,6 +55,7 @@ int virDomainSnapshotObjListNum(virDomainSnapshotObjListP= tr snapshots, unsigned int flags); virDomainSnapshotObjPtr virDomainSnapshotFindByName(virDomainSnapshotObjLi= stPtr snapshots, const char *name); +int virDomainSnapshotObjListSize(virDomainSnapshotObjListPtr snapshots); virDomainSnapshotObjPtr virDomainSnapshotGetCurrent(virDomainSnapshotObjLi= stPtr snapshots); const char *virDomainSnapshotGetCurrentName(virDomainSnapshotObjListPtr sn= apshots); bool virDomainSnapshotIsCurrentName(virDomainSnapshotObjListPtr snapshots, @@ -63,6 +64,7 @@ void virDomainSnapshotSetCurrent(virDomainSnapshotObjList= Ptr snapshots, virDomainSnapshotObjPtr snapshot); bool virDomainSnapshotObjListRemove(virDomainSnapshotObjListPtr snapshots, virDomainSnapshotObjPtr snapshot); +void virDomainSnapshotObjListRemoveAll(virDomainSnapshotObjListPtr snapsho= ts); int virDomainSnapshotForEach(virDomainSnapshotObjListPtr snapshots, virHashIterator iter, void *data); diff --git a/src/conf/virdomainsnapshotobj.c b/src/conf/virdomainsnapshotob= j.c index 7f92ac21d9..d6b216c7b2 100644 --- a/src/conf/virdomainsnapshotobj.c +++ b/src/conf/virdomainsnapshotobj.c @@ -121,3 +121,45 @@ virDomainSnapshotDropParent(virDomainSnapshotObjPtr sn= apshot) snapshot->parent =3D NULL; snapshot->sibling =3D NULL; } + + +/* Update @snapshot to no longer have children. */ +void +virDomainSnapshotDropChildren(virDomainSnapshotObjPtr snapshot) +{ + snapshot->nchildren =3D 0; + snapshot->first_child =3D NULL; +} + + +/* Add @snapshot to @parent's list of children. */ +void +virDomainSnapshotSetParent(virDomainSnapshotObjPtr snapshot, + virDomainSnapshotObjPtr parent) +{ + snapshot->parent =3D parent; + parent->nchildren++; + snapshot->sibling =3D parent->first_child; + parent->first_child =3D snapshot; +} + + +/* Take all children of @from and convert them into children of @to. */ +void +virDomainSnapshotMoveChildren(virDomainSnapshotObjPtr from, + virDomainSnapshotObjPtr to) +{ + virDomainSnapshotObjPtr child; + virDomainSnapshotObjPtr last; + + for (child =3D from->first_child; child; child =3D child->sibling) { + child->parent =3D to; + if (!child->sibling) + last =3D child; + } + to->nchildren +=3D from->nchildren; + last->sibling =3D to->first_child; + to->first_child =3D from->first_child; + from->nchildren =3D 0; + from->first_child =3D NULL; +} diff --git a/src/conf/virdomainsnapshotobjlist.c b/src/conf/virdomainsnapsh= otobjlist.c index 1eecb89a5d..9538521ab3 100644 --- a/src/conf/virdomainsnapshotobjlist.c +++ b/src/conf/virdomainsnapshotobjlist.c @@ -72,7 +72,7 @@ virDomainSnapshotObjListParse(const char *xmlStr, _("incorrect flags for bulk parse")); return -1; } - if (snapshots->metaroot.nchildren || snapshots->current) { + if (virDomainSnapshotObjListSize(snapshots)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("bulk define of snapshots only possible with " "no existing snapshot")); @@ -143,9 +143,7 @@ virDomainSnapshotObjListParse(const char *xmlStr, if (ret < 0) { /* There were no snapshots before this call; so on error, just * blindly delete anything created before the failure. */ - virHashRemoveAll(snapshots->objs); - snapshots->metaroot.nchildren =3D 0; - snapshots->metaroot.first_child =3D NULL; + virDomainSnapshotObjListRemoveAll(snapshots); } xmlXPathFreeContext(ctxt); xmlFreeDoc(xml); @@ -437,6 +435,14 @@ virDomainSnapshotFindByName(virDomainSnapshotObjListPt= r snapshots, } +/* Return the number of objects currently in the list */ +int +virDomainSnapshotObjListSize(virDomainSnapshotObjListPtr snapshots) +{ + return virHashSize(snapshots->objs); +} + + /* Return the current snapshot, or NULL */ virDomainSnapshotObjPtr virDomainSnapshotGetCurrent(virDomainSnapshotObjListPtr snapshots) @@ -484,6 +490,15 @@ bool virDomainSnapshotObjListRemove(virDomainSnapshotO= bjListPtr snapshots, return ret; } +/* Remove all snapshots tracked in the list */ +void +virDomainSnapshotObjListRemoveAll(virDomainSnapshotObjListPtr snapshots) +{ + virHashRemoveAll(snapshots->objs); + virDomainSnapshotDropChildren(&snapshots->metaroot); +} + + int virDomainSnapshotForEach(virDomainSnapshotObjListPtr snapshots, virHashIterator iter, @@ -511,28 +526,26 @@ virDomainSnapshotSetRelations(void *payload, virDomainSnapshotObjPtr obj =3D payload; struct snapshot_set_relation *curr =3D data; virDomainSnapshotObjPtr tmp; + virDomainSnapshotObjPtr parent; - obj->parent =3D virDomainSnapshotFindByName(curr->snapshots, - obj->def->parent); - if (!obj->parent) { + parent =3D virDomainSnapshotFindByName(curr->snapshots, obj->def->pare= nt); + if (!parent) { curr->err =3D -1; - obj->parent =3D &curr->snapshots->metaroot; + parent =3D &curr->snapshots->metaroot; VIR_WARN("snapshot %s lacks parent", obj->def->name); } else { - tmp =3D obj->parent; + tmp =3D parent; while (tmp && tmp->def) { if (tmp =3D=3D obj) { curr->err =3D -1; - obj->parent =3D &curr->snapshots->metaroot; + parent =3D &curr->snapshots->metaroot; VIR_WARN("snapshot %s in circular chain", obj->def->name); break; } tmp =3D tmp->parent; } } - obj->parent->nchildren++; - obj->sibling =3D obj->parent->first_child; - obj->parent->first_child =3D obj; + virDomainSnapshotSetParent(obj, parent); return 0; } @@ -545,8 +558,7 @@ virDomainSnapshotUpdateRelations(virDomainSnapshotObjLi= stPtr snapshots) { struct snapshot_set_relation act =3D { snapshots, 0 }; - snapshots->metaroot.nchildren =3D 0; - snapshots->metaroot.first_child =3D NULL; + virDomainSnapshotDropChildren(&snapshots->metaroot); virHashForEach(snapshots->objs, virDomainSnapshotSetRelations, &act); if (act.err) snapshots->current =3D NULL; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 72c5cef528..ffc1724850 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -980,9 +980,12 @@ virDomainObjListRename; # conf/virdomainsnapshotobj.h +virDomainSnapshotDropChildren; virDomainSnapshotDropParent; virDomainSnapshotForEachChild; virDomainSnapshotForEachDescendant; +virDomainSnapshotMoveChildren; +virDomainSnapshotSetParent; # conf/virdomainsnapshotobjlist.h diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6c71382b93..eb3d112b69 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15926,10 +15926,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, } else { other =3D virDomainSnapshotFindByName(vm->snapshots, snap->def->parent); - snap->parent =3D other; - other->nchildren++; - snap->sibling =3D other->first_child; - other->first_child =3D snap; + virDomainSnapshotSetParent(snap, other); } } else if (snap) { virDomainSnapshotObjListRemove(vm->snapshots, snap); @@ -16763,7 +16760,6 @@ struct _virQEMUSnapReparent { virCapsPtr caps; virDomainXMLOptionPtr xmlopt; int err; - virDomainSnapshotObjPtr last; }; @@ -16779,7 +16775,6 @@ qemuDomainSnapshotReparentChildren(void *payload, return 0; VIR_FREE(snap->def->parent); - snap->parent =3D rep->parent; if (rep->parent->def && VIR_STRDUP(snap->def->parent, rep->parent->def->name) < 0) { @@ -16787,9 +16782,6 @@ qemuDomainSnapshotReparentChildren(void *payload, return 0; } - if (!snap->sibling) - rep->last =3D snap; - rep->err =3D qemuDomainSnapshotWriteMetadata(rep->vm, snap, rep->caps, rep->xmlopt, rep->cfg->snapshotDir); @@ -16877,7 +16869,6 @@ qemuDomainSnapshotDelete(virDomainSnapshotPtr snaps= hot, rep.parent =3D snap->parent; rep.vm =3D vm; rep.err =3D 0; - rep.last =3D NULL; rep.caps =3D driver->caps; rep.xmlopt =3D driver->xmlopt; virDomainSnapshotForEachChild(snap, @@ -16885,15 +16876,11 @@ qemuDomainSnapshotDelete(virDomainSnapshotPtr sna= pshot, &rep); if (rep.err < 0) goto endjob; - /* Can't modify siblings during ForEachChild, so do it now. */ - snap->parent->nchildren +=3D snap->nchildren; - rep.last->sibling =3D snap->parent->first_child; - snap->parent->first_child =3D snap->first_child; + virDomainSnapshotMoveChildren(snap, snap->parent); } if (flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY) { - snap->nchildren =3D 0; - snap->first_child =3D NULL; + virDomainSnapshotDropChildren(snap); ret =3D 0; } else { ret =3D qemuDomainSnapshotDiscard(driver, vm, snap, true, metadata= _only); diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 9cbef70f1c..d3b76bfdbd 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -6416,10 +6416,7 @@ testDomainSnapshotCreateXML(virDomainPtr domain, virDomainSnapshotSetCurrent(vm->snapshots, snap); other =3D virDomainSnapshotFindByName(vm->snapshots, snap->def->parent); - snap->parent =3D other; - other->nchildren++; - snap->sibling =3D other->first_child; - other->first_child =3D snap; + virDomainSnapshotSetParent(snap, other); } virDomainObjEndAPI(&vm); } @@ -6454,7 +6451,6 @@ struct _testSnapReparentData { virDomainSnapshotObjPtr parent; virDomainObjPtr vm; int err; - virDomainSnapshotObjPtr last; }; static int @@ -6469,7 +6465,6 @@ testDomainSnapshotReparentChildren(void *payload, return 0; VIR_FREE(snap->def->parent); - snap->parent =3D rep->parent; if (rep->parent->def && VIR_STRDUP(snap->def->parent, rep->parent->def->name) < 0) { @@ -6477,8 +6472,6 @@ testDomainSnapshotReparentChildren(void *payload, return 0; } - if (!snap->sibling) - rep->last =3D snap; return 0; } @@ -6515,22 +6508,17 @@ testDomainSnapshotDelete(virDomainSnapshotPtr snaps= hot, rep.parent =3D snap->parent; rep.vm =3D vm; rep.err =3D 0; - rep.last =3D NULL; virDomainSnapshotForEachChild(snap, testDomainSnapshotReparentChildren, &rep); if (rep.err < 0) goto cleanup; - /* Can't modify siblings during ForEachChild, so do it now. */ - snap->parent->nchildren +=3D snap->nchildren; - rep.last->sibling =3D snap->parent->first_child; - snap->parent->first_child =3D snap->first_child; + virDomainSnapshotMoveChildren(snap, snap->parent); } if (flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY) { - snap->nchildren =3D 0; - snap->first_child =3D NULL; + virDomainSnapshotDropChildren(snap); } else { virDomainSnapshotDropParent(snap); if (snap =3D=3D virDomainSnapshotGetCurrent(vm->snapshots)) { --=20 2.20.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list