Re: [PATCH v5 07/10] block: Implement bdrv_{pread,pwrite,pwrite_zeroes}() using generated_co_wrapper

[Eric Blake]

On Thu, Jun 09, 2022 at 04:27:41PM +0100, Alberto Faria wrote:
> bdrv_{pread,pwrite}() now return -EIO instead of -EINVAL when 'bytes' is
> negative, making them consistent with bdrv_{preadv,pwritev}() and
> bdrv_co_{pread,pwrite,preadv,pwritev}().
> 
> bdrv_pwrite_zeroes() now also calls trace_bdrv_co_pwrite_zeroes() and
> clears the BDRV_REQ_MAY_UNMAP flag when appropriate, which it didn't
> previously.
> 
> Signed-off-by: Alberto Faria <afaria@redhat.com>
> ---
> 
> I audited all bdrv_{pread,pwrite}() callers to make sure that changing
> the -EINVAL return code to -EIO wont't break things. However, there are
> about 140 call sites, so the probability of me having missed something
> isn't negligible. If someone more accustomed to the code base is able to
> double-check this, that would be very much appreciated.

I did not get through all of the callers (you are right, there ARE a
lot), but the ones I checked, particularly in block/qcow2-*.c, appear
to handle -EIO just fine.

I did notice, however, that qcow2-bitmap.c:free_bitmap_clusters()
returns an int failure, but none of its three callers
(qcow2_co_remove_persistent_dirty_bitmap, and twice in
qcow2_store_persistent_dirty_bitmaps) care about the return value.
That may be worth a separate cleanup patch.

> 
> As a precaution, I also dropped Paolo's R-b.
> 
>  block/io.c               | 41 ----------------------------------------
>  include/block/block-io.h | 15 +++++++++------
>  2 files changed, 9 insertions(+), 47 deletions(-)

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

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