[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 1/2] qemu-iotests: fix cleanup of background
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v3 1/2] qemu-iotests: fix cleanup of background processes |
Date: |
Mon, 2 Nov 2015 09:32:18 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
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