[libvirt] [PATCH 2/6] virsh: remove limits on tree listing

Eric Blake eblake at redhat.com
Sat Jun 9 04:34:27 UTC 2012


I am not a fan of fixed-width buffers.  All it takes is a
linear chain of more than 100 snapshots to mess up 'virsh
snapshot-list --tree'.  Now that virBuffer is more powerful,
we might as well exploit its power.

* tools/virsh.c (cmdNodeListDevicesPrint): Simplify to use a
virBuffer instead of fixed-width prefix, factor guts, and rename...
(vshTreePrint, vshTreePrintInternal): ...along with new helper.
(cmdNodeListDevices, cmdSnapshotList): Update callers.
---
 tools/virsh.c |  172 ++++++++++++++++++++++++++-------------------------------
 1 file changed, 78 insertions(+), 94 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index b28dc49..0d67f4b 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -13165,60 +13165,34 @@ cmdVersion(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
     return true;
 }

-/*
- * "nodedev-list" command
- */
-static const vshCmdInfo info_node_list_devices[] = {
-    {"help", N_("enumerate devices on this host")},
-    {"desc", ""},
-    {NULL, NULL}
-};
-
-static const vshCmdOptDef opts_node_list_devices[] = {
-    {"tree", VSH_OT_BOOL, 0, N_("list devices in a tree")},
-    {"cap", VSH_OT_STRING, VSH_OFLAG_NONE, N_("capability name")},
-    {NULL, 0, 0, NULL}
-};
-
-#define MAX_DEPTH 100
-#define INDENT_SIZE 4
-#define INDENT_BUFLEN ((MAX_DEPTH * INDENT_SIZE) + 1)
-
-static void
-cmdNodeListDevicesPrint(vshControl *ctl,
-                        char **devices,
-                        char **parents,
-                        int num_devices,
-                        int devid,
-                        int lastdev,
-                        unsigned int depth,
-                        unsigned int indentIdx,
-                        char *indentBuf)
+/* Tree listing helpers.  */
+static int
+vshTreePrintInternal(vshControl *ctl,
+                     char **devices,
+                     char **parents,
+                     int num_devices,
+                     int devid,
+                     int lastdev,
+                     bool root,
+                     virBufferPtr indent)
 {
     int i;
     int nextlastdev = -1;
+    int ret = -1;

-    /* Prepare indent for this device, but not if at root */
-    if (depth && depth < MAX_DEPTH) {
-        indentBuf[indentIdx] = '+';
-        indentBuf[indentIdx+1] = '-';
-        indentBuf[indentIdx+2] = ' ';
-        indentBuf[indentIdx+3] = '\0';
-    }
-
-    /* Print this device */
-    vshPrint(ctl, "%s", indentBuf);
-    vshPrint(ctl, "%s\n", devices[devid]);
+    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]);

     /* Update indent to show '|' or ' ' for child devices */
-    if (depth && depth < MAX_DEPTH) {
-        if (devid == lastdev)
-            indentBuf[indentIdx] = ' ';
-        else
-            indentBuf[indentIdx] = '|';
-        indentBuf[indentIdx+1] = ' ';
-        indentIdx+=2;
+    if (!root) {
+        virBufferAddChar(indent, devid == lastdev ? ' ' : '|');
+        virBufferAddChar(indent, ' ');
+        if (virBufferError(indent))
+            goto cleanup;
     }

     /* Determine the index of the last child device */
@@ -13230,43 +13204,70 @@ cmdNodeListDevicesPrint(vshControl *ctl,
     }

     /* If there is a child device, then print another blank line */
-    if (nextlastdev != -1) {
-        vshPrint(ctl, "%s", indentBuf);
-        vshPrint(ctl, " |\n");
-    }
+    if (nextlastdev != -1)
+        vshPrint(ctl, "%s  |\n", virBufferCurrentContent(indent));

     /* Finally print all children */
-    if (depth < MAX_DEPTH)
-        indentBuf[indentIdx] = ' ';
+    virBufferAddLit(indent, "  ");
     for (i = 0 ; i < num_devices ; i++) {
-        if (depth < MAX_DEPTH) {
-            indentBuf[indentIdx] = ' ';
-            indentBuf[indentIdx+1] = ' ';
-        }
-        if (parents[i] &&
-            STREQ(parents[i], devices[devid]))
-            cmdNodeListDevicesPrint(ctl, devices, parents,
-                                    num_devices, i, nextlastdev,
-                                    depth + 1, indentIdx + 2, indentBuf);
-        if (depth < MAX_DEPTH)
-            indentBuf[indentIdx] = '\0';
+        if (parents[i] && STREQ(parents[i], devices[devid]) &&
+            vshTreePrintInternal(ctl, devices, parents,
+                                 num_devices, i, nextlastdev,
+                                 false, indent) < 0)
+            goto cleanup;
     }
+    virBufferTrim(indent, "  ", -1);

     /* If there was no child device, and we're the last in
      * a list of devices, then print another blank line */
-    if (nextlastdev == -1 && devid == lastdev) {
-        vshPrint(ctl, "%s", indentBuf);
-        vshPrint(ctl, "\n");
-    }
+    if (nextlastdev == -1 && devid == lastdev)
+        vshPrint(ctl, "%s\n", virBufferCurrentContent(indent));
+
+    if (!root)
+        virBufferTrim(indent, NULL, 2);
+    ret = 0;
+cleanup:
+    return ret;
+}
+
+static int
+vshTreePrint(vshControl *ctl, char **devices, char **parents,
+             int num_devices, int devid)
+{
+    int ret;
+    virBuffer indent = VIR_BUFFER_INITIALIZER;
+
+    ret = vshTreePrintInternal(ctl, devices, parents, num_devices,
+                               devid, devid, true, &indent);
+    if (ret < 0)
+        vshError(ctl, "%s", _("Failed to complete tree listing"));
+    virBufferFreeAndReset(&indent);
+    return ret;
 }

