[Qemu-devel] Re: [PATCH 03/11] raw-posix: Don't try harder for BDRV_TYPE_CDROM
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. Kevin
[Qemu-devel] Re: [PATCH 03/11] raw-posix: Don't try harder for BDRV_TYPE_CDROM
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 ;)
[Qemu-devel] Re: [PATCH 03/11] raw-posix: Don't try harder for BDRV_TYPE_CDROM
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 + Enhance raw io reliability, by Ben Guthro. Kevin
