Patchwork [v5,6/6] qemu-iotests: Additional info from qemu-img info

login
register
mail settings
Submitter Max Reitz
Date Sept. 23, 2013, 12:09 p.m.
Message ID <1379938162-14005-7-git-send-email-mreitz@redhat.com>
Download mbox | patch
Permalink /patch/277161/
State New
Headers show

Comments

Max Reitz - Sept. 23, 2013, 12:09 p.m.
Add a test for the additional information now provided by qemu-img info
when used on qcow2 images.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/065     | 72 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/065.out | 22 ++++++++++++++
 tests/qemu-iotests/group   |  1 +
 3 files changed, 95 insertions(+)
 create mode 100755 tests/qemu-iotests/065
 create mode 100644 tests/qemu-iotests/065.out
Eric Blake - Sept. 30, 2013, 5:19 p.m.
On 09/23/2013 06:09 AM, Max Reitz wrote:
> Add a test for the additional information now provided by qemu-img info
> when used on qcow2 images.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  tests/qemu-iotests/065     | 72 ++++++++++++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/065.out | 22 ++++++++++++++
>  tests/qemu-iotests/group   |  1 +
>  3 files changed, 95 insertions(+)
>  create mode 100755 tests/qemu-iotests/065
>  create mode 100644 tests/qemu-iotests/065.out

This patch only tests human output, not JSON.

> +# creator
> +owner=mreitz@redhat.com
> +
> +seq=`basename $0`
> +echo "QA output created by $seq"
> +
> +here=`pwd`

Not your fault (copy-and-paste from other tests), but as long as we are
requiring bash, $PWD is much faster than `pwd`, and $() is nicer than ``
where we don't have even faster shortcuts like $PWD.


> +=== Testing qcow2 image with -o compat=0.10 ===
> +
> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
> +Format specific information:
> +compat: 0.10

Should we be indenting the human output, to make it obvious how many
remaining fields are being output as a result of format specific
information?

I'm still not happy with 5/6 in its current form, but can live with this
patch as-is if we don't bother with testing JSON form.  Does 5/6 even
need to worry about stripping JSON form, if you aren't going to test
JSON form?  Depending on the answer to that question:

Reviewed-by: Eric Blake <eblake@redhat.com>
Max Reitz - Oct. 1, 2013, 9:19 a.m.
On 2013-09-30 19:19, Eric Blake wrote:
> On 09/23/2013 06:09 AM, Max Reitz wrote:
>> Add a test for the additional information now provided by qemu-img info
>> when used on qcow2 images.
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>>   tests/qemu-iotests/065     | 72 ++++++++++++++++++++++++++++++++++++++++++++++
>>   tests/qemu-iotests/065.out | 22 ++++++++++++++
>>   tests/qemu-iotests/group   |  1 +
>>   3 files changed, 95 insertions(+)
>>   create mode 100755 tests/qemu-iotests/065
>>   create mode 100644 tests/qemu-iotests/065.out
> This patch only tests human output, not JSON.
I'd not be happy at all with testing JSON output through a shell script 
(or rather, without properly parsing it). Since I don't really see the 
point of testing the JSON output as well: It is basically generated the 
same way as the human-readable output, however, the latter uses the new 
bdrv_image_info_specific_dump function, so I think the human-readable 
output is actually more "error-prone" (and if something breaks the JSON 
output that doesn't break other JSON tests, it should break the 
human-readable output as well).

So I'll leave the JSON test out and include a note in the commit message.

>> +# creator
>> +owner=mreitz@redhat.com
>> +
>> +seq=`basename $0`
>> +echo "QA output created by $seq"
>> +
>> +here=`pwd`
> Not your fault (copy-and-paste from other tests), but as long as we are
> requiring bash, $PWD is much faster than `pwd`, and $() is nicer than ``
> where we don't have even faster shortcuts like $PWD.
Well, it's executed just once per test, so it shouldn't be that much of 
a performance killer, but I'll change it anyway, thanks. ;)

>> +=== Testing qcow2 image with -o compat=0.10 ===
>> +
>> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
>> +Format specific information:
>> +compat: 0.10
> Should we be indenting the human output, to make it obvious how many
> remaining fields are being output as a result of format specific
> information?
Seems very reasonable. I'll have to change one of the other patches for 
this as well, but this should be a very minor change.

> I'm still not happy with 5/6 in its current form, but can live with this
> patch as-is if we don't bother with testing JSON form.  Does 5/6 even
> need to worry about stripping JSON form, if you aren't going to test
> JSON form?
As I've said in my answer to patch 5, stripping JSON isn't for this new 
test, but rather for compatibility with the old tests (specifically, 
test 043).

Max

Patch

diff --git a/tests/qemu-iotests/065 b/tests/qemu-iotests/065
new file mode 100755
index 0000000..5c56b56
--- /dev/null
+++ b/tests/qemu-iotests/065
@@ -0,0 +1,72 @@ 
+#!/bin/bash
+#
+# Test for additional information emitted by qemu-img info on qcow2
+# images
+#
+# Copyright (C) 2013 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/>.
+#
+
+# creator
+owner=mreitz@redhat.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+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
+
+# This tests qocw2-specific low-level functionality
+_supported_fmt qcow2
+_supported_proto generic
+_supported_os Linux
+
+IMG_SIZE=64M
+
+echo
+echo "=== Testing qcow2 image with -o compat=0.10 ==="
+echo
+IMGOPTS="compat=0.10" _make_test_img $IMG_SIZE
+# don't use _img_info, since that function will filter out the
+# additional information we're about to test for
+$QEMU_IMG info "$TEST_IMG" | sed -n '/^Format specific information:$/,$p'
+
+echo
+echo "=== Testing qcow2 image with -o compat=1.1,lazy_refcounts=off ==="
+echo
+IMGOPTS="compat=1.1,lazy_refcounts=off" _make_test_img $IMG_SIZE
+$QEMU_IMG info "$TEST_IMG" | sed -n '/^Format specific information:$/,$p'
+
+echo
+echo "=== Testing qcow2 image with -o compat=1.1,lazy_refcounts=on ==="
+echo
+IMGOPTS="compat=1.1,lazy_refcounts=on" _make_test_img $IMG_SIZE
+$QEMU_IMG info "$TEST_IMG" | sed -n '/^Format specific information:$/,$p'
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/065.out b/tests/qemu-iotests/065.out
new file mode 100644
index 0000000..4dd7f2b
--- /dev/null
+++ b/tests/qemu-iotests/065.out
@@ -0,0 +1,22 @@ 
+QA output created by 065
+
+=== Testing qcow2 image with -o compat=0.10 ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
+Format specific information:
+compat: 0.10
+
+=== Testing qcow2 image with -o compat=1.1,lazy_refcounts=off ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
+Format specific information:
+compat: 1.1
+lazy refcounts: false
+
+=== Testing qcow2 image with -o compat=1.1,lazy_refcounts=on ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
+Format specific information:
+compat: 1.1
+lazy refcounts: true
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 1ad02e5..f1a68b0 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -69,3 +69,4 @@ 
 061 rw auto
 062 rw auto
 063 rw auto
+065 rw auto