diff mbox series

block: Fix VM size column width in bdrv_snapshot_dump()

Message ID 20210202155911.179865-1-kwolf@redhat.com (mailing list archive)
State New, archived
Headers show
Series block: Fix VM size column width in bdrv_snapshot_dump() | expand

Commit Message

Kevin Wolf Feb. 2, 2021, 3:59 p.m. UTC
size_to_str() can return a size like "4.24 MiB", with a single digit
integer part and two fractional digits. This is eight characters, but
commit b39847a5 changed the format string to only reserve seven
characters for the column.

This can result in unaligned columns, which in turn changes the output of
iotests case 267 because exceeding the column size defeats the attempt
to filter the size out of the output (observed with the ppc64 emulator).
The resulting change is only a whitespace change, but since commit
f203080b this is enough for iotests to consider the test failed.

Taking a character away from the tag name column and adding it to the VM
size column doesn't change anything in the common case (the tag name is
left justified, the VM size is right justified), but fixes this case.

Fixes: b39847a50553b7679d6d7fefbe6a108a17aacf8d
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qapi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Eric Blake Feb. 2, 2021, 4:18 p.m. UTC | #1
On 2/2/21 9:59 AM, Kevin Wolf wrote:
> size_to_str() can return a size like "4.24 MiB", with a single digit
> integer part and two fractional digits. This is eight characters, but
> commit b39847a5 changed the format string to only reserve seven
> characters for the column.
> 
> This can result in unaligned columns, which in turn changes the output of
> iotests case 267 because exceeding the column size defeats the attempt
> to filter the size out of the output (observed with the ppc64 emulator).
> The resulting change is only a whitespace change, but since commit
> f203080b this is enough for iotests to consider the test failed.
> 
> Taking a character away from the tag name column and adding it to the VM
> size column doesn't change anything in the common case (the tag name is
> left justified, the VM size is right justified), but fixes this case.
> 
> Fixes: b39847a50553b7679d6d7fefbe6a108a17aacf8d
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block/qapi.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>
Vladimir Sementsov-Ogievskiy Feb. 2, 2021, 4:26 p.m. UTC | #2
02.02.2021 18:59, Kevin Wolf wrote:
> size_to_str() can return a size like "4.24 MiB", with a single digit
> integer part and two fractional digits. This is eight characters, but
> commit b39847a5 changed the format string to only reserve seven
> characters for the column.
> 
> This can result in unaligned columns, which in turn changes the output of
> iotests case 267 because exceeding the column size defeats the attempt
> to filter the size out of the output (observed with the ppc64 emulator).
> The resulting change is only a whitespace change, but since commit
> f203080b this is enough for iotests to consider the test failed.
> 
> Taking a character away from the tag name column and adding it to the VM
> size column doesn't change anything in the common case (the tag name is
> left justified, the VM size is right justified), but fixes this case.
> 
> Fixes: b39847a50553b7679d6d7fefbe6a108a17aacf8d
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

> ---
>   block/qapi.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/block/qapi.c b/block/qapi.c
> index 0a96099e36..84a0aadc09 100644
> --- a/block/qapi.c
> +++ b/block/qapi.c
> @@ -677,7 +677,7 @@ void bdrv_snapshot_dump(QEMUSnapshotInfo *sn)
>       char *sizing = NULL;
>   
>       if (!sn) {
> -        qemu_printf("%-10s%-18s%7s%20s%13s%11s",
> +        qemu_printf("%-10s%-17s%8s%20s%13s%11s",

Hmm, the sum is 79, so I think it's safe to keep 18 for tag and still update 7 to 8.. But it doesn't really matter.

>                       "ID", "TAG", "VM SIZE", "DATE", "VM CLOCK", "ICOUNT");
>       } else {
>           ti = sn->date_sec;
> @@ -696,7 +696,7 @@ void bdrv_snapshot_dump(QEMUSnapshotInfo *sn)
>               snprintf(icount_buf, sizeof(icount_buf),
>                   "%"PRId64, sn->icount);
>           }
> -        qemu_printf("%-9s %-17s %7s%20s%13s%11s",
> +        qemu_printf("%-9s %-16s %8s%20s%13s%11s",
>                       sn->id_str, sn->name,
>                       sizing,
>                       date_buf,
>
diff mbox series

Patch

diff --git a/block/qapi.c b/block/qapi.c
index 0a96099e36..84a0aadc09 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -677,7 +677,7 @@  void bdrv_snapshot_dump(QEMUSnapshotInfo *sn)
     char *sizing = NULL;
 
     if (!sn) {
-        qemu_printf("%-10s%-18s%7s%20s%13s%11s",
+        qemu_printf("%-10s%-17s%8s%20s%13s%11s",
                     "ID", "TAG", "VM SIZE", "DATE", "VM CLOCK", "ICOUNT");
     } else {
         ti = sn->date_sec;
@@ -696,7 +696,7 @@  void bdrv_snapshot_dump(QEMUSnapshotInfo *sn)
             snprintf(icount_buf, sizeof(icount_buf),
                 "%"PRId64, sn->icount);
         }
-        qemu_printf("%-9s %-17s %7s%20s%13s%11s",
+        qemu_printf("%-9s %-16s %8s%20s%13s%11s",
                     sn->id_str, sn->name,
                     sizing,
                     date_buf,