Re: [Qemu-devel] [PATCH] block: Merge .bdrv_co_writev{, _flags} in drivers

2018-04-25 Thread Kevin Wolf
Am 25.04.2018 um 10:06 hat Daniel P. Berrangé geschrieben:
> On Tue, Apr 24, 2018 at 05:01:57PM -0500, Eric Blake wrote:
> > We have too many driver callback interfaces; simplify the mess
> > somewhat by merging the flags parameter of .bdrv_co_writev_flags()
> > into .bdrv_co_writev_flags().  Note that as long as a driver doesn't
> 
> Typo - this should be just  .bdrv_co_writev

Thanks, fixed up the typo and applied to block-next.

Kevin



Re: [Qemu-devel] [PATCH] block: Merge .bdrv_co_writev{, _flags} in drivers

2018-04-25 Thread Daniel P . Berrangé
On Tue, Apr 24, 2018 at 05:01:57PM -0500, Eric Blake wrote:
> We have too many driver callback interfaces; simplify the mess
> somewhat by merging the flags parameter of .bdrv_co_writev_flags()
> into .bdrv_co_writev_flags().  Note that as long as a driver doesn't

Typo - this should be just  .bdrv_co_writev

> set .supported_write_flags, the flags argument will be 0 and behavior
> is identical.  Also note that the public function bdrv_co_writev()
> still lacks a flags argument; so the driver signature is thus
> intentionally slightly different.  But that's not the end of the
> world, nor the first time that the driver interface differs slightly
> from the public interface.
> 
> Ideally, we should be rewriting all of these drivers to use modern
> byte-based interfaces.  But that's a more invasive patch to write
> and audit, compared to the simplification done here.
> 
> Signed-off-by: Eric Blake 
> ---
> 
> Based-on: <20180424192506.149089-1-ebl...@redhat.com>
> ([PATCH v2 0/6] block: byte-based AIO read/write)
> 
>  include/block/block_int.h |  2 --
>  block/io.c| 13 -
>  block/gluster.c   |  4 +++-
>  block/iscsi.c |  8 
>  block/parallels.c |  4 +++-
>  block/qcow.c  |  6 --
>  block/qed.c   |  3 ++-
>  block/replication.c   |  4 +++-
>  block/sheepdog.c  |  4 +++-
>  block/ssh.c   |  4 +++-
>  block/vhdx.c  |  4 +++-
>  11 files changed, 32 insertions(+), 24 deletions(-)

Reviewed-by: Daniel P. Berrangé 


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



[Qemu-devel] [PATCH] block: Merge .bdrv_co_writev{, _flags} in drivers

2018-04-24 Thread Eric Blake
We have too many driver callback interfaces; simplify the mess
somewhat by merging the flags parameter of .bdrv_co_writev_flags()
into .bdrv_co_writev_flags().  Note that as long as a driver doesn't
set .supported_write_flags, the flags argument will be 0 and behavior
is identical.  Also note that the public function bdrv_co_writev()
still lacks a flags argument; so the driver signature is thus
intentionally slightly different.  But that's not the end of the
world, nor the first time that the driver interface differs slightly
from the public interface.

Ideally, we should be rewriting all of these drivers to use modern
byte-based interfaces.  But that's a more invasive patch to write
and audit, compared to the simplification done here.

Signed-off-by: Eric Blake 
---

