On 10/28/2015 07:15 PM, 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 <js...@redhat.com>
> Signed-off-by: Jeff Cody <jc...@redhat.com>
> ---
> 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(-)
>
> diff --git a/tests/qemu-iotests/common.config
> b/tests/qemu-iotests/common.config
> index 596bb2b..5fd4ca8 100644
> --- a/tests/qemu-iotests/common.config
> +++ b/tests/qemu-iotests/common.config
> @@ -44,6 +44,8 @@ export HOST_OPTIONS=${HOST_OPTIONS:=local.config}
> export CHECK_OPTIONS=${CHECK_OPTIONS:="-g auto"}
> export PWD=`pwd`
>
> +export _QEMU_HANDLE=0
> +
> # $1 = prog to look for, $2* = default pathnames if not found in $PATH
> set_prog_path()
> {
> @@ -105,7 +107,12 @@ fi
>
> _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=`cat "${TEST_DIR}/qemu-${i}.pid"`
...then you could avoid the subshell and useless use of cat here by doing:
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.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
