hmp: Make "info block" output more readable

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

Signed-off-by Kevin Wolf <[email protected]>

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

Signed-off-by Kevin Wolf <[email protected]>

---
 hmp.c | 94 +++++++++++++++++++++++++++++++++++++++----------------------------
 1 file changed, 55 insertions(+), 39 deletions(-)

hmp.c +55 -40

index 494a9aa..dddfaf4 100644
@@ -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

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

Signed-off-by Kevin Wolf <[email protected]>

---
 hmp.c | 94 +++++++++++++++++++++++++++++++++++++++----------------------------
 1 file changed, 55 insertions(+), 39 deletions(-)

hmp.c +55 -39

index 494a9aa..dddfaf4 100644
@@ -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);

Quoted 2 lines · On Wed, 19 Jun 2013 16:10:55 +0200 Kevin Wolf <[email protected]> wrote:

HMP is meant for humans and you should notice it.

Quoted 4 lines

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.

Am 21.06.2013 um 05:27 hat Luiz Capitulino geschrieben:

Quoted 10 lines

Quoted 6 lines · On Wed, 19 Jun 2013 16:10:55 +0200 Kevin Wolf <[email protected]> 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; ...)

Quoted 2 lines

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

Sure, if you like the suggestions, go ahead and include them.

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

Signed-off-by Kevin Wolf <[email protected]>

---
 hmp.c | 94 +++++++++++++++++++++++++++++++++++++++----------------------------
 1 file changed, 55 insertions(+), 39 deletions(-)

hmp.c +55 -39

index 494a9aa..dddfaf4 100644
@@ -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);

Signature · 1 line

1.8.1.4

Quoted 32 lines · Kevin Wolf

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

Quoted 2 lines

sd0: [not inserted]
Removable device: not locked, tray closed

Acked-by Anthony Liguori <[email protected]>

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.