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 ebl...@redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
