Re: [Qemu-devel] [PATCH v3 1/2] qemu-iotests: fix cleanup of background processes

[Markus Armbruster]

Max Reitz <address@hidden> 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 <address@hidden>
>> Signed-off-by: Jeff Cody <address@hidden>
>> ---
>>  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 <address@hidden>
>
>> +            kill "$NBD_SNAPSHOT_PID"
>> +        fi
>>      fi
>>      rm -f "$nbd_unix_socket"
>>  }