Re: [PATCH 01/23] iotests: Introduce $SOCK_DIR
On 10.10.19 20:18, Eric Blake wrote: > On 10/10/19 10:24 AM, Max Reitz wrote: >> Unix sockets generally have a maximum path length. Depending on your >> $TEST_DIR, it may be exceeded and then all tests that create and use >> Unix sockets there may fail. >> >> Circumvent this by adding a new scratch directory specifically for >> Unix socket files. It defaults to a temporary directory (mktemp -d) >> that is completely removed after the iotests are done. >> >> (By default, mktemp -d creates a /tmp/tmp.XX directory, which >> should be short enough for our use cases.) >> >> Signed-off-by: Max Reitz >> --- >> tests/qemu-iotests/check | 17 + > >> +tmp_sock_dir=false >> +if [ -z "$SOCK_DIR" ]; then >> + SOCK_DIR=$(mktemp -d) >> + tmp_sock_dir=true >> +fi >> + >> +if [ ! -d "$SOCK_DIR" ]; then >> + mkdir "$SOCK_DIR" >> +fi > > Should this use mkdir -p, in case two parallel processes compete with > the same SOCK_DIR? I would have used mkdir -p, but I saw we used this construct for TEST_DIR, so I thought I‘d just go for the same. > What if SOCK_DIR is set to something that is not a directory (say a > file), at which point mkdir fails, but you don't seem to be catching > that failure. Well, the same applies to TEST_DIR. And technically, as long as we don’t use mkdir -p for either, not catching the error at least helps circumvent the potential race. O:-) (I’ll convert both to mkdir -p with error handling.) Max > Otherwise looks good. signature.asc Description: OpenPGP digital signature
Re: [PATCH 01/23] iotests: Introduce $SOCK_DIR
On 10/10/19 10:24 AM, Max Reitz wrote: Unix sockets generally have a maximum path length. Depending on your $TEST_DIR, it may be exceeded and then all tests that create and use Unix sockets there may fail. Circumvent this by adding a new scratch directory specifically for Unix socket files. It defaults to a temporary directory (mktemp -d) that is completely removed after the iotests are done. (By default, mktemp -d creates a /tmp/tmp.XX directory, which should be short enough for our use cases.) Signed-off-by: Max Reitz --- tests/qemu-iotests/check | 17 + +tmp_sock_dir=false +if [ -z "$SOCK_DIR" ]; then +SOCK_DIR=$(mktemp -d) +tmp_sock_dir=true +fi + +if [ ! -d "$SOCK_DIR" ]; then +mkdir "$SOCK_DIR" +fi Should this use mkdir -p, in case two parallel processes compete with the same SOCK_DIR? What if SOCK_DIR is set to something that is not a directory (say a file), at which point mkdir fails, but you don't seem to be catching that failure. Otherwise looks good. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
[PATCH 01/23] iotests: Introduce $SOCK_DIR
Unix sockets generally have a maximum path length. Depending on your
$TEST_DIR, it may be exceeded and then all tests that create and use
Unix sockets there may fail.
Circumvent this by adding a new scratch directory specifically for
Unix socket files. It defaults to a temporary directory (mktemp -d)
that is completely removed after the iotests are done.
(By default, mktemp -d creates a /tmp/tmp.XX directory, which
should be short enough for our use cases.)
Signed-off-by: Max Reitz
---
tests/qemu-iotests/check | 17 +
1 file changed, 17 insertions(+)
diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index 588c453a94..a257215448 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -97,6 +97,7 @@ IMGFMT-- $FULL_IMGFMT_DETAILS
IMGPROTO -- $IMGPROTO
PLATFORM -- $FULL_HOST_DETAILS
TEST_DIR -- $TEST_DIR
+SOCK_DIR -- $SOCK_DIR
SOCKET_SCM_HELPER -- $SOCKET_SCM_HELPER
EOF
@@ -121,6 +122,16 @@ if [ ! -e "$TEST_DIR" ]; then
mkdir "$TEST_DIR"
fi
+tmp_sock_dir=false
+if [ -z "$SOCK_DIR" ]; then
+SOCK_DIR=$(mktemp -d)
+tmp_sock_dir=true
+fi
+
+if [ ! -d "$SOCK_DIR" ]; then
+mkdir "$SOCK_DIR"
+fi
+
diff="diff -u"
verbose=false
debug=false
@@ -534,6 +545,7 @@ if [ -z "$SAMPLE_IMG_DIR" ]; then
fi
export TEST_DIR
+export SOCK_DIR
export SAMPLE_IMG_DIR
if [ -s $tmp.list ]
@@ -716,6 +728,11 @@ END{ if (NR > 0) {
rm -f "${TEST_DIR}"/*.out "${TEST_DIR}"/*.err "${TEST_DIR}"/*.time
rm -f "${TEST_DIR}"/check.pid "${TEST_DIR}"/check.sts
rm -f $tmp.*
+
+if $tmp_sock_dir
+then
+rm -rf "$SOCK_DIR"
+fi
}
trap "_wrapup; exit \$status" 0 1 2 3 15
--
2.21.0
