Re: [PATCH v4 4/5] block: drop bdrv_prwv

[Eric Blake <eblake@redhat.com>]

On 5/25/20 5:08 AM, Vladimir Sementsov-Ogievskiy wrote:
> Now, when we are not more paying extra code for coroutine wrappers,
> there no more sence in extra indirection layer: bdrv_prwv(). Let's drop
> it and instead genereate pure bdrv_preadv() and bdrv_pwritev().

Typos and grammar; I suggest:

Now that we are not maintaining boilerplate code for coroutine wrappers, 
there is no more sense in keeping the extra indirection layer of 
bdrv_prwv().  Let's drop it and instead generate pure bdrv_preadv() and 
bdrv_pwritev().

> 
> Currently, bdrv_pwritev() and bdrv_preadv() are returning bytes on
> success, auto generated functions will instead return zero, as their
> _co_ prototype. Still, it's simple to make the conversion safe: the
> only external user of bdrv_pwritev() is test-bdrv-drain, and it is
> comfortable enough with bdrv_co_pwritev() instead. So prototypes are
> moved to local block/coroutines.h. Next, the only internal use is
> bdrv_pread() and bdrv_pwrite(), which are modified to return bytes on
> success.

Does returning bytes on success buy us anything useful?  We don't allow 
partial success, so blindly returning 0 on success is no less useful. 
True, we'd have to audit callers to make sure we aren't doing an 
inadvertent semantic change.

> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   block/coroutines.h      | 10 ++++-----
>   include/block/block.h   |  2 --
>   block/io.c              | 49 ++++++++---------------------------------
>   tests/test-bdrv-drain.c |  2 +-
>   4 files changed, 15 insertions(+), 48 deletions(-)
> 

At any rate, I think this patch is reasonable.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org