[libvirt] [PATCH v3] virsh: Add more human-friendly output of domblkstat command
Osier Yang
jyang at redhat.com
Wed Sep 7 11:45:21 UTC 2011
于 2011年09月06日 21:20, Peter Krempa 写道:
> 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.
>
> https://bugzilla.redhat.com/show_bug.cgi?id=731656
>
> Changes to v2:
> - Modify for new fields in virDomainBlockStatsFlags
>
> Changes to v1:
> - Rebase to current head
> ---
> tools/virsh.c | 114 ++++++++++++++++++++++++++++++++++++++++++++++--------
> tools/virsh.pod | 16 +++++++-
> 2 files changed, 112 insertions(+), 18 deletions(-)
>
> diff --git a/tools/virsh.c b/tools/virsh.c
> index 629233f..458252b 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -1054,16 +1054,46 @@ 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.\n\n"
> + " Explanation of fields:\n"
> + " rd_req - count of read operations (old format)\n"
> + " rd_operations - count of read operations (new format)\n"
> + " rd_bytes - count of read bytes\n"
> + " rd_total_times - total time read operations took\n"
Should also document the units IMHO, it's nano-seconds.
> + " wr_req - count of write operations (old format)\n"
> + " wr_operations - count of write operations (new format)\n"
> + " wr_bytes - count of written bytes\n"
Missed docs for wr_total_times.
> + " flush_operations - count of flush operations\n"
> + " flush_total_times - total time flush operations took\n"
likewise
> + " errs - error count")},
> {NULL,NULL}
> };
Think it would be better if converting "rd_operations", "wr_operations",
and "flush_operations" to the old names. They have the same meaning,
and we used "rd_req", "wr_req" in virsh for a long time. After conversion,
we can have less and not duplicate documention.
> 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_human_readable {
> + const char *field;
> + const char *human;
> +};
> +
> +static const struct _domblkstat_human_readable domblkstat_translate[] = {
> + { VIR_DOMAIN_BLOCK_STATS_READ_BYTES, N_("number of read bytes: ") }, /* 0 */
> + { VIR_DOMAIN_BLOCK_STATS_READ_REQ, N_("number of read operations: ") }, /* 1 */
> + { VIR_DOMAIN_BLOCK_STATS_READ_TOTAL_TIMES, N_("total duration of reads: ") }, /* 2 */
> + { VIR_DOMAIN_BLOCK_STATS_WRITE_BYTES, N_("number of bytes written: ") }, /* 3 */
> + { VIR_DOMAIN_BLOCK_STATS_WRITE_REQ, N_("number of write operations:") }, /* 4 */
> + { VIR_DOMAIN_BLOCK_STATS_WRITE_TOTAL_TIMES, N_("total duration of writes: ") }, /* 5 */
> + { VIR_DOMAIN_BLOCK_STATS_FLUSH_REQ, N_("number of flush operations:") }, /* 6 */
> + { VIR_DOMAIN_BLOCK_STATS_FLUSH_TOTAL_TIMES, N_("total duration of flushes: ") }, /* 7 */
Not sure if we should keep the same docs for these fields defined in struct
info_domblkstat and virsh.pod? it looks a bit long though. :-)
> + { VIR_DOMAIN_BLOCK_STATS_ERRS, N_("error count: ") }, /* 8 */
> + { NULL, NULL }
> +};
> +
> static bool
> cmdDomblkstat (vshControl *ctl, const vshCmd *cmd)
> {
> @@ -1071,8 +1101,11 @@ cmdDomblkstat (vshControl *ctl, const vshCmd *cmd)
> const char *name = NULL, *device = NULL;
> struct _virDomainBlockStats stats;
> virTypedParameterPtr params = NULL;
> + const char *field_name = NULL;
> + int j = 0;
> int rc, nparams = 0;
> bool ret = false;
> + bool human = vshCommandOptBool(cmd, "human"); /* enable human readable output */
>
> if (!vshConnectionUsability (ctl, ctl->conn))
> return false;
> @@ -1104,20 +1137,41 @@ cmdDomblkstat (vshControl *ctl, const vshCmd *cmd)
> goto cleanup;
> }
>
> - if (stats.rd_req>= 0)
> - vshPrint (ctl, "%s rd_req %lld\n", device, stats.rd_req);
> + if (human) {
> + /* human friendly output */
> + vshPrint(ctl, N_("Device: %s\n"), device);
> +
> + if (stats.rd_req>= 0)
> + vshPrint (ctl, "%s %lld\n", domblkstat_translate[1].human, stats.rd_req);
> +
> + if (stats.rd_bytes>= 0)
> + vshPrint (ctl, "%s %lld\n", domblkstat_translate[0].human, stats.rd_bytes);
> +
> + if (stats.wr_req>= 0)
> + vshPrint (ctl, "%s %lld\n", domblkstat_translate[4].human, stats.wr_req);
> +
> + if (stats.wr_bytes>= 0)
> + vshPrint (ctl, "%s %lld\n", domblkstat_translate[3].human, stats.wr_bytes);
> +
> + if (stats.errs>= 0)
> + vshPrint (ctl, "%s %lld\n", domblkstat_translate[8].human, stats.errs);
> + } else {
> +
> + 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.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);
> + if (stats.wr_req>= 0)
> + vshPrint (ctl, "%s wr_req %lld\n", device, stats.wr_req);
>
> - if (stats.wr_bytes>= 0)
> - vshPrint (ctl, "%s wr_bytes %lld\n", device, stats.wr_bytes);
> + if (stats.wr_bytes>= 0)
> + vshPrint (ctl, "%s wr_bytes %lld\n", device, stats.wr_bytes);
>
> - if (stats.errs>= 0)
> - vshPrint (ctl, "%s errs %lld\n", device, stats.errs);
> + if (stats.errs>= 0)
> + vshPrint (ctl, "%s errs %lld\n", device, stats.errs);
> + }
> }
> } else {
> params = vshMalloc(ctl, sizeof(*params) * nparams);
> @@ -1129,32 +1183,58 @@ cmdDomblkstat (vshControl *ctl, const vshCmd *cmd)
> }
>
> int i;
> +
> + /* set for prettier output */
> + if (human) {
> + vshPrint(ctl, N_("Device: %s\n"), device);
> + device = "";
> + }
> +
> /* XXX: The output sequence will be different. */
> for (i = 0; i< nparams; i++) {
> + /* translate messages into a human readable form, if requested */
> + if (human) {
> + /* try to look up the translation into a more human readable form */
> + field_name = NULL;
> + for (j = 0; domblkstat_translate[j].field != NULL; j++) {
> + if (STREQ(params[i].field, domblkstat_translate[j].field)) {
> + field_name = domblkstat_translate[j].human;
> +
> + break;
> + }
> + }
> +
> + /* translation not found, stick with the default field name */
> + if (!field_name)
> + field_name = params[i].field;
> + } else {
> + field_name = params[i].field;
Probly should do the conversion here.
> + }
> +
> switch(params[i].type) {
> case VIR_TYPED_PARAM_INT:
> vshPrint (ctl, "%s %s %d\n", device,
> - params[i].field, params[i].value.i);
> + field_name, params[i].value.i);
> break;
> case VIR_TYPED_PARAM_UINT:
> vshPrint (ctl, "%s %s %u\n", device,
> - params[i].field, params[i].value.ui);
> + field_name, params[i].value.ui);
> break;
> case VIR_TYPED_PARAM_LLONG:
> vshPrint (ctl, "%s %s %lld\n", device,
> - params[i].field, params[i].value.l);
> + field_name, params[i].value.l);
> break;
> case VIR_TYPED_PARAM_ULLONG:
> vshPrint (ctl, "%s %s %llu\n", device,
> - params[i].field, params[i].value.ul);
> + field_name, params[i].value.ul);
> break;
> case VIR_TYPED_PARAM_DOUBLE:
> vshPrint (ctl, "%s %s %f\n", device,
> - params[i].field, params[i].value.d);
> + field_name, 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"));
> + field_name, params[i].value.b ? _("yes") : _("no"));
> break;
> default:
> vshError(ctl, _("unimplemented block statistics parameter type"));
> diff --git a/tools/virsh.pod b/tools/virsh.pod
> index d826997..c8d88c9 100644
> --- a/tools/virsh.pod
> +++ b/tools/virsh.pod
> @@ -501,13 +501,27 @@ 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.
> +
> +B<Explanation of fields:>
> + rd_req - count of read operations (old format)
> + rd_operations - count of read operations (new format)
> + rd_bytes - count of read bytes
> + rd_total_times - total time read operations took
Units
> + wr_req - count of write operations (old format)
> + wr_operations - count of write operations (new format)
> + wr_bytes - count of written bytes
Missed wr_total_times
> + flush_operations - count of flush operations
> + flush_total_times - total time flush operations took
Units.
> + errs - error count
> +
> =item B<domifstat> I<domain> I<interface-device>
>
> Get network interface stats for a running domain.
Others look good to me.
Regards
Osier
More information about the libvir-list
mailing list