+/*
+ * "nodedev-list" command
+ */
+static const vshCmdInfo info_node_list_devices[] = {
+    {"help", N_("enumerate devices on this host")},
+    {"desc", ""},
+    {NULL, NULL}
+};
+
+static const vshCmdOptDef opts_node_list_devices[] = {
+    {"tree", VSH_OT_BOOL, 0, N_("list devices in a tree")},
+    {"cap", VSH_OT_STRING, VSH_OFLAG_NONE, N_("capability name")},
+    {NULL, 0, 0, NULL}
+};
+
 static bool
-cmdNodeListDevices (vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
+cmdNodeListDevices(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
 {
     const char *cap = NULL;
     char **devices;
     int num_devices, i;
     bool tree = vshCommandOptBool(cmd, "tree");
+    bool ret = true;

     if (!vshConnectionUsability(ctl, ctl->conn))
         return false;
@@ -13292,8 +13293,8 @@ cmdNodeListDevices (vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
     }
     qsort(&devices[0], num_devices, sizeof(char*), namesorter);
     if (tree) {
-        char indentBuf[INDENT_BUFLEN];
         char **parents = vshMalloc(ctl, sizeof(char *) * num_devices);
+
         for (i = 0; i < num_devices; i++) {
             virNodeDevicePtr dev = virNodeDeviceLookupByName(ctl->conn, devices[i]);
             if (dev && STRNEQ(devices[i], "computer")) {
@@ -13305,17 +13306,9 @@ cmdNodeListDevices (vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
             virNodeDeviceFree(dev);
         }
         for (i = 0 ; i < num_devices ; i++) {
-            memset(indentBuf, '\0', sizeof(indentBuf));
-            if (parents[i] == NULL)
-                cmdNodeListDevicesPrint(ctl,
-                                        devices,
-                                        parents,
-                                        num_devices,
-                                        i,
-                                        i,
-                                        0,
-                                        0,
-                                        indentBuf);
+            if (parents[i] == NULL &&
+                vshTreePrint(ctl, devices, parents, num_devices, i) < 0)
+                ret = false;
         }
         for (i = 0 ; i < num_devices ; i++) {
             VIR_FREE(devices[i]);
@@ -13329,7 +13322,7 @@ cmdNodeListDevices (vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
         }
     }
     VIR_FREE(devices);
-    return true;
+    return ret;
 }

 /*
@@ -16763,20 +16756,11 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
         }
     }
     if (tree) {
-        char indentBuf[INDENT_BUFLEN];
         for (i = 0 ; i < actual ; i++) {
-            memset(indentBuf, '\0', sizeof(indentBuf));
             if ((from && ctl->useSnapshotOld) ? STREQ(names[i], from) :
-                !parents[i])
-                cmdNodeListDevicesPrint(ctl,
-                                        names,
-                                        parents,
-                                        actual,
-                                        i,
-                                        i,
-                                        0,
-                                        0,
-                                        indentBuf);
+                !parents[i] &&
+                vshTreePrint(ctl, names, parents, actual, i) < 0)
+                goto cleanup;
         }

         ret = true;
-- 
1.7.10.2




More information about the libvir-list mailing list