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

[libvirt] [PATCHv2 9/9] virsh: add support for virConnectListAllDomains and clean up cmdList



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 function vshDomList was added 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. The helper
function also simulates filtering of some of the vlags supported by
virConnectListAllDomains().

This patch also cleans up the "list" command handler to use the new
helpers.
---
Diff to v1:
- moved filtering to the helper function
- cleaned up flag handling in cmdList
---
 tools/virsh.c |  469 ++++++++++++++++++++++++++++++++++++---------------------
 1 files changed, 297 insertions(+), 172 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index abcfbff..f258f59 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -64,6 +64,7 @@
 #include "util/bitmap.h"
 #include "conf/domain_conf.h"
 #include "virtypedparam.h"
+#include "intprops.h"

 static char *progname;

@@ -490,22 +491,252 @@ _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 inactive = (unsigned int) -1;
+
+    if (ida == inactive && idb == inactive)
+        return strcasecmp(virDomainGetName(*da), virDomainGetName(*db));
+
+    if (ida != inactive && idb != inactive) {
+        if (ida > idb)
+            return 1;
+        else if (ida < idb)
+            return -1;
+    }
+
+    if (ida != inactive)
+        return -1;
+    else
+        return 1;
+}
+
+struct vshDomainList {
+    virDomainPtr *domains;
+    int ndomains;
+};
+typedef struct vshDomainList *vshDomainListPtr;
+
+/* return 0 if false, 1 if true -1 if error */
+typedef int (*vshDomainListFilter)(vshControl *ctl, virDomainPtr dom);
+
+/* collection of filters */
+static int
+vshDomListPersistFilter(vshControl *ctl, virDomainPtr dom)
+{
+    int persistent = virDomainIsPersistent(dom);
+    if (persistent < 0)
+        vshError(ctl, "%s", _("Failed to determine domain's persistent state"));
+    return persistent;
+}
+
+static int
+vshDomListFilter(vshControl *ctl, vshDomainListPtr domlist,
+                 vshDomainListFilter filter, bool expect_result)
+{
+    int i = 0;
+    int ret;
+
+    while (i < domlist->ndomains) {
+        ret = filter(ctl, domlist->domains[i]);
+        if (ret < 0)
+            return -1;
+
+        if (!ret == !expect_result) {
+            virDomainFree(domlist->domains[i]);
+            if (i != --domlist->ndomains)
+                memmove(&domlist->domains[i],
+                        &domlist->domains[i+1],
+                        sizeof(*domlist->domains) * (domlist->ndomains - i));
+
+            if (VIR_REALLOC_N(domlist->domains, domlist->ndomains) < 0) {
+                ; /* not fatal */
+            }
+            continue;
+        }
+        i++;
+    }
+    return 0;
+}
+
+static int
+vshDomList(vshControl *ctl, vshDomainListPtr domlist, unsigned int flags)
+{
+    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;
+
+    /* try the list with flags support */
+    if ((ndoms = virConnectListAllDomains(ctl->conn, &doms, flags)) >= 0) {
+        domlist->ndomains = ndoms;
+        domlist->domains = doms;
+        doms = NULL;
+        goto finished;
+    }
+
+    /* check if the command is actualy supported or it was an error */
+    if ((err = virGetLastError()) &&
+        !(err->code == VIR_ERR_NO_SUPPORT ||
+          err->code == VIR_ERR_INVALID_ARG)) {
+        vshError(ctl, "%s", _("Failed to list domains"));
+        goto cleanup;
+    }
+
+    /* fall back to old method */
+    virResetLastError();
+
+    /* list active domains, if necessary */
+    if (!(flags &
+         (VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+          VIR_CONNECT_LIST_DOMAINS_INACTIVE)) ||
+        flags & VIR_CONNECT_LIST_DOMAINS_ACTIVE) {
+
+        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 (!(flags &
+         (VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+          VIR_CONNECT_LIST_DOMAINS_INACTIVE)) ||
+        flags & VIR_CONNECT_LIST_DOMAINS_INACTIVE) {
+
+        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));
+    ndoms = 0;
+
+    /* 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;
+    }
+
+    /* filter list the list if the list was acquired by fallback means */
+    domlist->domains = doms;
+    domlist->ndomains = ndoms;
+    doms = NULL;
+
+    /* filter by active state */
+    /* already done while getting the list by fallback means */
+
+    /* filter by persistent state */
+    if (!(flags & VIR_CONNECT_LIST_DOMAINS_PERSISTENT &&
+          flags & VIR_CONNECT_LIST_DOMAINS_TRANSIENT)) {
+        /* remove transient domains */
+        if (flags & VIR_CONNECT_LIST_DOMAINS_PERSISTENT &&
+            vshDomListFilter(ctl, domlist, vshDomListPersistFilter, false) < 0)
+            goto cleanup;
+
+        /* remove persistent domains */
+        if (flags & VIR_CONNECT_LIST_DOMAINS_TRANSIENT &&
+            vshDomListFilter(ctl, domlist, vshDomListPersistFilter, true) < 0)
+            goto cleanup;
+    }
+
+    /* reject all other filters - they are not needed by now */
+    if (flags &
+        ~(VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+          VIR_CONNECT_LIST_DOMAINS_INACTIVE |
+          VIR_CONNECT_LIST_DOMAINS_PERSISTENT |
+          VIR_CONNECT_LIST_DOMAINS_TRANSIENT)) {
+        vshError(ctl, "%s", _("Filter flags unsupported in fallback filter"));
+        goto cleanup;
+    }
+
+finished:
+    qsort(domlist->domains, domlist->ndomains,
+          sizeof(*domlist->domains), domsorter);
+
+    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 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 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);
 }

 static double
