[libvirt] [PATCHv2 01/13] snapshot: new virsh function factored from snapshot-list

Peter Krempa pkrempa at redhat.com
Fri Jun 15 09:16:03 UTC 2012


On 06/15/12 06:18, Eric Blake wrote:
> This patch is based on the fallback code out of cmdSnapshotList,
> with tweaks to keep the snapshot objects around rather than just
> their name, and to remove unwanted elements before returning.
> It looks forward to a future patch when we add a way to list all
> snapshot objects at once, and the next patch will simplify
> cmdSnapshotList to take advantage of this factorization.
>
> * tools/virsh.c (vshSnapshotList, vshSnapshotListFree): New functions.
> ---
>
> v2: fix lots of stupid bugs, such as a qsort callback that returned
> 1 where -1 was meant and led to a segv.  This no longer resembles
> the code from cmdSnapshotList quite as nicely, but I'm more confident
> that I have tested it across multiple server versions and all possible
> combinations of command line options, and across several snapshot
> hierarchies (linear list, all roots, random tree).
>
>   tools/virsh.c |  278 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 278 insertions(+)
>
> diff --git a/tools/virsh.c b/tools/virsh.c
> index e3cf159..ac748d9 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
>

> +    /* Collect parents when needed.  With the new API, --tree and
> +     * --from together put from as the first element without a parent;
> +     * with the old API we still need to do a post-process filtering
> +     * based on all parent information.  */
> +    if (tree || (from && ctl->useSnapshotOld) || roots) {
> +        for (i = (from && !ctl->useSnapshotOld); i < count; i++) {

That initialization of "i" isn't intuitive on the first read, but It's a 
correct optimalisation.


ACK.

Peter




More information about the libvir-list mailing list