[PATCH 0/2] block: Fix VM size field width in snapshot dump

[PATCH 0/2] block: Fix VM size field width in snapshot dump

[Max Reitz]

Hi,

https://bugs.launchpad.net/qemu/+bug/1859989 reports that fields in
"qemu-img snapshot -l"s output are not always separated by spaces in
4.1.1.  Fix that.


Branch: https://github.com/XanClic/qemu.git lp-1859989-v1
Branch: https://git.xanclic.moe/XanClic/qemu.git lp-1859989-v1


Max Reitz (2):
  block: Fix VM size field width in snapshot dump
  iotests: Test snapshot -l field separation

 block/qapi.c               |  4 +-
 tests/qemu-iotests/284     | 76 ++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/284.out |  8 ++++
 tests/qemu-iotests/group   |  1 +
 4 files changed, 87 insertions(+), 2 deletions(-)
 create mode 100755 tests/qemu-iotests/284
 create mode 100644 tests/qemu-iotests/284.out

-- 
2.24.1

[PATCH 1/2] block: Fix VM size field width in snapshot dump

[Max Reitz]

When printing the snapshot list (e.g. with qemu-img snapshot -l), the VM
size field is only seven characters wide.  As of de38b5005e9, this is
not necessarily sufficient: We generally print three digits, and this
may require a decimal point.  Also, the unit field grew from something
as plain as "M" to " MiB".  This means that number and unit may take up
eight characters in total; but we also want spaces in front.

Considering previously the maximum width was four characters and the
field width was chosen to be three characters wider, let us adjust the
field width to be eleven now.

