* [Qemu-devel] [PATCH] block: Merge .bdrv_co_writev{, _flags} in drivers
@ 2018-04-24 22:01 Eric Blake
2018-04-25 8:06 ` Daniel P. Berrangé
0 siblings, 1 reply; 3+ messages in thread
From: Eric Blake @ 2018-04-24 22:01 UTC (permalink / raw)
To: qemu-devel
Cc: Jeff Cody, Kevin Wolf, Max Reitz, Stefan Hajnoczi, Fam Zheng,
Ronnie Sahlberg, Paolo Bonzini, Peter Lieven, Denis V. Lunev,
Wen Congyang, Xie Changlong, Hitoshi Mitake, Liu Yuan,
Richard W.M. Jones, open list:GLUSTER, open list:Sheepdog
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 <eblake@redhat.com>
---
Based-on: <20180424192506.149089-1-eblake@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 = iscsi_co_writev,
.bdrv_co_flush_to_disk = iscsi_co_flush,
#ifdef __linux__
diff --git a/block/parallels.c b/block/parallels.c
index 799215e079f..3f74fcb877a 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -311,13 +311,15 @@ static int coroutine_fn parallels_co_block_status(BlockDriverState *bs,
}
static coroutine_fn int parallels_co_writev(BlockDriverState *bs,
- int64_t sector_num, int nb_sectors, QEMUIOVector *qiov)
+ int64_t sector_num, int nb_sectors,
+ QEMUIOVector *qiov, int flags)
{
BDRVParallelsState *s = bs->opaque;
uint64_t bytes_done = 0;
QEMUIOVector hd_qiov;
int ret = 0;
+ assert(!flags);
qemu_iovec_init(&hd_qiov, qiov->niov);
while (nb_sectors > 0) {
diff --git a/block/qcow.c b/block/qcow.c
index f92891676c9..dd042b8ddbe 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -720,7 +720,8 @@ static coroutine_fn int qcow_co_readv(BlockDriverState *bs, int64_t sector_num,
}
static coroutine_fn int qcow_co_writev(BlockDriverState *bs, int64_t sector_num,
- int nb_sectors, QEMUIOVector *qiov)
+ int nb_sectors, QEMUIOVector *qiov,
+ int flags)
{
BDRVQcowState *s = bs->opaque;
int index_in_cluster;
@@ -731,6 +732,7 @@ static coroutine_fn int qcow_co_writev(BlockDriverState *bs, int64_t sector_num,
uint8_t *buf;
void *orig_buf;
+ assert(!flags);
s->cluster_cache_offset = -1; /* disable compressed cache */
/* We must always copy the iov when encrypting, so we
@@ -1110,7 +1112,7 @@ qcow_co_pwritev_compressed(BlockDriverState *bs, uint64_t offset,
if (ret != Z_STREAM_END || out_len >= s->cluster_size) {
/* could not compress: write normal cluster */
ret = qcow_co_writev(bs, offset >> BDRV_SECTOR_BITS,
- bytes >> BDRV_SECTOR_BITS, qiov);
+ bytes >> BDRV_SECTOR_BITS, qiov, 0);
if (ret < 0) {
goto fail;
}
diff --git a/block/qed.c b/block/qed.c
index 35ff505066f..a3600bd2d07 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -1437,8 +1437,9 @@ static int coroutine_fn bdrv_qed_co_readv(BlockDriverState *bs,
static int coroutine_fn bdrv_qed_co_writev(BlockDriverState *bs,
int64_t sector_num, int nb_sectors,
- QEMUIOVector *qiov)
+ QEMUIOVector *qiov, int flags)
{
+ assert(!flags);
return qed_co_request(bs, sector_num, qiov, nb_sectors, QED_AIOCB_WRITE);
}
diff --git a/block/replication.c b/block/replication.c
index 6c0c7186d9a..48148b884a5 100644
--- a/block/replication.c
+++ b/block/replication.c
@@ -260,7 +260,8 @@ out:
static coroutine_fn int replication_co_writev(BlockDriverState *bs,
int64_t sector_num,
int remaining_sectors,
- QEMUIOVector *qiov)
+ QEMUIOVector *qiov,
+ int flags)
{
BDRVReplicationState *s = bs->opaque;
QEMUIOVector hd_qiov;
@@ -271,6 +272,7 @@ static coroutine_fn int replication_co_writev(BlockDriverState *bs,
int ret;
int64_t n;
+ assert(!flags);
ret = replication_get_io_status(s);
if (ret < 0) {
goto out;
diff --git a/block/sheepdog.c b/block/sheepdog.c
index 387f59c8aa5..a2f26efc0e2 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -2612,13 +2612,15 @@ static void sd_aio_complete(SheepdogAIOCB *acb)
}
static coroutine_fn int sd_co_writev(BlockDriverState *bs, int64_t sector_num,
- int nb_sectors, QEMUIOVector *qiov)
+ int nb_sectors, QEMUIOVector *qiov,
+ int flags)
{
SheepdogAIOCB acb;
int ret;
int64_t offset = (sector_num + nb_sectors) * BDRV_SECTOR_SIZE;
BDRVSheepdogState *s = bs->opaque;
+ assert(!flags);
if (offset > s->inode.vdi_size) {
ret = sd_truncate(bs, offset, PREALLOC_MODE_OFF, NULL);
if (ret < 0) {
diff --git a/block/ssh.c b/block/ssh.c
index ab3acf0c228..e8ad3d4c46a 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -1164,11 +1164,13 @@ static int ssh_write(BDRVSSHState *s, BlockDriverState *bs,
static coroutine_fn int ssh_co_writev(BlockDriverState *bs,
int64_t sector_num,
- int nb_sectors, QEMUIOVector *qiov)
+ int nb_sectors, QEMUIOVector *qiov,
+ int flags)
{
BDRVSSHState *s = bs->opaque;
int ret;
+ assert(!flags);
qemu_co_mutex_lock(&s->lock);
ret = ssh_write(s, bs, sector_num * BDRV_SECTOR_SIZE,
nb_sectors * BDRV_SECTOR_SIZE, qiov);
diff --git a/block/vhdx.c b/block/vhdx.c
index 6ac0424f61a..0b72d895db4 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -1226,7 +1226,8 @@ int vhdx_user_visible_write(BlockDriverState *bs, BDRVVHDXState *s)
}
static coroutine_fn int vhdx_co_writev(BlockDriverState *bs, int64_t sector_num,
- int nb_sectors, QEMUIOVector *qiov)
+ int nb_sectors, QEMUIOVector *qiov,
+ int flags)
{
int ret = -ENOTSUP;
BDRVVHDXState *s = bs->opaque;
@@ -1242,6 +1243,7 @@ static coroutine_fn int vhdx_co_writev(BlockDriverState *bs, int64_t sector_num,
uint64_t bat_prior_offset = 0;
bool bat_update = false;
+ assert(!flags);
qemu_iovec_init(&hd_qiov, qiov->niov);
qemu_co_mutex_lock(&s->lock);
--
2.14.3
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] block: Merge .bdrv_co_writev{, _flags} in drivers
2018-04-24 22:01 [Qemu-devel] [PATCH] block: Merge .bdrv_co_writev{, _flags} in drivers Eric Blake
@ 2018-04-25 8:06 ` Daniel P. Berrangé
2018-04-25 11:15 ` Kevin Wolf
0 siblings, 1 reply; 3+ messages in thread
From: Daniel P. Berrangé @ 2018-04-25 8:06 UTC (permalink / raw)
To: Eric Blake
Cc: qemu-devel, Kevin Wolf, Fam Zheng, Stefan Hajnoczi,
open list:GLUSTER, Hitoshi Mitake, Wen Congyang, Jeff Cody,
Peter Lieven, Richard W.M. Jones, Max Reitz, open list:Sheepdog,
Ronnie Sahlberg, Denis V. Lunev, Paolo Bonzini, Liu Yuan,
Xie Changlong
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 <eblake@redhat.com>
> ---
>
> Based-on: <20180424192506.149089-1-eblake@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é <berrange@redhat.com>
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 :|
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] block: Merge .bdrv_co_writev{, _flags} in drivers
2018-04-25 8:06 ` Daniel P. Berrangé
@ 2018-04-25 11:15 ` Kevin Wolf
0 siblings, 0 replies; 3+ messages in thread
From: Kevin Wolf @ 2018-04-25 11:15 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Eric Blake, qemu-devel, Fam Zheng, Stefan Hajnoczi,
open list:GLUSTER, Hitoshi Mitake, Wen Congyang, Jeff Cody,
Peter Lieven, Richard W.M. Jones, Max Reitz, open list:Sheepdog,
Ronnie Sahlberg, Denis V. Lunev, Paolo Bonzini, Liu Yuan,
Xie Changlong
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-04-25 11:15 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-04-24 22:01 [Qemu-devel] [PATCH] block: Merge .bdrv_co_writev{, _flags} in drivers Eric Blake
2018-04-25 8:06 ` Daniel P. Berrangé
2018-04-25 11:15 ` Kevin Wolf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).