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

[libvirt] [PATCH 2/7] virsh-domain: Refactor cmdVcpucount and fix output on inactive domains



This patch factors out the vCPU count retrieval including fallback means into
vshCPUCountCollect() and removes the duplicated code to retrieve individual
counts.

The --current flag (this flag is assumed by default) now works also with
--maximum or --active without the need to explicitly specify the state of the
domain that is requested.

This patch also fixes the output of "virsh vcpucount domain" on inactive
domains:

Before:
$ virsh vcpucount domain
maximum      config         4
error: Requested operation is not valid: domain is not running
current      config         4
error: Requested operation is not valid: domain is not running

After:
$virsh vcpucount domain
maximum      config         4
current      config         4
---
 tools/virsh-domain.c | 258 +++++++++++++++++++++++----------------------------
 1 file changed, 115 insertions(+), 143 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 4d0cc8f..5722d69 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -5066,7 +5066,7 @@ static const vshCmdOptDef opts_vcpucount[] = {
     },
     {.name = "maximum",
      .type = VSH_OT_BOOL,
-     .help = N_("get maximum cap on vcpus")
+     .help = N_("get maximum count of vcpus")
     },
     {.name = "active",
      .type = VSH_OT_BOOL,
@@ -5087,36 +5087,103 @@ static const vshCmdOptDef opts_vcpucount[] = {
     {.name = NULL}
 };

+/**
+ * Collect the number of vCPUs for a guest possibly with fallback means.
+ *
+ * Returns the count of vCPUs for a domain and certain flags. Returns -2 in case
+ * of error. In case live stats can't be collected when the domain is inactive,
+ * -1 is returned and no error is reported.
+ */
+
+static int
+vshCPUCountCollect(vshControl *ctl,
+                   virDomainPtr dom,
+                   unsigned int flags)
+{
+    int ret = -2;
+    virDomainInfo info;
+    int count;
+    char *def = NULL;
+    xmlDocPtr xml = NULL;
+    xmlXPathContextPtr ctxt = NULL;
+
+    if (flags & VIR_DOMAIN_AFFECT_LIVE &&
+        virDomainIsActive(dom) < 1)
+        return -1;
+
+    /* In all cases, try the new API first; if it fails because we are talking
+     * to an older daemon, generally we try a fallback API before giving up.
+     * --current requires the new API, since we don't know whether the domain is
+     *  running or inactive. */
+    if ((count = virDomainGetVcpusFlags(dom, flags)) >= 0)
+        return count;
+
+    /* fallback code */
+     if (!(last_error->code == VIR_ERR_NO_SUPPORT ||
+           last_error->code == VIR_ERR_INVALID_ARG))
+         goto cleanup;
+
+     if (!(flags & (VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG)) &&
+         virDomainIsActive(dom) == 1)
+         flags |= VIR_DOMAIN_AFFECT_LIVE;
+
+     vshResetLibvirtError();
+
+     if (flags & VIR_DOMAIN_AFFECT_LIVE) {
+         if (flags & VIR_DOMAIN_VCPU_MAXIMUM) {
+            count = virDomainGetMaxVcpus(dom);
+         } else {
+            if (virDomainGetInfo(dom, &info) < 0)
+                goto cleanup;
+
+            count = info.nrVirtCpu;
+         }
+     } else {
+         if (!(def = virDomainGetXMLDesc(dom, VIR_DOMAIN_XML_INACTIVE)))
+             goto cleanup;
+
+        if (!(xml = virXMLParseStringCtxt(def, _("(domain_definition)"), &ctxt)))
+            goto cleanup;
+
+         if (flags & VIR_DOMAIN_VCPU_MAXIMUM) {
+             if (virXPathInt("string(/domain/vcpus)", ctxt, &count) < 0) {
+                 vshError(ctl, "%s", _("Failed to retrieve maximum vcpu count"));
+                 goto cleanup;
+             }
+         } else {
+             if (virXPathInt("string(/domain/vcpus/@current)",
+                             ctxt, &count) < 0) {
+                 vshError(ctl, "%s", _("Failed to retrieve current vcpu count"));
+                 goto cleanup;
+             }
+         }
+     }
+
+    ret = count;
+cleanup:
+    VIR_FREE(def);
+    xmlXPathFreeContext(ctxt);
+    xmlFreeDoc(xml);
+
+    return ret;
+}
+
 static bool
 cmdVcpucount(vshControl *ctl, const vshCmd *cmd)
 {
     virDomainPtr dom;
-    bool ret = true;
+    bool ret = false;
     bool maximum = vshCommandOptBool(cmd, "maximum");
     bool active = vshCommandOptBool(cmd, "active");
     bool config = vshCommandOptBool(cmd, "config");
     bool live = vshCommandOptBool(cmd, "live");
     bool current = vshCommandOptBool(cmd, "current");
     bool all = maximum + active + current + config + live == 0;
-    int count;
+    unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;

-    /* We want one of each pair of mutually exclusive options; that
-     * is, use of flags requires exactly two options.  We reject the
-     * use of more than 2 flags later on.  */
-    if (maximum + active + current + config + live == 1) {
-        if (maximum || active) {
-            vshError(ctl,
-                     _("when using --%s, one of --config, --live, or --current "
-                       "must be specified"),
-                     maximum ? "maximum" : "active");
-        } else {
-            vshError(ctl,
-                     _("when using --%s, either --maximum or --active must be "
-                       "specified"),
-                     (current ? "current" : config ? "config" : "live"));
-        }
-        return false;
-    }
+    VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
+    VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
+    VSH_EXCLUSIVE_OPTIONS_VAR(active, maximum);

     /* Backwards compatibility: prior to 0.9.4,
      * VIR_DOMAIN_AFFECT_CURRENT was unsupported, and --current meant
@@ -5129,140 +5196,45 @@ cmdVcpucount(vshControl *ctl, const vshCmd *cmd)
         active = true;
     }

-    if (maximum && active) {
-        vshError(ctl, "%s",
-                 _("--maximum and --active cannot both be specified"));
-        return false;
-    }
-    if (current + config + live > 1) {
-        vshError(ctl, "%s",
-                 _("--config, --live, and --current are mutually exclusive"));
-        return false;
-    }
+    if (live)
+        flags |= VIR_DOMAIN_AFFECT_LIVE;
+    if (config)
+        flags |= VIR_DOMAIN_AFFECT_CONFIG;
+    if (maximum)
+        flags |= VIR_DOMAIN_VCPU_MAXIMUM;

     if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
         return false;

-    /* In all cases, try the new API first; if it fails because we are
-     * talking to an older client, generally we try a fallback API
-     * before giving up.  --current requires the new API, since we
-     * don't know whether the domain is running or inactive.  */
-    if (current) {
-        count = virDomainGetVcpusFlags(dom,
-                                       maximum ? VIR_DOMAIN_VCPU_MAXIMUM : 0);
-        if (count < 0) {
-            vshReportError(ctl);
-            ret = false;
-        } else {
-            vshPrint(ctl, "%d\n", count);
-        }
-    }
-
-    if (all || (maximum && config)) {
-        count = virDomainGetVcpusFlags(dom, (VIR_DOMAIN_VCPU_MAXIMUM |
-                                             VIR_DOMAIN_AFFECT_CONFIG));
-        if (count < 0 && (last_error->code == VIR_ERR_NO_SUPPORT
-                          || last_error->code == VIR_ERR_INVALID_ARG)) {
-            char *tmp;
-            char *xml = virDomainGetXMLDesc(dom, VIR_DOMAIN_XML_INACTIVE);
-            if (xml && (tmp = strstr(xml, "<vcpu"))) {
-                tmp = strchr(tmp, '>');
-                if (!tmp || virStrToLong_i(tmp + 1, &tmp, 10, &count) < 0)
-                    count = -1;
-            }
-            vshResetLibvirtError();
-            VIR_FREE(xml);
-        }
+    if (all) {
+        int conf_max = vshCPUCountCollect(ctl, dom, VIR_DOMAIN_AFFECT_CONFIG |
+                                                    VIR_DOMAIN_VCPU_MAXIMUM);
+        int conf_cur = vshCPUCountCollect(ctl, dom, VIR_DOMAIN_AFFECT_CONFIG);
+        int live_max = vshCPUCountCollect(ctl, dom, VIR_DOMAIN_AFFECT_LIVE |
+                                                    VIR_DOMAIN_VCPU_MAXIMUM);
+        int live_cur = vshCPUCountCollect(ctl, dom, VIR_DOMAIN_AFFECT_LIVE);

-        if (count < 0) {
-            vshReportError(ctl);
-            ret = false;
-        } else if (all) {
-            vshPrint(ctl, "%-12s %-12s %3d\n", _("maximum"), _("config"),
-                     count);
-        } else {
-            vshPrint(ctl, "%d\n", count);
-        }
-        vshResetLibvirtError();
-    }
-
-    if (all || (maximum && live)) {
-        count = virDomainGetVcpusFlags(dom, (VIR_DOMAIN_VCPU_MAXIMUM |
-                                             VIR_DOMAIN_AFFECT_LIVE));
-        if (count < 0 && (last_error->code == VIR_ERR_NO_SUPPORT
-                          || last_error->code == VIR_ERR_INVALID_ARG)) {
-            count = virDomainGetMaxVcpus(dom);
-        }
+        if (conf_max == -2 || conf_cur == -2 || live_max == -2 || live_cur ==  -2)
+            goto cleanup;

-        if (count < 0) {
-            vshReportError(ctl);
-            ret = false;
-        } else if (all) {
-            vshPrint(ctl, "%-12s %-12s %3d\n", _("maximum"), _("live"),
-                     count);
-        } else {
-            vshPrint(ctl, "%d\n", count);
-        }
-        vshResetLibvirtError();
-    }
+        vshPrint(ctl, "%-12s %-12s %3d\n", _("maximum"), _("config"), conf_max);
+        if (live_max > 0)
+            vshPrint(ctl, "%-12s %-12s %3d\n", _("maximum"), _("live"), live_max);
+        vshPrint(ctl, "%-12s %-12s %3d\n", _("current"), _("config"), conf_cur);
+        if (live_cur > 0)
+            vshPrint(ctl, "%-12s %-12s %3d\n", _("current"), _("live"), live_cur);
+    } else {
+        int count = vshCPUCountCollect(ctl, dom, flags);

-    if (all || (active && config)) {
-        count = virDomainGetVcpusFlags(dom, VIR_DOMAIN_AFFECT_CONFIG);
-        if (count < 0 && (last_error->code == VIR_ERR_NO_SUPPORT
-                          || last_error->code == VIR_ERR_INVALID_ARG)) {
-            char *tmp, *end;
-            char *xml = virDomainGetXMLDesc(dom, VIR_DOMAIN_XML_INACTIVE);
-            if (xml && (tmp = strstr(xml, "<vcpu"))) {
-                end = strchr(tmp, '>');
-                if (end) {
-                    *end = '\0';
-                    tmp = strstr(tmp, "current=");
-                    if (!tmp)
-                        tmp = end + 1;
-                    else {
-                        tmp += strlen("current=");
-                        tmp += *tmp == '\'' || *tmp == '"';
-                    }
-                }
-                if (!tmp || virStrToLong_i(tmp, &tmp, 10, &count) < 0)
-                    count = -1;
-            }
-            VIR_FREE(xml);
-        }
+        if (count < 0)
+            goto cleanup;

-        if (count < 0) {
-            vshReportError(ctl);
-            ret = false;
-        } else if (all) {
-            vshPrint(ctl, "%-12s %-12s %3d\n", _("current"), _("config"),
-                     count);
-        } else {
-            vshPrint(ctl, "%d\n", count);
-        }
-        vshResetLibvirtError();
+        vshPrint(ctl, "%d\n", count);
     }

-    if (all || (active && live)) {
-        count = virDomainGetVcpusFlags(dom, VIR_DOMAIN_AFFECT_LIVE);
-        if (count < 0 && (last_error->code == VIR_ERR_NO_SUPPORT
-                          || last_error->code == VIR_ERR_INVALID_ARG)) {
-            virDomainInfo info;
-            if (virDomainGetInfo(dom, &info) == 0)
-                count = info.nrVirtCpu;
-        }
-
-        if (count < 0) {
-            vshReportError(ctl);
-            ret = false;
-        } else if (all) {
-            vshPrint(ctl, "%-12s %-12s %3d\n", _("current"), _("live"),
-                     count);
-        } else {
-            vshPrint(ctl, "%d\n", count);
-        }
-        vshResetLibvirtError();
-    }
+    ret = true;

+cleanup:
     virDomainFree(dom);
     return ret;
 }
-- 
1.8.1.5


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