* [Qemu-devel] [PATCH] hmp: Make "info block" output more readable
@ 2013-06-19 14:10 Kevin Wolf
2013-06-20 1:50 ` Fam Zheng
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Kevin Wolf @ 2013-06-19 14:10 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, lcapitulino
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 <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);
--
1.8.1.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] hmp: Make "info block" output more readable
2013-06-19 14:10 [Qemu-devel] [PATCH] hmp: Make "info block" output more readable Kevin Wolf
@ 2013-06-20 1:50 ` Fam Zheng
2013-06-20 1:58 ` Zhi Yong Wu
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: Fam Zheng @ 2013-06-20 1:50 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-devel, lcapitulino
On Wed, 06/19 16:10, Kevin Wolf 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; ...)
>
> 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>
> + if (info->value->has_io_status && info->value->io_status != BLOCK_DEVICE_IO_STATUS_OK) {
> + monitor_printf(mon, " I/O Status: %s\n",
Would lowercase be more consistent? s/Status/status/
This is getting much readable!
Reviewed-by: Fam Zheng <famz@redhat.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] hmp: Make "info block" output more readable
2013-06-19 14:10 [Qemu-devel] [PATCH] hmp: Make "info block" output more readable Kevin Wolf
2013-06-20 1:50 ` Fam Zheng
@ 2013-06-20 1:58 ` Zhi Yong Wu
2013-06-21 3:27 ` Luiz Capitulino
2013-06-21 15:43 ` Anthony Liguori
3 siblings, 0 replies; 8+ messages in thread
From: Zhi Yong Wu @ 2013-06-20 1:58 UTC (permalink / raw)
To: Kevin Wolf; +Cc: QEMU Developers, Luiz Capitulino
On Wed, Jun 19, 2013 at 10:10 PM, 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; ...)
>
> 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
I prefer to one consistent term. s/limits/throttling/
>
> 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);
> --
> 1.8.1.4
>
>
--
Regards,
Zhi Yong Wu
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] hmp: Make "info block" output more readable
2013-06-19 14:10 [Qemu-devel] [PATCH] hmp: Make "info block" output more readable Kevin Wolf
2013-06-20 1:50 ` Fam Zheng
2013-06-20 1:58 ` Zhi Yong Wu
@ 2013-06-21 3:27 ` Luiz Capitulino
2013-06-21 9:07 ` Kevin Wolf
2013-06-21 15:43 ` Anthony Liguori
3 siblings, 1 reply; 8+ messages in thread
From: Luiz Capitulino @ 2013-06-21 3:27 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-devel
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);
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] hmp: Make "info block" output more readable
2013-06-21 3:27 ` Luiz Capitulino
@ 2013-06-21 9:07 ` Kevin Wolf
2013-06-21 14:39 ` Luiz Capitulino
0 siblings, 1 reply; 8+ messages in thread
From: Kevin Wolf @ 2013-06-21 9:07 UTC (permalink / raw)
To: Luiz Capitulino; +Cc: qemu-devel
Am 21.06.2013 um 05:27 hat Luiz Capitulino geschrieben:
> 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.
Sure, if you like the suggestions, go ahead and include them.
Kevin
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] hmp: Make "info block" output more readable
2013-06-21 9:07 ` Kevin Wolf
@ 2013-06-21 14:39 ` Luiz Capitulino
0 siblings, 0 replies; 8+ messages in thread
From: Luiz Capitulino @ 2013-06-21 14:39 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-devel
On Fri, 21 Jun 2013 11:07:51 +0200
Kevin Wolf <kwolf@redhat.com> wrote:
> Am 21.06.2013 um 05:27 hat Luiz Capitulino geschrieben:
> > 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.
>
> Sure, if you like the suggestions, go ahead and include them.
Done.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] hmp: Make "info block" output more readable
2013-06-19 14:10 [Qemu-devel] [PATCH] hmp: Make "info block" output more readable Kevin Wolf
` (2 preceding siblings ...)
2013-06-21 3:27 ` Luiz Capitulino
@ 2013-06-21 15:43 ` Anthony Liguori
2013-06-21 15:51 ` Luiz Capitulino
3 siblings, 1 reply; 8+ messages in thread
From: Anthony Liguori @ 2013-06-21 15:43 UTC (permalink / raw)
To: Kevin Wolf, qemu-devel; +Cc: lcapitulino
Kevin Wolf <kwolf@redhat.com> 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 <aliguori@us.ibm.com>
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 <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);
> --
> 1.8.1.4
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] hmp: Make "info block" output more readable
2013-06-21 15:43 ` Anthony Liguori
@ 2013-06-21 15:51 ` Luiz Capitulino
0 siblings, 0 replies; 8+ messages in thread
From: Luiz Capitulino @ 2013-06-21 15:51 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Kevin Wolf, qemu-devel
On Fri, 21 Jun 2013 10:43:19 -0500
Anthony Liguori <anthony@codemonkey.ws> wrote:
> Kevin Wolf <kwolf@redhat.com> 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 <aliguori@us.ibm.com>
>
> 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.
Yeah. I've already applied this one. I would consider not doing this
kind of change for HMP commands w/o QMP counterparts.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2013-06-21 15:51 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-06-19 14:10 [Qemu-devel] [PATCH] hmp: Make "info block" output more readable Kevin Wolf
2013-06-20 1:50 ` Fam Zheng
2013-06-20 1:58 ` Zhi Yong Wu
2013-06-21 3:27 ` Luiz Capitulino
2013-06-21 9:07 ` Kevin Wolf
2013-06-21 14:39 ` Luiz Capitulino
2013-06-21 15:43 ` Anthony Liguori
2013-06-21 15:51 ` Luiz Capitulino
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).