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

[libvirt] [PATCH v7] virsh: Add more human-friendly output of domblkstat command



Users of virsh complain that output of the domblkstat command
is not intuitive enough. This patch adds explanation of fields
returned by this command to the help section for domblkstat and
the man page of virsh. Also a switch --human is added for
domblkstat that prints the fields with more descriptive
texts.

This patch also changes sequence of the output fields and their
names back to the order and spelling established by previous
versions of virsh to mantain compatibility with scripts.

Example of ordered and "translated" output:

PRE-patch:

virsh # domblkstat 1 vda
vda wr_bytes 5170176
vda wr_operations 511
vda rd_bytes 82815488
vda rd_operations 3726

POST-patch:

virsh # domblkstat 1 vda
vda rd_req 3726
vda rd_bytes 82815488
vda wr_req 478
vda wr_bytes 4965376

Example of human readable output:

virsh # domblkstat 1 vda --human
Device: vda
 number of read operations:      3726
 number of read bytes:           82815488
 number of write operations:     478
 number of bytes written:        4965376

https://bugzilla.redhat.com/show_bug.cgi?id=731656

Changes to v6:
- post correct version
- add examples of output

Changes to v5:
- Do not fix VIR_ERR_RPC failure of this function (unknown procedure 243)
    ( will be fixed in a separate patch adressing more similar errors)
- Clean up legacy printout using a macro
- Remove newlines from "desc" in vshCmdInfo
- use calloc() instead of malloc() + memset(..., 0, ...)

Changes to v4:
- Fields are printed in order established by previous versions
- Fix translation macros
- Run old api unconditionaly (while comunicating with old libvirtd
  "error: unknown procedure: 243" occured)
- Remove redundant field description from command help and add reference
  to man page.
- Fix formating (unnecessary space after function name)
- Fix man page, mentioning that fields may be missig.

Changes to v3:
- Add units to duration values
- Add missing write doc/stranslation
- Add translation from new api names to legacy names used in previous
  versions in virsh

Changes to v2:
- Modify for new fields in virDomainBlockStatsFlags

Changes to v1:
- Rebase to current head
---
IMHO this patch is worth getting into 0.9.5 as it fixes the changed output
order since addition of the new api, and translates field names into those
that have been used in previous versions.

( sorry for the mess with v6 :/ )

 tools/virsh.c   |  236 +++++++++++++++++++++++++++++++++++++++++-------------
 tools/virsh.pod |   20 +++++-
 2 files changed, 198 insertions(+), 58 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index d575425..f2bb2db 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -369,6 +369,8 @@ static const char *vshDomainStateReasonToString(int state, int reason);
 static const char *vshDomainControlStateToString(int state);
 static const char *vshDomainVcpuStateToString(int state);
 static bool vshConnectionUsability(vshControl *ctl, virConnectPtr conn);
+static virTypedParameterPtr vshFindTypedParamByName(const char *name, virTypedParameterPtr list, int count);
+static char *vshGetTypedParamValue(vshControl *ctl, virTypedParameterPtr item);

 static char *editWriteToTempFile (vshControl *ctl, const char *doc);
 static int   editFile (vshControl *ctl, const char *filename);
