From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41287) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZjRw7-0005h2-Jy for qemu-devel@nongnu.org; Tue, 06 Oct 2015 09:05:44 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZjRw1-0004Fo-Lt for qemu-devel@nongnu.org; Tue, 06 Oct 2015 09:05:43 -0400 References: <1442838328-23117-1-git-send-email-pl@kamp.de> <1442838328-23117-2-git-send-email-pl@kamp.de> From: Laszlo Ersek Message-ID: <5613C713.1080305@redhat.com> Date: Tue, 6 Oct 2015 15:05:23 +0200 MIME-Version: 1.0 In-Reply-To: <1442838328-23117-2-git-send-email-pl@kamp.de> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit 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: Peter Lieven , qemu-devel@nongnu.org, qemu-block@nongnu.org Cc: kwolf@redhat.com, stefanha@gmail.com, jcody@redhat.com, jsnow@redhat.com On 09/21/15 14:25, Peter Lieven wrote: > PIO read requests on the ATAPI interface used to be sync blk requests. > This has to siginificant drawbacks. First the main loop hangs util an Trivial comments: - s/to/two/ - s/siginificant/significant/ Thanks Laszlo > I/O request is completed and secondly if the I/O request does not > complete (e.g. due to an unresponsive storage) Qemu hangs completely. > > Signed-off-by: Peter Lieven > --- > hw/ide/atapi.c | 69 ++++++++++++++++++++++++++++++++++++---------------------- > 1 file changed, 43 insertions(+), 26 deletions(-) > > diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c > index 747f466..9257e1c 100644 > --- a/hw/ide/atapi.c > +++ b/hw/ide/atapi.c > @@ -105,31 +105,51 @@ static void cd_data_to_raw(uint8_t *buf, int lba) > memset(buf, 0, 288); > } > > -static int cd_read_sector(IDEState *s, int lba, uint8_t *buf, int sector_size) > +static void cd_read_sector_cb(void *opaque, int ret) > { > - int ret; > + IDEState *s = opaque; > > - switch(sector_size) { > - case 2048: > - block_acct_start(blk_get_stats(s->blk), &s->acct, > - 4 * BDRV_SECTOR_SIZE, BLOCK_ACCT_READ); > - ret = blk_read(s->blk, (int64_t)lba << 2, buf, 4); > - block_acct_done(blk_get_stats(s->blk), &s->acct); > - break; > - case 2352: > - block_acct_start(blk_get_stats(s->blk), &s->acct, > - 4 * BDRV_SECTOR_SIZE, BLOCK_ACCT_READ); > - ret = blk_read(s->blk, (int64_t)lba << 2, buf + 16, 4); > - block_acct_done(blk_get_stats(s->blk), &s->acct); > - if (ret < 0) > - return ret; > - cd_data_to_raw(buf, lba); > - break; > - default: > - ret = -EIO; > - break; > + block_acct_done(blk_get_stats(s->blk), &s->acct); > + > + if (ret < 0) { > + ide_atapi_io_error(s, ret); > + return; > + } > + > + if (s->cd_sector_size == 2352) { > + cd_data_to_raw(s->io_buffer, s->lba); > } > - return ret; > + > + s->lba++; > + s->io_buffer_index = 0; > + s->status &= ~BUSY_STAT; > + > + ide_atapi_cmd_reply_end(s); > +} > + > +static int cd_read_sector(IDEState *s, int lba, void *buf, int sector_size) > +{ > + if (sector_size != 2048 && sector_size != 2352) { > + return -EINVAL; > + } > + > + s->iov.iov_base = buf; > + if (sector_size == 2352) { > + buf += 4; > + } > + > + s->iov.iov_len = 4 * BDRV_SECTOR_SIZE; > + qemu_iovec_init_external(&s->qiov, &s->iov, 1); > + > + if (blk_aio_readv(s->blk, (int64_t)lba << 2, &s->qiov, 4, > + cd_read_sector_cb, s) == NULL) { > + return -EIO; > + } > + > + block_acct_start(blk_get_stats(s->blk), &s->acct, > + 4 * BDRV_SECTOR_SIZE, BLOCK_ACCT_READ); > + s->status |= BUSY_STAT; > + return 0; > } > > void ide_atapi_cmd_ok(IDEState *s) > @@ -190,10 +210,8 @@ void ide_atapi_cmd_reply_end(IDEState *s) > ret = cd_read_sector(s, s->lba, s->io_buffer, s->cd_sector_size); > if (ret < 0) { > ide_atapi_io_error(s, ret); > - return; > } > - s->lba++; > - s->io_buffer_index = 0; > + return; > } > if (s->elementary_transfer_size > 0) { > /* there are some data left to transmit in this elementary > @@ -275,7 +293,6 @@ static void ide_atapi_cmd_read_pio(IDEState *s, int lba, int nb_sectors, > s->io_buffer_index = sector_size; > s->cd_sector_size = sector_size; > > - s->status = READY_STAT | SEEK_STAT; > ide_atapi_cmd_reply_end(s); > } > >