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

[Eric Blake]

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 <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.

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 ?.


> > +++ 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).

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.

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.

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org