| Message ID | 20191010152457.17713-2-mreitz@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | iotests: Add and use $SOCK_DIR | expand |
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.XXXXXXXXXX directory, which > should be short enough for our use cases.) > > Signed-off-by: Max Reitz <mreitz@redhat.com> > --- > 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.
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.XXXXXXXXXX directory, which >> should be short enough for our use cases.) >> >> Signed-off-by: Max Reitz <mreitz@redhat.com> >> --- >> 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.
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
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.XXXXXXXXXX directory, which should be short enough for our use cases.) Signed-off-by: Max Reitz <mreitz@redhat.com> --- tests/qemu-iotests/check | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)