Fixes: de38b5005e946aa3714963ea4c501e279e7d3666
Buglink: https://bugs.launchpad.net/qemu/+bug/1859989
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/qapi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/qapi.c b/block/qapi.c
index 9a5d0c9b27..ffa539250d 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -657,7 +657,7 @@ void bdrv_snapshot_dump(QEMUSnapshotInfo *sn)
     char *sizing = NULL;
 
     if (!sn) {
-        qemu_printf("%-10s%-20s%7s%20s%15s",
+        qemu_printf("%-10s%-20s%11s%20s%15s",
                     "ID", "TAG", "VM SIZE", "DATE", "VM CLOCK");
     } else {
         ti = sn->date_sec;
@@ -672,7 +672,7 @@ void bdrv_snapshot_dump(QEMUSnapshotInfo *sn)
                  (int)(secs % 60),
                  (int)((sn->vm_clock_nsec / 1000000) % 1000));
         sizing = size_to_str(sn->vm_state_size);
-        qemu_printf("%-10s%-20s%7s%20s%15s",
+        qemu_printf("%-10s%-20s%11s%20s%15s",
                     sn->id_str, sn->name,
                     sizing,
                     date_buf,
-- 
2.24.1

[PATCH 2/2] iotests: Test snapshot -l field separation

[Max Reitz]

Add a test that all fields in "qemu-img snapshot -l"s output are
separated by spaces.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/284     | 76 ++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/284.out |  8 ++++
 tests/qemu-iotests/group   |  1 +
 3 files changed, 85 insertions(+)
 create mode 100755 tests/qemu-iotests/284
 create mode 100644 tests/qemu-iotests/284.out

diff --git a/tests/qemu-iotests/284 b/tests/qemu-iotests/284
new file mode 100755
index 0000000000..f14445ba4a
--- /dev/null
+++ b/tests/qemu-iotests/284
@@ -0,0 +1,76 @@
+#!/usr/bin/env bash
+#
+# Test qemu-img snapshot -l
+#
+# Copyright (C) 2019 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+seq=$(basename "$0")
+echo "QA output created by $seq"
+
+status=1	# failure is the default!
+
+_cleanup()
+{
+    _cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+. ./common.qemu
+
+_supported_fmt qcow2
+_supported_proto file
+# Internal snapshots are (currently) impossible with refcount_bits=1,
+# and generally impossible with external data files
+_unsupported_imgopts 'refcount_bits=1[^0-9]' data_file
+
+_make_test_img 64M
+
+# Should be so long as to take up the whole field width
+sn_name=abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz
+
+# More memory will give us a larger VM state, i.e. one above 1 MB.
+# This way, we get a number with a decimal point.
+qemu_comm_method=monitor _launch_qemu -m 512 "$TEST_IMG"
+
+_send_qemu_cmd $QEMU_HANDLE "savevm $sn_name" '(qemu)'
+_send_qemu_cmd $QEMU_HANDLE 'quit' '(qemu)'
+wait=yes _cleanup_qemu
+
+# Check that all fields are separated by spaces.
+# We first collapse all space sequences into one space each;
+# then we turn every space-separated field into a '.';
+# and finally, we name the '.'s so the output is not just a confusing
+# sequence of dots.
+
+echo 'Output structure:'
+$QEMU_IMG snapshot -l "$TEST_IMG" | tail -n 1 | tr -s ' ' \
+    | sed -e 's/\S\+/./g' \
+    | sed -e 's/\./(snapshot ID)/' \
+          -e 's/\./(snapshot name)/' \
+          -e 's/\./(VM state size value)/' \
+          -e 's/\./(VM state size unit)/' \
+          -e 's/\./(snapshot date)/' \
+          -e 's/\./(snapshot time)/' \
+          -e 's/\./(VM clock)/'
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/284.out b/tests/qemu-iotests/284.out
new file mode 100644
index 0000000000..13ced439be
--- /dev/null
+++ b/tests/qemu-iotests/284.out
@@ -0,0 +1,8 @@
+QA output created by 284
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) savevm abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz
+(qemu) quit
+Output structure:
+(snapshot ID) (snapshot name) (VM state size value) (VM state size unit) (snapshot date) (snapshot time) (VM clock)
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index cb2b789e44..f8ce344f83 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -288,3 +288,4 @@
 277 rw quick
 279 rw backing quick
 280 rw migration quick
+284 rw quick
-- 
2.24.1

Re: [PATCH 1/2] block: Fix VM size field width in snapshot dump

[Eric Blake]

On 1/17/20 4:58 AM, Max Reitz wrote:
> When printing the snapshot list (e.g. with qemu-img snapshot -l), the VM
> size field is only seven characters wide.  As of de38b5005e9, this is
> not necessarily sufficient: We generally print three digits, and this
> may require a decimal point.  Also, the unit field grew from something
> as plain as "M" to " MiB".  This means that number and unit may take up
> eight characters in total; but we also want spaces in front.
> 
> Considering previously the maximum width was four characters and the
> field width was chosen to be three characters wider, let us adjust the
> field width to be eleven now.
> 
> Fixes: de38b5005e946aa3714963ea4c501e279e7d3666

My patch - hmm, yet another example of an "obvious bugfix" having 
non-obvious consequences :)

> Buglink: https://bugs.launchpad.net/qemu/+bug/1859989
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>   block/qapi.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH 2/2] iotests: Test snapshot -l field separation

[Eric Blake]

On 1/17/20 4:58 AM, Max Reitz wrote:
> Add a test that all fields in "qemu-img snapshot -l"s output are
> separated by spaces.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>   tests/qemu-iotests/284     | 76 ++++++++++++++++++++++++++++++++++++++
>   tests/qemu-iotests/284.out |  8 ++++
>   tests/qemu-iotests/group   |  1 +
>   3 files changed, 85 insertions(+)
>   create mode 100755 tests/qemu-iotests/284
>   create mode 100644 tests/qemu-iotests/284.out
> 

> +# Check that all fields are separated by spaces.
> +# We first collapse all space sequences into one space each;
> +# then we turn every space-separated field into a '.';
> +# and finally, we name the '.'s so the output is not just a confusing
> +# sequence of dots.
> +
> +echo 'Output structure:'
> +$QEMU_IMG snapshot -l "$TEST_IMG" | tail -n 1 | tr -s ' ' \
> +    | sed -e 's/\S\+/./g' \
> +    | sed -e 's/\./(snapshot ID)/' \
> +          -e 's/\./(snapshot name)/' \
> +          -e 's/\./(VM state size value)/' \
> +          -e 's/\./(VM state size unit)/' \
> +          -e 's/\./(snapshot date)/' \
> +          -e 's/\./(snapshot time)/' \
> +          -e 's/\./(VM clock)/'

Cute conversion.  If you had picked some other character (like 
s/\S\+/=/g), you wouldn't have to use \. everywhere in the second sed, 
for less typing, but that's aesthetic, so no need to change if you don't 
want.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH 0/2] block: Fix VM size field width in snapshot dump

[Max Reitz]

[-- Attachment #1.1: Type: text/plain, Size: 523 bytes --]

On 17.01.20 11:58, Max Reitz wrote:
> Hi,
> 
> https://bugs.launchpad.net/qemu/+bug/1859989 reports that fields in
> "qemu-img snapshot -l"s output are not always separated by spaces in
> 4.1.1.  Fix that.
> 
> 
> Branch: https://github.com/XanClic/qemu.git lp-1859989-v1
> Branch: https://git.xanclic.moe/XanClic/qemu.git lp-1859989-v1

Thanks for the review, I renamed the test from 284 to 286, and applied
the series to my block branch:

https://git.xanclic.moe/XanClic/qemu/commits/branch/block

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]