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

[Anthony Liguori]

Kevin Wolf <address@hidden> writes:

> 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; ...)
>
> 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

Acked-by: Anthony Liguori <address@hidden>

I made a note on the changelog for 1.6 about this change.  Folks have
had plenty of time to move to QMP so changing HMP output is reasonable
at this point.

Regards,

Anthony Liguori


>
> Signed-off-by: Kevin Wolf <address@hidden>
> ---
>  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);
> -- 
> 1.8.1.4