Re: [Qemu-devel] [PATCH] hmp: Make "info block" output more readable

[Luiz Capitulino <lcapitulino@redhat.com>]

On Wed, 19 Jun 2013 16:10:55 +0200
Kevin Wolf <kwolf@redhat.com> wrote:

> HMP is meant for humans and you should notice it.
> 
> This changes the output format to use a bit more space to display the
> information more readable and leaves out irrelevant information (e.g.
> mention only that an image is encrypted, but not when it's not; display
> I/O limits only if throttling is in effect; ...)

I've applied this one. I can make the small suggestions that have been
made if you're OK with them.

> 
> Before:
> 
>     (qemu) info block
>     ide0-hd0: removable=0 io-status=ok file=/tmp/overlay.qcow2
>     backing_file=/tmp/backing.img backing_file_depth=1 ro=0 drv=qcow2
>     encrypted=1 bps=0 bps_rd=0 bps_wr=0 iops=1024 iops_rd=0 iops_wr=0
>     ide1-cd0: removable=1 locked=0 tray-open=0 io-status=ok
>     file=/home/kwolf/images/iso/Fedora-18-x86_64-Live-Desktop.iso ro=1
>     drv=raw encrypted=0 bps=0 bps_rd=0 bps_wr=0 iops=0 iops_rd=0 iops_wr=0
>     floppy0: removable=1 locked=0 tray-open=0 [not inserted]
>     sd0: removable=1 locked=0 tray-open=0 [not inserted]
> 
> After:
> 
>     (qemu) info block
>     ide0-hd0: /tmp/overlay.qcow2 (qcow2, encrypted)
>         Backing file:     /tmp/backing.img (chain depth: 1)
>         I/O limits:       bps=0 bps_rd=0 bps_wr=0 iops=1024 iops_rd=0 iops_wr=0
> 
>     ide1-cd0: /home/kwolf/images/iso/Fedora-18-x86_64-Live-Desktop.iso (raw, read-only)
>         Removable device: not locked, tray closed
> 
>     floppy0: [not inserted]
>         Removable device: not locked, tray closed
> 
>     sd0: [not inserted]
>         Removable device: not locked, tray closed
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  hmp.c | 94 +++++++++++++++++++++++++++++++++++++++----------------------------
>  1 file changed, 55 insertions(+), 39 deletions(-)
> 
> diff --git a/hmp.c b/hmp.c
> index 494a9aa..dddfaf4 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -289,62 +289,78 @@ void hmp_info_block(Monitor *mon, const QDict *qdict)
>          if (device && strcmp(device, info->value->device)) {
>              continue;
>          }
> -        monitor_printf(mon, "%s: removable=%d",
> -                       info->value->device, info->value->removable);
>  
> -        if (info->value->removable) {
> -            monitor_printf(mon, " locked=%d", info->value->locked);
> -            monitor_printf(mon, " tray-open=%d", info->value->tray_open);
> +        if (info != block_list) {
> +            monitor_printf(mon, "\n");
> +        }
> +
> +        monitor_printf(mon, "%s", info->value->device);
> +        if (info->value->has_inserted) {
> +            monitor_printf(mon, ": %s (%s%s%s)\n",
> +                           info->value->inserted->file,
> +                           info->value->inserted->drv,
> +                           info->value->inserted->ro ? ", read-only" : "",
> +                           info->value->inserted->encrypted ? ", encrypted" : "");
> +        } else {
> +            monitor_printf(mon, ": [not inserted]\n");
>          }
>  
> -        if (info->value->has_io_status) {
> -            monitor_printf(mon, " io-status=%s",
> +        if (info->value->has_io_status && info->value->io_status != BLOCK_DEVICE_IO_STATUS_OK) {
> +            monitor_printf(mon, "    I/O Status:       %s\n",
>                             BlockDeviceIoStatus_lookup[info->value->io_status]);
>          }
>  
> -        if (info->value->has_inserted) {
> -            monitor_printf(mon, " file=");
> -            monitor_print_filename(mon, info->value->inserted->file);
> -
> -            if (info->value->inserted->has_backing_file) {
> -                monitor_printf(mon, " backing_file=");
> -                monitor_print_filename(mon, info->value->inserted->backing_file);
> -                monitor_printf(mon, " backing_file_depth=%" PRId64,
> -                    info->value->inserted->backing_file_depth);
> -            }
> -            monitor_printf(mon, " ro=%d drv=%s encrypted=%d",
> -                           info->value->inserted->ro,
> -                           info->value->inserted->drv,
> -                           info->value->inserted->encrypted);
> +        if (info->value->removable) {
> +            monitor_printf(mon, "    Removable device: %slocked, tray %s\n",
> +                           info->value->locked ? "" : "not ",
> +                           info->value->tray_open ? "open" : "closed");
> +        }
>  
> -            monitor_printf(mon, " bps=%" PRId64 " bps_rd=%" PRId64
> -                            " bps_wr=%" PRId64 " iops=%" PRId64
> -                            " iops_rd=%" PRId64 " iops_wr=%" PRId64,
> +
> +        if (!info->value->has_inserted) {
> +            continue;
> +        }
> +
> +        if (info->value->inserted->has_backing_file) {
> +            monitor_printf(mon,
> +                           "    Backing file:     %s "
> +                           "(chain depth: %" PRId64 ")\n",
> +                           info->value->inserted->backing_file,
> +                           info->value->inserted->backing_file_depth);
> +        }
> +
> +        if (info->value->inserted->bps
> +            || info->value->inserted->bps_rd
> +            || info->value->inserted->bps_wr
> +            || info->value->inserted->iops
> +            || info->value->inserted->iops_rd
> +            || info->value->inserted->iops_wr)
> +        {
> +            monitor_printf(mon, "    I/O limits:       bps=%" PRId64
> +                            " bps_rd=%" PRId64  " bps_wr=%" PRId64
> +                            " iops=%" PRId64 " iops_rd=%" PRId64
> +                            " iops_wr=%" PRId64 "\n",
>                              info->value->inserted->bps,
>                              info->value->inserted->bps_rd,
>                              info->value->inserted->bps_wr,
>                              info->value->inserted->iops,
>                              info->value->inserted->iops_rd,
>                              info->value->inserted->iops_wr);
> +        }
>  
> -            if (verbose) {
> -                monitor_printf(mon, " images:\n");
> -                image_info = info->value->inserted->image;
> -                while (1) {
> -                        bdrv_image_info_dump((fprintf_function)monitor_printf,
> -                                             mon, image_info);
> -                    if (image_info->has_backing_image) {
> -                        image_info = image_info->backing_image;
> -                    } else {
> -                        break;
> -                    }
> +        if (verbose) {
> +            monitor_printf(mon, "\nImages:\n");
> +            image_info = info->value->inserted->image;
> +            while (1) {
> +                    bdrv_image_info_dump((fprintf_function)monitor_printf,
> +                                         mon, image_info);
> +                if (image_info->has_backing_image) {
> +                    image_info = image_info->backing_image;
> +                } else {
> +                    break;
>                  }
>              }
> -        } else {
> -            monitor_printf(mon, " [not inserted]");
>          }
> -
> -        monitor_printf(mon, "\n");
>      }
>  
>      qapi_free_BlockInfoList(block_list);