Currently, 233 cannot reliably run concurrently to other NBD TCP tests.
When it starts, it looks for a free port and then attempts to use that
for the whole duration of the test run. This is a TOCTTOU race
condition: It does not reserve that port, so another NBD TCP test that
runs in parallel can grab it.
To fix this, we must not use the same port all the time, but always
choose a new one when qemu-nbd is started. We cannot check whether it
is free, but must let qemu-nbd do so and take it atomically. We can
achieve this by using the existing --fork option.
There are two problems with --fork, however. First, it does not give us
a chance to reliably get the server’s PID, which we need. We can change
that by letting qemu-nbd (optionally) write a PID file, though. (Which
makes sense if we have a daemon mode.)
Second, it currently discards all output after the server has been
started. That looks like an accident to me, because we clearly try to
restore the old stderr channel after having redirected all startup
messages to the parent process. If it is a bug, we can fix it.
v3:
- Patch 1: Dropped “pid_file” variable, so it actually compiles...
git backport-diff of v3 against v2:
Key:
[----] : patches are identical
[####] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively
001/5:[0001] [FC] 'qemu-nbd: Add --pid-file option'
002/5:[----] [--] 'iotests.py: Add qemu_nbd_early_pipe()'
003/5:[----] [--] 'qemu-nbd: Do not close stderr'
004/5:[----] [--] 'iotests: Use qemu-nbd's --pid-file'
005/5:[----] [--] 'iotests: Let 233 run concurrently'
v2:
- Patch 1:
- Use qemu_write_pidfile() [Dan]
- %s/pid_path/pid_filename/ [Eric]
- Patch 4: Drop the now superfluous subshell [Eric]
(Didn’t touch _qemu_img_wrapper, because, well, it doesn’t belong in
this series?)
- Patch 5:
- s/racey/racy/ [Eric]
- Unite the “rm -f”s [Eric]
(Did not address the “FIFO filling up” problem, because 64 kB of FIFO
space ought to be enough. Also, cat-ing around that felt weird.)
git backport-diff of v2 against v1:
Key:
[----] : patches are identical
[####] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively
001/5:[0025] [FC] 'qemu-nbd: Add --pid-file option'
002/5:[----] [--] 'iotests.py: Add qemu_nbd_early_pipe()'
003/5:[----] [--] 'qemu-nbd: Do not close stderr'
004/5:[0006] [FC] 'iotests: Use qemu-nbd's --pid-file'
005/5:[0003] [FC] 'iotests: Let 233 run concurrently'
Max Reitz (5):
qemu-nbd: Add --pid-file option
iotests.py: Add qemu_nbd_early_pipe()
qemu-nbd: Do not close stderr
iotests: Use qemu-nbd's --pid-file
iotests: Let 233 run concurrently
qemu-nbd.c | 14 +++++-
qemu-nbd.texi | 2 +
tests/qemu-iotests/147 | 4 +-
tests/qemu-iotests/233 | 1 -
tests/qemu-iotests/common.nbd | 94 ++++++++++++++++-------------------
tests/qemu-iotests/common.rc | 6 +--
tests/qemu-iotests/iotests.py | 9 ++--
7 files changed, 67 insertions(+), 63 deletions(-)
--
2.21.0