[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH 1/4] ide/atapi: make PIO read requests async
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-block] [PATCH 1/4] ide/atapi: make PIO read requests async |
Date: |
Thu, 22 Oct 2015 17:17:53 +0100 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
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?
Re: [Qemu-block] [PATCH V2 0/4] ide: avoid main-loop hang on CDROM/NFS failure, Stefan Hajnoczi, 2015/10/26