virsh: Add snapshot-list --topological

For snapshots, virsh already has a (shockingly naive [1]) client-side
topological sorter with the --tree option. But as a series of REDEFINE
calls must be presented in topological order, it's worth letting the
server do the work for us, especially since the server can give us a
topological sorting with less effort than our naive client
reconstruction.

[1] The XXX comment in virshSnapshotListCollect() about --tree being
O(n^3) is telling; https://en.wikipedia.org/wiki/Topological_sorting
is an interesting resource describing Kahn's algorithm and other
approaches for O(n) topological sorting for anyone motivated to use a
more elegant algorithm than brute force - but that doesn't affect this
patch.

For now, I am purposefully NOT implementing virsh fallback code to
provide a topological sort when the flag was rejected as unsupported;
we can worry about that down the road if users actually demonstrate
that they use new virsh but old libvirt to even need the fallback.
(The code we use for --tree could be repurposed to be such a fallback,
whether or not we keep it naive or improve it to be faster - but
again, no one should spend time on a fallback without evidence that we
need it.)

The test driver makes it easy to test:
$ virsh -c test:///default '
snapshot-create-as test a
snapshot-create-as test c
snapshot-create-as test b
snapshot-list test
snapshot-list test --topological
snapshot-list test --descendants a
snapshot-list test --descendants a --topological
snapshot-list test --tree
snapshot-list test --tree --topological
'

Without any flags, virsh does client-side sorting alphabetically, and
lists 'b' before 'c' (even though 'c' is the parent of 'b'); with the
flag, virsh skips sorting, and you can now see that the server handed
back data in a correct ordering. As shown here with a simple linear
chain, there isn't any other possible ordering, so --tree mode doesn't
seem to care whether --topological is used.  But it is possible to
compose more complicated DAGs with multiple children to a parent
(representing reverting back to a snapshot then creating more
snapshots along those divergent execution timelines), where it is then
possible (but not guaranteed) that adding the --topological flag
changes the --tree output (the client-side --tree algorithm breaks
ties based on alphabetical sorting between two nodes that share the
same parent, while the --topological sort skips the client-side
alphabetical sort and ends up exposing the server's internal order for
siblings, whether that be historical creation order or dependent on a
random hash seed).  But even if the results differ, they will still be
topologically correct.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
This commit is contained in:
Eric Blake 2019-03-07 22:12:01 -06:00
parent e3e8fa1fb4
commit c615c14246
2 changed files with 17 additions and 4 deletions

View File

@ -1,7 +1,7 @@
/*
* virsh-snapshot.c: Commands to manage domain snapshot
*
* Copyright (C) 2005, 2007-2016 Red Hat, Inc.
* Copyright (C) 2005-2019 Red Hat, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@ -1351,8 +1351,9 @@ virshSnapshotListCollect(vshControl *ctl, virDomainPtr dom,
}
}
}
qsort(snaplist->snaps, snaplist->nsnaps, sizeof(*snaplist->snaps),
virshSnapSorter);
if (!(orig_flags & VIR_DOMAIN_SNAPSHOT_LIST_TOPOLOGICAL))
qsort(snaplist->snaps, snaplist->nsnaps, sizeof(*snaplist->snaps),
virshSnapSorter);
snaplist->nsnaps -= deleted;
VIR_STEAL_PTR(ret, snaplist);
@ -1451,6 +1452,10 @@ static const vshCmdOptDef opts_snapshot_list[] = {
.type = VSH_OT_BOOL,
.help = N_("list snapshot names only")
},
{.name = "topological",
.type = VSH_OT_BOOL,
.help = N_("sort list topologically rather than by name"),
},
{.name = NULL}
};
@ -1512,6 +1517,9 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
FILTER("external", EXTERNAL);
#undef FILTER
if (vshCommandOptBool(cmd, "topological"))
flags |= VIR_DOMAIN_SNAPSHOT_LIST_TOPOLOGICAL;
if (roots)
flags |= VIR_DOMAIN_SNAPSHOT_LIST_ROOTS;

View File

@ -4726,7 +4726,7 @@ Output basic information about a named <snapshot>, or the current snapshot
with I<--current>.
=item B<snapshot-list> I<domain> [I<--metadata>] [I<--no-metadata>]
[{I<--parent> | I<--roots> | [{I<--tree> | I<--name>}]}]
[{I<--parent> | I<--roots> | [{I<--tree> | I<--name>}]}] [I<--topological>]
[{[I<--from>] B<snapshot> | I<--current>} [I<--descendants>]]
[I<--leaves>] [I<--no-leaves>] [I<--inactive>] [I<--active>]
[I<--disk-only>] [I<--internal>] [I<--external>]
@ -4734,6 +4734,11 @@ with I<--current>.
List all of the available snapshots for the given domain, defaulting
to show columns for the snapshot name, creation time, and domain state.
Normally, table form output is sorted by snapshot name; using
I<--topological> instead sorts so that no child is listed before its
ancestors (although there may be more than one possible ordering with
this property).
If I<--parent> is specified, add a column to the output table giving
the name of the parent of each snapshot. If I<--roots> is specified,
the list will be filtered to just snapshots that have no parents.