[libvirt] [PATCH v2 03/10] virsh: Add support for list -reason
Martin Kletzander
mkletzan at redhat.com
Fri Dec 11 10:40:05 UTC 2015
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 at redhat.com>
>> ---
>> 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.
>
>
>John
>
>> Example:
>>
>> B<virsh> list --title
>>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20151211/1e5cf1c1/attachment-0001.sig>
More information about the libvir-list
mailing list