@@ -990,35 +1221,32 @@ static const vshCmdOptDef opts_list[] = {
 static bool
 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");
     bool optUUID = vshCommandOptBool(cmd, "uuid");
     bool optName = vshCommandOptBool(cmd, "name");
-    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)];
+    unsigned int flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE;

-    inactive |= all;
+    if (vshCommandOptBool(cmd, "inactive"))
+        flags = VIR_CONNECT_LIST_DOMAINS_INACTIVE;

-    /* process arguments */
-    if (!optPersistent && !optTransient) {
-        optPersistent = true;
-        optTransient = true;
-        persistUsed = false;
-    }
+    if (vshCommandOptBool(cmd, "all"))
+        flags = VIR_CONNECT_LIST_DOMAINS_INACTIVE |
+                VIR_CONNECT_LIST_DOMAINS_ACTIVE;
+
+    if (vshCommandOptBool(cmd, "persistent"))
+        flags |= VIR_CONNECT_LIST_DOMAINS_PERSISTENT;
+
+    if (vshCommandOptBool(cmd, "transient"))
+        flags |= VIR_CONNECT_LIST_DOMAINS_TRANSIENT;

     if (optTable + optName + optUUID > 1) {
         vshError(ctl, "%s",
@@ -1033,167 +1261,64 @@ 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 ((maxid = virConnectListDomains(ctl->conn, ids, maxid)) < 0) {
-                vshError(ctl, "%s", _("Failed to list active domains"));
-                goto cleanup;
-            }
-
-            qsort(ids, maxid, sizeof(int), idsorter);
-        }
-    }
-
-    if (inactive) {
-        maxname = virConnectNumOfDefinedDomains(ctl->conn);
-        if (maxname < 0) {
-            vshError(ctl, "%s", _("Failed to list inactive domains"));
-            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 (vshDomList(ctl, &list, flags) < 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)
-            continue;
+    for (i = 0; i < list.ndomains; i++) {
+        if (virDomainGetID(list.domains[i]) != (unsigned int) -1)
+            snprintf(id, sizeof(id), "%d", virDomainGetID(list.domains[i]));
+        else
+            virStrncpy(id, "-", sizeof(id));

-        if (persistUsed) {
-            persistent = virDomainIsPersistent(dom);
-            if (persistent < 0) {
-                vshError(ctl, "%s",
-                         _("Failed to determine domain's persistent state"));
-                goto cleanup;
-            }
+        state = vshDomainState(ctl, list.domains[i], NULL);
+        if (optTable && managed && state == VIR_DOMAIN_SHUTOFF &&
+            virDomainHasManagedSaveImage(list.domains[i], 0) > 0)
+            state = -2;

-            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;
+        if (optTable) {
+            if (optTitle) {
+                if (!(title = vshGetDomainDescription(ctl, list.domains[i], true, 0)))
+                    goto cleanup;

-            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]);
-           }
+                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


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