Re: [Qemu-devel] [PATCH V12 6/6] add-cow: add qemu-iotests support

[Kevin Wolf]

Am 10.08.2012 17:39, schrieb Dong Xu Wang:
> Add qemu-iotests support for add-cow.
> 
> Signed-off-by: Dong Xu Wang <address@hidden>
> ---
>  tests/qemu-iotests/017       |    2 +-
>  tests/qemu-iotests/020       |    2 +-
>  tests/qemu-iotests/check     |    4 ++--
>  tests/qemu-iotests/common    |    6 ++++++
>  tests/qemu-iotests/common.rc |   19 +++++++++++++++++++
>  5 files changed, 29 insertions(+), 4 deletions(-)

> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
> index 432732c..122267b 100755
> --- a/tests/qemu-iotests/check
> +++ b/tests/qemu-iotests/check
> @@ -243,7 +243,7 @@ do
>               echo " - no qualified output"
>               err=true
>           else
> -             if diff -w $seq.out $tmp.out >/dev/null 2>&1
> +        if diff -w -I "^Formatting" $seq.out $tmp.out >/dev/null 2>&1
>               then
>                   echo ""
>                   if $err
> @@ -255,7 +255,7 @@ do
>               else
>                   echo " - output mismatch (see $seq.out.bad)"
>                   mv $tmp.out $seq.out.bad
> -                 $diff -w $seq.out $seq.out.bad
> +            $diff -w -I "^Formatting" $seq.out $seq.out.bad
>                   err=true
>               fi
>           fi

These two hunks don't look right. You probably want to amend the sed
command in _make_test_img().

> diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
> index 7782808..ec5afd7 100644
> --- a/tests/qemu-iotests/common.rc
> +++ b/tests/qemu-iotests/common.rc
> @@ -97,6 +97,18 @@ _make_test_img()
>      fi
>      if [ \( "$IMGFMT" = "qcow2" -o "$IMGFMT" = "qed" \) -a -n 
> "$CLUSTER_SIZE" ]; then
>          optstr=$(_optstr_add "$optstr" "cluster_size=$CLUSTER_SIZE")
> +    elif [ "$IMGFMT" = "add-cow" ]; then
> +        local BACKING="$TEST_IMG"".qcow2"
> +        local IMG="$TEST_IMG"".raw"
> +        if [ "$1" = "-b" ]; then
> +            IMG="$IMG"".b"
> +            $QEMU_IMG create -f raw $IMG $image_size>/dev/null
> +            extra_img_options="-o image_file=$IMG $extra_img_options"
> +        else
> +            $QEMU_IMG create -f raw $IMG $image_size>/dev/null
> +            $QEMU_IMG create -f qcow2 $BACKING $image_size>/dev/null
> +            extra_img_options="-o backing_file=$BACKING,image_file=$IMG"
> +        fi

This looks a bit hackish... Doesn't it completely ignore the requested
backing file name? I'm not sure if this is a good idea.

Can't you just create the raw image file and then use _optstr_add to add
the right -o image_file=... option? It should automatically get the
backing file right.

>      fi
>  
>      if [ -n "$optstr" ]; then
> @@ -125,6 +137,13 @@ _cleanup_test_img()
>              rm -f $TEST_DIR/t.$IMGFMT
>              rm -f $TEST_DIR/t.$IMGFMT.orig
>              rm -f $TEST_DIR/t.$IMGFMT.base
> +            if [ "$IMGFMT" = "add-cow" ]; then
> +                rm -f $TEST_DIR/t.$IMGFMT.qcow2
> +                rm -f $TEST_DIR/t.$IMGFMT.raw
> +                rm -f $TEST_DIR/t.$IMGFMT.raw.b
> +                rm -f $TEST_DIR/t.$IMGFMT.ct.qcow2
> +                rm -f $TEST_DIR/t.$IMGFMT.ct.raw

What are the .ct files?

Kevin