Re: [libvirt] [PATCH 07/16] snapshot: Add accessors for updating snapshot list relations

Eric Blake <eblake(a)redhat.com&gt; [2019-03-21, 01:03PM -0500]: > On 3/20/19 4:28 PM, John Ferlan wrote: > > > > > > On 3/20/19 1:40 AM, Eric Blake wrote: > >> 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. > > > > Understatement ;-) Some black magic occurs > > > > The qemuDomainSnapshotReparentChildren "snap->parent = rep->parent" gets > > replaced by the new for loop... Tough to see without watching really > > closely. > > > > >> +/* Take all children of @from and convert them into children of @to. */ > >> +void > >> +virDomainSnapshotMoveChildren(virDomainSnapshotObjPtr from, > >> + virDomainSnapshotObjPtr to) > >> +{ > >> + virDomainSnapshotObjPtr child; > >> + virDomainSnapshotObjPtr last; > >> + > >> + for (child = from->first_child; child; child = child->sibling) { > >> + child->parent = to; > >> + if (!child->sibling) > >> + last = child; > >> + } > >> + to->nchildren += from->nchildren; > >> + last->sibling = to->first_child; > > > > Silly Coverity compiler gets quite confused thinking that @last couldn't > > be set while not considering the above loop couldn't end without it > > unless of course from->first_child == NULL I suppose, which would be a > > different issue. Still if before the for loop we check "if > > (!from->first_child) return;", then coverity is happy. > > Good find from Coverity. If there are no children to move, I do need the > early exit, so I'll squash that in. Did you forget this? Function is pushed in this (broken?) version and I get a warning/error on GCC 8.0.1.

-- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list