@@ -1054,16 +1056,43 @@ cleanup:
  */
 static const vshCmdInfo info_domblkstat[] = {
     {"help", N_("get device block stats for a domain")},
-    {"desc", N_("Get device block stats for a running domain.")},
+    {"desc", N_("Get device block stats for a running domain. See man page or use --human for explanation of fields")},
     {NULL,NULL}
 };

 static const vshCmdOptDef opts_domblkstat[] = {
     {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")},
     {"device", VSH_OT_DATA, VSH_OFLAG_REQ, N_("block device")},
+    {"human",  VSH_OT_BOOL, 0, N_("print a more human readable output")},
     {NULL, 0, 0, NULL}
 };

+struct _domblkstat_sequence {
+    const char *field;  /* field name */
+    const char *legacy; /* legacy name from previous releases */
+    const char *human;  /* human-friendly explanation */
+};
+
+/* sequence of values for output to honor legacy format from previous versions */
+static const struct _domblkstat_sequence domblkstat_output[] = {
+    { VIR_DOMAIN_BLOCK_STATS_READ_REQ,          "rd_req",   N_("number of read operations:     ") }, /* 0 */
+    { VIR_DOMAIN_BLOCK_STATS_READ_BYTES,        "rd_bytes", N_("number of read bytes:          ") }, /* 1 */
+    { VIR_DOMAIN_BLOCK_STATS_WRITE_REQ,         "wr_req",   N_("number of write operations:    ") }, /* 2 */
+    { VIR_DOMAIN_BLOCK_STATS_WRITE_BYTES,       "wr_bytes", N_("number of bytes written:       ") }, /* 3 */
+    { VIR_DOMAIN_BLOCK_STATS_ERRS,              "errs",     N_("error count:                   ") }, /* 4 */
+    { VIR_DOMAIN_BLOCK_STATS_FLUSH_REQ,         NULL,       N_("number of flush operations:    ") }, /* 5 */
+    { VIR_DOMAIN_BLOCK_STATS_READ_TOTAL_TIMES,  NULL,       N_("total duration of reads (ns):  ") }, /* 6 */
+    { VIR_DOMAIN_BLOCK_STATS_WRITE_TOTAL_TIMES, NULL,       N_("total duration of writes (ns): ") }, /* 7 */
+    { VIR_DOMAIN_BLOCK_STATS_FLUSH_TOTAL_TIMES, NULL,       N_("total duration of flushes (ns):") }, /* 8 */
+    { NULL, NULL, NULL }
+};
+
+#define DOMBLKSTAT_LEGACY_PRINT(ID, VALUE) \
+    if (VALUE >= 0) \
+        vshPrint(ctl, "%s %s %lld\n", device, \
+                 human?_(domblkstat_output[ID].human):domblkstat_output[ID].legacy, \
+                 VALUE);
+
 static bool
 cmdDomblkstat (vshControl *ctl, const vshCmd *cmd)
 {
@@ -1071,8 +1100,13 @@ cmdDomblkstat (vshControl *ctl, const vshCmd *cmd)
     const char *name = NULL, *device = NULL;
     struct _virDomainBlockStats stats;
     virTypedParameterPtr params = NULL;
+    virTypedParameterPtr par = NULL;
+    char *value = NULL;
+    const char *field = NULL;
     int rc, nparams = 0;
+    int i = 0;
     bool ret = false;
+    bool human = vshCommandOptBool(cmd, "human"); /* enable human readable output */

     if (!vshConnectionUsability (ctl, ctl->conn))
         return false;
@@ -1090,76 +1124,84 @@ cmdDomblkstat (vshControl *ctl, const vshCmd *cmd)
      * then.
      */
     if (rc < 0) {
-        if (last_error->code != VIR_ERR_NO_SUPPORT) {
-            virshReportError(ctl);
+        /* try older API if newer is not supported */
+        if (last_error->code != VIR_ERR_NO_SUPPORT)
             goto cleanup;
-        } else {
-            virFreeError(last_error);
-            last_error = NULL;
-
-            if (virDomainBlockStats (dom, device, &stats,
-                                     sizeof stats) == -1) {
-                vshError(ctl, _("Failed to get block stats %s %s"),
-                         name, device);
-                goto cleanup;
-            }
-
-            if (stats.rd_req >= 0)
-                vshPrint (ctl, "%s rd_req %lld\n", device, stats.rd_req);

-            if (stats.rd_bytes >= 0)
-                vshPrint (ctl, "%s rd_bytes %lld\n", device, stats.rd_bytes);
-
-            if (stats.wr_req >= 0)
-                vshPrint (ctl, "%s wr_req %lld\n", device, stats.wr_req);
+        virFreeError(last_error);
+        last_error = NULL;

-            if (stats.wr_bytes >= 0)
-                vshPrint (ctl, "%s wr_bytes %lld\n", device, stats.wr_bytes);
+        if (virDomainBlockStats (dom, device, &stats,
+                                 sizeof stats) == -1) {
+            vshError(ctl, _("Failed to get block stats %s %s"),
+                     name, device);
+            goto cleanup;
+        }

-            if (stats.errs >= 0)
-                vshPrint (ctl, "%s errs %lld\n", device, stats.errs);
+        /* human friendly output */
+        if (human) {
+            vshPrint(ctl, N_("Device: %s\n"), device);
+            device = "";
         }
+
+        DOMBLKSTAT_LEGACY_PRINT(0, stats.rd_req);
+        DOMBLKSTAT_LEGACY_PRINT(1, stats.rd_bytes);
+        DOMBLKSTAT_LEGACY_PRINT(2, stats.wr_req);
+        DOMBLKSTAT_LEGACY_PRINT(3, stats.wr_bytes);
+        DOMBLKSTAT_LEGACY_PRINT(4, stats.errs);
     } else {
-        params = vshMalloc(ctl, sizeof(*params) * nparams);
-        memset(params, 0, sizeof(*params) * nparams);
+        params = vshCalloc(ctl, nparams, sizeof(*params));

         if (virDomainBlockStatsFlags (dom, device, params, &nparams, 0) < 0) {
             vshError(ctl, _("Failed to get block stats %s %s"), name, device);
             goto cleanup;
         }

-        int i;
-        /* XXX: The output sequence will be different. */
+        /* set for prettier output */
+        if (human) {
+            vshPrint(ctl, N_("Device: %s\n"), device);
+            device = "";
+        }
+
+        /* at first print all known values in desired order */
+        for (i = 0; domblkstat_output[i].field != NULL; i++) {
+            if (!(par = vshFindTypedParamByName(domblkstat_output[i].field,
+                                          params,
+                                          nparams)))
+                continue;
+
+            if (!(value = vshGetTypedParamValue(ctl, par)))
+                continue;
+
+            /* to print other not supported fields, mark the already printed */
+            par->field[0] = '\0'; /* set the name to empty string */
+
+            /* translate into human readable or legacy spelling */
+            field = NULL;
+            if (human)
+                field = _(domblkstat_output[i].human);
+            else
+                field = domblkstat_output[i].legacy;
+
+            /* use the provided spelling if no translation is available */
+            if (!field)
+                field = domblkstat_output[i].field;
+
+            vshPrint(ctl, "%s %s %s\n", device, field, value);
+
+            VIR_FREE(value);
+        }
+
+        /* go through the fields again, looking for fields that were not printed*/
         for (i = 0; i < nparams; i++) {
-            switch(params[i].type) {
-            case VIR_TYPED_PARAM_INT:
-                vshPrint (ctl, "%s %s %d\n", device,
-                          params[i].field, params[i].value.i);
-                break;
-            case VIR_TYPED_PARAM_UINT:
-                vshPrint (ctl, "%s %s %u\n", device,
-                          params[i].field, params[i].value.ui);
-                break;
-            case VIR_TYPED_PARAM_LLONG:
-                vshPrint (ctl, "%s %s %lld\n", device,
-                          params[i].field, params[i].value.l);
-                break;
-            case VIR_TYPED_PARAM_ULLONG:
-                vshPrint (ctl, "%s %s %llu\n", device,
-                          params[i].field, params[i].value.ul);
-                break;
-            case VIR_TYPED_PARAM_DOUBLE:
-                vshPrint (ctl, "%s %s %f\n", device,
-                          params[i].field, params[i].value.d);
-                break;
-            case VIR_TYPED_PARAM_BOOLEAN:
-                vshPrint (ctl, "%s %s %s\n", device,
-                          params[i].field, params[i].value.b ? _("yes") : _("no"));
-                break;
-            default:
-                vshError(ctl, _("unimplemented block statistics parameter type"));
-            }
+            if (strlen(params[i].field) == 0)
+                continue;

+            if (!(value = vshGetTypedParamValue(ctl, params+i)))
+                continue;
+
+            vshPrint(ctl, "%s %s %s\n", device, params[i].field, value);
+            VIR_FREE(value);
         }
     }

@@ -1170,6 +1212,7 @@ cleanup:
     virDomainFree(dom);
     return ret;
 }
+#undef DOMBLKSTAT_LEGACY_PRINT

 /* "domifstat" command
  */
@@ -15202,6 +15245,85 @@ vshDomainStateReasonToString(int state, int reason)
     return N_("unknown");
 }

+static char *
+vshGetTypedParamValue(vshControl *ctl, virTypedParameterPtr item)
+{
+    int size = 0;
+    char *str = NULL;
+
+    if (!ctl || !item)
+        return NULL;
+
+    switch(item->type) {
+    case VIR_TYPED_PARAM_INT:
+        size = snprintf(NULL, 0, "%d", item->value.i);
+        str = vshMalloc(ctl, size+1);
+        snprintf(str, size+1, "%d", item->value.i);
+        return str;
+        break;
+
+    case VIR_TYPED_PARAM_UINT:
+        size = snprintf(NULL, 0, "%u", item->value.ui);
+        str = vshMalloc(ctl, size+1);
+        snprintf(str, size+1, "%u", item->value.ui);
+        return str;
+        break;
+
+    case VIR_TYPED_PARAM_LLONG:
+        size = snprintf(NULL, 0, "%lld", item->value.l);
+        str = vshMalloc(ctl, size+1);
+        snprintf(str, size+1, "%lld", item->value.l);
+        return str;
+        break;
+
+    case VIR_TYPED_PARAM_ULLONG:
+        size = snprintf(NULL, 0, "%llu", item->value.ul);
+        str = vshMalloc(ctl, size+1);
+        snprintf(str, size+1, "%llu", item->value.ul);
+        return str;
+        break;
+
+    case VIR_TYPED_PARAM_DOUBLE:
+        size = snprintf(NULL, 0, "%f", item->value.d);
+        str = vshMalloc(ctl, size+1);
+        snprintf(str, size+1, "%f", item->value.d);
+        return str;
+        break;
+
+    case VIR_TYPED_PARAM_BOOLEAN:
+        size = snprintf(NULL, 0, "%s", item->value.b ? _("yes") : _("no"));
+        str = vshMalloc(ctl, size+1);
+        snprintf(str, size+1, "%s", item->value.b ? _("yes") : _("no"));
+        return str;
+        break;
+
+    default:
+        vshError(ctl, _("unimplemented block statistics parameter type"));
+    }
+
+    return NULL;
+}
+
+static virTypedParameterPtr
+vshFindTypedParamByName(const char *name, virTypedParameterPtr list, int count)
+{
+    int i = count;
+    virTypedParameterPtr found = list;
+
+    if (!list || !name)
+        return NULL;
+
+    while (i-- > 0) {
+        if (STREQ(name, found->field))
+            return found;
+
+        found++; /* go to next struct in array */
+    }
+
+    /* not found */
+    return NULL;
+}
+
 static const char *
 vshDomainControlStateToString(int state)
 {
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 02726f3..edf451f 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -501,13 +501,31 @@ be lost once the guest stops running, but the snapshot contents still
 exist, and a new domain with the same name and UUID can restore the
 snapshot metadata with B<snapshot-create>.

-=item B<domblkstat> I<domain> I<block-device>
+=item B<domblkstat> I<domain> I<block-device> [I<--human>]

 Get device block stats for a running domain.  A I<block-device> corresponds
 to a unique target name (<target dev='name'/>) or source file (<source
 file='name'/>) for one of the disk devices attached to I<domain> (see
 also B<domblklist> for listing these names).

+Use I<--human> for a more human readable output.
+
+Availabilty of these fields depends on hypervisor. Unsupported fields are
+missing from the output. Other fields may appear if comunicating with a newer
+version of libvirtd.
+
+B<Explanation of fields> (fields appear in the folowing order):
+  rd_req            - count of read operations
+  rd_bytes          - count of read bytes
+  wr_req            - count of write operations
+  wr_bytes          - count of written bytes
+  errs              - error count
+  flush_operations  - count of flush operations
+  rd_total_times    - total time read operations took (ns)
+  wr_total_times    - total time write operations took (ns)
+  flush_total_times - total time flush operations took (ns)
+    <-- other fields provided by hypervisor -->
+
 =item B<domifstat> I<domain> I<interface-device>

 Get network interface stats for a running domain.
-- 
1.7.3.4


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