[libvirt] [PATCH 5/5] virsh: add support for virConnectListAllDomains and clean up cmdList

Peter Krempa pkrempa at redhat.com
Sun May 20 15:56:26 UTC 2012


This patch makes use of the newly added api virConnectListAllDomains()
to list domains in virsh.

To enable fallback virsh now represents lists of domains using an
internal structure vshDomainList. This structure contains the
virDomainPtr list as provided by virConnectListAllDomains() and the
count of domains in the list.

For backwards compatiblity I've added function vshDomList that tries to
enumerate the domains using the new API and if the API is not supported
falls back to the older approach with the two list functions.

This patch also adds helpers to filter domain lists provided by
vshDomList().

As a last improvement I've cleaned up code that handles the "list"
command using the newly added helper and filter functions.
---
 tools/virsh.c |  423 +++++++++++++++++++++++++++++++++++++--------------------
 1 files changed, 273 insertions(+), 150 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index 46239fa..7f3f9ac 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -63,6 +63,7 @@
 #include "util/bitmap.h"
 #include "conf/domain_conf.h"
 #include "virtypedparam.h"
+#include "intprops.h"

 static char *progname;

@@ -489,22 +490,225 @@ _vshStrdup(vshControl *ctl, const char *s, const char *filename, int line)
 #define realloc use_vshRealloc_instead_of_realloc
 #define strdup use_vshStrdup_instead_of_strdup

