Re: [Qemu-block] [PATCH] qemu-iotests: fix -valgrind option for check

[Max Reitz]

On 30.10.2015 19:04, Jeff Cody wrote:
> On Fri, Oct 30, 2015 at 06:16:29PM +0100, Max Reitz wrote:
>> On 29.10.2015 19:04, Jeff Cody wrote:
>>> Commit 934659c switched the iotests to run qemu-io from a bash subshell,
>>> in order to catch segfaults.  This method is incompatible with the
>>> current valgrind_qemu_io() bash function.
>>>
>>> Move the valgrind usage into the exec subshell in _qemu_io_wrapper(),
>>> while making sure the original return value is passed back to the
>>> caller.
>>>
>>> Update test output for tests 039, 061, and 137 as it looks for the
>>> specific subshell command when the process is terminated.
>>>
>>> Reported-by: Kevin Wolf <address@hidden>
>>> Signed-off-by: Jeff Cody <address@hidden>
>>> ---
>>>  tests/qemu-iotests/039.out       | 30 +++++++++++++++++++++++++-----
>>>  tests/qemu-iotests/061.out       | 12 ++++++++++--
>>>  tests/qemu-iotests/137.out       |  6 +++++-
>>>  tests/qemu-iotests/common        |  9 ++-------
>>>  tests/qemu-iotests/common.config | 18 +++++++++++++++++-
>>>  tests/qemu-iotests/common.rc     | 10 ----------
>>>  6 files changed, 59 insertions(+), 26 deletions(-)
>>>

[...]

>>> diff --git a/tests/qemu-iotests/common.config 
>>> b/tests/qemu-iotests/common.config
>>> index 4d8665f..db9702b 100644
>>> --- a/tests/qemu-iotests/common.config
>>> +++ b/tests/qemu-iotests/common.config
>>> @@ -122,7 +122,23 @@ _qemu_img_wrapper()
>>>  
>>>  _qemu_io_wrapper()
>>>  {
>>> -    (exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@")
>>> +    local VALGRIND_LOGFILE=/tmp/$$.valgrind
>>> +    local RETVAL
>>> +    (
>>> +        if [ "${VALGRIND_QEMU}" == "y" ]; then
>>> +            exec valgrind --log-file="${VALGRIND_LOGFILE}" 
>>> --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@"
>>> +        else
>>> +            exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@"
>>> +        fi
>>> +    )
>>> +    RETVAL=$?
>>
>> Er, well, this is nice... When just invoking $QEMU_IO -c 'sigraise 9', I
>> get the appropriate error message. ("$PID Killed [...]"). But the
>> instant an image is opened, it just disappears. Yes, using -c open makes
>> it disappear, too.
>>
>> Since all of our qemu-io invocations do use image files (that's its
>> purpose after all), that means that all of them seem to exit just fine
>> when running under valgrind. That is... strange.
>>
>> Is it just me? Maybe I have a broken valgrind, I don't know (3.11.0 here).
>>
> 
> I have valgrind-3.9.0 here, and I get the same behavior... it is not
> just you.  There are also some tests where valgrind itself segfaults.
> 
> I was going to suggest using kill -l TERM instead of kill -l KILL.
> However, I just tried that with test 137, and it causes valgrind to
> segfault. :(

Hm, well, then I guess we can just ignore this and declare it broken. If
someone complains, it'll be his/her job to fix it. ;-)

>>> +    if [ "${VALGRIND_QEMU}" == "y" ]; then
>>> +        if [ $RETVAL != 0 ]; then
>>> +            cat "${VALGRIND_LOGFILE}"
>>
>> If I got the error message and RETVAL would be correctly set to 137,
>> this would print the log file. I'm not sure whether that's what we
>> want...? If valgrind exits with any error code but 99, the log file will
>> probably not contain anything interesting.
>>
>> But if the qemu-io process was killed on purpose, this breaks the test,
>> which I don't think is necessary.
>>
> 
> Good point... How about just:
> 
>  if [ $RETVAL == 99 ]; then
>       cat "${VALGRIND_LOGFILE}"
>  fi

Yes, that's what I had in mind.

Max

>>> +        fi
>>> +        rm -f "${VALGRIND_LOGFILE}"
>>> +    fi
>>> +    (exit $RETVAL)
>>>  }
>>>  
>>>  _qemu_nbd_wrapper()
>>> diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
>>> index 4878e99..d9913f8 100644
>>> --- a/tests/qemu-iotests/common.rc
>>> +++ b/tests/qemu-iotests/common.rc
>>> @@ -70,16 +70,6 @@ else
>>>      TEST_IMG=$IMGPROTO:$TEST_DIR/t.$IMGFMT
>>>  fi
>>>  
>>> -function valgrind_qemu_io()
>>> -{
>>> -    valgrind --log-file=/tmp/$$.valgrind --error-exitcode=99 $REAL_QEMU_IO 
>>> "$@"
>>> -    if [ $? != 0 ]; then
>>> -        cat /tmp/$$.valgrind
>>> -    fi
>>> -    rm -f /tmp/$$.valgrind
>>> -}
>>> -
>>> -
>>>  _optstr_add()
>>>  {
>>>      if [ -n "$1" ]; then
>>>
>>
>>
> 
>

signature.asc
Description: OpenPGP digital signature