[libvirt] [PATCH v2 0/2] virsh expand virsh list command options

Chen Hanxiao (2): virsh: allow both --uuid and --name at same cmd virsh: enable --table with --name or --uuid tools/virsh-domain-monitor.c | 56 +++++++++++++++++++++++++++++++++++++------- 1 file changed, 47 insertions(+), 9 deletions(-) -- 2.5.0

From: Chen Hanxiao <chenhanxiao@gmail.com> virsh # list --uuid --name c7 49c765a0-25e7-40d0-964f-dac99724b32c f23 918f1dd6-b19f-412b-ba17-d113bad89af8 Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com> --- v2: drop uuid-name option, enable both --uuid and --name tools/virsh-domain-monitor.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index 0a93949..c712fa5 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -1844,12 +1844,8 @@ cmdList(vshControl *ctl, const vshCmd *cmd) FILTER("state-shutoff", VIR_CONNECT_LIST_DOMAINS_SHUTOFF); FILTER("state-other", VIR_CONNECT_LIST_DOMAINS_OTHER); - if (optTable + optName + optUUID > 1) { - vshError(ctl, "%s", - _("Only one argument from --table, --name and --uuid " - "may be specified.")); - return false; - } + VSH_EXCLUSIVE_OPTIONS("table", "name"); + VSH_EXCLUSIVE_OPTIONS("table", "uuid"); if (!optUUID && !optName) optTable = true; @@ -1907,6 +1903,12 @@ cmdList(vshControl *ctl, const vshCmd *cmd) state == -2 ? _("saved") : virshDomainStateToString(state)); } + } else if (optUUID && optName) { + if (virDomainGetUUIDString(dom, uuid) < 0) { + vshError(ctl, "%s", _("Failed to get domain's UUID")); + goto cleanup; + } + vshPrint(ctl, "%-36s %-30s\n", uuid, virDomainGetName(dom)); } else if (optUUID) { if (virDomainGetUUIDString(dom, uuid) < 0) { vshError(ctl, "%s", _("Failed to get domain's UUID")); -- 2.5.0

