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

[Kevin Wolf]

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