Am 14.06.2017 um 19:44 hat John Snow geschrieben: > > > 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.
Yes, the bdrv_pread/pwrite() functions have always returned byte counts and still do. They always return the full byte count or -errno, and this is fully contained in higher level wrappers. > >> (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. I expected someone to complain about compatibility, but if nobody objects, let's do this. Kevin