-static int idsorter(const void *a, const void *b) {
-  const int *ia = (const int *)a;
-  const int *ib = (const int *)b;
+static int
+namesorter(const void *a, const void *b)
+{
+    const char **sa = (const char**)a;
+    const char **sb = (const char**)b;

-  if (*ia > *ib)
-    return 1;
-  else if (*ia < *ib)
-    return -1;
-  return 0;
+    /* User visible sort, so we want locale-specific case comparison.  */
+    return strcasecmp(*sa, *sb);
+}
+
+static int
+domsorter(const void *a, const void *b)
+{
+    virDomainPtr *da = (virDomainPtr *) a;
+    virDomainPtr *db = (virDomainPtr *) b;
+    unsigned int ida = virDomainGetID(*da);
+    unsigned int idb = virDomainGetID(*db);
+    unsigned int err = (unsigned int) -1;
+
+    if (ida == err && idb == err) {
+        if (virDomainGetName(*da) ||
+            virDomainGetName(*db))
+                return strcasecmp(virDomainGetName(*da),
+                                  virDomainGetName(*db));
+        else
+            return 0;
+    }
+
+    if (ida != err && idb != err) {
+        if (ida > idb)
+            return 1;
+        else if (ida < idb)
+            return -1;
+
+        return 0;
+    }
+
+    if (ida != err)
+        return -1;
+    else
+        return 1;
+}
+
+struct vshDomainList {
+    virDomainPtr *domains;
+    int ndomains;
+};
+typedef struct vshDomainList *vshDomainListPtr;
+
+static int
+vshDomList(vshControl *ctl, vshDomainListPtr domlist)
+{
+    int ret;
+    int rv = -1;
+    virErrorPtr err = NULL;
+    int i;
+    int *ids = NULL;
+    int nids = 0;
+    char **names = NULL;
+    int nnames = 0;
+    virDomainPtr dom;
+    virDomainPtr *doms = NULL;
+    int ndoms = 0;
+
+    domlist->domains = NULL;
+    domlist->ndomains = 0;
+
+    if ((ret = virConnectListAllDomains(ctl->conn,
+                                        &(domlist->domains),
+                                        -1, 0)) >= 0) {
+        domlist->ndomains = ret;
+        qsort(domlist->domains, domlist->ndomains,
+              sizeof(virDomainPtr), domsorter);
+        return ret;
+    }
+
+    if ((err = virGetLastError()) &&
+        err->code != VIR_ERR_NO_SUPPORT) {
+        vshError(ctl, "%s", _("Failed to list domains"));
+        goto cleanup;
+    }
+
+    /* fall back to old method */
+    virResetLastError();
+
+    if ((nids = virConnectNumOfDomains(ctl->conn)) < 0) {
+        vshError(ctl, "%s", _("Failed to list active domains"));
+        goto cleanup;
+    }
+
+    if (nids) {
+        ids = vshMalloc(ctl, sizeof(int) * nids);
+
+        if ((nids = virConnectListDomains(ctl->conn, ids, nids)) < 0) {
+            vshError(ctl, "%s", _("Failed to list active domains"));
+            goto cleanup;
+        }
+    }
+
+    if ((nnames = virConnectNumOfDefinedDomains(ctl->conn)) < 0) {
+        vshError(ctl, "%s", _("Failed to list inactive domains"));
+        goto cleanup;
+    }
+
+    if (nnames) {
+        names = vshMalloc(ctl, sizeof(char *) * nnames);
+
+        if ((nnames = virConnectListDefinedDomains(ctl->conn,
+                                                  names,
+                                                  nnames)) < 0) {
+            vshError(ctl, "%s", _("Failed to list inactive domains"));
+            goto cleanup;
+        }
+    }
+
+    doms = vshMalloc(ctl, sizeof(virDomainPtr) * (nids + nnames));
+
+    /* get active domains */
+    for (i = 0; i < nids; i++) {
+        if (!(dom = virDomainLookupByID(ctl->conn, ids[i])))
+            continue;
+
+        doms[ndoms++] = dom;
+    }
+
+    /* get inctive domains */
+    for (i = 0; i < nnames; i++) {
+        if (!(dom = virDomainLookupByName(ctl->conn, names[i])))
+            continue;
+
+        doms[ndoms++] = dom;
+    }
+
+    qsort(doms, ndoms, sizeof(virDomainPtr), domsorter);
+
+    domlist->domains = doms;
+    doms = NULL;
+    domlist->ndomains = ndoms;
+    rv = ndoms;
+
+cleanup:
+    for (i = 0; i < nnames; i++)
+        VIR_FREE(names[i]);
+    if (doms) {
+        for (i = 0; i < ndoms; i++) {
+            if (doms[i])
+                virDomainFree(doms[i]);
+        }
+    }
+    VIR_FREE(doms);
+    VIR_FREE(names);
+    VIR_FREE(ids);
+    return rv;
+}
+
+static void
+vshDomListFree(vshDomainListPtr domlist)
+{
+    int i;
+
+    if (!domlist || !domlist->domains)
+        return;
+
+    for (i = 0; i < domlist->ndomains; i++) {
+        if (domlist->domains[i])
+            virDomainFree(domlist->domains[i]);
+    }
+
+    domlist->ndomains = 0;
+    VIR_FREE(domlist->domains);
+}
+
+/* return 0 if false, 1 if true -1 if error */
+typedef int (*vshDomainListFilter)(vshControl *ctl, virDomainPtr dom);
+
+/* collection of filters */
+static int
+vshDomainListActiveFilter(vshControl *ctl ATTRIBUTE_UNUSED,
+                          virDomainPtr dom)
+{
+    return virDomainGetID(dom) != (unsigned int) -1;
 }
-static int namesorter(const void *a, const void *b) {
-  const char **sa = (const char**)a;
-  const char **sb = (const char**)b;

-  /* User visible sort, so we want locale-specific case comparison.  */
-  return strcasecmp(*sa, *sb);
+static int
+vshDomainListPersistentFilter(vshControl *ctl,
+                             virDomainPtr dom)
+{
+    int persistent = virDomainIsPersistent(dom);
+    if (persistent < 0) {
+        vshError(ctl, "%s",
+                 _("Failed to determine domain's persistent state"));
+        return -1;
+    }
+    return persistent;
+}
+
+static int
+vshDomListFreeFilter(vshControl *ctl,
+                     vshDomainListPtr domlist,
+                     vshDomainListFilter filter,
+                     bool expect_result)
+{
+    int i;
+    int ret;
+
+    for (i = 0; i < domlist->ndomains; i++) {
+        if (!domlist->domains[i])
+            continue;
+
+        ret = filter(ctl, domlist->domains[i]);
+        if (ret < 0)
+            return -1;
+
+        if (!ret == !expect_result) {
+            virDomainFree(domlist->domains[i]);
+            domlist->domains[i] = NULL;
+        }
+    }
+    return 0;
 }

 static double
@@ -936,9 +1140,6 @@ cmdList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
     bool inactive = vshCommandOptBool(cmd, "inactive");
     bool all = vshCommandOptBool(cmd, "all");
     bool active = !inactive || all;
-    int *ids = NULL, maxid = 0, i;
-    char **names = NULL;
-    int maxname = 0;
     bool managed = vshCommandOptBool(cmd, "managed-save");
     bool optTitle = vshCommandOptBool(cmd, "title");
     bool optTable = vshCommandOptBool(cmd, "table");
@@ -947,12 +1148,13 @@ cmdList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
     bool optPersistent = vshCommandOptBool(cmd, "persistent");
     bool optTransient = vshCommandOptBool(cmd, "transient");
     bool persistUsed = true;
-    virDomainPtr dom = NULL;
+    int i;
     char *title;
     char uuid[VIR_UUID_STRING_BUFLEN];
     int state;
-    int persistent;
     bool ret = false;
+    struct vshDomainList list;
+    char id[INT_BUFSIZE_BOUND(unsigned int)];

     inactive |= all;

@@ -976,167 +1178,88 @@ cmdList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
     if (!vshConnectionUsability(ctl, ctl->conn))
         return false;

-    if (active) {
-        maxid = virConnectNumOfDomains(ctl->conn);
-        if (maxid < 0) {
-            vshError(ctl, "%s", _("Failed to list active domains"));
-            return false;
-        }
-        if (maxid) {
-            ids = vshMalloc(ctl, sizeof(int) * maxid);
+    if (vshDomList(ctl, &list) < 0)
+        goto cleanup;

-            if ((maxid = virConnectListDomains(ctl->conn, ids, maxid)) < 0) {
-                vshError(ctl, "%s", _("Failed to list active domains"));
-                goto cleanup;
-            }
+    if (!active)
+        vshDomListFreeFilter(ctl, &list, vshDomainListActiveFilter, true);

-            qsort(ids, maxid, sizeof(int), idsorter);
-        }
-    }
+    if (!inactive)
+        vshDomListFreeFilter(ctl, &list, vshDomainListActiveFilter, false);

-    if (inactive) {
-        maxname = virConnectNumOfDefinedDomains(ctl->conn);
-        if (maxname < 0) {
-            vshError(ctl, "%s", _("Failed to list inactive domains"));
+    if (persistUsed) {
+        if (!optPersistent &&
+            vshDomListFreeFilter(ctl, &list,
+                                 vshDomainListPersistentFilter, true) < 0)
             goto cleanup;
-        }
-        if (maxname) {
-            names = vshMalloc(ctl, sizeof(char *) * maxname);

-            if ((maxname = virConnectListDefinedDomains(ctl->conn,
-                                                        names,
-                                                        maxname)) < 0) {
-                vshError(ctl, "%s", _("Failed to list inactive domains"));
-                goto cleanup;
-            }
-
-            qsort(&names[0], maxname, sizeof(char*), namesorter);
-        }
+        if (!optTransient &&
+            vshDomListFreeFilter(ctl, &list,
+                                 vshDomainListPersistentFilter, false) < 0)
+            goto cleanup;
     }

     /* print table header in legacy mode */
     if (optTable) {
-        if (optTitle) {
+        if (optTitle)
             vshPrintExtra(ctl, " %-5s %-30s %-10s %-20s\n%s\n",
                           _("Id"), _("Name"), _("State"), _("Title"),
                           "-----------------------------------------"
                           "-----------------------------------------");
-        } else {
+        else
             vshPrintExtra(ctl, " %-5s %-30s %s\n%s\n",
                           _("Id"), _("Name"), _("State"),
                           "-----------------------------------------"
                           "-----------");
-        }
     }

-    for (i = 0; i < maxid; i++) {
-         dom = virDomainLookupByID(ctl->conn, ids[i]);
-
-        /* this kind of work with domains is not atomic operation */
-        if (!dom)
+    for (i = 0; i < list.ndomains; i++) {
+        if (!list.domains[i])
             continue;

-        if (persistUsed) {
-            persistent = virDomainIsPersistent(dom);
-            if (persistent < 0) {
-                vshError(ctl, "%s",
-                         _("Failed to determine domain's persistent state"));
-                goto cleanup;
-            }
+        if (virDomainGetID(list.domains[i]) != (unsigned int) -1)
+            snprintf(id, sizeof(id), "%d", virDomainGetID(list.domains[i]));
+        else
+            strncpy(id, "-", sizeof(id));

-            if (!(persistent ? optPersistent : optTransient)) {
-                virDomainFree(dom);
-                dom = NULL;
-                continue;
-            }
-       }
-
-       if (optTable) {
-           if (optTitle) {
-               if (!(title = vshGetDomainDescription(ctl, dom, true, 0)))
-                   goto cleanup;
-
-               vshPrint(ctl, " %-5d %-30s %-10s %-20s\n",
-                        virDomainGetID(dom),
-                        virDomainGetName(dom),
-                        _(vshDomainStateToString(vshDomainState(ctl, dom, NULL))),
-                        title);
-               VIR_FREE(title);
-           } else {
-               vshPrint(ctl, " %-5d %-30s %s\n",
-                        virDomainGetID(dom),
-                        virDomainGetName(dom),
-                        _(vshDomainStateToString(vshDomainState(ctl, dom, NULL))));
-           }
-       } else if (optUUID) {
-           if (virDomainGetUUIDString(dom, uuid) < 0) {
-               vshError(ctl, "%s",
-                        _("Failed to get domain's UUID"));
-               goto cleanup;
-           }
-           vshPrint(ctl, "%s\n", uuid);
-       } else if (optName) {
-           vshPrint(ctl, "%s\n", virDomainGetName(dom));
-       }
-
-       virDomainFree(dom);
-       dom = NULL;
-    }
-
-    if (optPersistent) {
-        for (i = 0; i < maxname; i++) {
-            dom = virDomainLookupByName(ctl->conn, names[i]);
-
-            /* this kind of work with domains is not atomic operation */
-            if (!dom)
-                continue;
+        state = vshDomainState(ctl, list.domains[i], NULL);
+        if (managed && state == VIR_DOMAIN_SHUTOFF &&
+            virDomainHasManagedSaveImage(list.domains[i], 0) > 0)
+            state = -2;

-            if (optTable) {
-                state = vshDomainState(ctl, dom, NULL);
-                if (managed && state == VIR_DOMAIN_SHUTOFF &&
-                    virDomainHasManagedSaveImage(dom, 0) > 0)
-                    state = -2;
-
-                if (optTitle) {
-                    if (!(title = vshGetDomainDescription(ctl, dom, true, 0)))
-                        goto cleanup;
-
-                    vshPrint(ctl, " %-5s %-30s %-10s %-20s\n",
-                             "-",
-                             names[i],
-                             state == -2 ? _("saved") : _(vshDomainStateToString(state)),
-                             title);
-                    VIR_FREE(title);
-                } else {
-                    vshPrint(ctl, " %-5s %-30s %s\n",
-                             "-",
-                             names[i],
-                             state == -2 ? _("saved") : _(vshDomainStateToString(state)));
-                }
-           } else if (optUUID) {
-               if (virDomainGetUUIDString(dom, uuid) < 0) {
-                   vshError(ctl, "%s",
-                            _("Failed to get domain's UUID"));
-                   goto cleanup;
-               }
-               vshPrint(ctl, "%s\n", uuid);
-           } else if (optName) {
-               vshPrint(ctl, "%s\n", names[i]);
-           }
+        if (optTable) {
+            if (optTitle) {
+                if (!(title = vshGetDomainDescription(ctl, list.domains[i], true, 0)))
+                    goto cleanup;
+
+                vshPrint(ctl, " %-5s %-30s %-10s %-20s\n",
+                         id,
+                         virDomainGetName(list.domains[i]),
+                         state == -2 ? _("saved") : _(vshDomainStateToString(state)),
+                         title);

-           virDomainFree(dom);
-           dom = NULL;
+                VIR_FREE(title);
+            } else {
+                vshPrint(ctl, " %-5s %-30s %s\n",
+                         id,
+                         virDomainGetName(list.domains[i]),
+                         state == -2 ? _("saved") : _(vshDomainStateToString(state)));
+            }
+        } else if (optUUID) {
+            if (virDomainGetUUIDString(list.domains[i], uuid) < 0) {
+                vshError(ctl, "%s",
+                         _("Failed to get domain's UUID"));
+                goto cleanup;
+            }
+            vshPrint(ctl, "%s\n", uuid);
+        } else if (optName) {
+            vshPrint(ctl, "%s\n", virDomainGetName(list.domains[i]));
         }
     }

     ret = true;
 cleanup:
-    if (dom)
-        virDomainFree(dom);
-    VIR_FREE(ids);
-    for (i = 0; i < maxname; i++)
-        VIR_FREE(names[i]);
-    VIR_FREE(names);
+    vshDomListFree(&list);
     return ret;
 }

-- 
1.7.3.4




More information about the libvir-list mailing list