[libvirt] [PATCH 5/6] snapshot: new virsh function factored from snapshot-list

[Eric Blake]

On 06/12/2012 05:27 PM, Eric Blake wrote:

> I'm off on my versioning - this was 0.9.6 and earlier.
> 
>>      if (from) {
> 
> There's multiple things going on here:
> 
> First, the valid combinations:

> 
> Then the API that satisfy those combinations:
> 

> I really need to list this as a comment.  I'll post a v2, adding
> comments and including your improvements.

Here's what I'll squash for my v2:

diff --git i/tools/virsh.c w/tools/virsh.c
index 768d189..537496f 100644
--- i/tools/virsh.c
+++ w/tools/virsh.c
@@ -16848,7 +16848,7 @@ vshSnapshotListCollect(vshControl *ctl,
virDomainPtr dom,
 {
     int i;
     char **names = NULL;
-    int count = 0;
+    int count = -1;
     bool descendants = false;
     bool roots = false;
     vshSnapshotListPtr snaplist = vshMalloc(ctl, sizeof(*snaplist));
@@ -16859,8 +16859,24 @@ vshSnapshotListCollect(vshControl *ctl,
virDomainPtr dom,

     /* 0.9.13 will be adding a new listing API.  */

-    /* This is the interface available in 0.9.12 and earlier,
-     * including fallbacks to 0.9.4 and earlier.  */
+    /* This uses the interfaces available in 0.8.0-0.9.6
+     * (virDomainSnapshotListNames, global list only) and in
+     * 0.9.7-0.9.12 (addition of virDomainSnapshotListChildrenNames
+     * for child listing, and new flags), as follows, with [*] by the
+     * combinations that need parent info (either for filtering
+     * purposes or for the resulting tree listing):
+     *                              old               new
+     * list                         global as-is      global as-is
+     * list --roots                *global + filter   global + flags
+     * list --from                 *global + filter   child + flags
+     * list --from --descendants   *global + filter   child + flags
+     * list --tree                 *global as-is     *global as-is
+     * list --tree --from          *global + filter  *child + flags
+     *
+     * Additionally, when --tree and --from are both used, from is
+     * added to the final list as the only element without a parent.
+     * Otherwise, --from does not appear in the final list.
+     */
     if (from) {
         fromname = virDomainSnapshotGetName(from);
         if (!fromname) {
@@ -16870,28 +16886,30 @@ vshSnapshotListCollect(vshControl *ctl,
virDomainPtr dom,
         descendants = (flags & VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS) != 0;
         if (tree)
             flags |= VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS;
-        count = ctl->useSnapshotOld ? -1 :
-            virDomainSnapshotNumChildren(from, flags);
-        if (count < 0) {
-            if (ctl->useSnapshotOld ||
-                last_error->code == VIR_ERR_NO_SUPPORT) {
-                /* We can emulate --from.  */
-                /* XXX can we also emulate --leaves? */
-                virFreeError(last_error);
-                last_error = NULL;
-                ctl->useSnapshotOld = true;
-                flags &= ~VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS;
-                count = virDomainSnapshotNum(dom, flags);
-            }
-        } else if (tree) {
-            count++;
+
+        /* First, determine if we can use the new child listing API.  */
+        if (!ctl->useSnapshotOld &&
+            (count = virDomainSnapshotNumChildren(from, flags) < 0) &&
+            last_error->code == VIR_ERR_NO_SUPPORT) {
+            /* We can emulate --from.  */
+            /* XXX can we also emulate --leaves? */
+            virFreeError(last_error);
+            last_error = NULL;
+            ctl->useSnapshotOld = true;
+            flags &= ~VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS;
         }
-    } else {
+        if (tree && count >= 0)
+            count++;
+    }
+
+    /* Global listing (including fallback when --from failed with
+     * child listing).  */
+    if (count < 0) {
         count = virDomainSnapshotNum(dom, flags);

         /* Fall back to simulation if --roots was unsupported. */
         /* XXX can we also emulate --leaves? */
-        if (count < 0 && last_error->code == VIR_ERR_INVALID_ARG &&
+        if (!from && count < 0 && last_error->code ==
VIR_ERR_INVALID_ARG &&
             (flags & VIR_DOMAIN_SNAPSHOT_LIST_ROOTS)) {
             virFreeError(last_error);
             last_error = NULL;
@@ -16903,7 +16921,7 @@ vshSnapshotListCollect(vshControl *ctl,
virDomainPtr dom,

     if (count < 0) {
         if (!last_error)
-            vshError(ctl, _("missing support"));
+            vshError(ctl, _("failed to collect snapshot list"));
         goto cleanup;
     }

@@ -16912,6 +16930,7 @@ vshSnapshotListCollect(vshControl *ctl,
virDomainPtr dom,

     names = vshCalloc(ctl, sizeof(*names), count);

+    /* Now that we have a count, collect the list.  */
     if (from && !ctl->useSnapshotOld) {
         if (tree) {
             if (count)
@@ -16940,6 +16959,10 @@ vshSnapshotListCollect(vshControl *ctl,
virDomainPtr dom,
             goto cleanup;
     }

+    /* 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)) {
         for (i = (from && !ctl->useSnapshotOld); i < count; i++) {
             if (from && ctl->useSnapshotOld && STREQ(names[i], fromname)) {

-- 
Eric Blake   eblake at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120612/2e4028ce/attachment-0001.sig>