[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH 1/5] ide/atapi: make PIO read reque
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-block] [Qemu-devel] [PATCH 1/5] ide/atapi: make PIO read requests async |
Date: |
Fri, 9 Oct 2015 10:21:01 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
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
- Re: [Qemu-block] [Qemu-devel] [PATCH 1/5] ide/atapi: make PIO read requests async, John Snow, 2015/10/08
- Re: [Qemu-block] [Qemu-devel] [PATCH 1/5] ide/atapi: make PIO read requests async, Laszlo Ersek, 2015/10/08
- Re: [Qemu-block] [Qemu-devel] [PATCH 1/5] ide/atapi: make PIO read requests async, John Snow, 2015/10/08
- Re: [Qemu-block] [Qemu-devel] [PATCH 1/5] ide/atapi: make PIO read requests async, Peter Lieven, 2015/10/08
- Re: [Qemu-block] [Qemu-devel] [PATCH 1/5] ide/atapi: make PIO read requests async, Peter Lieven, 2015/10/08
- Re: [Qemu-block] [Qemu-devel] [PATCH 1/5] ide/atapi: make PIO read requests async, John Snow, 2015/10/08
- Re: [Qemu-block] [Qemu-devel] [PATCH 1/5] ide/atapi: make PIO read requests async, Peter Lieven, 2015/10/08
- Re: [Qemu-block] [Qemu-devel] [PATCH 1/5] ide/atapi: make PIO read requests async, Peter Lieven, 2015/10/08
- Re: [Qemu-block] [Qemu-devel] [PATCH 1/5] ide/atapi: make PIO read requests async, John Snow, 2015/10/09
- Re: [Qemu-block] [Qemu-devel] [PATCH 1/5] ide/atapi: make PIO read requests async,
Kevin Wolf <=
- Re: [Qemu-block] [Qemu-devel] [PATCH 1/5] ide/atapi: make PIO read requests async, John Snow, 2015/10/09
- Re: [Qemu-block] [Qemu-devel] [PATCH 1/5] ide/atapi: make PIO read requests async, Peter Lieven, 2015/10/09
- Re: [Qemu-block] [Qemu-devel] [PATCH 1/5] ide/atapi: make PIO read requests async, Peter Lieven, 2015/10/14
- Re: [Qemu-block] [Qemu-devel] [PATCH 1/5] ide/atapi: make PIO read requests async, John Snow, 2015/10/14
- Re: [Qemu-block] [Qemu-devel] [PATCH 1/5] ide/atapi: make PIO read requests async, Peter Lieven, 2015/10/16
Re: [Qemu-block] [Qemu-devel] [PATCH 1/5] ide/atapi: make PIO read requests async, Kevin Wolf, 2015/10/08