Re: [libvirt] [PATCH v2 3/3] virsh: Implement domblkerror command
On 01/30/2012 09:01 AM, Jiri Denemark wrote:
This command lists all disk devices with errors
---
tools/virsh.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
tools/virsh.pod | 7 ++++
2 files changed, 96 insertions(+), 0 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 3a59746..1b93852 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -16037,6 +16037,94 @@ cleanup:
}
/*
+ * "domioerror" command
+ */
+
+static const vshCmdInfo info_domblkerror[] = {
Looks like you missed a rename; s/domioerror/domblkerror/ in the comment.
+ {"help", N_("Show errors on block
devices")},
+ {"desc", N_("Show block device errors")},
+ {NULL, NULL}
+};
+
+static const vshCmdOptDef opts_domblkerror[] = {
+ {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id, or
uuid")},
+ {NULL, 0, 0, NULL}
Should we also allow this additional usage:
virsh domblkerror dom vda
which lists the status of just vda (no error, no space left, ...)? That
would mean adding:
{"disk", VSH_OT_DATA, VSH_OFLAG_OPT, N_("particular disk to check")}
then filtering through the libvirt API to match just that disk? You can
say "no, that's overkill" and not implement it, and I won't be hurt.
+ if (!(xml = virDomainGetXMLDesc(dom, 0)))
+ goto cleanup;
+
+ xmldoc = virXMLParseStringCtxt(xml, _("(domain_definition)"), &ctxt);
+ VIR_FREE(xml);
+ if (!xmldoc)
+ goto cleanup;
+
+ if (virXPathInt("count(./devices/disk)", ctxt, &ndisks) < 0)
+ goto cleanup;
Guess what - if we made the API take NULL,0 as a shorthand to query how
big to make our array, you wouldn't have to resort to this XML parsing.
+ if ((count = virDomainGetDiskErrors(dom, disks, ndisks, 0)) ==
-1)
+ goto cleanup;
+
+ for (i = 0; i < count; i++) {
+ vshPrint(ctl, "%s: %s\n",
+ disks[i].disk,
+ vshDomainIOErrorToString(disks[i].error));
+ }
Are we okay that if there are no disk errors (count is 0), we have no
output, not even mentioning that the command succeeded?
@@ -16240,6 +16328,7 @@ static const vshCmdDef domMonitoringCmds[] =
{
{"domblklist", cmdDomblklist, opts_domblklist, info_domblklist, 0},
{"domblkstat", cmdDomblkstat, opts_domblkstat, info_domblkstat, 0},
{"domcontrol", cmdDomControl, opts_domcontrol, info_domcontrol, 0},
+ {"domblkerror", cmdDomBlkError, opts_domblkerror, info_domblkerror, 0},
Sort this line earlier.
Overall, I like the series, and want this API in 0.9.10; can we get a v3
prior to RC1?
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 620 bytes)