[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 05/14] block: Make blk_co_preadv/pwritev() pu
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH v2 05/14] block: Make blk_co_preadv/pwritev() public |
Date: |
Wed, 25 May 2016 11:07:47 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
Am 25.05.2016 um 02:24 hat Eric Blake geschrieben:
> On 05/24/2016 07:47 AM, Kevin Wolf wrote:
> > Also add trace points now that the function can be directly called.
> >
> > Signed-off-by: Kevin Wolf <address@hidden>
> > ---
> > block/block-backend.c | 21 ++++++++++++++-------
> > include/sysemu/block-backend.h | 6 ++++++
> > trace-events | 4 ++++
> > 3 files changed, 24 insertions(+), 7 deletions(-)
> >
>
> > @@ -741,11 +742,15 @@ static int blk_check_request(BlockBackend *blk,
> > int64_t sector_num,
> > nb_sectors * BDRV_SECTOR_SIZE);
> > }
> >
> > -static int coroutine_fn blk_co_preadv(BlockBackend *blk, int64_t offset,
> > - unsigned int bytes, QEMUIOVector
> > *qiov,
> > - BdrvRequestFlags flags)
> > +int coroutine_fn blk_co_preadv(BlockBackend *blk, int64_t offset,
> > + unsigned int bytes, QEMUIOVector *qiov,
>
> Isn't bytes redundant with qiov->size? Or can qiov be NULL? Should we
> assert(!qiov || qiov->size == bytes)?
It's redundant for reads, as far as I know. I'm not completely sure yet
if the redundancy is completely useless; it might help to catch bugs
with the qiov initialisation in devices.
In any case, if we want to remove bytes from the interface, I think
that's a change that should be done consistently across all functions in
a series of its own and not hidden in something that is only meant to
touch block jobs.
> > +int coroutine_fn blk_co_pwritev(BlockBackend *blk, int64_t offset,
> > + unsigned int bytes, QEMUIOVector *qiov,
> > + BdrvRequestFlags flags)
> > {
>
> Ditto. When doing write_zeroes, do we want qiov == NULL, must we always
> have qiov but just leave qiov->iov[0].base as NULL? Probably worth
> documenting as part of making it public.
Here it's currently not redundant because we use qiov == NULL in some
callers. Other callers do the .iov_base = NULL, .iov_len = bytes thing.
Perhaps worth cleaning up, but again, not in this series.
Kevin
pgp17iFUnvJFb.pgp
Description: PGP signature
- Re: [Qemu-devel] [PATCH v2 08/14] mirror: Use BlockBackend for I/O, (continued)
[Qemu-devel] [PATCH v2 06/14] stream: Use BlockBackend for I/O, Kevin Wolf, 2016/05/24
[Qemu-devel] [PATCH v2 11/14] backup: Remove bs parameter from backup_do_cow(), Kevin Wolf, 2016/05/24
[Qemu-devel] [PATCH v2 14/14] blockjob: Remove BlockJob.bs, Kevin Wolf, 2016/05/24
[Qemu-devel] [PATCH v2 05/14] block: Make blk_co_preadv/pwritev() public, Kevin Wolf, 2016/05/24
[Qemu-devel] [PATCH v2 12/14] backup: Use BlockBackend for I/O, Kevin Wolf, 2016/05/24
[Qemu-devel] [PATCH v2 13/14] commit: Use BlockBackend for I/O, Kevin Wolf, 2016/05/24