From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60863) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zt9gg-0005ww-I4 for qemu-devel@nongnu.org; Mon, 02 Nov 2015 02:37:55 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zt9gf-0008Ut-DU for qemu-devel@nongnu.org; Mon, 02 Nov 2015 02:37:54 -0500 From: Markus Armbruster References: <9e4f958b3895b7259b98d845bb46f000ba362869.1446232490.git.jcody@redhat.com> <5633DD87.1090302@redhat.com> Date: Mon, 02 Nov 2015 08:37:43 +0100 In-Reply-To: <5633DD87.1090302@redhat.com> (Max Reitz's message of "Fri, 30 Oct 2015 22:13:43 +0100") Message-ID: <87a8qw4zso.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH v3 1/2] qemu-iotests: fix cleanup of background processes List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz Cc: kwolf@redhat.com, Jeff Cody , jsnow@redhat.com, qemu-devel@nongnu.org, qemu-block@nongnu.org Max Reitz 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 >> Signed-off-by: Jeff Cody >> --- >> 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). > Reviewed-by: Max Reitz > >> + kill "$NBD_SNAPSHOT_PID" >> + fi >> fi >> rm -f "$nbd_unix_socket" >> }