Re: [Qemu-devel] [PULL 01/13] iotests: Set read-zeroes on in null block driver for Valgrind

[Andrey Shinkevich]

On 24/07/2019 11:05, Kevin Wolf wrote:
> Am 24.07.2019 um 09:57 hat Andrey Shinkevich geschrieben:
>>
>>
>> On 24/07/2019 10:38, Kevin Wolf wrote:
>>> Am 24.07.2019 um 09:30 hat Andrey Shinkevich geschrieben:
>>>>
>>>>
>>>> On 24/07/2019 10:18, Christian Borntraeger wrote:
>>>>>
>>>>> On 19.07.19 15:43, Kevin Wolf wrote:
>>>>>> From: Andrey Shinkevich <address@hidden>
>>>>>>
>>>>>> The Valgrind tool reports about the uninitialised buffer 'buf'
>>>>>> instantiated on the stack of the function guess_disk_lchs().
>>>>>> Pass 'read-zeroes=on' to the null block driver to make it deterministic.
>>>>>> The output of the tests 051, 186 and 227 now includes the parameter
>>>>>> 'read-zeroes'. So, the benchmark output files are being changed too.
>>>>>>
>>>>>> Suggested-by: Kevin Wolf <address@hidden>
>>>>>> Signed-off-by: Andrey Shinkevich <address@hidden>
>>>>>> Signed-off-by: Kevin Wolf <address@hidden>
>>>>>> ---
>>>>>>     tests/qemu-iotests/051        |  10 +--
>>>>>>     tests/qemu-iotests/051.pc.out |  10 +--
>>>>>>     tests/qemu-iotests/093        |   9 +-
>>>>> [...9
>>>>>
>>>>>
>>>>> I now get the following on s390.
>>>>> Seems that you only fixed 051.pc.out but not 051.out
>>>>>
>>>>>        051      ...        [09:01:49]             ...              051    
>>>>>   fail       [09:01:49] [09:01:50]                    output 
>>>>> mismatch (see 051.out.bad)
>>>>>        --- tests/qemu-iotests/051.out     2019-07-09 18:34:26.734654933 
>>>>> +0200
>>>>>        +++ build/tests/qemu-iotests/051.out.bad   2019-07-24 
>>>>> 09:01:50.015024901 +0200
>>>>>        @@ -149,23 +149,23 @@
>>>>>         
>>>>>         === Cache modes ===
>>>>>         
>>>>>        -Testing: -drive driver=null-co,cache=none
>>>>>        +Testing: -drive driver=null-co,read-zeroes=on,cache=none
>>>>>         QEMU X.Y.Z monitor - type 'help' for more information
>>>>>         (qemu) quit
>>>>>         
>>>>>        -Testing: -drive driver=null-co,cache=directsync
>>>>>        +Testing: -drive driver=null-co,read-zeroes=on,cache=directsync
>>>>>         QEMU X.Y.Z monitor - type 'help' for more information
>>>>>         (qemu) quit
>>>>>         
>>>>>        -Testing: -drive driver=null-co,cache=writeback
>>>>>        +Testing: -drive driver=null-co,read-zeroes=on,cache=writeback
>>>>>         QEMU X.Y.Z monitor - type 'help' for more information
>>>>>         (qemu) quit
>>>>>         
>>>>>        -Testing: -drive driver=null-co,cache=writethrough
>>>>>        +Testing: -drive driver=null-co,read-zeroes=on,cache=writethrough
>>>>>         QEMU X.Y.Z monitor - type 'help' for more information
>>>>>         (qemu) quit
>>>>>         
>>>>>        -Testing: -drive driver=null-co,cache=unsafe
>>>>>        +Testing: -drive driver=null-co,read-zeroes=on,cache=unsafe
>>>>>         QEMU X.Y.Z monitor - type 'help' for more information
>>>>>         (qemu) quit
>>>>>         
>>>>>
>>>>
>>>> Thank you Christian for your report.
>>>> Would you please send the command line you ran the 051 test with?
>>>>
>>>> Kevin,
>>>> How do add the 051.out file to the patch? New version?
>>>
>>> No, we need a follow-up patch. This one is already merged, so we can't
>>> change it any more.
>>>
>>> Kevin
>>>
>>
>> Thank you and Christian both.
>> It is my first time I have been doing the fix patch.
>> Should it be named a specific way like "iotests: Fix for..."?
> 
> People sometimes use a "Fixes: <commit number>" tag in the body of the
> commit message, which is probably a good idea. The subject line just
> describes the change as usual.
> 
> Kevin
> 
Got it.
Thank you.
Andrey
-- 
With the best regards,
Andrey Shinkevich