Based-on: <20180424192506.149089-1-ebl...@redhat.com>
([PATCH v2 0/6] block: byte-based AIO read/write)

 include/block/block_int.h |  2 --
 block/io.c| 13 -
 block/gluster.c   |  4 +++-
 block/iscsi.c |  8 
 block/parallels.c |  4 +++-
 block/qcow.c  |  6 --
 block/qed.c   |  3 ++-
 block/replication.c   |  4 +++-
 block/sheepdog.c  |  4 +++-
 block/ssh.c   |  4 +++-
 block/vhdx.c  |  4 +++-
 11 files changed, 32 insertions(+), 24 deletions(-)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index 0bba7ed024a..e3d6219f4e3 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -174,8 +174,6 @@ struct BlockDriver {
 int coroutine_fn (*bdrv_co_preadv)(BlockDriverState *bs,
 uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags);
 int coroutine_fn (*bdrv_co_writev)(BlockDriverState *bs,
-int64_t sector_num, int nb_sectors, QEMUIOVector *qiov);
-int coroutine_fn (*bdrv_co_writev_flags)(BlockDriverState *bs,
 int64_t sector_num, int nb_sectors, QEMUIOVector *qiov, int flags);
 /**
  * @offset: position in bytes to write at
diff --git a/block/io.c b/block/io.c
index 6b110b207a0..4fad5ac2fef 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1000,15 +1000,10 @@ static int coroutine_fn 
bdrv_driver_pwritev(BlockDriverState *bs,
 assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0);
 assert((bytes >> BDRV_SECTOR_BITS) <= BDRV_REQUEST_MAX_SECTORS);

-if (drv->bdrv_co_writev_flags) {
-ret = drv->bdrv_co_writev_flags(bs, sector_num, nb_sectors, qiov,
-flags & bs->supported_write_flags);
-flags &= ~bs->supported_write_flags;
-} else {
-assert(drv->bdrv_co_writev);
-assert(!bs->supported_write_flags);
-ret = drv->bdrv_co_writev(bs, sector_num, nb_sectors, qiov);
-}
+assert(drv->bdrv_co_writev);
+ret = drv->bdrv_co_writev(bs, sector_num, nb_sectors, qiov,
+  flags & bs->supported_write_flags);
+flags &= ~bs->supported_write_flags;

 emulate_flags:
 if (ret == 0 && (flags & BDRV_REQ_FUA)) {
diff --git a/block/gluster.c b/block/gluster.c
index 4adc1a875b1..15e7d7fc351 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -1194,8 +1194,10 @@ static coroutine_fn int 
qemu_gluster_co_readv(BlockDriverState *bs,
 static coroutine_fn int qemu_gluster_co_writev(BlockDriverState *bs,
int64_t sector_num,
int nb_sectors,
-   QEMUIOVector *qiov)
+   QEMUIOVector *qiov,
+   int flags)
 {
+assert(!flags);
 return qemu_gluster_co_rw(bs, sector_num, nb_sectors, qiov, 1);
 }

diff --git a/block/iscsi.c b/block/iscsi.c
index f5aecfc8834..35423ded03b 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -556,8 +556,8 @@ static inline bool iscsi_allocmap_is_valid(IscsiLun 
*iscsilun,
 }

 static int coroutine_fn
-iscsi_co_writev_flags(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
-  QEMUIOVector *iov, int flags)
+iscsi_co_writev(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
+QEMUIOVector *iov, int flags)
 {
 IscsiLun *iscsilun = bs->opaque;
 struct IscsiTask iTask;
@@ -2220,7 +2220,7 @@ static BlockDriver bdrv_iscsi = {
 .bdrv_co_pdiscard  = iscsi_co_pdiscard,
 .bdrv_co_pwrite_zeroes = iscsi_co_pwrite_zeroes,
 .bdrv_co_readv = iscsi_co_readv,
-.bdrv_co_writev_flags  = iscsi_co_writev_flags,
+.bdrv_co_writev= iscsi_co_writev,
 .bdrv_co_flush_to_disk = iscsi_co_flush,

 #ifdef __linux__
@@ -2255,7 +2255,7 @@ static BlockDriver bdrv_iser = {
 .bdrv_co_pdiscard  = iscsi_co_pdiscard,
 .bdrv_co_pwrite_zeroes = iscsi_co_pwrite_zeroes,
 .bdrv_co_readv = iscsi_co_readv,
-.bdrv_co_writev_flags  = iscsi_co_writev_flags,
+.bdrv_co_writev=