| Message ID | 1458569512-22970-9-git-send-email-berrange@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 03/21/2016 08:11 AM, Daniel P. Berrange wrote: > Currently all block tests use the traditional syntax for images > just specifying a filename. To support the LUKS driver without > resorting to JSON, the tests need to be able to use the new > --image-opts argument to qemu-img and qemu-io. > > This introduces a new env variable IMGOPTSSYNTAX. If this is Would IMG_OPTS_SYNTAX be any more legible? > set to 'true', then qemu-img/qemu-io should use --image-opts. > > Signed-off-by: Daniel P. Berrange <berrange@redhat.com> > --- > tests/qemu-iotests/common | 7 ++++- > tests/qemu-iotests/common.config | 15 +++++++++-- > tests/qemu-iotests/common.rc | 58 +++++++++++++++++++++++++++++----------- > 3 files changed, 62 insertions(+), 18 deletions(-) > > diff --git a/tests/qemu-iotests/common b/tests/qemu-iotests/common > index ff84f4b..05c9df2 100644 > --- a/tests/qemu-iotests/common > +++ b/tests/qemu-iotests/common > @@ -53,6 +53,7 @@ export QEMU_IO_OPTIONS="" > export CACHEMODE_IS_DEFAULT=true > export QEMU_OPTIONS="-nodefaults" > export VALGRIND_QEMU= > +export IMGOPTSSYNTAX=false Particularly since we use _ between words in other variables above. > @@ -199,7 +221,13 @@ _cleanup_test_img() > > _check_test_img() > { > - $QEMU_IMG check "$@" -f $IMGFMT "$TEST_IMG" 2>&1 | _filter_testdir | \ > + ( > + if [ "$IMGOPTSSYNTAX" = "true" ]; then > + $QEMU_IMG check --image-opts "$@" "$TEST_IMG" 2>&1 > + else > + $QEMU_IMG check "$@" -f $IMGFMT "$TEST_IMG" 2>&1 > + fi > + ) | _filter_testdir | \ Would '{ if ... fi; } |' be any better than a subshell? But the idea looks to be on track.
On Mon, Mar 21, 2016 at 02:08:16PM -0600, Eric Blake wrote: > On 03/21/2016 08:11 AM, Daniel P. Berrange wrote: > > Currently all block tests use the traditional syntax for images > > just specifying a filename. To support the LUKS driver without > > resorting to JSON, the tests need to be able to use the new > > --image-opts argument to qemu-img and qemu-io. > > > > This introduces a new env variable IMGOPTSSYNTAX. If this is > > Would IMG_OPTS_SYNTAX be any more legible? [snip] > > @@ -53,6 +53,7 @@ export QEMU_IO_OPTIONS="" > > export CACHEMODE_IS_DEFAULT=true > > export QEMU_OPTIONS="-nodefaults" > > export VALGRIND_QEMU= > > +export IMGOPTSSYNTAX=false > > Particularly since we use _ between words in other variables above. It isn't visible from the diff context but just above these quoted lines we have IMGFMT, IMGPROTO and IMGOPTS, though we then also have the inconssitent IMGFMT_GENERIC :-) > > _check_test_img() > > { > > - $QEMU_IMG check "$@" -f $IMGFMT "$TEST_IMG" 2>&1 | _filter_testdir | \ > > + ( > > + if [ "$IMGOPTSSYNTAX" = "true" ]; then > > + $QEMU_IMG check --image-opts "$@" "$TEST_IMG" 2>&1 > > + else > > + $QEMU_IMG check "$@" -f $IMGFMT "$TEST_IMG" 2>&1 > > + fi > > + ) | _filter_testdir | \ > > Would '{ if ... fi; } |' be any better than a subshell? FWIW this is the style used elsewhere in the I/O tests Regards, Daniel
On 21.03.2016 15:11, Daniel P. Berrange wrote: > Currently all block tests use the traditional syntax for images > just specifying a filename. To support the LUKS driver without > resorting to JSON, the tests need to be able to use the new > --image-opts argument to qemu-img and qemu-io. > > This introduces a new env variable IMGOPTSSYNTAX. If this is > set to 'true', then qemu-img/qemu-io should use --image-opts. > > Signed-off-by: Daniel P. Berrange <berrange@redhat.com> > --- > tests/qemu-iotests/common | 7 ++++- > tests/qemu-iotests/common.config | 15 +++++++++-- > tests/qemu-iotests/common.rc | 58 +++++++++++++++++++++++++++++----------- > 3 files changed, 62 insertions(+), 18 deletions(-) > [...] > diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc > index d9913f8..5eb654b 100644 > --- a/tests/qemu-iotests/common.rc > +++ b/tests/qemu-iotests/common.rc > @@ -53,21 +53,43 @@ fi > # make sure we have a standard umask > umask 022 > > -if [ "$IMGPROTO" = "file" ]; then > - TEST_IMG=$TEST_DIR/t.$IMGFMT > -elif [ "$IMGPROTO" = "nbd" ]; then > - TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT > - TEST_IMG="nbd:127.0.0.1:10810" > -elif [ "$IMGPROTO" = "ssh" ]; then > - TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT > - TEST_IMG="ssh://127.0.0.1$TEST_IMG_FILE" > -elif [ "$IMGPROTO" = "nfs" ]; then > - TEST_DIR="nfs://127.0.0.1/$TEST_DIR" > - TEST_IMG=$TEST_DIR/t.$IMGFMT > -elif [ "$IMGPROTO" = "archipelago" ]; then > - TEST_IMG="archipelago:at.$IMGFMT" > +if [ "$IMGOPTSSYNTAX" = "true" ]; then > + DRIVER="driver=$IMGFMT" > + if [ "$IMGPROTO" = "file" ]; then > + TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT > + TEST_IMG="$DRIVER,file.filename=$TEST_DIR/t.$IMGFMT" > + elif [ "$IMGPROTO" = "nbd" ]; then > + TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT > + TEST_IMG="$DRIVER,file.driver=nbd,file.host=127.0.0.1,file.port=10810" > + elif [ "$IMGPROTO" = "ssh" ]; then > + TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT > + TEST_IMG="$DRIVER,file.driver=ssh,file.host=127.0.0.1,file.path=$TEST_IMG_FILE" > + elif [ "$IMGPROTO" = "nfs" ]; then > + TEST_DIR="$DRIVER,file.driver=nfs,file.filename=nfs://127.0.0.1/$TEST_DIR" > + TEST_IMG=$TEST_DIR_OPTS/t.$IMGFMT > + elif [ "$IMGPROTO" = "archipelago" ]; then > + TEST_IMG="$DRIVER,file.driver=archipelago,file.volume=:at.$IMGFMT" > + else > + TEST_IMG="$DRIVER,file.driver=$IMGPROTO,file.filename=$TEST_DIR/t.$IMGFMT" > + fi > else > - TEST_IMG=$IMGPROTO:$TEST_DIR/t.$IMGFMT > + if [ "$IMGPROTO" = "file" ]; then > + TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT This wasn't set before (in this case). Doing so breaks many qcow2+file tests (28, to be exact), because they rely on being able to do something like TEST_IMG="${TEST_IMG}.base" _make_test_img which fails now because _make_test_img resorts to TEST_IMG_FILE. I guess the fix would be for them to use TEST_IMG_FILE in those places instead of TEST_IMG; but it's not always so simple. For instance, test 017 sets TEST_IMG and then relies on the io() function provided by common.pattern to use that image, so maybe 017 would need to set both TEST_IMG and TEST_IMG_FILE. Max > + TEST_IMG=$TEST_DIR/t.$IMGFMT > + elif [ "$IMGPROTO" = "nbd" ]; then > + TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT > + TEST_IMG="nbd:127.0.0.1:10810" > + elif [ "$IMGPROTO" = "ssh" ]; then > + TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT > + TEST_IMG="ssh://127.0.0.1$TEST_IMG_FILE" > + elif [ "$IMGPROTO" = "nfs" ]; then > + TEST_DIR="nfs://127.0.0.1/$TEST_DIR" > + TEST_IMG=$TEST_DIR/t.$IMGFMT > + elif [ "$IMGPROTO" = "archipelago" ]; then > + TEST_IMG="archipelago:at.$IMGFMT" > + else > + TEST_IMG=$IMGPROTO:$TEST_DIR/t.$IMGFMT > + fi > fi > > _optstr_add()
diff --git a/tests/qemu-iotests/common b/tests/qemu-iotests/common index ff84f4b..05c9df2 100644 --- a/tests/qemu-iotests/common +++ b/tests/qemu-iotests/common @@ -53,6 +53,7 @@ export QEMU_IO_OPTIONS="" export CACHEMODE_IS_DEFAULT=true export QEMU_OPTIONS="-nodefaults" export VALGRIND_QEMU= +export IMGOPTSSYNTAX=false for r do @@ -398,7 +399,11 @@ BEGIN { for (t='$start'; t<='$end'; t++) printf "%03d\n",t }' \ done # Set qemu-io cache mode with $CACHEMODE we have -QEMU_IO_OPTIONS="$QEMU_IO_OPTIONS -f $IMGFMT --cache $CACHEMODE" +if [ "$IMGOPTSSYNTAX" = "true" ]; then + QEMU_IO_OPTIONS="$QEMU_IO_OPTIONS --cache $CACHEMODE" +else + QEMU_IO_OPTIONS="$QEMU_IO_OPTIONS -f $IMGFMT --cache $CACHEMODE" +fi # Set default options for qemu-img create -o if they were not specified _set_default_imgopts diff --git a/tests/qemu-iotests/common.config b/tests/qemu-iotests/common.config index 60bfabf..6d4c829 100644 --- a/tests/qemu-iotests/common.config +++ b/tests/qemu-iotests/common.config @@ -123,12 +123,16 @@ _qemu_img_wrapper() _qemu_io_wrapper() { local VALGRIND_LOGFILE=/tmp/$$.valgrind + local QEMU_IO_ARGS="$QEMU_IO_OPTIONS" + if [ "$IMGOPTSSYNTAX" = "true" ]; then + QEMU_IO_ARGS="--image-opts $QEMU_IO_ARGS" + fi local RETVAL ( if [ "${VALGRIND_QEMU}" == "y" ]; then - exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@" + exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" else - exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@" + exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" fi ) RETVAL=$? @@ -154,6 +158,13 @@ export QEMU_IMG=_qemu_img_wrapper export QEMU_IO=_qemu_io_wrapper export QEMU_NBD=_qemu_nbd_wrapper +QEMU_IMG_EXTRA_ARGS= +if [ "$IMGOPTSSYNTAX" = "true" ]; then + QEMU_IMG_EXTRA_ARGS="--image-opts $QEMU_IMG_EXTRA_ARGS" +fi +export QEMU_IMG_EXTRA_ARGS + + default_machine=$($QEMU -machine help | sed -n '/(default)/ s/ .*//p') default_alias_machine=$($QEMU -machine help | \ sed -n "/(alias of $default_machine)/ { s/ .*//p; q; }") diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc index d9913f8..5eb654b 100644 --- a/tests/qemu-iotests/common.rc +++ b/tests/qemu-iotests/common.rc @@ -53,21 +53,43 @@ fi # make sure we have a standard umask umask 022 -if [ "$IMGPROTO" = "file" ]; then - TEST_IMG=$TEST_DIR/t.$IMGFMT -elif [ "$IMGPROTO" = "nbd" ]; then - TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT - TEST_IMG="nbd:127.0.0.1:10810" -elif [ "$IMGPROTO" = "ssh" ]; then - TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT - TEST_IMG="ssh://127.0.0.1$TEST_IMG_FILE" -elif [ "$IMGPROTO" = "nfs" ]; then - TEST_DIR="nfs://127.0.0.1/$TEST_DIR" - TEST_IMG=$TEST_DIR/t.$IMGFMT -elif [ "$IMGPROTO" = "archipelago" ]; then - TEST_IMG="archipelago:at.$IMGFMT" +if [ "$IMGOPTSSYNTAX" = "true" ]; then + DRIVER="driver=$IMGFMT" + if [ "$IMGPROTO" = "file" ]; then + TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT + TEST_IMG="$DRIVER,file.filename=$TEST_DIR/t.$IMGFMT" + elif [ "$IMGPROTO" = "nbd" ]; then + TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT + TEST_IMG="$DRIVER,file.driver=nbd,file.host=127.0.0.1,file.port=10810" + elif [ "$IMGPROTO" = "ssh" ]; then + TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT + TEST_IMG="$DRIVER,file.driver=ssh,file.host=127.0.0.1,file.path=$TEST_IMG_FILE" + elif [ "$IMGPROTO" = "nfs" ]; then + TEST_DIR="$DRIVER,file.driver=nfs,file.filename=nfs://127.0.0.1/$TEST_DIR" + TEST_IMG=$TEST_DIR_OPTS/t.$IMGFMT + elif [ "$IMGPROTO" = "archipelago" ]; then + TEST_IMG="$DRIVER,file.driver=archipelago,file.volume=:at.$IMGFMT" + else + TEST_IMG="$DRIVER,file.driver=$IMGPROTO,file.filename=$TEST_DIR/t.$IMGFMT" + fi else - TEST_IMG=$IMGPROTO:$TEST_DIR/t.$IMGFMT + if [ "$IMGPROTO" = "file" ]; then + TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT + TEST_IMG=$TEST_DIR/t.$IMGFMT + elif [ "$IMGPROTO" = "nbd" ]; then + TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT + TEST_IMG="nbd:127.0.0.1:10810" + elif [ "$IMGPROTO" = "ssh" ]; then + TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT + TEST_IMG="ssh://127.0.0.1$TEST_IMG_FILE" + elif [ "$IMGPROTO" = "nfs" ]; then + TEST_DIR="nfs://127.0.0.1/$TEST_DIR" + TEST_IMG=$TEST_DIR/t.$IMGFMT + elif [ "$IMGPROTO" = "archipelago" ]; then + TEST_IMG="archipelago:at.$IMGFMT" + else + TEST_IMG=$IMGPROTO:$TEST_DIR/t.$IMGFMT + fi fi _optstr_add() @@ -199,7 +221,13 @@ _cleanup_test_img() _check_test_img() { - $QEMU_IMG check "$@" -f $IMGFMT "$TEST_IMG" 2>&1 | _filter_testdir | \ + ( + if [ "$IMGOPTSSYNTAX" = "true" ]; then + $QEMU_IMG check --image-opts "$@" "$TEST_IMG" 2>&1 + else + $QEMU_IMG check "$@" -f $IMGFMT "$TEST_IMG" 2>&1 + fi + ) | _filter_testdir | \ sed -e '/allocated.*fragmented.*compressed clusters/d' \ -e 's/qemu-img: This image format does not support checks/No errors were found on the image./' \ -e '/Image end offset: [0-9]\+/d'
Currently all block tests use the traditional syntax for images just specifying a filename. To support the LUKS driver without resorting to JSON, the tests need to be able to use the new --image-opts argument to qemu-img and qemu-io. This introduces a new env variable IMGOPTSSYNTAX. If this is set to 'true', then qemu-img/qemu-io should use --image-opts. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- tests/qemu-iotests/common | 7 ++++- tests/qemu-iotests/common.config | 15 +++++++++-- tests/qemu-iotests/common.rc | 58 +++++++++++++++++++++++++++++----------- 3 files changed, 62 insertions(+), 18 deletions(-)