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

[Max Reitz]

On 02.11.2015 08:37, Markus Armbruster wrote:
> 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).

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 <address@hidden>
>>
>>> +            kill "$NBD_SNAPSHOT_PID"
>>> +        fi
>>>      fi
>>>      rm -f "$nbd_unix_socket"
>>>  }

signature.asc
Description: OpenPGP digital signature