Re: [Qemu-block] null-co undefined read behavior

[John Snow]

On 06/13/2017 03:52 AM, Kevin Wolf wrote:
> Am 12.06.2017 um 23:20 hat John Snow geschrieben:
>> I noticed while debugging an unrelated test that our use of the null
>> driver has a habit of making functions like find_image_format trigger a
>> lot of uninitialized memory errors in valgrind, because it will return a
>> successful read without actually touching the buffer.
>>
>> I see that in March there was a bit of a debate over whether or not the
>> null driver SHOULD zero-write memory for performance reasons, but when
>> this option isn't specified, the semantics of read are arguably broken.
>>
>> This isn't terribly important to fix, but for actual debugging purposes
>> it's nice to not have valgrind screaming at you with spurious junk. A
>> few options:
>>
>> (1) Admit that it's weird to allow reads to succeed with null-co driver.
>> Annoy people who apparently wanted performance for their do-nothing
>> driver and force the initialization of memory.
>>
>> --or--
>>
>> (1a) Use valgrind macros to pretend the memory has been initialized.
>>
>> (2) Band-aid find_image_format to zero-initialize memory. Wonder if
>> there are other usages of read() getting called from tests that utilize
>> null-co that will make debugging difficult in other contexts.
>>
>> (3) Edit any tests to always use the zero option when using the null
>> driver, and then periodically keep updating this option in a losing
>> battle over time.
>>
>> (4) Find a way to adjust return value of null-co's read implementation
>> to return 0 instead of a lie over the number of bytes it read/wrote.
>> Correctly represents "No bytes written, but no error occurred either."
> 
> The block layer doesn't support short reads. Therefore, the return value
> is 0/-errno even today, the byte count isn't returned anywhere.
> 

Run the blockjob transaction test and take a look at what the read is
returning in find_image_format. It's 512! I didn't look further than
that, I assumed it was intentional.

>> (5) Shut up, John, really, who cares? Please shut up. You are the only
>> idiot who would even think to use valgrind ever, and we just don't like
>> you that much.
> 
> (6) Make read-zeroes=on the default. Gives people worse performance if
> they expect that null-co:// without any options is the fastest thing
> possible, but it has defined behaviour by default which seems good.
> 
> Whatever solution we choose, I think it should be local to the null
> driver, i.e. I don't like (2).
> 

Neither do I, really.

Having read all of the responses, I think I'd like your approach of
making read-zeroes the default, and people who "really know what they
want" can turn it off and enter the danger zone if they'd like to.