Re: [Qemu-devel] [PATCH v1 01/18] block/io: add bdrv_aio_{preadv, pwritev}

[Pavel Butsykin <pbutsykin@virtuozzo.com>]

On 23.11.2016 17:28, Kevin Wolf wrote:
> Am 15.11.2016 um 07:36 hat Pavel Butsykin geschrieben:
>> It's just byte-based wrappers over bdrv_co_aio_prw_vector(), which provide
>>   a byte-based interface for AIO read/write.
>>
>> Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
>
> I'm in the process to phase out the last users of bdrv_aio_*() so that
> this set of interfaces can be removed. I'm doing this because it's an
> unnecessary redundancy, we have too many wrapper functions that expose
> the same functionality with different syntax. So let's not add new
> users.
>
> At first sight, you don't even seem to use bdrv_aio_preadv() for actual
> parallelism, but you often have a pattern like this:
>
>      void foo_cb(void *opaque)
>      {
>          ...
>          qemu_coroutine_enter(acb->co);
>      }
>
>      void caller()
>      {
>          ...
>          acb = bdrv_aio_preadv(...);
>          qemu_coroutine_yield();
>      }
>
> The code will actually become a lot simpler if you use bdrv_co_preadv()
> instead because you don't have to have a callback, but you get pure
> sequential code.
>
> The part that actually has some parallelism, pcache_readahead_request(),
> already creates its own coroutine, so it runs in the background without
> using callback-style interfaces.

I used bdrv_co_preadv(), because it conveniently solves the partial
cache hit. To solve the partial cache hit, we need to split a request
into smaller parts, make asynchronous requests and wait for all
requests in one place.

Do you propose to create a coroutine for each part of request? It
seemed to me that bdrv_co_preadv() is a wrapper that allows us to get
rid of the same code.

> Kevin
>