From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:32803) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZreWC-0002BM-CL for qemu-devel@nongnu.org; Thu, 29 Oct 2015 00:08:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZreWB-0002C9-8j for qemu-devel@nongnu.org; Thu, 29 Oct 2015 00:08:52 -0400 References: <1aebb2401ecdeff9a85f9f19c981caacf0ed69d3.1446080991.git.jcody@redhat.com> From: Eric Blake Message-ID: <56319BCA.7000708@redhat.com> Date: Wed, 28 Oct 2015 22:08:42 -0600 MIME-Version: 1.0 In-Reply-To: <1aebb2401ecdeff9a85f9f19c981caacf0ed69d3.1446080991.git.jcody@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="ln2oLj4ldO592ELQ1AIcLGLOGDjocNVjU" Subject: Re: [Qemu-devel] [PATCH] qemu-iotests: fix cleanup of background processes List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jeff Cody , qemu-devel@nongnu.org Cc: kwolf@redhat.com, jsnow@redhat.com, qemu-block@nongnu.org, mreitz@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --ln2oLj4ldO592ELQ1AIcLGLOGDjocNVjU Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 10/28/2015 07:15 PM, Jeff Cody wrote: > Commit 934659c switched the iotests to run qemu and qemu-nbd from a bas= h > 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. >=20 > Since the process is using exec in the subshell, the PID is the > same as the subshell PID. >=20 > 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. >=20 > Reported-by: John Snow > Signed-off-by: Jeff Cody > --- > tests/qemu-iotests/common.config | 14 ++++++++++++-- > tests/qemu-iotests/common.qemu | 17 +++++++++++------ > tests/qemu-iotests/common.rc | 6 +++--- > 3 files changed, 26 insertions(+), 11 deletions(-) >=20 > diff --git a/tests/qemu-iotests/common.config b/tests/qemu-iotests/comm= on.config > index 596bb2b..5fd4ca8 100644 > --- a/tests/qemu-iotests/common.config > +++ b/tests/qemu-iotests/common.config > @@ -44,6 +44,8 @@ export HOST_OPTIONS=3D${HOST_OPTIONS:=3Dlocal.config}= > export CHECK_OPTIONS=3D${CHECK_OPTIONS:=3D"-g auto"} > export PWD=3D`pwd` > =20 > +export _QEMU_HANDLE=3D0 > + > # $1 =3D prog to look for, $2* =3D default pathnames if not found in $= PATH > set_prog_path() > { > @@ -105,7 +107,12 @@ fi > =20 > _qemu_wrapper() > { > - (exec "$QEMU_PROG" $QEMU_OPTIONS "$@") > + ( > + if [ ! -z ${QEMU_NEED_PID} ]; then > + echo -n $BASHPID > "${TEST_DIR}/qemu-${_QEMU_HANDLE}.pid" 'echo -n' is a non-portable bashism; even in bash, it can be made to behave differently by 'set -o posix; shopt -s xpg_echo'. It's safer, and shorter, to use 'printf', if you don't need the newline. On the other hand, if you use plain 'echo', and include the newline,... > @@ -196,10 +194,17 @@ function _cleanup_qemu() > # QEMU_PID[], QEMU_IN[], QEMU_OUT[] all use same indices > for i in "${!QEMU_OUT[@]}" > do > - if [ -z "${wait}" ]; then > - kill -KILL ${QEMU_PID[$i]} 2>/dev/null > + local QEMU_PID > + if [ -f "${TEST_DIR}/qemu-${i}.pid" ]; then > + QEMU_PID=3D`cat "${TEST_DIR}/qemu-${i}.pid"` =2E..then you could avoid the subshell and useless use of cat here by doi= ng: read QEMU_PID < "${TEST_DIR}/qemu-${i}.pid" > + rm -f "${TEST_DIR}/qemu-${i}.pid" > + fi > + if [ -z "${wait}" ] && [ ! -z ${QEMU_PID} ]; then Missing quotes around ${QEMU_PID}. But you got lucky: if it is empty, then you are evaluating [ ! -z ], which is false; where the intended [ ! -z "" ] would also be false. Still, it's bad form to abuse [] like that.= --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --ln2oLj4ldO592ELQ1AIcLGLOGDjocNVjU Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJWMZvKAAoJEKeha0olJ0NqPMoIAILgGz3g0Yf4DEX5MNweAjbC meSpv5KsSiys1n7ipdCdMnQKRkk6KT0LpMU+gvre8ridNNl9hsXbFMluyXnYa6uX aVapvxJxgFpjZdlNXHhXVu7JmguXgUzigHjVcmF+XRSyJrFhBkmCZ3gOjKkIo0ij 1RzSkGrPZ5J/pDpI/Omn7erD5NX81Fdtdkk82z9SloR9iSADxxlHd8TCBdD30VP0 HOmWQp4XozE8wCKnsrMiYbfBCNp72O7OccHjFilaly7dncgaukG5m0IjctHTptYN RpLJtTKMvceqMKmRu5rGDOnIkCWbEfgvmeNCT63+i+WrSecMWGMK7pgHVIx2IFY= =5tIW -----END PGP SIGNATURE----- --ln2oLj4ldO592ELQ1AIcLGLOGDjocNVjU--