| Message ID | 1461750767-23273-5-git-send-email-kwolf@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 04/27/2016 03:52 AM, Kevin Wolf wrote: > It used to be an internal helper function just for implementing > bdrv_co_do_readv/writev(), but now that it's a public interface, it > deserves a name without "do" in it. > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > block/block-backend.c | 4 ++-- > block/io.c | 20 ++++++++++---------- > block/raw_bsd.c | 4 ++-- > hw/ide/macio.c | 4 ++-- > include/block/block_int.h | 4 ++-- > 5 files changed, 18 insertions(+), 18 deletions(-) > > @@ -1127,7 +1127,7 @@ static int coroutine_fn bdrv_co_do_readv(BlockDriverState *bs, > return -EINVAL; > } > > - return bdrv_co_do_preadv(bs, sector_num << BDRV_SECTOR_BITS, > + return bdrv_co_preadv(bs, sector_num << BDRV_SECTOR_BITS, > nb_sectors << BDRV_SECTOR_BITS, qiov, flags); > } Missed alignment. > @@ -1523,7 +1523,7 @@ static int coroutine_fn bdrv_co_do_writev(BlockDriverState *bs, > return -EINVAL; > } > > - return bdrv_co_do_pwritev(bs, sector_num << BDRV_SECTOR_BITS, > + return bdrv_co_pwritev(bs, sector_num << BDRV_SECTOR_BITS, > nb_sectors << BDRV_SECTOR_BITS, qiov, flags); > } and again > +++ b/hw/ide/macio.c > @@ -55,8 +55,8 @@ static const int debug_macio = 0; > /* > * Unaligned DMA read/write access functions required for OS X/Darwin which > * don't perform DMA transactions on sector boundaries. These functions are > - * modelled on bdrv_co_do_preadv()/bdrv_co_do_pwritev() and so should be > - * easy to remove if the unaligned block APIs are ever exposed. > + * modelled on bdrv_co_preadv()/bdrv_co_pwritev() and so should be easy to > + * remove if the unaligned block APIs are ever exposed. > */ Is this comment now stale as a result of your series? > +++ b/include/block/block_int.h > @@ -517,10 +517,10 @@ extern BlockDriver bdrv_qcow2; > */ > void bdrv_setup_io_funcs(BlockDriver *bdrv); > > -int coroutine_fn bdrv_co_do_preadv(BlockDriverState *bs, > +int coroutine_fn bdrv_co_preadv(BlockDriverState *bs, > int64_t offset, unsigned int bytes, QEMUIOVector *qiov, > BdrvRequestFlags flags); > -int coroutine_fn bdrv_co_do_pwritev(BlockDriverState *bs, > +int coroutine_fn bdrv_co_pwritev(BlockDriverState *bs, > int64_t offset, unsigned int bytes, QEMUIOVector *qiov, > BdrvRequestFlags flags); Should alignment be attempted here, while touching it? My comments are minor, so whether or not you make those changes: Reviewed-by: Eric Blake <eblake@redhat.com>
Am 27.04.2016 um 16:34 hat Eric Blake geschrieben: > On 04/27/2016 03:52 AM, Kevin Wolf wrote: > > It used to be an internal helper function just for implementing > > bdrv_co_do_readv/writev(), but now that it's a public interface, it > > deserves a name without "do" in it. > > > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > > +++ b/hw/ide/macio.c > > @@ -55,8 +55,8 @@ static const int debug_macio = 0; > > /* > > * Unaligned DMA read/write access functions required for OS X/Darwin which > > * don't perform DMA transactions on sector boundaries. These functions are > > - * modelled on bdrv_co_do_preadv()/bdrv_co_do_pwritev() and so should be > > - * easy to remove if the unaligned block APIs are ever exposed. > > + * modelled on bdrv_co_preadv()/bdrv_co_pwritev() and so should be easy to > > + * remove if the unaligned block APIs are ever exposed. > > */ > > Is this comment now stale as a result of your series? No, as I mentioned in the cover letter, bdrv_co_preadv() and bdrv_co_pwritev() still enforce a minimum alignment of 512. The next steps towards using unaligned I/O in macio.c are removing that minimum (which we can now do for all drivers that implement .bdrv_co_preadv/pwritev) and then using these functions in dma-helpers.c. Kevin
diff --git a/block/block-backend.c b/block/block-backend.c index 16c9d5e..115b069 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -692,7 +692,7 @@ static int coroutine_fn blk_co_preadv(BlockBackend *blk, int64_t offset, return ret; } - return bdrv_co_do_preadv(blk_bs(blk), offset, bytes, qiov, flags); + return bdrv_co_preadv(blk_bs(blk), offset, bytes, qiov, flags); } static int coroutine_fn blk_co_pwritev(BlockBackend *blk, int64_t offset, @@ -710,7 +710,7 @@ static int coroutine_fn blk_co_pwritev(BlockBackend *blk, int64_t offset, flags |= BDRV_REQ_FUA; } - return bdrv_co_do_pwritev(blk_bs(blk), offset, bytes, qiov, flags); + return bdrv_co_pwritev(blk_bs(blk), offset, bytes, qiov, flags); } typedef struct BlkRwCo { diff --git a/block/io.c b/block/io.c index 86d97c3..11ff421 100644 --- a/block/io.c +++ b/block/io.c @@ -569,13 +569,13 @@ static void coroutine_fn bdrv_rw_co_entry(void *opaque) RwCo *rwco = opaque; if (!rwco->is_write) { - rwco->ret = bdrv_co_do_preadv(rwco->bs, rwco->offset, - rwco->qiov->size, rwco->qiov, - rwco->flags); + rwco->ret = bdrv_co_preadv(rwco->bs, rwco->offset, + rwco->qiov->size, rwco->qiov, + rwco->flags); } else { - rwco->ret = bdrv_co_do_pwritev(rwco->bs, rwco->offset, - rwco->qiov->size, rwco->qiov, - rwco->flags); + rwco->ret = bdrv_co_pwritev(rwco->bs, rwco->offset, + rwco->qiov->size, rwco->qiov, + rwco->flags); } } @@ -1045,7 +1045,7 @@ out: /* * Handle a read request in coroutine context */ -int coroutine_fn bdrv_co_do_preadv(BlockDriverState *bs, +int coroutine_fn bdrv_co_preadv(BlockDriverState *bs, int64_t offset, unsigned int bytes, QEMUIOVector *qiov, BdrvRequestFlags flags) { @@ -1127,7 +1127,7 @@ static int coroutine_fn bdrv_co_do_readv(BlockDriverState *bs, return -EINVAL; } - return bdrv_co_do_preadv(bs, sector_num << BDRV_SECTOR_BITS, + return bdrv_co_preadv(bs, sector_num << BDRV_SECTOR_BITS, nb_sectors << BDRV_SECTOR_BITS, qiov, flags); } @@ -1388,7 +1388,7 @@ fail: /* * Handle a write request in coroutine context */ -int coroutine_fn bdrv_co_do_pwritev(BlockDriverState *bs, +int coroutine_fn bdrv_co_pwritev(BlockDriverState *bs, int64_t offset, unsigned int bytes, QEMUIOVector *qiov, BdrvRequestFlags flags) { @@ -1523,7 +1523,7 @@ static int coroutine_fn bdrv_co_do_writev(BlockDriverState *bs, return -EINVAL; } - return bdrv_co_do_pwritev(bs, sector_num << BDRV_SECTOR_BITS, + return bdrv_co_pwritev(bs, sector_num << BDRV_SECTOR_BITS, nb_sectors << BDRV_SECTOR_BITS, qiov, flags); } diff --git a/block/raw_bsd.c b/block/raw_bsd.c index 9c9d39b..5e65fb0 100644 --- a/block/raw_bsd.c +++ b/block/raw_bsd.c @@ -105,8 +105,8 @@ raw_co_writev_flags(BlockDriverState *bs, int64_t sector_num, int nb_sectors, } BLKDBG_EVENT(bs->file, BLKDBG_WRITE_AIO); - ret = bdrv_co_do_pwritev(bs->file->bs, sector_num * BDRV_SECTOR_SIZE, - nb_sectors * BDRV_SECTOR_SIZE, qiov, flags); + ret = bdrv_co_pwritev(bs->file->bs, sector_num * BDRV_SECTOR_SIZE, + nb_sectors * BDRV_SECTOR_SIZE, qiov, flags); fail: if (qiov == &local_qiov) { diff --git a/hw/ide/macio.c b/hw/ide/macio.c index 76256eb..ae29b6f 100644 --- a/hw/ide/macio.c +++ b/hw/ide/macio.c @@ -55,8 +55,8 @@ static const int debug_macio = 0; /* * Unaligned DMA read/write access functions required for OS X/Darwin which * don't perform DMA transactions on sector boundaries. These functions are - * modelled on bdrv_co_do_preadv()/bdrv_co_do_pwritev() and so should be - * easy to remove if the unaligned block APIs are ever exposed. + * modelled on bdrv_co_preadv()/bdrv_co_pwritev() and so should be easy to + * remove if the unaligned block APIs are ever exposed. */ static void pmac_dma_read(BlockBackend *blk, diff --git a/include/block/block_int.h b/include/block/block_int.h index 10d8759..f0171e3 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -517,10 +517,10 @@ extern BlockDriver bdrv_qcow2; */ void bdrv_setup_io_funcs(BlockDriver *bdrv); -int coroutine_fn bdrv_co_do_preadv(BlockDriverState *bs, +int coroutine_fn bdrv_co_preadv(BlockDriverState *bs, int64_t offset, unsigned int bytes, QEMUIOVector *qiov, BdrvRequestFlags flags); -int coroutine_fn bdrv_co_do_pwritev(BlockDriverState *bs, +int coroutine_fn bdrv_co_pwritev(BlockDriverState *bs, int64_t offset, unsigned int bytes, QEMUIOVector *qiov, BdrvRequestFlags flags);
It used to be an internal helper function just for implementing bdrv_co_do_readv/writev(), but now that it's a public interface, it deserves a name without "do" in it. Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- block/block-backend.c | 4 ++-- block/io.c | 20 ++++++++++---------- block/raw_bsd.c | 4 ++-- hw/ide/macio.c | 4 ++-- include/block/block_int.h | 4 ++-- 5 files changed, 18 insertions(+), 18 deletions(-)