On Mon, Oct 12, 2015 at 02:27:22PM +0200, Peter Lieven wrote: > @@ -129,9 +134,71 @@ static int cd_read_sector(IDEState *s, int lba, uint8_t > *buf, int sector_size) > ret = -EIO; > break; > } > + > + if (!ret) { > + s->lba++;
This function probably shouldn't take the lba argument if it modifies s->lba. You dropped the sector_size argument and I think the lba argument should be dropped for the same reason. > +static int cd_read_sector(IDEState *s, int lba, void *buf) > +{ > + if (s->cd_sector_size != 2048 && s->cd_sector_size != 2352) { > + return -EINVAL; > + } > + > + s->iov.iov_base = buf; > + if (s->cd_sector_size == 2352) { > + buf += 16; > + } > + > + s->iov.iov_len = 4 * BDRV_SECTOR_SIZE; > + qemu_iovec_init_external(&s->qiov, &s->iov, 1); > + > +#ifdef DEBUG_IDE_ATAPI > + printf("cd_read_sector: lba=%d\n", lba); > +#endif > + > + if (blk_aio_readv(s->blk, (int64_t)lba << 2, &s->qiov, 4, > + cd_read_sector_cb, s) == NULL) { This function never returns NULL, the if statement can be removed. Doesn't the aiocb need to be stored for cancellation (e.g. device reset)? > + return -EIO; > + } > + > + block_acct_start(blk_get_stats(s->blk), &s->acct, > + 4 * BDRV_SECTOR_SIZE, BLOCK_ACCT_READ); Why does accounting start *after* the read request has been submitted?