Am 05.07.2010 18:15, schrieb Markus Armbruster: > Kevin Wolf <kw...@redhat.com> writes: > >> Am 30.06.2010 13:55, schrieb Markus Armbruster: >>> raw_pread_aligned() retries up to two times if the block device backs >>> a virtual CD-ROM. This makes no sense. Whether retrying reads can >>> correct read errors may depend on what we're reading, not on how the >>> result gets used. >>> >>> Also clean up gratuitous use of goto. >>> >>> This reverts what's left of commit 8c05dbf9. >>> >>> Signed-off-by: Markus Armbruster <arm...@redhat.com> >> >> Are you sure that this won't cause a regression? I mean if there is a >> patch specifically adding this behaviour, there probably was a problem >> that made someone touch the code in the first place. >> >> Arguably checking for the type hint is nonsense, however I think the >> case for which this was written is passing through a real CD-ROM to a VM >> - in which case the condition would be true anyway. >> >> So instead of removing the code, the fix to achieve what was probably >> intended is to check for bs->drv == &bdrv_host_cdrom. > > I can do that. But does it make sense? How can retrying failed reads > help? Isn't the OS in a much better position to retry? > > Keeping the retry code feels like voodoo-programming to me: I have no > idea how waving around this dead chicken could help, but we've always > done it, so keep waving ;)
I would agree that someone tried to be clever without real reason if this was buried in one of those big Fabrice-style commits. But is was added as a commit for itself, and I'd be surprised if someone sent a patch if it didn't change anything for him. Let's try if I've got a valid email address of Ben for CCing him... Ben, do you remember this patch and can you help us? commit 8c05dbf9b68cc8444573116063582e01a0442b0b Author: ths <t...@c046a42c-6fe2-441c-8c8c-71466251a162> Date: Thu Sep 13 12:29:23 2007 +0000 Enhance raw io reliability, by Ben Guthro. Kevin