From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44487) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZkSvU-0001F7-4A for qemu-devel@nongnu.org; Fri, 09 Oct 2015 04:21:16 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZkSvQ-0003if-7b for qemu-devel@nongnu.org; Fri, 09 Oct 2015 04:21:16 -0400 Date: Fri, 9 Oct 2015 10:21:01 +0200 From: Kevin Wolf Message-ID: <20151009082101.GB3956@noname.redhat.com> References: <1442838328-23117-1-git-send-email-pl@kamp.de> <1442838328-23117-2-git-send-email-pl@kamp.de> <5612E873.1090503@redhat.com> <56138A76.1030804@kamp.de> <56154B71.1060001@redhat.com> <56165C60.1010806@kamp.de> <56169D7F.5010403@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <56169D7F.5010403@redhat.com> Subject: Re: [Qemu-devel] [PATCH 1/5] ide/atapi: make PIO read requests async List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: John Snow Cc: stefanha@gmail.com, jcody@redhat.com, Peter Lieven , qemu-devel@nongnu.org, qemu-block@nongnu.org Am 08.10.2015 um 18:44 hat John Snow geschrieben: > On 10/08/2015 08:06 AM, Peter Lieven wrote: > > Hi all, > > > > short summary from my side. The whole thing seems to get complicated, > > let me explain why: > > > > 1) During review I found that the code in ide_atapi_cmd_reply_end can't > > work correctly if the > > byte_count_limit is not a divider or a multiple of cd_sector_size. The > > reason is that as soon > > as we load the next sector we start at io_buffer offset 0 overwriting > > whatever is left in there > > for transfer. We also reset the io_buffer_index to 0 which means if we > > continue with the > > elementary transfer we always transfer a whole sector (of corrupt data) > > regardless if we > > are allowed to transfer that much data. Before we consider fixing this I > > wonder if it > > is legal at all to have an unaligned byte_count_limit. It obviously has > > never caused trouble in > > practice so maybe its not happening in real life. > > > > I had overlooked that part. Good catch. I do suspect that in practice > nobody will be asking for bizarre values. > > There's no rule against an unaligned byte_count_limit as far as I have > read, but suspect nobody would have a reason to use it in practice. If we know that our code is technically wrong, "nobody uses it" is not a good reason not to fix it. Because sooner or later someone will use it. > > 2) I found that whatever cool optimization I put in to buffer multiple > > sectors at once I end > > up with code that breaks migration because older versions would either > > not fill the io_buffer > > as expected or we introduce variables that older versions do not > > understand. This will > > lead to problems if we migrate in the middle of a transfer. > > > > Ech. This sounds like a bit of a problem. I'll need to think about this > one... Sounds like a textbook example for subsections to me? Kevin