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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]