[libvirt] [PATCH v2 2/2] virsh: enable --table with --name or --uuid

[John Ferlan]

On 06/24/2016 12:44 PM, Chen Hanxiao wrote:
> From: Chen Hanxiao <chenhanxiao at 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 at 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),
>