From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=39452 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OVpZO-0005By-5F for qemu-devel@nongnu.org; Mon, 05 Jul 2010 13:35:03 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OVpZM-0005NE-6T for qemu-devel@nongnu.org; Mon, 05 Jul 2010 13:35:01 -0400 Received: from mx1.redhat.com ([209.132.183.28]:23130) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OVpZL-0005N3-UI for qemu-devel@nongnu.org; Mon, 05 Jul 2010 13:35:00 -0400 Message-ID: <4C3209DD.3040701@redhat.com> Date: Mon, 05 Jul 2010 18:35:41 +0200 From: Kevin Wolf MIME-Version: 1.0 References: <1277898942-6501-1-git-send-email-armbru@redhat.com> <1277898942-6501-4-git-send-email-armbru@redhat.com> <4C31FABA.5080906@redhat.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [PATCH 03/11] raw-posix: Don't "try harder" for BDRV_TYPE_CDROM List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: ben@guthro.net, hch@lst.de, qemu-devel@nongnu.org, kraxel@redhat.com Am 05.07.2010 18:15, schrieb Markus Armbruster: > Kevin Wolf 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 >> >> 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 Date: Thu Sep 13 12:29:23 2007 +0000 Enhance raw io reliability, by Ben Guthro. Kevin