Re: [PATCH v3 2/4] iotests: Switch nbd tests to use Unix rather than TCP

[Max Reitz]

On 14.11.19 22:34, Eric Blake wrote:
> Up to now, all it took to cause a lot of iotest failures was to have a
> background process such as 'nbdkit -p 10810 null' running, because we
> hard-coded the TCP port.  Switching to a Unix socket eliminates this
> contention.  We still have TCP coverage in test 233, and that test is
> more careful to not pick a hard-coded port.

For me, all it took was to run qcow2 and nbd tests in parallel (some
qcow2 tests create nbd servers, too), so this is great.

> Signed-off-by: Eric Blake <address@hidden>
> ---
>  tests/qemu-iotests/common.filter | 6 ++++--
>  tests/qemu-iotests/common.rc     | 8 ++++----
>  2 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/qemu-iotests/common.filter 
> b/tests/qemu-iotests/common.filter
> index f870e00e4421..5367deea398e 100644
> --- a/tests/qemu-iotests/common.filter
> +++ b/tests/qemu-iotests/common.filter
> @@ -127,7 +127,8 @@ _filter_img_create()
>          -e "s#$TEST_DIR#TEST_DIR#g" \
>          -e "s#$SOCK_DIR#SOCK_DIR#g" \
>          -e "s#$IMGFMT#IMGFMT#g" \
> -        -e 's#nbd:127.0.0.1:10810#TEST_DIR/t.IMGFMT#g' \
> +        -e 's#nbd:127.0.0.1:[0-9]\\+#TEST_DIR/t.IMGFMT#g' \
> +        -e 's#nbd+unix:///\??socket=SOCK_DIR/nbd#TEST_DIR/t.IMGFMT#g' \

Why the second question mark?  I thought the ? after the /// was mandatory.

>          -e "s# encryption=off##g" \
>          -e "s# cluster_size=[0-9]\\+##g" \
>          -e "s# table_size=[0-9]\\+##g" \
> @@ -164,7 +165,8 @@ _filter_img_info()
>          -e "s#$TEST_DIR#TEST_DIR#g" \
>          -e "s#$SOCK_DIR#SOCK_DIR#g" \
>          -e "s#$IMGFMT#IMGFMT#g" \
> -        -e 's#nbd://127.0.0.1:10810$#TEST_DIR/t.IMGFMT#g' \
> +        -e 's#nbd://127.0.0.1:[0-9]\\+$#TEST_DIR/t.IMGFMT#g' \
> +        -e 's#nbd+unix:///\??socket=SOCK_DIR/nbd#TEST_DIR/t.IMGFMT#g' \
>          -e 's#json.*vdisk-id.*vxhs"}}#TEST_DIR/t.IMGFMT#' \
>          -e "/encrypted: yes/d" \
>          -e "/cluster_size: [0-9]\\+/d" \
> diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
> index fa7bae24226a..f772dcb67322 100644
> --- a/tests/qemu-iotests/common.rc
> +++ b/tests/qemu-iotests/common.rc
> @@ -217,7 +217,7 @@ if [ "$IMGOPTSSYNTAX" = "true" ]; then
>          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"
> +        
> TEST_IMG="$DRIVER,file.driver=nbd,file.type=unix,file.path=$SOCKDIR/$IMGFMT"

Maybe nbd.$IMGFMT?

>      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"
> @@ -233,7 +233,7 @@ else
>          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"
> +        TEST_IMG="nbd+unix:///?socket=$SOCK_DIR/nbd"

Shouldn’t this be $IMGFMT, too (instead of nbd)?  (Or maybe nbd.$IMGFMT)

Max

>      elif [ "$IMGPROTO" = "ssh" ]; then
>          TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT
>          
> REMOTE_TEST_DIR="ssh://\\($USER@\\)\\?127.0.0.1\\(:[0-9]\\+\\)\\?$TEST_DIR"
> @@ -293,7 +293,7 @@ _stop_nbd_server()
>          local QEMU_NBD_PID
>          read QEMU_NBD_PID < "${QEMU_TEST_DIR}/qemu-nbd.pid"
>          kill ${QEMU_NBD_PID}
> -        rm -f "${QEMU_TEST_DIR}/qemu-nbd.pid"
> +        rm -f "${QEMU_TEST_DIR}/qemu-nbd.pid" "$SOCK_DIR/nbd"
>      fi
>  }
> 
> @@ -353,7 +353,7 @@ _make_test_img()
>      if [ $IMGPROTO = "nbd" ]; then
>          # Pass a sufficiently high number to -e that should be enough for all
>          # tests
> -        eval "$QEMU_NBD -v -t -b 127.0.0.1 -p 10810 -f $IMGFMT -e 42 -x '' 
> $TEST_IMG_FILE >/dev/null &"
> +        eval "$QEMU_NBD -v -t -k '$SOCK_DIR/nbd' -f $IMGFMT -e 42 -x '' 
> $TEST_IMG_FILE >/dev/null &"
>          sleep 1 # FIXME: qemu-nbd needs to be listening before we continue
>      fi
>

signature.asc
Description: OpenPGP digital signature