On 06/24/2016 12:44 PM, Chen Hanxiao wrote:
From: Chen Hanxiao <chenhanxiao@gmail.com>
virsh # list --uuid --name c7 49c765a0-25e7-40d0-964f-dac99724b32c f23 918f1dd6-b19f-412b-ba17-d113bad89af8
The actual output is reversed - perhaps a late change not properly reflected in the /commit message... # virsh list --all --name --uuid 56c1f811-3ffc-4363-b2d9-06bdc9fbbe2b dom1 abeb5797-df38-42c7-9265-c3359d409ab5 dom2
Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com> --- v2: drop uuid-name option, enable both --uuid and --name
tools/virsh-domain-monitor.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)
virsh.pod needs be changed to indicate, right after "If I<--name> is specified, domain names are printed instead of the table formatted one per line. If I<--uuid> is specified domain's UUID's are printed instead of names." The following is a suggestion and would replace the remainder of that paragraph: If both I<--name> and I<--uuid> are specified, domain UUID's and names are printed side by side without any header. Flag I<--table> specifies that the legacy table-formatted output should be used. This is the default if neither I<--name> nor I<--uuid> are specified. Options I<--uuid> and I<--name> are mutually exclusive if option I<--table> is specified. John
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index 0a93949..c712fa5 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -1844,12 +1844,8 @@ cmdList(vshControl *ctl, const vshCmd *cmd) FILTER("state-shutoff", VIR_CONNECT_LIST_DOMAINS_SHUTOFF); FILTER("state-other", VIR_CONNECT_LIST_DOMAINS_OTHER);
- if (optTable + optName + optUUID > 1) { - vshError(ctl, "%s", - _("Only one argument from --table, --name and --uuid " - "may be specified.")); - return false; - } + VSH_EXCLUSIVE_OPTIONS("table", "name"); + VSH_EXCLUSIVE_OPTIONS("table", "uuid");
if (!optUUID && !optName) optTable = true; @@ -1907,6 +1903,12 @@ cmdList(vshControl *ctl, const vshCmd *cmd) state == -2 ? _("saved") : virshDomainStateToString(state)); } + } else if (optUUID && optName) { + if (virDomainGetUUIDString(dom, uuid) < 0) { + vshError(ctl, "%s", _("Failed to get domain's UUID")); + goto cleanup; + } + vshPrint(ctl, "%-36s %-30s\n", uuid, virDomainGetName(dom)); } else if (optUUID) { if (virDomainGetUUIDString(dom, uuid) < 0) { vshError(ctl, "%s", _("Failed to get domain's UUID"));

At 2016-06-29 05:36:25, "John Ferlan" <jferlan@redhat.com> wrote:
On 06/24/2016 12:44 PM, Chen Hanxiao wrote:
From: Chen Hanxiao <chenhanxiao@gmail.com>
virsh # list --uuid --name c7 49c765a0-25e7-40d0-964f-dac99724b32c f23 918f1dd6-b19f-412b-ba17-d113bad89af8
The actual output is reversed - perhaps a late change not properly reflected in the /commit message...
# virsh list --all --name --uuid 56c1f811-3ffc-4363-b2d9-06bdc9fbbe2b dom1 abeb5797-df38-42c7-9265-c3359d409ab5 dom2
Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com> --- v2: drop uuid-name option, enable both --uuid and --name
tools/virsh-domain-monitor.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)
virsh.pod needs be changed to indicate, right after "If I<--name> is specified, domain names are printed instead of the table formatted one per line. If I<--uuid> is specified domain's UUID's are printed instead of names."
The following is a suggestion and would replace the remainder of that paragraph:
If both I<--name> and I<--uuid> are specified, domain UUID's and names are printed side by side without any header. Flag I<--table> specifies that the legacy table-formatted output should be used. This is the default if neither I<--name> nor I<--uuid> are specified. Options I<--uuid> and I<--name> are mutually exclusive if option I<--table> is specified.
Sure, v3 will add virsh.pod. Regards, - Chen

From: Chen Hanxiao <chenhanxiao@gmail.com> remove restrictions of --table with --name or --uid. Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com> --- tools/virsh-domain-monitor.c | 46 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 41 insertions(+), 5 deletions(-) diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index c712fa5..2596504 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -1844,22 +1844,33 @@ cmdList(vshControl *ctl, const vshCmd *cmd) FILTER("state-shutoff", VIR_CONNECT_LIST_DOMAINS_SHUTOFF); FILTER("state-other", VIR_CONNECT_LIST_DOMAINS_OTHER); - VSH_EXCLUSIVE_OPTIONS("table", "name"); - VSH_EXCLUSIVE_OPTIONS("table", "uuid"); - if (!optUUID && !optName) optTable = true; + if (optUUID && optTitle) + optTable = true; if (!(list = virshDomainListCollect(ctl, flags))) goto cleanup; /* print table header in legacy mode */ if (optTable) { - if (optTitle) + if (optTitle && optUUID) + vshPrintExtra(ctl, " %-5s %-30s %-10s %-36s %-20s\n%s\n", + _("Id"), _("Name"), _("State"), _("UUID"), _("Title"), + "-----------------------------------------" + "-----------------------------------------" + "-----------------------------------------"); + else if (optTitle) vshPrintExtra(ctl, " %-5s %-30s %-10s %-20s\n%s\n", _("Id"), _("Name"), _("State"), _("Title"), "-----------------------------------------" "-----------------------------------------"); + else if (optUUID) + vshPrintExtra(ctl, " %-5s %-30s %-10s %-36s\n%s\n", + _("Id"), _("Name"), _("State"), _("UUID"), + "-----------------------------------------" + "-----------------------------------------" + "----"); else vshPrintExtra(ctl, " %-5s %-30s %s\n%s\n", _("Id"), _("Name"), _("State"), @@ -1886,7 +1897,22 @@ cmdList(vshControl *ctl, const vshCmd *cmd) virDomainHasManagedSaveImage(dom, 0) > 0) state = -2; - if (optTitle) { + if (optTitle && optUUID) { + if (!(title = virshGetDomainDescription(ctl, dom, true, 0))) + goto cleanup; + if (virDomainGetUUIDString(dom, uuid) < 0) { + vshError(ctl, "%s", _("Failed to get domain's UUID")); + goto cleanup; + } + vshPrint(ctl, " %-5s %-30s %-10s %-36s %-20s\n", id_buf, + virDomainGetName(dom), + state == -2 ? _("saved") + : virshDomainStateToString(state), + uuid, + title); + + VIR_FREE(title); + } else if (optTitle) { if (!(title = virshGetDomainDescription(ctl, dom, true, 0))) goto cleanup; @@ -1897,6 +1923,16 @@ cmdList(vshControl *ctl, const vshCmd *cmd) title); VIR_FREE(title); + } else if (optUUID) { + if (virDomainGetUUIDString(dom, uuid) < 0) { + vshError(ctl, "%s", _("Failed to get domain's UUID")); + goto cleanup; + } + vshPrint(ctl, " %-5s %-30s %-10s %-36s\n", id_buf, + virDomainGetName(dom), + state == -2 ? _("saved") + : virshDomainStateToString(state), + uuid); } else { vshPrint(ctl, " %-5s %-30s %s\n", id_buf, virDomainGetName(dom), -- 2.5.0

On 06/24/2016 12:44 PM, Chen Hanxiao wrote:
From: Chen Hanxiao <chenhanxiao@gmail.com>
remove restrictions of --table with --name or --uid.
I think what you're trying to do is allow --uuid to be printed in the --table output. I don't htink --name should be mentioned/modified.
Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com> --- tools/virsh-domain-monitor.c | 46 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 41 insertions(+), 5 deletions(-)
again, no virsh.pod change. But how exactly it changes depends on the final result here, so no suggestions yet. Adding UUID easily goes beyond 80 columns (e.g. normal screen width). Although it wouldn't be the first... Another option for display is using 2 columns, e.g.: Id: - Name: dom1 UUID: 56c1f811-3ffc-4363-b2d9-06bdc9fbbe2b State: shut off Title: But that's a lot like {vol|pool}-info command output and probably should be reserved for a similarly verbose dom-info type output.
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index c712fa5..2596504 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -1844,22 +1844,33 @@ cmdList(vshControl *ctl, const vshCmd *cmd) FILTER("state-shutoff", VIR_CONNECT_LIST_DOMAINS_SHUTOFF); FILTER("state-other", VIR_CONNECT_LIST_DOMAINS_OTHER);
- VSH_EXCLUSIVE_OPTIONS("table", "name");
This one is unrelated and I think stays; otherwise, one could get the impression that --name is optional, which it's not.
- VSH_EXCLUSIVE_OPTIONS("table", "uuid"); - if (!optUUID && !optName) optTable = true; + if (optUUID && optTitle) + optTable = true;
The rest works, but is really repetitive... let's see what/if anyone else has comments on this. Maybe there's someone else with "thoughts" on how to print out the headers "nicer" or "more cleanly". John
if (!(list = virshDomainListCollect(ctl, flags))) goto cleanup;
/* print table header in legacy mode */ if (optTable) { - if (optTitle) + if (optTitle && optUUID) + vshPrintExtra(ctl, " %-5s %-30s %-10s %-36s %-20s\n%s\n", + _("Id"), _("Name"), _("State"), _("UUID"), _("Title"), + "-----------------------------------------" + "-----------------------------------------" + "-----------------------------------------"); + else if (optTitle) vshPrintExtra(ctl, " %-5s %-30s %-10s %-20s\n%s\n", _("Id"), _("Name"), _("State"), _("Title"), "-----------------------------------------" "-----------------------------------------"); + else if (optUUID) + vshPrintExtra(ctl, " %-5s %-30s %-10s %-36s\n%s\n", + _("Id"), _("Name"), _("State"), _("UUID"), + "-----------------------------------------" + "-----------------------------------------" + "----"); else vshPrintExtra(ctl, " %-5s %-30s %s\n%s\n", _("Id"), _("Name"), _("State"), @@ -1886,7 +1897,22 @@ cmdList(vshControl *ctl, const vshCmd *cmd) virDomainHasManagedSaveImage(dom, 0) > 0) state = -2;
- if (optTitle) { + if (optTitle && optUUID) { + if (!(title = virshGetDomainDescription(ctl, dom, true, 0))) + goto cleanup; + if (virDomainGetUUIDString(dom, uuid) < 0) { + vshError(ctl, "%s", _("Failed to get domain's UUID")); + goto cleanup; + } + vshPrint(ctl, " %-5s %-30s %-10s %-36s %-20s\n", id_buf, + virDomainGetName(dom), + state == -2 ? _("saved") + : virshDomainStateToString(state), + uuid, + title); + + VIR_FREE(title); + } else if (optTitle) { if (!(title = virshGetDomainDescription(ctl, dom, true, 0))) goto cleanup;
@@ -1897,6 +1923,16 @@ cmdList(vshControl *ctl, const vshCmd *cmd) title);
VIR_FREE(title); + } else if (optUUID) { + if (virDomainGetUUIDString(dom, uuid) < 0) { + vshError(ctl, "%s", _("Failed to get domain's UUID")); + goto cleanup; + } + vshPrint(ctl, " %-5s %-30s %-10s %-36s\n", id_buf, + virDomainGetName(dom), + state == -2 ? _("saved") + : virshDomainStateToString(state), + uuid); } else { vshPrint(ctl, " %-5s %-30s %s\n", id_buf, virDomainGetName(dom),

At 2016-06-29 05:36:30, "John Ferlan" <jferlan@redhat.com> wrote:
On 06/24/2016 12:44 PM, Chen Hanxiao wrote:
From: Chen Hanxiao <chenhanxiao@gmail.com>
remove restrictions of --table with --name or --uid.
I think what you're trying to do is allow --uuid to be printed in the --table output. I don't htink --name should be mentioned/modified.
Yes, I want to remove that restrictions.
Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com> --- tools/virsh-domain-monitor.c | 46 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 41 insertions(+), 5 deletions(-)
again, no virsh.pod change. But how exactly it changes depends on the final result here, so no suggestions yet.
Adding UUID easily goes beyond 80 columns (e.g. normal screen width). Although it wouldn't be the first...
--title may easily goes beyond that limit too :)
Another option for display is using 2 columns, e.g.:
Id: - Name: dom1 UUID: 56c1f811-3ffc-4363-b2d9-06bdc9fbbe2b State: shut off Title:
But that's a lot like {vol|pool}-info command output and probably should be reserved for a similarly verbose dom-info type output.
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index c712fa5..2596504 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -1844,22 +1844,33 @@ cmdList(vshControl *ctl, const vshCmd *cmd) FILTER("state-shutoff", VIR_CONNECT_LIST_DOMAINS_SHUTOFF); FILTER("state-other", VIR_CONNECT_LIST_DOMAINS_OTHER);
- VSH_EXCLUSIVE_OPTIONS("table", "name");
This one is unrelated and I think stays; otherwise, one could get the impression that --name is optional, which it's not.
- VSH_EXCLUSIVE_OPTIONS("table", "uuid"); - if (!optUUID && !optName) optTable = true; + if (optUUID && optTitle) + optTable = true;
The rest works, but is really repetitive... let's see what/if anyone else has comments on this. Maybe there's someone else with "thoughts" on how to print out the headers "nicer" or "more cleanly".
The movitvation of 2/2 is to remove the restrictions of: if (optTable + optName + optUUID > 1) Comments are welcome. Regards, - Chen
participants (2)
-
Chen Hanxiao
-
John Ferlan