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.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
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' \
-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"
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"
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
--
2.21.0
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 <eblake@redhat.com>
> ---
> 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
>
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 <eblake@redhat.com> >> --- >> 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
On 11/18/19 11:42 AM, Eric Blake wrote: >>> +++ 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=... Re-reading https://github.com/NetworkBlockDevice/nbd/blob/master/doc/uri.md and RFC 3986, I see that both forms appear to be valid URIs (both have empty authority, the first has empty path and the second has path '/', but NBD says leading / in path is stripped to form the export name). However, the NBD document does not mention the 2-slash form with no URI authority or export name; perhaps we should amend that document to make it obvious that it is indeed valid. > > When there IS an export name, it outputs > > nbd+unix:///name?socket=... While in this case, the 3-slash form is essential - with only two slashes, the URI 'nbd+unix://name?socket' would be trying to access a URI authority of 'name' with an empty path, rather than the intended empty authority and path of '/name' which gets translated to the NBD export 'name'. > > So the regex is matching 2 or 3 / (using \? to make the third optional), > then a mandatory ?. > -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
On 11/18/19 11:42 AM, Eric Blake wrote: >>> - >>> 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) > > 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. Naming the socket $SOCK_DIR/nbd.raw means that filters tend to rename it to $SOCK_DIR/nbd.IMGFMT before my attempt to rename everything to TEST_DIR/t.IMGFMT. So I'm now leaning towards just naming the socket $SOCK_DIR/nbd and leave it at that. > > 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
On 11/18/19 4:18 PM, Eric Blake wrote:
>>
>> 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.
>
> Naming the socket $SOCK_DIR/nbd.raw means that filters tend to rename it
> to $SOCK_DIR/nbd.IMGFMT before my attempt to rename everything to
> TEST_DIR/t.IMGFMT. So I'm now leaning towards just naming the socket
> $SOCK_DIR/nbd and leave it at that.
>
>>
>> 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.
In order to get my NBD 4.2-rc2 pull request out, I'll be squashing this
in (having tested that my usual iotest configurations still pass)::
diff --git i/tests/qemu-iotests/common.rc w/tests/qemu-iotests/common.rc
index f772dcb67322..0cc8acc9edd2 100644
--- i/tests/qemu-iotests/common.rc
+++ w/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.type=unix,file.path=$SOCKDIR/$IMGFMT"
+
TEST_IMG="$DRIVER,file.driver=nbd,file.type=unix,file.path=$SOCKDIR/nbd"
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"
@@ -349,7 +349,10 @@ _make_test_img()
fi
) | _filter_img_create
- # Start an NBD server on the image file, which is what we'll be
talking to
+ # Start an NBD server on the image file, which is what we'll be
talking to.
+ # Once NBD gains resize support, we may also want to use -f raw at the
+ # server and interpret format over NBD, but for now, the format is
+ # interpreted at the server and raw data sent over NBD.
if [ $IMGPROTO = "nbd" ]; then
# Pass a sufficiently high number to -e that should be enough
for all
# tests
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
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 <eblake@redhat.com> >>> --- >>> 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
© 2016 - 2026 Red Hat, Inc.