On 04/22/2016 07:40 PM, Eric Blake wrote:
> Sector-based blk_read() should die; switch to byte-based
> blk_pread() instead.
>
> Signed-off-by: Eric Blake <ebl...@redhat.com>
> ---
> hw/ide/atapi.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
> index 2bb606c..81000d8 100644
> --- a/hw/ide/atapi.c
> +++ b/hw/ide/atapi.c
> @@ -119,12 +119,12 @@ cd_read_sector_sync(IDEState *s)
>
> switch (s->cd_sector_size) {
> case 2048:
> - ret = blk_read(s->blk, (int64_t)s->lba << 2,
> - s->io_buffer, 4);
> + ret = blk_pread(s->blk, (int64_t)s->lba << (2 + BDRV_SECTOR_BITS),
> + s->io_buffer, 4 << BDRV_SECTOR_BITS);
> break;
> case 2352:
> - ret = blk_read(s->blk, (int64_t)s->lba << 2,
> - s->io_buffer + 16, 4);
> + ret = blk_pread(s->blk, (int64_t)s->lba << (2 + BDRV_SECTOR_BITS),
Uh, hm. So what we have is a cdrom-sector-based LBA, that we need to
transform into IDE-based sectors, then to bytes.
We could just define an ATAPI_SECTOR_BITS to be (2 + BDRV_SECTOR_BITS).
Then, the lba conversion would be just:
s->lba << ATAPI_SECTOR_BITS
and the size would be just:
1 << ATAPI_SECTOR_BITS
And that's probably better on the eyes.
> + s->io_buffer + 16, 4 << BDRV_SECTOR_BITS);
> if (ret >= 0) {
> cd_data_to_raw(s->io_buffer, s->lba);
> }
>
The code already uses lots of different stuff like
4 * BDRV_SECTOR_SIZE
or
"2048"
so we probably need some staple definition here.
...Otherwise, none of this is a problem you've created, just one this
patch highlights. Fix at your own peril.
Acked-by: John Snow <js...@redhat.com>
