On 02.11.2015 08:37, Markus Armbruster wrote: > Max Reitz <[email protected]> writes: > >> On 30.10.2015 20:25, Jeff Cody wrote: >>> Commit 934659c switched the iotests to run qemu and qemu-nbd from a bash >>> subshell, in order to catch segfaults. Unfortunately, this means the >>> process PID cannot be captured via '$!'. We stopped killing qemu and >>> qemu-nbd processes, leaving a lot of orphaned, running qemu processes >>> after executing iotests. >>> >>> Since the process is using exec in the subshell, the PID is the >>> same as the subshell PID. >>> >>> Track these PIDs for cleanup using pidfiles in the $TEST_DIR. Only >>> track the qemu PID, however, if requested - not all usage requires >>> killing the process. >>> >>> Reported-by: John Snow <[email protected]> >>> Signed-off-by: Jeff Cody <[email protected]> >>> --- >>> tests/qemu-iotests/058 | 12 ++++++++---- >>> tests/qemu-iotests/common.config | 14 ++++++++++++-- >>> tests/qemu-iotests/common.qemu | 18 ++++++++++++------ >>> tests/qemu-iotests/common.rc | 8 +++++--- >>> 4 files changed, 37 insertions(+), 15 deletions(-) >>> >>> diff --git a/tests/qemu-iotests/058 b/tests/qemu-iotests/058 >>> index f2bdd0b..63a6598 100755 >>> --- a/tests/qemu-iotests/058 >>> +++ b/tests/qemu-iotests/058 >>> @@ -32,11 +32,17 @@ status=1 # failure is the default! >>> >>> nbd_unix_socket=$TEST_DIR/test_qemu_nbd_socket >>> nbd_snapshot_img="nbd:unix:$nbd_unix_socket" >>> +rm -f "${TEST_DIR}/qemu-nbd.pid" >>> >>> _cleanup_nbd() >>> { >>> - if [ -n "$NBD_SNAPSHOT_PID" ]; then >>> - kill "$NBD_SNAPSHOT_PID" >>> + local NBD_SNAPSHOT_PID >>> + if [ -f "${TEST_DIR}/qemu-nbd.pid" ]; then >>> + read NBD_SNAPSHOT_PID < "${TEST_DIR}/qemu-nbd.pid" >>> + rm -f "${TEST_DIR}/qemu-nbd.pid" >>> + if [ -n "$NBD_SNAPSHOT_PID" ]; then >> >> No, I won't complain about using ! -z "" elsewhere and -n "" here. :-) > > The little pedant in me screams "but I will!", and the little prankster > next to him is clapping enthusiastically. > > Kidding aside: not worth a respin, but could be cleaned up on commit > (maintainer's discretion).
Oh, if that's the case, then I have another thing for you: The use of == in patch 2! ;-) (I'm a bit disappointed Eric doesn't have a mail filter for #!/bin/(ba)?sh ... if.*== for his mail client.) Max > >> Reviewed-by: Max Reitz <[email protected]> >> >>> + kill "$NBD_SNAPSHOT_PID" >>> + fi >>> fi >>> rm -f "$nbd_unix_socket" >>> }
signature.asc
Description: OpenPGP digital signature
