[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [Qemu-block] [PATCH v5 4/6] iotests: Valgrind fails wit
From: |
John Snow |
Subject: |
Re: [Qemu-devel] [Qemu-block] [PATCH v5 4/6] iotests: Valgrind fails with nonexistent directory |
Date: |
Tue, 27 Aug 2019 15:45:38 -0400 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 |
On 8/25/19 11:24 AM, Andrey Shinkevich wrote:
>
>
> On 16/08/2019 03:55, John Snow wrote:
>>
>>
>> On 7/19/19 12:30 PM, Andrey Shinkevich wrote:
>>> The Valgrind uses the exported variable TMPDIR and fails if the
>>> directory does not exist. Let us exclude such a test case from
>>> being run under the Valgrind and notify the user of it.
>>>
>>> Suggested-by: Kevin Wolf <address@hidden>
>>> Signed-off-by: Andrey Shinkevich <address@hidden>
>>> ---
>>> tests/qemu-iotests/051 | 4 ++++
>>> 1 file changed, 4 insertions(+)
>>>
>>> diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051
>>> index ce942a5..f8141ca 100755
>>> --- a/tests/qemu-iotests/051
>>> +++ b/tests/qemu-iotests/051
>>> @@ -377,6 +377,10 @@ printf %b "qemu-io $device_id \"write -P 0x33 0
>>> 4k\"\ncommit $device_id\n" |
>>> $QEMU_IO -c "read -P 0x33 0 4k" "$TEST_IMG" | _filter_qemu_io
>>>
>>> # Using snapshot=on with a non-existent TMPDIR
>>> +if [ "${VALGRIND_QEMU}" == "y" ]; then
>>> + _casenotrun "Valgrind needs a valid TMPDIR for itself"
>>> +fi
>>> +VALGRIND_QEMU="" \
>>> TMPDIR=/nonexistent run_qemu -drive driver=null-co,snapshot=on
>>>
>>> # Using snapshot=on together with read-only=on
>>>
>>
>> The only other way around this would be a complicated mechanism to set
>> the TMPDIR for valgrind's sub-processes only, with e.g.
>>
>> valgrind ... env TMPDIR=/nonexistent qemu ...
>>
>> ... It's probably not worth trying to concoct such a thing; but I
>> suppose it is possible. You'd have to set up a generic layer for setting
>> environment variables, then in the qemu shim, you could either set them
>> directly (non-valgrind invocation) or set them as part of the valgrind
>> command-line.
>>
>> Or you could just take my R-B:
>>
>> Reviewed-by: John Snow <address@hidden>
>>
>
> Thanks again John for your review and the advice.
> Probably, it doesn't worth efforts to manage that case because QEMU
> should fail anyway with the error message "Could not get temporary
> filename: No such file or directory". So, we would not benefit much from
> that run. We have other test cases that cover the main functionality.
> It's just to check the QEMU error path for possible memory issues. Shall we?
>
> Andrey
>
Yeah, don't bother with this for now. I just have a personal compulsion
to try to concretely measure how much work it would take to avoid a
"hack" and then make my decision.
You're free to just take the R-B :)
--js