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

[libvirt] [PATCH 3/6] virsh: make tree listing more flexible



Requiring the user to pass in parallel arrays of names and parents
is annoying; it means that you can't qsort one of the arrays without
invalidating the ordering of the other.  By refactoring this function
to use callbacks, we isolate the layout to be independent of the
printing, and a future patch can exploit that to improve layout.

* tools/virsh.c (vshTreePrintInternal): Use callbacks rather than
requiring a char** array.
(vshTreeArrayLookup): New helper function.
(vshTreePrint, cmdNodeListDevices, cmdSnapshotList): Update callers.
---
 tools/virsh.c |   50 ++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 38 insertions(+), 12 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index 0d67f4b..85e618c 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -13166,10 +13166,15 @@ cmdVersion(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
 }

 /* Tree listing helpers.  */
+
+/* Given an index, return either the name of that device (non-NULL) or
+ * of its parent (NULL if a root).  */
+typedef const char * (*vshTreeLookup)(int devid, bool parent, void *opaque);
+
 static int
 vshTreePrintInternal(vshControl *ctl,
-                     char **devices,
-                     char **parents,
+                     vshTreeLookup lookup,
+                     void *opaque,
                      int num_devices,
                      int devid,
                      int lastdev,
@@ -13179,13 +13184,14 @@ vshTreePrintInternal(vshControl *ctl,
     int i;
     int nextlastdev = -1;
     int ret = -1;
+    const char *dev = (lookup)(devid, false, opaque);

     if (virBufferError(indent))
         goto cleanup;

     /* Print this device, with indent if not at root */
     vshPrint(ctl, "%s%s%s\n", virBufferCurrentContent(indent),
-             root ? "" : "+- ", devices[devid]);
+             root ? "" : "+- ", dev);

     /* Update indent to show '|' or ' ' for child devices */
     if (!root) {
@@ -13197,10 +13203,10 @@ vshTreePrintInternal(vshControl *ctl,

     /* Determine the index of the last child device */
     for (i = 0 ; i < num_devices ; i++) {
-        if (parents[i] &&
-            STREQ(parents[i], devices[devid])) {
+        const char *parent = (lookup)(i, true, opaque);
+
+        if (parent && STREQ(parent, dev))
             nextlastdev = i;
-        }
     }

     /* If there is a child device, then print another blank line */
@@ -13210,8 +13216,10 @@ vshTreePrintInternal(vshControl *ctl,
     /* Finally print all children */
     virBufferAddLit(indent, "  ");
     for (i = 0 ; i < num_devices ; i++) {
-        if (parents[i] && STREQ(parents[i], devices[devid]) &&
-            vshTreePrintInternal(ctl, devices, parents,
+        const char *parent = (lookup)(i, true, opaque);
+
+        if (parent && STREQ(parent, dev) &&
+            vshTreePrintInternal(ctl, lookup, opaque,
                                  num_devices, i, nextlastdev,
                                  false, indent) < 0)
             goto cleanup;
@@ -13231,13 +13239,13 @@ cleanup:
 }

 static int
-vshTreePrint(vshControl *ctl, char **devices, char **parents,
+vshTreePrint(vshControl *ctl, vshTreeLookup lookup, void *opaque,
              int num_devices, int devid)
 {
     int ret;
     virBuffer indent = VIR_BUFFER_INITIALIZER;

-    ret = vshTreePrintInternal(ctl, devices, parents, num_devices,
+    ret = vshTreePrintInternal(ctl, lookup, opaque, num_devices,
                                devid, devid, true, &indent);
     if (ret < 0)
         vshError(ctl, "%s", _("Failed to complete tree listing"));
@@ -13245,6 +13253,20 @@ vshTreePrint(vshControl *ctl, char **devices, char **parents,
     return ret;
 }

+struct vshTreeArray {
+    char **names;
+    char **parents;
+};
+
+static const char *
+vshTreeArrayLookup(int devid, bool parent, void *opaque)
+{
+    struct vshTreeArray *arrays = opaque;
+    if (parent)
+        return arrays->parents[devid];
+    return arrays->names[devid];
+}
+
 /*
  * "nodedev-list" command
  */
@@ -13294,6 +13316,7 @@ cmdNodeListDevices(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
     qsort(&devices[0], num_devices, sizeof(char*), namesorter);
     if (tree) {
         char **parents = vshMalloc(ctl, sizeof(char *) * num_devices);
+        struct vshTreeArray arrays = { devices, parents };

         for (i = 0; i < num_devices; i++) {
             virNodeDevicePtr dev = virNodeDeviceLookupByName(ctl->conn, devices[i]);
@@ -13307,7 +13330,8 @@ cmdNodeListDevices(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
         }
         for (i = 0 ; i < num_devices ; i++) {
             if (parents[i] == NULL &&
-                vshTreePrint(ctl, devices, parents, num_devices, i) < 0)
+                vshTreePrint(ctl, vshTreeArrayLookup, &arrays, num_devices,
+                             i) < 0)
                 ret = false;
         }
         for (i = 0 ; i < num_devices ; i++) {
@@ -16756,10 +16780,12 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
         }
     }
     if (tree) {
+        struct vshTreeArray arrays = { names, parents };
+
         for (i = 0 ; i < actual ; i++) {
             if ((from && ctl->useSnapshotOld) ? STREQ(names[i], from) :
                 !parents[i] &&
-                vshTreePrint(ctl, names, parents, actual, i) < 0)
+                vshTreePrint(ctl, vshTreeArrayLookup, &arrays, actual, i) < 0)
                 goto cleanup;
         }

-- 
1.7.10.2


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