Re: [Qemu-devel] [PATCH 3/4] ide: add support for cancelable read requests

[Stefan Hajnoczi <stefanha@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1551 bytes --]

On Tue, Oct 27, 2015 at 11:58:55AM +0100, Peter Lieven wrote:
> Am 26.10.2015 um 11:39 schrieb Stefan Hajnoczi:
> >On Mon, Oct 12, 2015 at 02:27:24PM +0200, Peter Lieven wrote:
> >>+BlockAIOCB *ide_readv_cancelable(IDEState *s, int64_t sector_num,
> >>+                                 QEMUIOVector *iov, int nb_sectors,
> >>+                                 BlockCompletionFunc *cb, void *opaque)
> >>+{
> >>+    BlockAIOCB *aioreq;
> >>+    IDECancelableRequest *req;
> >>+    int c = 0;
> >>+
> >>+    QLIST_FOREACH(req, &s->cancelable_requests, list) {
> >>+        c++;
> >>+    }
> >>+    if (c > MAX_CANCELABLE_REQS) {
> >>+        return NULL;
> >>+    }
> >A BH is probably needed here to schedule an cb(-EIO) call since this
> >function isn't supposed to return NULL if it's a direct replacement for
> >blk_aio_readv().
> 
> You mean sth like:
> 
> acb = qemu_aio_get(&bdrv_em_aiocb_info, bs, cb, opaque);
> acb->bh = aio_bh_new(bdrv_get_aio_context(bs), bdrv_aio_bh_cb, acb);
> acb->ret = -EIO;
> qemu_bh_schedule(acb->bh);
> 
> return &acb->common;

Yes.

> As pointed out in my comment to your requestion about write/discard I think it should
> be feasible to use buffered readv requests for all read-only IDE devices.
> Only thing I'm unsure about is reopening. A reopen seems to only flush the device not
> drain all requests.

bdrv_reopen_prepare() callers should drain requests.  For example,
bdrv_reopen_multiple() (and indirectly bdrv_reopen()) call
bdrv_drain_all().  Is this what you mean?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]