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

[Eric Blake]

On 11/02/2015 09:03 AM, Max Reitz wrote:

>>>> +++ 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.)

I already know that most (if not all) of qemu-iotests is specifically
/bin/bash.  But if we want to, we can ditch -n and -z, and just use:

if [[ $NBD_SNAPSHOT_PID ]]; then

and similarly.  In fact, I actually prefer embracing bash-isms when we
know we are using bash, to make it obvious that we know we are not
catering to /bin/sh.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature