[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v5 07/10] block: Implement bdrv_{pread,pwrite,pwrite_zeroes}(
From: |
Eric Blake |
Subject: |
Re: [PATCH v5 07/10] block: Implement bdrv_{pread,pwrite,pwrite_zeroes}() using generated_co_wrapper |
Date: |
Thu, 23 Jun 2022 16:47:06 -0500 |
User-agent: |
NeoMutt/20220429-136-41baff |
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
- [PATCH v5 00/10] Implement bdrv_{pread, pwrite, pwrite_sync, pwrite_zeroes}() using generated_co_wrapper, Alberto Faria, 2022/06/09
- [PATCH v5 01/10] block: Add a 'flags' param to bdrv_{pread, pwrite, pwrite_sync}(), Alberto Faria, 2022/06/09
- [PATCH v5 02/10] block: Change bdrv_{pread, pwrite, pwrite_sync}() param order, Alberto Faria, 2022/06/09
- [PATCH v5 03/10] block: Make bdrv_{pread, pwrite}() return 0 on success, Alberto Faria, 2022/06/09
- [PATCH v5 04/10] crypto: Make block callbacks return 0 on success, Alberto Faria, 2022/06/09
- [PATCH v5 05/10] block: Make bdrv_co_pwrite() take a const buffer, Alberto Faria, 2022/06/09
- [PATCH v5 07/10] block: Implement bdrv_{pread, pwrite, pwrite_zeroes}() using generated_co_wrapper, Alberto Faria, 2022/06/09
- Re: [PATCH v5 07/10] block: Implement bdrv_{pread,pwrite,pwrite_zeroes}() using generated_co_wrapper,
Eric Blake <=
- [PATCH v5 06/10] block: Make 'bytes' param of bdrv_co_{pread, pwrite, preadv, pwritev}() an int64_t, Alberto Faria, 2022/06/09
- [PATCH v5 08/10] block: Add bdrv_co_pwrite_sync(), Alberto Faria, 2022/06/09
- [PATCH v5 09/10] block: Use bdrv_co_pwrite_sync() when caller is coroutine_fn, Alberto Faria, 2022/06/09
- [PATCH v5 10/10] block/qcow2: Use bdrv_pwrite_sync() in qcow2_mark_dirty(), Alberto Faria, 2022/06/09
- Re: [PATCH v5 00/10] Implement bdrv_{pread, pwrite, pwrite_sync, pwrite_zeroes}() using generated_co_wrapper, Alberto Faria, 2022/06/23