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

[Peter Lieven <pl@kamp.de>]

Am 28.10.2015 um 12:26 schrieb Stefan Hajnoczi:
> 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?
Yes, I have only found a flush in bdrv_reopen_prepare, but if you say they need to drain before
I think it is safe to use the buffered_ide_readv for all read-only IDE devices not only CDROMs.

Peter