Re: [Qemu-devel] [PATCH] qemu-iotests: Test qcow2 image creation options

[Markus Armbruster]

Kevin Wolf <address@hidden> writes:

> Just create lots of images and try out each of the creation options that
> qcow2 provides (except backing_file/fmt for now)
>
> I'm not totally happy with the behaviour of qemu-img in each of the
> cases, but let's be explicit and update the test when we do change
> things later.
>
> Signed-off-by: Kevin Wolf <address@hidden>
> ---
>  tests/qemu-iotests/048     |  117 ++++++++++++++++++++++++
>  tests/qemu-iotests/048.out |  213 
> ++++++++++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/group   |    1 +
>  3 files changed, 331 insertions(+), 0 deletions(-)
>  create mode 100755 tests/qemu-iotests/048
>  create mode 100644 tests/qemu-iotests/048.out
>
> diff --git a/tests/qemu-iotests/048 b/tests/qemu-iotests/048
> new file mode 100755
> index 0000000..89d3d32
> --- /dev/null
> +++ b/tests/qemu-iotests/048
> @@ -0,0 +1,117 @@
> +#!/bin/bash
> +#
> +# Check qemu-img option parsing
> +#
> +# 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
> address@hidden
> +
> +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
> +
> +_supported_fmt qcow2
> +_supported_proto file
> +_supported_os Linux
> +
> +function test_qemu_img()
> +{
> +    echo qemu-img "$@"
> +    $QEMU_IMG "$@"
> +    echo
> +}
> +
> +echo "=== Check correct interpretation of suffixes for image size ==="
> +echo
> +sizes="1024 1024b 1k 1K 1M 1G 1T "
> +sizes+="1024.0 1024.0b 1.5k 1.5K 1.5M 1.5G 1.5T"
> +
> +echo "== 1. Traditional size parameter =="
> +echo
> +for s in $sizes; do
> +    test_qemu_img create -f $IMGFMT $TEST_IMG $s
> +done

Do we have a qemu-option.c unit test?  If not, should we have one?

Even if yes, testing qemu-img like this still makes sense.  But fewer
test cases could do.

> +
> +echo "== 2. Specifying size via -o =="
> +echo
> +for s in $sizes; do
> +    test_qemu_img create -f $IMGFMT -o size=$s $TEST_IMG
> +done
> +
> +echo "== 3. Invalid sizes =="
> +echo
> +sizes="-1024 -1k 1kilobyte foobar"
> +
> +for s in $sizes; do
> +    test_qemu_img create -f $IMGFMT $TEST_IMG -- $s
> +    test_qemu_img create -f $IMGFMT -o size=$s $TEST_IMG
> +done

Valid sizes are tested in two loops, one for size parameter, one for -o,
invalid sizes in a single combined loop.  I don't mind, I just find it
odd.

> +
> +echo "== Check correct interpretation of suffixes for cluster size =="
> +echo
> +sizes="1024 1024b 1k 1K 1M "
> +sizes+="1024.0 1024.0b 0.5k 0.5K 0.5M"
> +
> +for s in $sizes; do
> +    test_qemu_img create -f $IMGFMT -o cluster_size=$s $TEST_IMG 64M
> +done
> +
> +echo "== Check compat level option =="
> +echo
> +test_qemu_img create -f $IMGFMT -o compat=0.10 $TEST_IMG 64M
> +test_qemu_img create -f $IMGFMT -o compat=1.1 $TEST_IMG 64M
> +
> +test_qemu_img create -f $IMGFMT -o compat=0.42 $TEST_IMG 64M
> +test_qemu_img create -f $IMGFMT -o compat=foobar $TEST_IMG 64M
> +
> +echo "== Check preallocation option =="
> +echo
> +test_qemu_img create -f $IMGFMT -o preallocation=off $TEST_IMG 64M
> +test_qemu_img create -f $IMGFMT -o preallocation=metadata $TEST_IMG 64M
> +test_qemu_img create -f $IMGFMT -o preallocation=1234 $TEST_IMG 64M
> +
> +echo "== Check encryption option =="
> +echo
> +test_qemu_img create -f $IMGFMT -o encryption=off $TEST_IMG 64M
> +test_qemu_img create -f $IMGFMT -o encryption=on $TEST_IMG 64M
> +
> +echo "== Check lazy_refcounts option (only with v3) =="
> +echo
> +test_qemu_img create -f $IMGFMT -o compat=1.1,lazy_refcounts=off $TEST_IMG 
> 64M
> +test_qemu_img create -f $IMGFMT -o compat=1.1,lazy_refcounts=on $TEST_IMG 64M
> +
> +test_qemu_img create -f $IMGFMT -o compat=0.10,lazy_refcounts=off $TEST_IMG 
> 64M
> +test_qemu_img create -f $IMGFMT -o compat=0.10,lazy_refcounts=on $TEST_IMG 
> 64M
> +
> +# success, all done
> +echo "*** done"
> +rm -f $seq.full
> +status=0

Let's examine coverage:

    #define BLOCK_OPT_SIZE              "size"
    Check

    #define BLOCK_OPT_ENCRYPT           "encryption"
    Check

    #define BLOCK_OPT_COMPAT6           "compat6"
    Not covered, only used by vmdk.

    #define BLOCK_OPT_BACKING_FILE      "backing_file"
    Not covered, mentioned in commit message.  Okay.

    #define BLOCK_OPT_BACKING_FMT       "backing_fmt"
    Likewise

    #define BLOCK_OPT_CLUSTER_SIZE      "cluster_size"
    Check

    #define BLOCK_OPT_TABLE_SIZE        "table_size"
    Not covered, only used by qed.

    #define BLOCK_OPT_PREALLOC          "preallocation"
    Check

    #define BLOCK_OPT_SUBFMT            "subformat"
    Not covered, used by vmdk and vpc.

    #define BLOCK_OPT_COMPAT_LEVEL      "compat"
    Check

    #define BLOCK_OPT_LAZY_REFCOUNTS    "lazy_refcounts"
    Check

Good enough for me, and matches commit message.

> diff --git a/tests/qemu-iotests/048.out b/tests/qemu-iotests/048.out
> new file mode 100644
> index 0000000..8569f3e
> --- /dev/null
> +++ b/tests/qemu-iotests/048.out
[...]

I figure this is simply current output, and we assume it's good.

> diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
> index 1bbd2bf..a4175b2 100644
> --- a/tests/qemu-iotests/group
> +++ b/tests/qemu-iotests/group
> @@ -54,3 +54,4 @@
>  045 rw auto
>  046 rw auto aio
>  047 rw auto
> +048 rw auto

Reviewed-by: Markus Armbruster <address@hidden>