On 18.11.19 18:42, Eric Blake wrote: > On 11/18/19 11:29 AM, Max Reitz wrote: >> 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 <[email protected]> >>> --- >>> 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. > > Some of our code outputs: > > nbd+unix://?socket=... > > when there is no export name, while other outputs: > > nbd+unix:///?socket=... > > When there IS an export name, it outputs > > nbd+unix:///name?socket=... > > So the regex is matching 2 or 3 / (using \? to make the third optional), > then a mandatory ?.
Ah, I mixed up the escaping, as I often do when dealing with regexes. >>> +++ 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? > > At first glance, it seems reasonable. But reading further, > >> >>> 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) > > Now I'm starting to wonder. With NBD and non-raw, there are two places > to do the image format: > > qcow2 file -> qemu-nbd -f qcow2 -> raw bytes over NBD -> qemu client -f > raw -> guest (our typical usage) > > qcow2 file -> qemu-nbd -f raw -> qcow2 bytes over NBD -> qemu client -f > qcow2 -> guest (limited use, since NBD does not [yet] have resize support] > > so naming the socket $SOCK_DIR/nbd.qcow2 when the socket carries raw > data (our typical use) seems awkward. But then again, running './check > -qcow2 -nbd' shows that we seldom test qcow2 format over nbd protocol > (precisely because nbd does not yet have resize). Yes, the socket would carry qcow2 data. Which doesn’t work (and I don’t see a reason why we’d want it), but, well. > If anything, I'm inclined to use $SOCK_DIR/nbd.raw to indicate that the > NBD client sees raw format, regardless of the format in use by the > server, to leave the door open for $SOCK_DIR/nbd.qcow2 when we finally > are happy to test qcow2 format over NBD. Sure, works for me. > Or stick to just $SOCK_DIR/nbd hard-coded everywhere, and quit trying to > use $IMGFMT in the socket name, to make all the usage consistent. Works for me, too. Max
signature.asc
Description: OpenPGP digital signature
