On Thu, Dec 10, 2015 at 03:15:43PM -0500, John Ferlan wrote:
On 12/01/2015 12:35 PM, Martin Kletzander wrote:Add new parameter for the list command, --reason. This adds another optional column in the table output of listed domains. Signed-off-by: Martin Kletzander <mkletzan@xxxxxxxxxx> --- tests/virshtest.c | 16 ++++++++++++++++ tools/virsh-domain-monitor.c | 19 ++++++++++++++++++- tools/virsh.pod | 5 ++++- 3 files changed, 38 insertions(+), 2 deletions(-) diff --git a/tests/virshtest.c b/tests/virshtest.c index 5983b5b190d6..ecec898c7264 100644 --- a/tests/virshtest.c +++ b/tests/virshtest.c @@ -117,6 +117,18 @@ static int testCompareListCustom(const void *data ATTRIBUTE_UNUSED) return testCompareOutputLit(exp, NULL, argv); } +static int testCompareListReason(const void *data ATTRIBUTE_UNUSED) +{ + const char *const argv[] = { VIRSH_CUSTOM, "list", "--reason", NULL }; + const char *exp = "\ + Id Name State Reason \n\ +------------------------------------------------------------\n\ + 1 fv0 running unknown \n\ + 2 fc4 running unknown \n\ +\n"; + return testCompareOutputLit(exp, NULL, argv); +} +Nice to have this test... Would be better to have one with Table too...
You mean Title, not Table, right? This is Table output (default). With Title I would find out that there is extra newline, I'll include that as well.
static int testCompareNodeinfoDefault(const void *data ATTRIBUTE_UNUSED) { const char *const argv[] = { VIRSH_DEFAULT, "nodeinfo", NULL }; @@ -266,6 +278,10 @@ mymain(void) testCompareListCustom, NULL) != 0) ret = -1; + if (virtTestRun("virsh list (with reason)", + testCompareListReason, NULL) != 0) + ret = -1; + if (virtTestRun("virsh nodeinfo (default)", testCompareNodeinfoDefault, NULL) != 0) ret = -1; diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index 24b902905968..e2ef2c566f84 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -1848,6 +1848,10 @@ static const vshCmdOptDef opts_list[] = { .type = VSH_OT_BOOL, .help = N_("show domain title") }, + {.name = "reason", + .type = VSH_OT_BOOL, + .help = N_("show domain state reason") + }, {.name = NULL} }; @@ -1862,10 +1866,12 @@ cmdList(vshControl *ctl, const vshCmd *cmd) bool optTable = vshCommandOptBool(cmd, "table"); bool optUUID = vshCommandOptBool(cmd, "uuid"); bool optName = vshCommandOptBool(cmd, "name"); + bool optReason = vshCommandOptBool(cmd, "reason"); size_t i; char *title; char uuid[VIR_UUID_STRING_BUFLEN]; int state; + int state_reason; bool ret = false; virshDomainListPtr list = NULL; virDomainPtr dom; @@ -1916,12 +1922,18 @@ cmdList(vshControl *ctl, const vshCmd *cmd) if (optTable) { vshPrintExtra(ctl, " %-5s %-30s %-10s", _("Id"), _("Name"), _("State")); + if (optReason) + vshPrintExtra(ctl, " %-10s", _("Reason")); +Fairly easy to "dashcount += 10;" or because the longest reason I can find is 19 characters, perhaps use 20... The only oddity is long state reason when title is also used. Your call on this.if (optTitle) vshPrintExtra(ctl, " %-20s\n", _("Title")); vshPrintExtra(ctl, "\n%s", "-------------------------------------------------"); + if (optReason) + vshPrintExtra(ctl, "%s", "-----------"); +not needing this if use dashcount...if (optTitle) vshPrintExtra(ctl, "%s", "---------------------"); @@ -1936,7 +1948,7 @@ cmdList(vshControl *ctl, const vshCmd *cmd) else ignore_value(virStrcpyStatic(id_buf, "-")); - state = virshDomainState(ctl, dom, NULL); + state = virshDomainState(ctl, dom, &state_reason); /* Domain could've been removed in the meantime */ if (state < 0) @@ -1952,6 +1964,11 @@ cmdList(vshControl *ctl, const vshCmd *cmd) state == -2 ? _("saved") : virshDomainStateToString(state)); + if (optReason) { + vshPrint(ctl, " %-10s", + virshDomainStateReasonToString(state, state_reason)); + } + if (optTitle) { if (!(title = virshGetDomainDescription(ctl, dom, true, 0))) goto cleanup; diff --git a/tools/virsh.pod b/tools/virsh.pod index 21ae4a3e5b46..7fddfc65d48c 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -393,7 +393,7 @@ value will generate output for the specific machine. Inject NMI to the guest. =item B<list> [I<--inactive> | I<--all>] - [I<--managed-save>] [I<--title>] + [I<--managed-save>] [I<--title>] [I<--reason>] { [I<--table>] | I<--name> | I<--uuid> } [I<--persistent>] [I<--transient>] [I<--with-managed-save>] [I<--without-managed-save>] @@ -531,6 +531,9 @@ If I<--title> is specified, then the short domain description (title) is printed in an extra column. This flag is usable only with the default I<--table> output. +If I<--reason> is specified, then the state reason is printed in an extra +column. This flag is usable only with the default I<--table> output. +This should go after the Example since it seems that's related to the --title option... Might be nice to explain what a "state reason" is.... Maybe even provide another example with a couple of different state reasons for different States (eg, runnning, migrating, paused, etc states) ACK w/ a few adjustments. Your call on dashcount - I just thought it'd be easier. JohnExample: B<virsh> list --title
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list