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

[Peter Krempa]

Dňa 19.9.2011 22:49, Eric Blake  wrote / napísal(a):

> On 09/19/2011 02:24 PM, Eric Blake wrote:
>>> 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
>> For consistency, I think "bytes read" matches better with "bytes written".
> And for all that I squashed in, I forgot to fix this one.  Also,
> translators hate trailing whitespace; we're better off using printf
> field widths to get padding.
> Can you give this a quick ACK (it's just big enough that I don't want to
> push it under the trivial rule).
> diff --git i/tools/virsh.c w/tools/virsh.c
> index df1e10e..e19f47a 100644
> --- i/tools/virsh.c
> +++ w/tools/virsh.c
> @@ -1080,21 +1080,21 @@ struct _domblkstat_sequence {
>    * versions */
>   static const struct _domblkstat_sequence domblkstat_output[] = {
>       { VIR_DOMAIN_BLOCK_STATS_READ_REQ,          "rd_req",
> -      N_("number of read operations:     ") }, /* 0 */
> +      N_("number of read operations:") }, /* 0 */
>       { VIR_DOMAIN_BLOCK_STATS_READ_BYTES,        "rd_bytes",
> -      N_("number of read bytes:          ") }, /* 1 */
> +      N_("number of bytes read:") }, /* 1 */
>       { VIR_DOMAIN_BLOCK_STATS_WRITE_REQ,         "wr_req",
> -      N_("number of write operations:    ") }, /* 2 */
> +      N_("number of write operations:") }, /* 2 */
>       { VIR_DOMAIN_BLOCK_STATS_WRITE_BYTES,       "wr_bytes",
> -      N_("number of bytes written:       ") }, /* 3 */
> +      N_("number of bytes written:") }, /* 3 */
>       { VIR_DOMAIN_BLOCK_STATS_ERRS,              "errs",
> -      N_("error count:                   ") }, /* 4 */
> +      N_("error count:") }, /* 4 */
>       { VIR_DOMAIN_BLOCK_STATS_FLUSH_REQ,         NULL,
> -      N_("number of flush operations:    ") }, /* 5 */
> +      N_("number of flush operations:") }, /* 5 */
>       { VIR_DOMAIN_BLOCK_STATS_READ_TOTAL_TIMES,  NULL,
> -      N_("total duration of reads (ns):  ") }, /* 6 */
> +      N_("total duration of reads (ns):") }, /* 6 */
>       { VIR_DOMAIN_BLOCK_STATS_WRITE_TOTAL_TIMES, NULL,
> -      N_("total duration of writes (ns): ") }, /* 7 */
> +      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 }
Looks fine to me.
> @@ -1102,7 +1102,8 @@ static const struct _domblkstat_sequence
> domblkstat_output[] = {
>   #define DOMBLKSTAT_LEGACY_PRINT(ID, VALUE)              \
>       if (VALUE>= 0)                                     \
> -        vshPrint(ctl, "%s %s %lld\n", device,           \
> +        vshPrint(ctl, "%-*s %s %lld\n",                 \
> +                 human ? 31 : 0, device,                \
>                    human ? _(domblkstat_output[ID].human) \
>                    : domblkstat_output[ID].legacy,        \
>                    VALUE);
See below please.
> @@ -1201,7 +1202,8 @@ cmdDomblkstat (vshControl *ctl, const vshCmd *cmd)
>               if (!field)
>                   field = domblkstat_output[i].field;
> -            vshPrint(ctl, "%s %s %s\n", device, field, value);
> +            vshPrint(ctl, "%-*s %s %s\n", human ? 31 : 0,
I think this is not completely correct (but the trick with the "*" is really nice), as
the first field is the device name and the second one is field name, that should be
formatted nice. When "human" is true, device is set to an empty string and only the
field name is printed.

vshPrint(ctl, "%s %-*s %s\n", device,
          human ? 31 : 0, field value);

> +                     device, field, value);
>               VIR_FREE(value);
>           }
I give my incompetent ACK with that fixed :)

Peter