[libvirt] [PATCH v7] virsh: Add more human-friendly output of domblkstat command
Eric Blake
eblake at redhat.com
Mon Sep 19 20:24:39 UTC 2011
On 09/19/2011 06:23 AM, Peter Krempa wrote:
> 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.
s/mantain/maintain/
> 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
For consistency, I think "bytes read" matches better with "bytes written".
> 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.
Agree. So I'll help out, so you don't have to make a v8 :)
> +++ 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);
Long line; wrapped to keep at 80 columns.
> +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")},
Another long line.
> +
> +#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, \
Spacing around ?: operator.
> - if (stats.wr_bytes>= 0)
> - vshPrint (ctl, "%s wr_bytes %lld\n", device, stats.wr_bytes);
> + if (virDomainBlockStats (dom, device,&stats,
No space before '(' in function call.
> + /* 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)))
Indentation.
> + if (strlen(params[i].field) == 0)
More efficient as:
if (!*params[i].field)
> @@ -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);
Yuck. Why not just use virAsprintf?
> +++ 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
s/Availabilty/Availability/
> +missing from the output. Other fields may appear if comunicating with a newer
s/comunicating/communicating/
ACK with my nits fixed, so I've pushed this. Here's what I squashed in:
diff --git i/tools/virsh.c w/tools/virsh.c
index f2bb2db..df1e10e 100644
--- i/tools/virsh.c
+++ w/tools/virsh.c
@@ -369,7 +369,9 @@ 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 virTypedParameterPtr vshFindTypedParamByName(const char *name,
+
virTypedParameterPtr list,
+ int count);
static char *vshGetTypedParamValue(vshControl *ctl,
virTypedParameterPtr item);
static char *editWriteToTempFile (vshControl *ctl, const char *doc);
@@ -1056,7 +1058,8 @@ 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. See man
page or use --human for explanation of fields")},
+ {"desc", N_("Get device block stats for a running domain. See man
page or "
+ "use --human for explanation of fields")},
{NULL,NULL}
};
@@ -1073,24 +1076,35 @@ struct _domblkstat_sequence {
const char *human; /* human-friendly explanation */
};
-/* sequence of values for output to honor legacy format from previous
versions */
+/* 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 */
+ { 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, \
+#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
@@ -1106,15 +1120,15 @@ cmdDomblkstat (vshControl *ctl, const vshCmd *cmd)
int rc, nparams = 0;
int i = 0;
bool ret = false;
- bool human = vshCommandOptBool(cmd, "human"); /* enable human
readable output */
+ bool human = vshCommandOptBool(cmd, "human"); /* human readable
output */
- if (!vshConnectionUsability (ctl, ctl->conn))
+ if (!vshConnectionUsability(ctl, ctl->conn))
return false;
- if (!(dom = vshCommandOptDomain (ctl, cmd, &name)))
+ if (!(dom = vshCommandOptDomain(ctl, cmd, &name)))
return false;
- if (vshCommandOptString (cmd, "device", &device) <= 0)
+ if (vshCommandOptString(cmd, "device", &device) <= 0)
goto cleanup;
rc = virDomainBlockStatsFlags(dom, device, NULL, &nparams, 0);
@@ -1131,8 +1145,8 @@ cmdDomblkstat (vshControl *ctl, const vshCmd *cmd)
virFreeError(last_error);
last_error = NULL;
- if (virDomainBlockStats (dom, device, &stats,
- sizeof stats) == -1) {
+ if (virDomainBlockStats(dom, device, &stats,
+ sizeof stats) == -1) {
vshError(ctl, _("Failed to get block stats %s %s"),
name, device);
goto cleanup;
@@ -1166,8 +1180,8 @@ cmdDomblkstat (vshControl *ctl, const vshCmd *cmd)
/* 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)))
+ params,
+ nparams)))
continue;
if (!(value = vshGetTypedParamValue(ctl, par)))
@@ -1192,9 +1206,9 @@ cmdDomblkstat (vshControl *ctl, const vshCmd *cmd)
VIR_FREE(value);
}
- /* go through the fields again, looking for fields that were
not printed*/
+ /* go through the fields again, for remaining fields */
for (i = 0; i < nparams; i++) {
- if (strlen(params[i].field) == 0)
+ if (!*params[i].field)
continue;
if (!(value = vshGetTypedParamValue(ctl, params+i)))
@@ -15248,7 +15262,7 @@ vshDomainStateReasonToString(int state, int reason)
static char *
vshGetTypedParamValue(vshControl *ctl, virTypedParameterPtr item)
{
- int size = 0;
+ int ret = 0;
char *str = NULL;
if (!ctl || !item)
@@ -15256,52 +15270,36 @@ vshGetTypedParamValue(vshControl *ctl,
virTypedParameterPtr item)
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;
+ ret = virAsprintf(&str, "%d", item->value.i);
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;
+ ret = virAsprintf(&str, "%u", item->value.ui);
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;
+ ret = virAsprintf(&str, "%lld", item->value.l);
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;
+ ret = virAsprintf(&str, "%llu", item->value.ul);
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;
+ ret = virAsprintf(&str, "%f", item->value.d);
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;
+ ret = virAsprintf(&str, "%s", item->value.b ? _("yes") : _("no"));
break;
default:
vshError(ctl, _("unimplemented block statistics parameter type"));
}
- return NULL;
+ if (ret < 0)
+ vshError(ctl, "%s", _("Out of memory"));
+ return str;
}
static virTypedParameterPtr
diff --git i/tools/virsh.pod w/tools/virsh.pod
index edf451f..6529cd6 100644
--- i/tools/virsh.pod
+++ w/tools/virsh.pod
@@ -510,8 +510,8 @@ 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
+Availability of these fields depends on hypervisor. Unsupported fields are
+missing from the output. Other fields may appear if communicating with
a newer
version of libvirtd.
B<Explanation of fields> (fields appear in the folowing order):
--
Eric Blake eblake at redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
More information about the libvir-list
mailing list