* [Qemu-devel] [PATCH 00/17] block: Convert .bdrv_read/write to .bdrv_preadv/pwritev
@ 2016-04-27 9:52 Kevin Wolf
2016-04-27 9:52 ` [Qemu-devel] [PATCH 01/17] block: Introduce bdrv_driver_preadv() Kevin Wolf
` (17 more replies)
0 siblings, 18 replies; 34+ messages in thread
From: Kevin Wolf @ 2016-04-27 9:52 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, stefanha, famz, sw, qemu-devel
This series introduces a new BlockDriver interface, which will hopefully be the
final one, or at least good enough for another few years: .bdrv_preadv/pwritev.
It is based on coroutines, vectored, has flags and uses a byte granularity.
This is now the preferred interface for new drivers.
All drivers still using the legacy interface .bdrv_read/write are converted to
the new interface and the emulation code we had for the old interface is
removed. Most of the drivers become zero-copy with these patches as they are
vectored now; only vvfat continues to allocate a bounce buffer.
The less obscure formats (vmdk, vdi, vpc) also natively support byte-aligned
requests now. The block layer is still enforcing a minimal alignment of 512, so
this isn't actually used yet, but in a next step, we can lift this restriction
for drivers that implement .bdrv_preadv/pwritev.
Kevin Wolf (17):
block: Introduce bdrv_driver_preadv()
block: Introduce bdrv_driver_pwritev()
block: Support AIO drivers in bdrv_driver_preadv/pwritev()
block: Rename bdrv_co_do_preadv/writev to bdrv_co_preadv/writev
block: Introduce .bdrv_co_preadv/pwritev BlockDriver function
bochs: Implement .bdrv_co_preadv() interface
cloop: Implement .bdrv_co_preadv() interface
dmg: Implement .bdrv_co_preadv() interface
vdi: Implement .bdrv_co_preadv() interface
vdi: Implement .bdrv_co_pwritev() interface
vmdk: Add vmdk_find_offset_in_cluster()
vmdk: Implement .bdrv_co_preadv() interface
vmdk: Implement .bdrv_co_pwritev() interface
vpc: Implement .bdrv_co_preadv() interface
vpc: Implement .bdrv_co_pwritev() interface
vvfat: Implement .bdrv_co_preadv/pwritev interfaces
block: Remove BlockDriver.bdrv_read/write
block.c | 2 -
block/block-backend.c | 4 +-
block/bochs.c | 46 ++++---
block/cloop.c | 33 ++---
block/dmg.c | 36 +++---
block/io.c | 317 +++++++++++++++++++--------------------------
block/iscsi.c | 8 --
block/nbd.c | 9 --
block/raw_bsd.c | 12 +-
block/vdi.c | 127 ++++++++++--------
block/vmdk.c | 320 ++++++++++++++++++++++++++++------------------
block/vpc.c | 165 ++++++++++++------------
block/vvfat.c | 55 ++++++--
hw/ide/macio.c | 4 +-
include/block/block_int.h | 12 +-
15 files changed, 609 insertions(+), 541 deletions(-)
--
1.8.3.1
^ permalink raw reply [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH 01/17] block: Introduce bdrv_driver_preadv()
2016-04-27 9:52 [Qemu-devel] [PATCH 00/17] block: Convert .bdrv_read/write to .bdrv_preadv/pwritev Kevin Wolf
@ 2016-04-27 9:52 ` Kevin Wolf
2016-04-27 13:52 ` Eric Blake
2016-04-27 9:52 ` [Qemu-devel] [PATCH 02/17] block: Introduce bdrv_driver_pwritev() Kevin Wolf
` (16 subsequent siblings)
17 siblings, 1 reply; 34+ messages in thread
From: Kevin Wolf @ 2016-04-27 9:52 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, stefanha, famz, sw, qemu-devel
This is a function that simply calls into the block driver for doing a
read, providing the byte granularity interface we want to eventually
have everywhere, and using whatever interface that driver supports.
For now, this is just a wrapper for calling bs->drv->bdrv_co_readv().
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/io.c | 30 +++++++++++++++++++++++-------
1 file changed, 23 insertions(+), 7 deletions(-)
diff --git a/block/io.c b/block/io.c
index a7dbf85..586a46a 100644
--- a/block/io.c
+++ b/block/io.c
@@ -800,6 +800,21 @@ int bdrv_pwrite_sync(BlockDriverState *bs, int64_t offset,
return 0;
}
+static int coroutine_fn bdrv_driver_preadv(BlockDriverState *bs,
+ uint64_t offset, uint64_t bytes,
+ QEMUIOVector *qiov, int flags)
+{
+ BlockDriver *drv = bs->drv;
+ int64_t sector_num = offset >> BDRV_SECTOR_BITS;
+ unsigned int nb_sectors = bytes >> BDRV_SECTOR_BITS;
+
+ assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0);
+ assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0);
+ assert((bytes >> BDRV_SECTOR_BITS) <= BDRV_REQUEST_MAX_SECTORS);
+
+ return drv->bdrv_co_readv(bs, sector_num, nb_sectors, qiov);
+}
+
static int coroutine_fn bdrv_co_do_copy_on_readv(BlockDriverState *bs,
int64_t sector_num, int nb_sectors, QEMUIOVector *qiov)
{
@@ -836,8 +851,9 @@ static int coroutine_fn bdrv_co_do_copy_on_readv(BlockDriverState *bs,
qemu_iovec_init_external(&bounce_qiov, &iov, 1);
- ret = drv->bdrv_co_readv(bs, cluster_sector_num, cluster_nb_sectors,
- &bounce_qiov);
+ ret = bdrv_driver_preadv(bs, cluster_sector_num * BDRV_SECTOR_SIZE,
+ cluster_nb_sectors * BDRV_SECTOR_SIZE,
+ &bounce_qiov, 0);
if (ret < 0) {
goto err;
}
@@ -880,7 +896,6 @@ static int coroutine_fn bdrv_aligned_preadv(BlockDriverState *bs,
BdrvTrackedRequest *req, int64_t offset, unsigned int bytes,
int64_t align, QEMUIOVector *qiov, int flags)
{
- BlockDriver *drv = bs->drv;
int ret;
int64_t sector_num = offset >> BDRV_SECTOR_BITS;
@@ -921,7 +936,7 @@ static int coroutine_fn bdrv_aligned_preadv(BlockDriverState *bs,
/* Forward the request to the BlockDriver */
if (!bs->zero_beyond_eof) {
- ret = drv->bdrv_co_readv(bs, sector_num, nb_sectors, qiov);
+ ret = bdrv_driver_preadv(bs, offset, bytes, qiov, 0);
} else {
/* Read zeros after EOF */
int64_t total_sectors, max_nb_sectors;
@@ -935,7 +950,7 @@ static int coroutine_fn bdrv_aligned_preadv(BlockDriverState *bs,
max_nb_sectors = ROUND_UP(MAX(0, total_sectors - sector_num),
align >> BDRV_SECTOR_BITS);
if (nb_sectors < max_nb_sectors) {
- ret = drv->bdrv_co_readv(bs, sector_num, nb_sectors, qiov);
+ ret = bdrv_driver_preadv(bs, offset, bytes, qiov, 0);
} else if (max_nb_sectors > 0) {
QEMUIOVector local_qiov;
@@ -943,8 +958,9 @@ static int coroutine_fn bdrv_aligned_preadv(BlockDriverState *bs,
qemu_iovec_concat(&local_qiov, qiov, 0,
max_nb_sectors * BDRV_SECTOR_SIZE);
- ret = drv->bdrv_co_readv(bs, sector_num, max_nb_sectors,
- &local_qiov);
+ ret = bdrv_driver_preadv(bs, offset,
+ max_nb_sectors * BDRV_SECTOR_SIZE,
+ &local_qiov, 0);
qemu_iovec_destroy(&local_qiov);
} else {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH 02/17] block: Introduce bdrv_driver_pwritev()
2016-04-27 9:52 [Qemu-devel] [PATCH 00/17] block: Convert .bdrv_read/write to .bdrv_preadv/pwritev Kevin Wolf
2016-04-27 9:52 ` [Qemu-devel] [PATCH 01/17] block: Introduce bdrv_driver_preadv() Kevin Wolf
@ 2016-04-27 9:52 ` Kevin Wolf
2016-04-27 14:03 ` Eric Blake
2016-04-27 9:52 ` [Qemu-devel] [PATCH 03/17] block: Support AIO drivers in bdrv_driver_preadv/pwritev() Kevin Wolf
` (15 subsequent siblings)
17 siblings, 1 reply; 34+ messages in thread
From: Kevin Wolf @ 2016-04-27 9:52 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, stefanha, famz, sw, qemu-devel
This is a function that simply calls into the block driver for doing a
write, providing the byte granularity interface we want to eventually
have everywhere, and using whatever interface that driver supports.
This one is a bit more interesting that the version for reads: It adds
support for .bdrv_co_writev_flags() everywhere, so that drivers
implementing this function can drop .bdrv_co_writev() now.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/io.c | 51 ++++++++++++++++++++++++++++++++++++---------------
block/iscsi.c | 8 --------
block/nbd.c | 9 ---------
block/raw_bsd.c | 8 --------
4 files changed, 36 insertions(+), 40 deletions(-)
diff --git a/block/io.c b/block/io.c
index 586a46a..c9b2864 100644
--- a/block/io.c
+++ b/block/io.c
@@ -815,6 +815,36 @@ static int coroutine_fn bdrv_driver_preadv(BlockDriverState *bs,
return drv->bdrv_co_readv(bs, sector_num, nb_sectors, qiov);
}
+static int coroutine_fn bdrv_driver_pwritev(BlockDriverState *bs,
+ uint64_t offset, uint64_t bytes,
+ QEMUIOVector *qiov, int flags)
+{
+ BlockDriver *drv = bs->drv;
+ int64_t sector_num = offset >> BDRV_SECTOR_BITS;
+ unsigned int nb_sectors = bytes >> BDRV_SECTOR_BITS;
+ int ret;
+
+ assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0);
+ 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);
+ } else {
+ assert(drv->supported_write_flags == 0);
+ ret = drv->bdrv_co_writev(bs, sector_num, nb_sectors, qiov);
+ }
+
+ if (ret == 0 && (flags & BDRV_REQ_FUA) &&
+ !(drv->supported_write_flags & BDRV_REQ_FUA))
+ {
+ ret = bdrv_co_flush(bs);
+ }
+
+ return ret;
+}
+
static int coroutine_fn bdrv_co_do_copy_on_readv(BlockDriverState *bs,
int64_t sector_num, int nb_sectors, QEMUIOVector *qiov)
{
@@ -866,8 +896,9 @@ static int coroutine_fn bdrv_co_do_copy_on_readv(BlockDriverState *bs,
/* This does not change the data on the disk, it is not necessary
* to flush even in cache=writethrough mode.
*/
- ret = drv->bdrv_co_writev(bs, cluster_sector_num, cluster_nb_sectors,
- &bounce_qiov);
+ ret = bdrv_driver_pwritev(bs, cluster_sector_num * BDRV_SECTOR_SIZE,
+ cluster_nb_sectors * BDRV_SECTOR_SIZE,
+ &bounce_qiov, 0);
}
if (ret < 0) {
@@ -1155,7 +1186,8 @@ static int coroutine_fn bdrv_co_do_write_zeroes(BlockDriverState *bs,
}
qemu_iovec_init_external(&qiov, &iov, 1);
- ret = drv->bdrv_co_writev(bs, sector_num, num, &qiov);
+ ret = bdrv_driver_pwritev(bs, sector_num * BDRV_SECTOR_SIZE,
+ num * BDRV_SECTOR_SIZE, &qiov, 0);
/* Keep bounce buffer around if it is big enough for all
* all future requests.
@@ -1215,23 +1247,12 @@ static int coroutine_fn bdrv_aligned_pwritev(BlockDriverState *bs,
} else if (flags & BDRV_REQ_ZERO_WRITE) {
bdrv_debug_event(bs, BLKDBG_PWRITEV_ZERO);
ret = bdrv_co_do_write_zeroes(bs, sector_num, nb_sectors, flags);
- } else if (drv->bdrv_co_writev_flags) {
- bdrv_debug_event(bs, BLKDBG_PWRITEV);
- ret = drv->bdrv_co_writev_flags(bs, sector_num, nb_sectors, qiov,
- flags);
} else {
- assert(drv->supported_write_flags == 0);
bdrv_debug_event(bs, BLKDBG_PWRITEV);
- ret = drv->bdrv_co_writev(bs, sector_num, nb_sectors, qiov);
+ ret = bdrv_driver_pwritev(bs, offset, bytes, qiov, flags);
}
bdrv_debug_event(bs, BLKDBG_PWRITEV_DONE);
- if (ret == 0 && (flags & BDRV_REQ_FUA) &&
- !(drv->supported_write_flags & BDRV_REQ_FUA))
- {
- ret = bdrv_co_flush(bs);
- }
-
bdrv_set_dirty(bs, sector_num, nb_sectors);
if (bs->wr_highest_offset < offset + bytes) {
diff --git a/block/iscsi.c b/block/iscsi.c
index 302baf8..4f75204 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -513,13 +513,6 @@ retry:
return 0;
}
-static int coroutine_fn
-iscsi_co_writev(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
- QEMUIOVector *iov)
-{
- return iscsi_co_writev_flags(bs, sector_num, nb_sectors, iov, 0);
-}
-
static bool iscsi_allocationmap_is_allocated(IscsiLun *iscsilun,
int64_t sector_num, int nb_sectors)
@@ -1847,7 +1840,6 @@ static BlockDriver bdrv_iscsi = {
.bdrv_co_discard = iscsi_co_discard,
.bdrv_co_write_zeroes = iscsi_co_write_zeroes,
.bdrv_co_readv = iscsi_co_readv,
- .bdrv_co_writev = iscsi_co_writev,
.bdrv_co_writev_flags = iscsi_co_writev_flags,
.supported_write_flags = BDRV_REQ_FUA,
.bdrv_co_flush_to_disk = iscsi_co_flush,
diff --git a/block/nbd.c b/block/nbd.c
index f7ea3b3..fccbfef 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -374,12 +374,6 @@ static int nbd_co_writev_flags(BlockDriverState *bs, int64_t sector_num,
return ret;
}
-static int nbd_co_writev(BlockDriverState *bs, int64_t sector_num,
- int nb_sectors, QEMUIOVector *qiov)
-{
- return nbd_co_writev_flags(bs, sector_num, nb_sectors, qiov, 0);
-}
-
static int nbd_co_flush(BlockDriverState *bs)
{
return nbd_client_co_flush(bs);
@@ -476,7 +470,6 @@ static BlockDriver bdrv_nbd = {
.bdrv_parse_filename = nbd_parse_filename,
.bdrv_file_open = nbd_open,
.bdrv_co_readv = nbd_co_readv,
- .bdrv_co_writev = nbd_co_writev,
.bdrv_co_writev_flags = nbd_co_writev_flags,
.supported_write_flags = BDRV_REQ_FUA,
.bdrv_close = nbd_close,
@@ -496,7 +489,6 @@ static BlockDriver bdrv_nbd_tcp = {
.bdrv_parse_filename = nbd_parse_filename,
.bdrv_file_open = nbd_open,
.bdrv_co_readv = nbd_co_readv,
- .bdrv_co_writev = nbd_co_writev,
.bdrv_co_writev_flags = nbd_co_writev_flags,
.supported_write_flags = BDRV_REQ_FUA,
.bdrv_close = nbd_close,
@@ -516,7 +508,6 @@ static BlockDriver bdrv_nbd_unix = {
.bdrv_parse_filename = nbd_parse_filename,
.bdrv_file_open = nbd_open,
.bdrv_co_readv = nbd_co_readv,
- .bdrv_co_writev = nbd_co_writev,
.bdrv_co_writev_flags = nbd_co_writev_flags,
.supported_write_flags = BDRV_REQ_FUA,
.bdrv_close = nbd_close,
diff --git a/block/raw_bsd.c b/block/raw_bsd.c
index a6cc7e9..9c9d39b 100644
--- a/block/raw_bsd.c
+++ b/block/raw_bsd.c
@@ -116,13 +116,6 @@ fail:
return ret;
}
-static int coroutine_fn
-raw_co_writev(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
- QEMUIOVector *qiov)
-{
- return raw_co_writev_flags(bs, sector_num, nb_sectors, qiov, 0);
-}
-
static int64_t coroutine_fn raw_co_get_block_status(BlockDriverState *bs,
int64_t sector_num,
int nb_sectors, int *pnum,
@@ -256,7 +249,6 @@ BlockDriver bdrv_raw = {
.bdrv_close = &raw_close,
.bdrv_create = &raw_create,
.bdrv_co_readv = &raw_co_readv,
- .bdrv_co_writev = &raw_co_writev,
.bdrv_co_writev_flags = &raw_co_writev_flags,
.supported_write_flags = BDRV_REQ_FUA,
.bdrv_co_write_zeroes = &raw_co_write_zeroes,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH 03/17] block: Support AIO drivers in bdrv_driver_preadv/pwritev()
2016-04-27 9:52 [Qemu-devel] [PATCH 00/17] block: Convert .bdrv_read/write to .bdrv_preadv/pwritev Kevin Wolf
2016-04-27 9:52 ` [Qemu-devel] [PATCH 01/17] block: Introduce bdrv_driver_preadv() Kevin Wolf
2016-04-27 9:52 ` [Qemu-devel] [PATCH 02/17] block: Introduce bdrv_driver_pwritev() Kevin Wolf
@ 2016-04-27 9:52 ` Kevin Wolf
2016-04-27 14:13 ` Eric Blake
2016-04-27 9:52 ` [Qemu-devel] [PATCH 04/17] block: Rename bdrv_co_do_preadv/writev to bdrv_co_preadv/writev Kevin Wolf
` (14 subsequent siblings)
17 siblings, 1 reply; 34+ messages in thread
From: Kevin Wolf @ 2016-04-27 9:52 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, stefanha, famz, sw, qemu-devel
Instead of registering emulation functions as .bdrv_co_writev, just
directly check whether the function is there or not, and use the AIO
interface if it isn't. This makes the read/write functions more
consistent with how things are done in other places (flush, discard,
etc.)
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/io.c | 126 +++++++++++++++++++++++++------------------------------------
1 file changed, 52 insertions(+), 74 deletions(-)
diff --git a/block/io.c b/block/io.c
index c9b2864..86d97c3 100644
--- a/block/io.c
+++ b/block/io.c
@@ -40,12 +40,6 @@ static BlockAIOCB *bdrv_aio_readv_em(BlockDriverState *bs,
static BlockAIOCB *bdrv_aio_writev_em(BlockDriverState *bs,
int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
BlockCompletionFunc *cb, void *opaque);
-static int coroutine_fn bdrv_co_readv_em(BlockDriverState *bs,
- int64_t sector_num, int nb_sectors,
- QEMUIOVector *iov);
-static int coroutine_fn bdrv_co_writev_em(BlockDriverState *bs,
- int64_t sector_num, int nb_sectors,
- QEMUIOVector *iov);
static BlockAIOCB *bdrv_co_aio_rw_vector(BlockDriverState *bs,
int64_t sector_num,
QEMUIOVector *qiov,
@@ -125,19 +119,13 @@ void bdrv_io_limits_update_group(BlockDriverState *bs, const char *group)
void bdrv_setup_io_funcs(BlockDriver *bdrv)
{
- /* Block drivers without coroutine functions need emulation */
- if (!bdrv->bdrv_co_readv) {
- bdrv->bdrv_co_readv = bdrv_co_readv_em;
- bdrv->bdrv_co_writev = bdrv_co_writev_em;
-
- /* bdrv_co_readv_em()/brdv_co_writev_em() work in terms of aio, so if
- * the block driver lacks aio we need to emulate that too.
- */
- if (!bdrv->bdrv_aio_readv) {
- /* add AIO emulation layer */
- bdrv->bdrv_aio_readv = bdrv_aio_readv_em;
- bdrv->bdrv_aio_writev = bdrv_aio_writev_em;
- }
+ /* bdrv_co_readv_em()/brdv_co_writev_em() work in terms of aio, so if
+ * the block driver lacks aio we need to emulate that.
+ */
+ if (!bdrv->bdrv_aio_readv) {
+ /* add AIO emulation layer */
+ bdrv->bdrv_aio_readv = bdrv_aio_readv_em;
+ bdrv->bdrv_aio_writev = bdrv_aio_writev_em;
}
}
@@ -800,6 +788,19 @@ int bdrv_pwrite_sync(BlockDriverState *bs, int64_t offset,
return 0;
}
+typedef struct CoroutineIOCompletion {
+ Coroutine *coroutine;
+ int ret;
+} CoroutineIOCompletion;
+
+static void bdrv_co_io_em_complete(void *opaque, int ret)
+{
+ CoroutineIOCompletion *co = opaque;
+
+ co->ret = ret;
+ qemu_coroutine_enter(co->coroutine, NULL);
+}
+
static int coroutine_fn bdrv_driver_preadv(BlockDriverState *bs,
uint64_t offset, uint64_t bytes,
QEMUIOVector *qiov, int flags)
@@ -812,7 +813,23 @@ static int coroutine_fn bdrv_driver_preadv(BlockDriverState *bs,
assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0);
assert((bytes >> BDRV_SECTOR_BITS) <= BDRV_REQUEST_MAX_SECTORS);
- return drv->bdrv_co_readv(bs, sector_num, nb_sectors, qiov);
+ if (drv->bdrv_co_readv) {
+ return drv->bdrv_co_readv(bs, sector_num, nb_sectors, qiov);
+ } else {
+ BlockAIOCB *acb;
+ CoroutineIOCompletion co = {
+ .coroutine = qemu_coroutine_self(),
+ };
+
+ acb = bs->drv->bdrv_aio_readv(bs, sector_num, qiov, nb_sectors,
+ bdrv_co_io_em_complete, &co);
+ if (acb == NULL) {
+ return -EIO;
+ } else {
+ qemu_coroutine_yield();
+ return co.ret;
+ }
+ }
}
static int coroutine_fn bdrv_driver_pwritev(BlockDriverState *bs,
@@ -831,9 +848,23 @@ static int coroutine_fn bdrv_driver_pwritev(BlockDriverState *bs,
if (drv->bdrv_co_writev_flags) {
ret = drv->bdrv_co_writev_flags(bs, sector_num, nb_sectors, qiov,
flags);
- } else {
+ } else if (drv->bdrv_co_writev) {
assert(drv->supported_write_flags == 0);
ret = drv->bdrv_co_writev(bs, sector_num, nb_sectors, qiov);
+ } else {
+ BlockAIOCB *acb;
+ CoroutineIOCompletion co = {
+ .coroutine = qemu_coroutine_self(),
+ };
+
+ acb = bs->drv->bdrv_aio_writev(bs, sector_num, qiov, nb_sectors,
+ bdrv_co_io_em_complete, &co);
+ if (acb == NULL) {
+ return -EIO;
+ } else {
+ qemu_coroutine_yield();
+ return co.ret;
+ }
}
if (ret == 0 && (flags & BDRV_REQ_FUA) &&
@@ -2351,59 +2382,6 @@ void qemu_aio_unref(void *p)
/**************************************************************/
/* Coroutine block device emulation */
-typedef struct CoroutineIOCompletion {
- Coroutine *coroutine;
- int ret;
-} CoroutineIOCompletion;
-
-static void bdrv_co_io_em_complete(void *opaque, int ret)
-{
- CoroutineIOCompletion *co = opaque;
-
- co->ret = ret;
- qemu_coroutine_enter(co->coroutine, NULL);
-}
-
-static int coroutine_fn bdrv_co_io_em(BlockDriverState *bs, int64_t sector_num,
- int nb_sectors, QEMUIOVector *iov,
- bool is_write)
-{
- CoroutineIOCompletion co = {
- .coroutine = qemu_coroutine_self(),
- };
- BlockAIOCB *acb;
-
- if (is_write) {
- acb = bs->drv->bdrv_aio_writev(bs, sector_num, iov, nb_sectors,
- bdrv_co_io_em_complete, &co);
- } else {
- acb = bs->drv->bdrv_aio_readv(bs, sector_num, iov, nb_sectors,
- bdrv_co_io_em_complete, &co);
- }
-
- trace_bdrv_co_io_em(bs, sector_num, nb_sectors, is_write, acb);
- if (!acb) {
- return -EIO;
- }
- qemu_coroutine_yield();
-
- return co.ret;
-}
-
-static int coroutine_fn bdrv_co_readv_em(BlockDriverState *bs,
- int64_t sector_num, int nb_sectors,
- QEMUIOVector *iov)
-{
- return bdrv_co_io_em(bs, sector_num, nb_sectors, iov, false);
-}
-
-static int coroutine_fn bdrv_co_writev_em(BlockDriverState *bs,
- int64_t sector_num, int nb_sectors,
- QEMUIOVector *iov)
-{
- return bdrv_co_io_em(bs, sector_num, nb_sectors, iov, true);
-}
-
static void coroutine_fn bdrv_flush_co_entry(void *opaque)
{
RwCo *rwco = opaque;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH 04/17] block: Rename bdrv_co_do_preadv/writev to bdrv_co_preadv/writev
2016-04-27 9:52 [Qemu-devel] [PATCH 00/17] block: Convert .bdrv_read/write to .bdrv_preadv/pwritev Kevin Wolf
` (2 preceding siblings ...)
2016-04-27 9:52 ` [Qemu-devel] [PATCH 03/17] block: Support AIO drivers in bdrv_driver_preadv/pwritev() Kevin Wolf
@ 2016-04-27 9:52 ` Kevin Wolf
2016-04-27 14:34 ` Eric Blake
2016-04-27 9:52 ` [Qemu-devel] [PATCH 05/17] block: Introduce .bdrv_co_preadv/pwritev BlockDriver function Kevin Wolf
` (13 subsequent siblings)
17 siblings, 1 reply; 34+ messages in thread
From: Kevin Wolf @ 2016-04-27 9:52 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, stefanha, famz, sw, qemu-devel
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(-)
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);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH 05/17] block: Introduce .bdrv_co_preadv/pwritev BlockDriver function
2016-04-27 9:52 [Qemu-devel] [PATCH 00/17] block: Convert .bdrv_read/write to .bdrv_preadv/pwritev Kevin Wolf
` (3 preceding siblings ...)
2016-04-27 9:52 ` [Qemu-devel] [PATCH 04/17] block: Rename bdrv_co_do_preadv/writev to bdrv_co_preadv/writev Kevin Wolf
@ 2016-04-27 9:52 ` Kevin Wolf
2016-04-27 15:44 ` Eric Blake
2016-04-27 9:52 ` [Qemu-devel] [PATCH 06/17] bochs: Implement .bdrv_co_preadv() interface Kevin Wolf
` (12 subsequent siblings)
17 siblings, 1 reply; 34+ messages in thread
From: Kevin Wolf @ 2016-04-27 9:52 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, stefanha, famz, sw, qemu-devel
Many parts of the block layer are already byte granularity. The block
driver interface, however, was still missing an interface that allows
making use of this. This patch introduces a new BlockDriver interface,
which is based on coroutines, vectored, has flags and uses a byte
granularity. This is now the preferred interface for new drivers.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/io.c | 28 ++++++++++++++++++++++------
include/block/block_int.h | 4 ++++
2 files changed, 26 insertions(+), 6 deletions(-)
diff --git a/block/io.c b/block/io.c
index 11ff421..4aac4ab 100644
--- a/block/io.c
+++ b/block/io.c
@@ -806,8 +806,15 @@ static int coroutine_fn bdrv_driver_preadv(BlockDriverState *bs,
QEMUIOVector *qiov, int flags)
{
BlockDriver *drv = bs->drv;
- int64_t sector_num = offset >> BDRV_SECTOR_BITS;
- unsigned int nb_sectors = bytes >> BDRV_SECTOR_BITS;
+ int64_t sector_num;
+ unsigned int nb_sectors;
+
+ if (drv->bdrv_co_preadv) {
+ return drv->bdrv_co_preadv(bs, offset, bytes, qiov, flags);
+ }
+
+ sector_num = offset >> BDRV_SECTOR_BITS;
+ nb_sectors = bytes >> BDRV_SECTOR_BITS;
assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0);
assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0);
@@ -837,10 +844,18 @@ static int coroutine_fn bdrv_driver_pwritev(BlockDriverState *bs,
QEMUIOVector *qiov, int flags)
{
BlockDriver *drv = bs->drv;
- int64_t sector_num = offset >> BDRV_SECTOR_BITS;
- unsigned int nb_sectors = bytes >> BDRV_SECTOR_BITS;
+ int64_t sector_num;
+ unsigned int nb_sectors;
int ret;
+ if (drv->bdrv_co_pwritev) {
+ ret = drv->bdrv_co_pwritev(bs, offset, bytes, qiov, flags);
+ goto emulate_flags;
+ }
+
+ sector_num = offset >> BDRV_SECTOR_BITS;
+ nb_sectors = bytes >> BDRV_SECTOR_BITS;
+
assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0);
assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0);
assert((bytes >> BDRV_SECTOR_BITS) <= BDRV_REQUEST_MAX_SECTORS);
@@ -860,13 +875,14 @@ static int coroutine_fn bdrv_driver_pwritev(BlockDriverState *bs,
acb = bs->drv->bdrv_aio_writev(bs, sector_num, qiov, nb_sectors,
bdrv_co_io_em_complete, &co);
if (acb == NULL) {
- return -EIO;
+ ret = -EIO;
} else {
qemu_coroutine_yield();
- return co.ret;
+ ret = co.ret;
}
}
+emulate_flags:
if (ret == 0 && (flags & BDRV_REQ_FUA) &&
!(drv->supported_write_flags & BDRV_REQ_FUA))
{
diff --git a/include/block/block_int.h b/include/block/block_int.h
index f0171e3..0ef6f2c 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -153,10 +153,14 @@ struct BlockDriver {
int coroutine_fn (*bdrv_co_readv)(BlockDriverState *bs,
int64_t sector_num, int nb_sectors, QEMUIOVector *qiov);
+ 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);
+ int coroutine_fn (*bdrv_co_pwritev)(BlockDriverState *bs,
+ uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags);
int supported_write_flags;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH 06/17] bochs: Implement .bdrv_co_preadv() interface
2016-04-27 9:52 [Qemu-devel] [PATCH 00/17] block: Convert .bdrv_read/write to .bdrv_preadv/pwritev Kevin Wolf
` (4 preceding siblings ...)
2016-04-27 9:52 ` [Qemu-devel] [PATCH 05/17] block: Introduce .bdrv_co_preadv/pwritev BlockDriver function Kevin Wolf
@ 2016-04-27 9:52 ` Kevin Wolf
2016-04-27 14:06 ` Stefan Hajnoczi
2016-04-27 15:51 ` Eric Blake
2016-04-27 9:52 ` [Qemu-devel] [PATCH 07/17] cloop: " Kevin Wolf
` (11 subsequent siblings)
17 siblings, 2 replies; 34+ messages in thread
From: Kevin Wolf @ 2016-04-27 9:52 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, stefanha, famz, sw, qemu-devel
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/bochs.c | 46 +++++++++++++++++++++++++++++-----------------
1 file changed, 29 insertions(+), 17 deletions(-)
diff --git a/block/bochs.c b/block/bochs.c
index af8b7ab..d148454 100644
--- a/block/bochs.c
+++ b/block/bochs.c
@@ -104,6 +104,7 @@ static int bochs_open(BlockDriverState *bs, QDict *options, int flags,
int ret;
bs->read_only = 1; // no write support yet
+ bs->request_alignment = BDRV_SECTOR_SIZE; /* No sub-sector I/O supported */
ret = bdrv_pread(bs->file->bs, 0, &bochs, sizeof(bochs));
if (ret < 0) {
@@ -221,39 +222,50 @@ static int64_t seek_to_sector(BlockDriverState *bs, int64_t sector_num)
return bitmap_offset + (512 * (s->bitmap_blocks + extent_offset));
}
-static int bochs_read(BlockDriverState *bs, int64_t sector_num,
- uint8_t *buf, int nb_sectors)
+static int coroutine_fn
+bochs_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
+ QEMUIOVector *qiov, int flags)
{
+ BDRVBochsState *s = bs->opaque;
+ uint64_t sector_num = offset >> BDRV_SECTOR_BITS;
+ int nb_sectors = bytes >> BDRV_SECTOR_BITS;
+ uint64_t bytes_done = 0;
+ QEMUIOVector local_qiov;
int ret;
+ assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0);
+ assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0);
+
+ qemu_iovec_init(&local_qiov, qiov->niov);
+ qemu_co_mutex_lock(&s->lock);
+
while (nb_sectors > 0) {
int64_t block_offset = seek_to_sector(bs, sector_num);
if (block_offset < 0) {
return block_offset;
- } else if (block_offset > 0) {
- ret = bdrv_pread(bs->file->bs, block_offset, buf, 512);
+ }
+
+ qemu_iovec_reset(&local_qiov);
+ qemu_iovec_concat(&local_qiov, qiov, bytes_done, 512);
+
+ if (block_offset > 0) {
+ ret = bdrv_co_preadv(bs->file->bs, block_offset, 512,
+ &local_qiov, 0);
if (ret < 0) {
return ret;
}
} else {
- memset(buf, 0, 512);
+ qemu_iovec_memset(&local_qiov, 0, 0, 512);
}
nb_sectors--;
sector_num++;
- buf += 512;
+ bytes_done += 512;
}
- return 0;
-}
-static coroutine_fn int bochs_co_read(BlockDriverState *bs, int64_t sector_num,
- uint8_t *buf, int nb_sectors)
-{
- int ret;
- BDRVBochsState *s = bs->opaque;
- qemu_co_mutex_lock(&s->lock);
- ret = bochs_read(bs, sector_num, buf, nb_sectors);
qemu_co_mutex_unlock(&s->lock);
- return ret;
+ qemu_iovec_destroy(&local_qiov);
+
+ return 0;
}
static void bochs_close(BlockDriverState *bs)
@@ -267,7 +279,7 @@ static BlockDriver bdrv_bochs = {
.instance_size = sizeof(BDRVBochsState),
.bdrv_probe = bochs_probe,
.bdrv_open = bochs_open,
- .bdrv_read = bochs_co_read,
+ .bdrv_co_preadv = bochs_co_preadv,
.bdrv_close = bochs_close,
};
--
1.8.3.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH 07/17] cloop: Implement .bdrv_co_preadv() interface
2016-04-27 9:52 [Qemu-devel] [PATCH 00/17] block: Convert .bdrv_read/write to .bdrv_preadv/pwritev Kevin Wolf
` (5 preceding siblings ...)
2016-04-27 9:52 ` [Qemu-devel] [PATCH 06/17] bochs: Implement .bdrv_co_preadv() interface Kevin Wolf
@ 2016-04-27 9:52 ` Kevin Wolf
2016-04-27 14:12 ` Stefan Hajnoczi
2016-04-27 9:52 ` [Qemu-devel] [PATCH 08/17] dmg: " Kevin Wolf
` (10 subsequent siblings)
17 siblings, 1 reply; 34+ messages in thread
From: Kevin Wolf @ 2016-04-27 9:52 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, stefanha, famz, sw, qemu-devel
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/cloop.c | 33 ++++++++++++++++++---------------
1 file changed, 18 insertions(+), 15 deletions(-)
diff --git a/block/cloop.c b/block/cloop.c
index a84f140..7289a80 100644
--- a/block/cloop.c
+++ b/block/cloop.c
@@ -66,6 +66,7 @@ static int cloop_open(BlockDriverState *bs, QDict *options, int flags,
int ret;
bs->read_only = 1;
+ bs->request_alignment = BDRV_SECTOR_SIZE; /* No sub-sector I/O supported */
/* read header */
ret = bdrv_pread(bs->file->bs, 128, &s->block_size, 4);
@@ -229,34 +230,36 @@ static inline int cloop_read_block(BlockDriverState *bs, int block_num)
return 0;
}
-static int cloop_read(BlockDriverState *bs, int64_t sector_num,
- uint8_t *buf, int nb_sectors)
+static int coroutine_fn
+cloop_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
+ QEMUIOVector *qiov, int flags)
{
BDRVCloopState *s = bs->opaque;
+ uint64_t sector_num = offset >> BDRV_SECTOR_BITS;
+ int nb_sectors = bytes >> BDRV_SECTOR_BITS;
int i;
+ assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0);
+ assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0);
+
+ qemu_co_mutex_lock(&s->lock);
+
for (i = 0; i < nb_sectors; i++) {
+ void *data;
uint32_t sector_offset_in_block =
((sector_num + i) % s->sectors_per_block),
block_num = (sector_num + i) / s->sectors_per_block;
if (cloop_read_block(bs, block_num) != 0) {
return -1;
}
- memcpy(buf + i * 512,
- s->uncompressed_block + sector_offset_in_block * 512, 512);
+
+ data = s->uncompressed_block + sector_offset_in_block * 512;
+ qemu_iovec_from_buf(qiov, i * 512, data, 512);
}
- return 0;
-}
-static coroutine_fn int cloop_co_read(BlockDriverState *bs, int64_t sector_num,
- uint8_t *buf, int nb_sectors)
-{
- int ret;
- BDRVCloopState *s = bs->opaque;
- qemu_co_mutex_lock(&s->lock);
- ret = cloop_read(bs, sector_num, buf, nb_sectors);
qemu_co_mutex_unlock(&s->lock);
- return ret;
+
+ return 0;
}
static void cloop_close(BlockDriverState *bs)
@@ -273,7 +276,7 @@ static BlockDriver bdrv_cloop = {
.instance_size = sizeof(BDRVCloopState),
.bdrv_probe = cloop_probe,
.bdrv_open = cloop_open,
- .bdrv_read = cloop_co_read,
+ .bdrv_co_preadv = cloop_co_preadv,
.bdrv_close = cloop_close,
};
--
1.8.3.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH 08/17] dmg: Implement .bdrv_co_preadv() interface
2016-04-27 9:52 [Qemu-devel] [PATCH 00/17] block: Convert .bdrv_read/write to .bdrv_preadv/pwritev Kevin Wolf
` (6 preceding siblings ...)
2016-04-27 9:52 ` [Qemu-devel] [PATCH 07/17] cloop: " Kevin Wolf
@ 2016-04-27 9:52 ` Kevin Wolf
2016-04-27 9:52 ` [Qemu-devel] [PATCH 09/17] vdi: " Kevin Wolf
` (9 subsequent siblings)
17 siblings, 0 replies; 34+ messages in thread
From: Kevin Wolf @ 2016-04-27 9:52 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, stefanha, famz, sw, qemu-devel
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/dmg.c | 36 ++++++++++++++++++++----------------
1 file changed, 20 insertions(+), 16 deletions(-)
diff --git a/block/dmg.c b/block/dmg.c
index a496eb7..ca37c92 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -440,6 +440,8 @@ static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
int ret;
bs->read_only = 1;
+ bs->request_alignment = BDRV_SECTOR_SIZE; /* No sub-sector I/O supported */
+
s->n_chunks = 0;
s->offsets = s->lengths = s->sectors = s->sectorcounts = NULL;
/* used by dmg_read_mish_block to keep track of the current I/O position */
@@ -659,14 +661,24 @@ static inline int dmg_read_chunk(BlockDriverState *bs, uint64_t sector_num)
return 0;
}
-static int dmg_read(BlockDriverState *bs, int64_t sector_num,
- uint8_t *buf, int nb_sectors)
+static int coroutine_fn
+dmg_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
+ QEMUIOVector *qiov, int flags)
{
BDRVDMGState *s = bs->opaque;
+ uint64_t sector_num = offset >> BDRV_SECTOR_BITS;
+ int nb_sectors = bytes >> BDRV_SECTOR_BITS;
int i;
+ assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0);
+ assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0);
+
+ qemu_co_mutex_lock(&s->lock);
+
for (i = 0; i < nb_sectors; i++) {
uint32_t sector_offset_in_chunk;
+ void *data;
+
if (dmg_read_chunk(bs, sector_num + i) != 0) {
return -1;
}
@@ -674,25 +686,17 @@ static int dmg_read(BlockDriverState *bs, int64_t sector_num,
* s->uncompressed_chunk may be too small to cover the large all-zeroes
* section. dmg_read_chunk is called to find s->current_chunk */
if (s->types[s->current_chunk] == 2) { /* all zeroes block entry */
- memset(buf + i * 512, 0, 512);
+ qemu_iovec_memset(qiov, i * 512, 0, 512);
continue;
}
sector_offset_in_chunk = sector_num + i - s->sectors[s->current_chunk];
- memcpy(buf + i * 512,
- s->uncompressed_chunk + sector_offset_in_chunk * 512, 512);
+ data = s->uncompressed_chunk + sector_offset_in_chunk * 512;
+ qemu_iovec_from_buf(qiov, i * 512, data, 512);
}
- return 0;
-}
-static coroutine_fn int dmg_co_read(BlockDriverState *bs, int64_t sector_num,
- uint8_t *buf, int nb_sectors)
-{
- int ret;
- BDRVDMGState *s = bs->opaque;
- qemu_co_mutex_lock(&s->lock);
- ret = dmg_read(bs, sector_num, buf, nb_sectors);
qemu_co_mutex_unlock(&s->lock);
- return ret;
+
+ return 0;
}
static void dmg_close(BlockDriverState *bs)
@@ -715,7 +719,7 @@ static BlockDriver bdrv_dmg = {
.instance_size = sizeof(BDRVDMGState),
.bdrv_probe = dmg_probe,
.bdrv_open = dmg_open,
- .bdrv_read = dmg_co_read,
+ .bdrv_co_preadv = dmg_co_preadv,
.bdrv_close = dmg_close,
};
--
1.8.3.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH 09/17] vdi: Implement .bdrv_co_preadv() interface
2016-04-27 9:52 [Qemu-devel] [PATCH 00/17] block: Convert .bdrv_read/write to .bdrv_preadv/pwritev Kevin Wolf
` (7 preceding siblings ...)
2016-04-27 9:52 ` [Qemu-devel] [PATCH 08/17] dmg: " Kevin Wolf
@ 2016-04-27 9:52 ` Kevin Wolf
2016-04-27 9:52 ` [Qemu-devel] [PATCH 10/17] vdi: Implement .bdrv_co_pwritev() interface Kevin Wolf
` (8 subsequent siblings)
17 siblings, 0 replies; 34+ messages in thread
From: Kevin Wolf @ 2016-04-27 9:52 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, stefanha, famz, sw, qemu-devel
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/vdi.c | 55 ++++++++++++++++++++++++++++++++-----------------------
1 file changed, 32 insertions(+), 23 deletions(-)
diff --git a/block/vdi.c b/block/vdi.c
index 75d4819..8295511 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -557,48 +557,57 @@ static int64_t coroutine_fn vdi_co_get_block_status(BlockDriverState *bs,
return BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID | offset;
}
-static int vdi_co_read(BlockDriverState *bs,
- int64_t sector_num, uint8_t *buf, int nb_sectors)
+static int coroutine_fn
+vdi_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
+ QEMUIOVector *qiov, int flags)
{
BDRVVdiState *s = bs->opaque;
+ QEMUIOVector local_qiov;
uint32_t bmap_entry;
uint32_t block_index;
- uint32_t sector_in_block;
- uint32_t n_sectors;
+ uint32_t offset_in_block;
+ uint32_t n_bytes;
+ uint64_t bytes_done = 0;
int ret = 0;
logout("\n");
- while (ret >= 0 && nb_sectors > 0) {
- block_index = sector_num / s->block_sectors;
- sector_in_block = sector_num % s->block_sectors;
- n_sectors = s->block_sectors - sector_in_block;
- if (n_sectors > nb_sectors) {
- n_sectors = nb_sectors;
- }
+ qemu_iovec_init(&local_qiov, qiov->niov);
- logout("will read %u sectors starting at sector %" PRIu64 "\n",
- n_sectors, sector_num);
+ while (ret >= 0 && bytes > 0) {
+ block_index = offset / s->block_size;
+ offset_in_block = offset % s->block_size;
+ n_bytes = MIN(bytes, s->block_size - offset_in_block);
+
+ logout("will read %u bytes starting at offset %" PRIu64 "\n",
+ n_bytes, offset);
/* prepare next AIO request */
bmap_entry = le32_to_cpu(s->bmap[block_index]);
if (!VDI_IS_ALLOCATED(bmap_entry)) {
/* Block not allocated, return zeros, no need to wait. */
- memset(buf, 0, n_sectors * SECTOR_SIZE);
+ qemu_iovec_memset(qiov, bytes_done, 0, n_bytes);
ret = 0;
} else {
- uint64_t offset = s->header.offset_data / SECTOR_SIZE +
- (uint64_t)bmap_entry * s->block_sectors +
- sector_in_block;
- ret = bdrv_read(bs->file->bs, offset, buf, n_sectors);
+ uint64_t data_offset = s->header.offset_data +
+ (uint64_t)bmap_entry * s->block_size +
+ offset_in_block;
+
+ qemu_iovec_reset(&local_qiov);
+ qemu_iovec_concat(&local_qiov, qiov, bytes_done, n_bytes);
+
+ ret = bdrv_co_preadv(bs->file->bs, data_offset, n_bytes,
+ &local_qiov, 0);
}
- logout("%u sectors read\n", n_sectors);
+ logout("%u bytes read\n", n_bytes);
- nb_sectors -= n_sectors;
- sector_num += n_sectors;
- buf += n_sectors * SECTOR_SIZE;
+ bytes -= n_bytes;
+ offset += n_bytes;
+ bytes_done += n_bytes;
}
+ qemu_iovec_destroy(&local_qiov);
+
return ret;
}
@@ -903,7 +912,7 @@ static BlockDriver bdrv_vdi = {
.bdrv_co_get_block_status = vdi_co_get_block_status,
.bdrv_make_empty = vdi_make_empty,
- .bdrv_read = vdi_co_read,
+ .bdrv_co_preadv = vdi_co_preadv,
#if defined(CONFIG_VDI_WRITE)
.bdrv_write = vdi_co_write,
#endif
--
1.8.3.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH 10/17] vdi: Implement .bdrv_co_pwritev() interface
2016-04-27 9:52 [Qemu-devel] [PATCH 00/17] block: Convert .bdrv_read/write to .bdrv_preadv/pwritev Kevin Wolf
` (8 preceding siblings ...)
2016-04-27 9:52 ` [Qemu-devel] [PATCH 09/17] vdi: " Kevin Wolf
@ 2016-04-27 9:52 ` Kevin Wolf
2016-04-27 14:17 ` Stefan Hajnoczi
2016-04-27 9:52 ` [Qemu-devel] [PATCH 11/17] vmdk: Add vmdk_find_offset_in_cluster() Kevin Wolf
` (7 subsequent siblings)
17 siblings, 1 reply; 34+ messages in thread
From: Kevin Wolf @ 2016-04-27 9:52 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, stefanha, famz, sw, qemu-devel
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/vdi.c | 72 +++++++++++++++++++++++++++++++++++--------------------------
1 file changed, 41 insertions(+), 31 deletions(-)
diff --git a/block/vdi.c b/block/vdi.c
index 8295511..e5fe4e8 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -611,53 +611,55 @@ vdi_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
return ret;
}
-static int vdi_co_write(BlockDriverState *bs,
- int64_t sector_num, const uint8_t *buf, int nb_sectors)
+static int coroutine_fn
+vdi_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
+ QEMUIOVector *qiov, int flags)
{
BDRVVdiState *s = bs->opaque;
+ QEMUIOVector local_qiov;
uint32_t bmap_entry;
uint32_t block_index;
- uint32_t sector_in_block;
- uint32_t n_sectors;
+ uint32_t offset_in_block;
+ uint32_t n_bytes;
uint32_t bmap_first = VDI_UNALLOCATED;
uint32_t bmap_last = VDI_UNALLOCATED;
uint8_t *block = NULL;
+ uint64_t bytes_done = 0;
int ret = 0;
logout("\n");
- while (ret >= 0 && nb_sectors > 0) {
- block_index = sector_num / s->block_sectors;
- sector_in_block = sector_num % s->block_sectors;
- n_sectors = s->block_sectors - sector_in_block;
- if (n_sectors > nb_sectors) {
- n_sectors = nb_sectors;
- }
+ qemu_iovec_init(&local_qiov, qiov->niov);
- logout("will write %u sectors starting at sector %" PRIu64 "\n",
- n_sectors, sector_num);
+ while (ret >= 0 && bytes > 0) {
+ block_index = offset / s->block_size;
+ offset_in_block = offset % s->block_size;
+ n_bytes = MIN(bytes, s->block_size - offset_in_block);
+
+ logout("will write %u bytes starting at offset %" PRIu64 "\n",
+ n_bytes, offset);
/* prepare next AIO request */
bmap_entry = le32_to_cpu(s->bmap[block_index]);
if (!VDI_IS_ALLOCATED(bmap_entry)) {
/* Allocate new block and write to it. */
- uint64_t offset;
+ uint64_t data_offset;
bmap_entry = s->header.blocks_allocated;
s->bmap[block_index] = cpu_to_le32(bmap_entry);
s->header.blocks_allocated++;
- offset = s->header.offset_data / SECTOR_SIZE +
- (uint64_t)bmap_entry * s->block_sectors;
+ data_offset = s->header.offset_data +
+ (uint64_t)bmap_entry * s->block_size;
if (block == NULL) {
block = g_malloc(s->block_size);
bmap_first = block_index;
}
bmap_last = block_index;
/* Copy data to be written to new block and zero unused parts. */
- memset(block, 0, sector_in_block * SECTOR_SIZE);
- memcpy(block + sector_in_block * SECTOR_SIZE,
- buf, n_sectors * SECTOR_SIZE);
- memset(block + (sector_in_block + n_sectors) * SECTOR_SIZE, 0,
- (s->block_sectors - n_sectors - sector_in_block) * SECTOR_SIZE);
+ memset(block, 0, offset_in_block);
+ qemu_iovec_to_buf(qiov, bytes_done, block + offset_in_block,
+ n_bytes);
+ memset(block + offset_in_block + n_bytes, 0,
+ s->block_size - n_bytes - offset_in_block);
/* Note that this coroutine does not yield anywhere from reading the
* bmap entry until here, so in regards to all the coroutines trying
@@ -667,12 +669,12 @@ static int vdi_co_write(BlockDriverState *bs,
* acquire the lock and thus the padded cluster is written before
* the other coroutines can write to the affected area. */
qemu_co_mutex_lock(&s->write_lock);
- ret = bdrv_write(bs->file->bs, offset, block, s->block_sectors);
+ ret = bdrv_pwrite(bs->file->bs, data_offset, block, s->block_size);
qemu_co_mutex_unlock(&s->write_lock);
} else {
- uint64_t offset = s->header.offset_data / SECTOR_SIZE +
- (uint64_t)bmap_entry * s->block_sectors +
- sector_in_block;
+ uint64_t data_offset = s->header.offset_data +
+ (uint64_t)bmap_entry * s->block_size +
+ offset_in_block;
qemu_co_mutex_lock(&s->write_lock);
/* This lock is only used to make sure the following write operation
* is executed after the write issued by the coroutine allocating
@@ -683,16 +685,23 @@ static int vdi_co_write(BlockDriverState *bs,
* that that write operation has returned (there may be other writes
* in flight, but they do not concern this very operation). */
qemu_co_mutex_unlock(&s->write_lock);
- ret = bdrv_write(bs->file->bs, offset, buf, n_sectors);
+
+ qemu_iovec_reset(&local_qiov);
+ qemu_iovec_concat(&local_qiov, qiov, bytes_done, n_bytes);
+
+ ret = bdrv_co_pwritev(bs->file->bs, data_offset, n_bytes,
+ &local_qiov, 0);
}
- nb_sectors -= n_sectors;
- sector_num += n_sectors;
- buf += n_sectors * SECTOR_SIZE;
+ bytes -= n_bytes;
+ offset += n_bytes;
+ bytes_done += n_bytes;
- logout("%u sectors written\n", n_sectors);
+ logout("%u bytes written\n", n_bytes);
}
+ qemu_iovec_destroy(&local_qiov);
+
logout("finished data write\n");
if (ret < 0) {
return ret;
@@ -703,6 +712,7 @@ static int vdi_co_write(BlockDriverState *bs,
VdiHeader *header = (VdiHeader *) block;
uint8_t *base;
uint64_t offset;
+ uint32_t n_sectors;
logout("now writing modified header\n");
assert(VDI_IS_ALLOCATED(bmap_first));
@@ -914,7 +924,7 @@ static BlockDriver bdrv_vdi = {
.bdrv_co_preadv = vdi_co_preadv,
#if defined(CONFIG_VDI_WRITE)
- .bdrv_write = vdi_co_write,
+ .bdrv_co_pwritev = vdi_co_pwritev,
#endif
.bdrv_get_info = vdi_get_info,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH 11/17] vmdk: Add vmdk_find_offset_in_cluster()
2016-04-27 9:52 [Qemu-devel] [PATCH 00/17] block: Convert .bdrv_read/write to .bdrv_preadv/pwritev Kevin Wolf
` (9 preceding siblings ...)
2016-04-27 9:52 ` [Qemu-devel] [PATCH 10/17] vdi: Implement .bdrv_co_pwritev() interface Kevin Wolf
@ 2016-04-27 9:52 ` Kevin Wolf
2016-04-27 9:52 ` [Qemu-devel] [PATCH 12/17] vmdk: Implement .bdrv_co_preadv() interface Kevin Wolf
` (6 subsequent siblings)
17 siblings, 0 replies; 34+ messages in thread
From: Kevin Wolf @ 2016-04-27 9:52 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, stefanha, famz, sw, qemu-devel
This is a byte granularity version of vmdk_find_index_in_cluster().
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/vmdk.c | 23 +++++++++++++++++------
1 file changed, 17 insertions(+), 6 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index 45f9d3c..f1e01f9 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1259,15 +1259,26 @@ static VmdkExtent *find_extent(BDRVVmdkState *s,
return NULL;
}
+static inline uint64_t vmdk_find_offset_in_cluster(VmdkExtent *extent,
+ int64_t offset)
+{
+ uint64_t offset_in_cluster, extent_begin_offset, extent_relative_offset;
+ uint64_t cluster_size = extent->cluster_sectors * BDRV_SECTOR_SIZE;
+
+ extent_begin_offset =
+ (extent->end_sector - extent->sectors) * BDRV_SECTOR_SIZE;
+ extent_relative_offset = offset - extent_begin_offset;
+ offset_in_cluster = extent_relative_offset % cluster_size;
+
+ return offset_in_cluster;
+}
+
static inline uint64_t vmdk_find_index_in_cluster(VmdkExtent *extent,
int64_t sector_num)
{
- uint64_t index_in_cluster, extent_begin_sector, extent_relative_sector_num;
-
- extent_begin_sector = extent->end_sector - extent->sectors;
- extent_relative_sector_num = sector_num - extent_begin_sector;
- index_in_cluster = extent_relative_sector_num % extent->cluster_sectors;
- return index_in_cluster;
+ uint64_t offset;
+ offset = vmdk_find_offset_in_cluster(extent, sector_num * BDRV_SECTOR_SIZE);
+ return offset / BDRV_SECTOR_SIZE;
}
static int64_t coroutine_fn vmdk_co_get_block_status(BlockDriverState *bs,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH 12/17] vmdk: Implement .bdrv_co_preadv() interface
2016-04-27 9:52 [Qemu-devel] [PATCH 00/17] block: Convert .bdrv_read/write to .bdrv_preadv/pwritev Kevin Wolf
` (10 preceding siblings ...)
2016-04-27 9:52 ` [Qemu-devel] [PATCH 11/17] vmdk: Add vmdk_find_offset_in_cluster() Kevin Wolf
@ 2016-04-27 9:52 ` Kevin Wolf
2016-04-27 9:52 ` [Qemu-devel] [PATCH 13/17] vmdk: Implement .bdrv_co_pwritev() interface Kevin Wolf
` (5 subsequent siblings)
17 siblings, 0 replies; 34+ messages in thread
From: Kevin Wolf @ 2016-04-27 9:52 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, stefanha, famz, sw, qemu-devel
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/vmdk.c | 88 +++++++++++++++++++++++++++++++-----------------------------
1 file changed, 46 insertions(+), 42 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index f1e01f9..d9b9b89 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1381,8 +1381,8 @@ static int vmdk_write_extent(VmdkExtent *extent, int64_t cluster_offset,
}
static int vmdk_read_extent(VmdkExtent *extent, int64_t cluster_offset,
- int64_t offset_in_cluster, uint8_t *buf,
- int nb_sectors)
+ int64_t offset_in_cluster, QEMUIOVector *qiov,
+ int bytes)
{
int ret;
int cluster_bytes, buf_bytes;
@@ -1394,14 +1394,13 @@ static int vmdk_read_extent(VmdkExtent *extent, int64_t cluster_offset,
if (!extent->compressed) {
- ret = bdrv_pread(extent->file->bs,
- cluster_offset + offset_in_cluster,
- buf, nb_sectors * 512);
- if (ret == nb_sectors * 512) {
- return 0;
- } else {
- return -EIO;
+ ret = bdrv_co_preadv(extent->file->bs,
+ cluster_offset + offset_in_cluster, bytes,
+ qiov, 0);
+ if (ret < 0) {
+ return ret;
}
+ return 0;
}
cluster_bytes = extent->cluster_sectors * 512;
/* Read two clusters in case GrainMarker + compressed data > one cluster */
@@ -1433,11 +1432,11 @@ static int vmdk_read_extent(VmdkExtent *extent, int64_t cluster_offset,
}
if (offset_in_cluster < 0 ||
- offset_in_cluster + nb_sectors * 512 > buf_len) {
+ offset_in_cluster + bytes > buf_len) {
ret = -EINVAL;
goto out;
}
- memcpy(buf, uncomp_buf + offset_in_cluster, nb_sectors * 512);
+ qemu_iovec_from_buf(qiov, 0, uncomp_buf + offset_in_cluster, bytes);
ret = 0;
out:
@@ -1446,65 +1445,70 @@ static int vmdk_read_extent(VmdkExtent *extent, int64_t cluster_offset,
return ret;
}
-static int vmdk_read(BlockDriverState *bs, int64_t sector_num,
- uint8_t *buf, int nb_sectors)
+static int coroutine_fn
+vmdk_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
+ QEMUIOVector *qiov, int flags)
{
BDRVVmdkState *s = bs->opaque;
int ret;
- uint64_t n, index_in_cluster;
+ uint64_t n_bytes, offset_in_cluster;
VmdkExtent *extent = NULL;
+ QEMUIOVector local_qiov;
uint64_t cluster_offset;
+ uint64_t bytes_done = 0;
- while (nb_sectors > 0) {
- extent = find_extent(s, sector_num, extent);
+ qemu_iovec_init(&local_qiov, qiov->niov);
+ qemu_co_mutex_lock(&s->lock);
+
+ while (bytes > 0) {
+ extent = find_extent(s, offset >> BDRV_SECTOR_BITS, extent);
if (!extent) {
return -EIO;
}
ret = get_cluster_offset(bs, extent, NULL,
- sector_num << 9, false, &cluster_offset,
- 0, 0);
- index_in_cluster = vmdk_find_index_in_cluster(extent, sector_num);
- n = extent->cluster_sectors - index_in_cluster;
- if (n > nb_sectors) {
- n = nb_sectors;
- }
+ offset, false, &cluster_offset, 0, 0);
+ offset_in_cluster = vmdk_find_offset_in_cluster(extent, offset);
+
+ n_bytes = MIN(bytes, extent->cluster_sectors * BDRV_SECTOR_SIZE
+ - offset_in_cluster);
+
if (ret != VMDK_OK) {
/* if not allocated, try to read from parent image, if exist */
if (bs->backing && ret != VMDK_ZEROED) {
if (!vmdk_is_cid_valid(bs)) {
return -EINVAL;
}
- ret = bdrv_read(bs->backing->bs, sector_num, buf, n);
+
+ qemu_iovec_reset(&local_qiov);
+ qemu_iovec_concat(&local_qiov, qiov, bytes_done, n_bytes);
+
+ ret = bdrv_co_preadv(bs->backing->bs, offset, n_bytes,
+ &local_qiov, 0);
if (ret < 0) {
return ret;
}
} else {
- memset(buf, 0, 512 * n);
+ qemu_iovec_memset(qiov, bytes_done, 0, n_bytes);
}
} else {
- ret = vmdk_read_extent(extent,
- cluster_offset, index_in_cluster * 512,
- buf, n);
+ qemu_iovec_reset(&local_qiov);
+ qemu_iovec_concat(&local_qiov, qiov, bytes_done, n_bytes);
+
+ ret = vmdk_read_extent(extent, cluster_offset, offset_in_cluster,
+ &local_qiov, n_bytes);
if (ret) {
return ret;
}
}
- nb_sectors -= n;
- sector_num += n;
- buf += n * 512;
+ bytes -= n_bytes;
+ offset += n_bytes;
+ bytes_done += n_bytes;
}
- return 0;
-}
-static coroutine_fn int vmdk_co_read(BlockDriverState *bs, int64_t sector_num,
- uint8_t *buf, int nb_sectors)
-{
- int ret;
- BDRVVmdkState *s = bs->opaque;
- qemu_co_mutex_lock(&s->lock);
- ret = vmdk_read(bs, sector_num, buf, nb_sectors);
qemu_co_mutex_unlock(&s->lock);
- return ret;
+ qemu_iovec_destroy(&local_qiov);
+
+ return 0;
}
/**
@@ -2332,7 +2336,7 @@ static BlockDriver bdrv_vmdk = {
.bdrv_open = vmdk_open,
.bdrv_check = vmdk_check,
.bdrv_reopen_prepare = vmdk_reopen_prepare,
- .bdrv_read = vmdk_co_read,
+ .bdrv_co_preadv = vmdk_co_preadv,
.bdrv_write = vmdk_co_write,
.bdrv_write_compressed = vmdk_write_compressed,
.bdrv_co_write_zeroes = vmdk_co_write_zeroes,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH 13/17] vmdk: Implement .bdrv_co_pwritev() interface
2016-04-27 9:52 [Qemu-devel] [PATCH 00/17] block: Convert .bdrv_read/write to .bdrv_preadv/pwritev Kevin Wolf
` (11 preceding siblings ...)
2016-04-27 9:52 ` [Qemu-devel] [PATCH 12/17] vmdk: Implement .bdrv_co_preadv() interface Kevin Wolf
@ 2016-04-27 9:52 ` Kevin Wolf
2016-04-27 14:21 ` Stefan Hajnoczi
2016-04-27 9:52 ` [Qemu-devel] [PATCH 14/17] vpc: Implement .bdrv_co_preadv() interface Kevin Wolf
` (4 subsequent siblings)
17 siblings, 1 reply; 34+ messages in thread
From: Kevin Wolf @ 2016-04-27 9:52 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, stefanha, famz, sw, qemu-devel
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/vmdk.c | 209 ++++++++++++++++++++++++++++++++++++++---------------------
1 file changed, 135 insertions(+), 74 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index d9b9b89..f177ffd 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1016,27 +1016,26 @@ static void vmdk_refresh_limits(BlockDriverState *bs, Error **errp)
*/
static int get_whole_cluster(BlockDriverState *bs,
VmdkExtent *extent,
- uint64_t cluster_sector_num,
- uint64_t sector_num,
- uint64_t skip_start_sector,
- uint64_t skip_end_sector)
+ uint64_t cluster_offset,
+ uint64_t offset,
+ uint64_t skip_start_bytes,
+ uint64_t skip_end_bytes)
{
int ret = VMDK_OK;
int64_t cluster_bytes;
uint8_t *whole_grain;
/* For COW, align request sector_num to cluster start */
- sector_num = QEMU_ALIGN_DOWN(sector_num, extent->cluster_sectors);
cluster_bytes = extent->cluster_sectors << BDRV_SECTOR_BITS;
+ offset = QEMU_ALIGN_DOWN(offset, cluster_bytes);
whole_grain = qemu_blockalign(bs, cluster_bytes);
if (!bs->backing) {
- memset(whole_grain, 0, skip_start_sector << BDRV_SECTOR_BITS);
- memset(whole_grain + (skip_end_sector << BDRV_SECTOR_BITS), 0,
- cluster_bytes - (skip_end_sector << BDRV_SECTOR_BITS));
+ memset(whole_grain, 0, skip_start_bytes);
+ memset(whole_grain + skip_end_bytes, 0, cluster_bytes - skip_end_bytes);
}
- assert(skip_end_sector <= extent->cluster_sectors);
+ assert(skip_end_bytes <= cluster_bytes);
/* we will be here if it's first write on non-exist grain(cluster).
* try to read from parent image, if exist */
if (bs->backing && !vmdk_is_cid_valid(bs)) {
@@ -1045,42 +1044,43 @@ static int get_whole_cluster(BlockDriverState *bs,
}
/* Read backing data before skip range */
- if (skip_start_sector > 0) {
+ if (skip_start_bytes > 0) {
if (bs->backing) {
- ret = bdrv_read(bs->backing->bs, sector_num,
- whole_grain, skip_start_sector);
+ ret = bdrv_pread(bs->backing->bs, offset, whole_grain,
+ skip_start_bytes);
if (ret < 0) {
ret = VMDK_ERROR;
goto exit;
}
}
- ret = bdrv_write(extent->file->bs, cluster_sector_num, whole_grain,
- skip_start_sector);
+ ret = bdrv_pwrite(extent->file->bs, cluster_offset, whole_grain,
+ skip_start_bytes);
if (ret < 0) {
ret = VMDK_ERROR;
goto exit;
}
}
/* Read backing data after skip range */
- if (skip_end_sector < extent->cluster_sectors) {
+ if (skip_end_bytes < cluster_bytes) {
if (bs->backing) {
- ret = bdrv_read(bs->backing->bs, sector_num + skip_end_sector,
- whole_grain + (skip_end_sector << BDRV_SECTOR_BITS),
- extent->cluster_sectors - skip_end_sector);
+ ret = bdrv_pread(bs->backing->bs, offset + skip_end_bytes,
+ whole_grain + skip_end_bytes,
+ cluster_bytes - skip_end_bytes);
if (ret < 0) {
ret = VMDK_ERROR;
goto exit;
}
}
- ret = bdrv_write(extent->file->bs, cluster_sector_num + skip_end_sector,
- whole_grain + (skip_end_sector << BDRV_SECTOR_BITS),
- extent->cluster_sectors - skip_end_sector);
+ ret = bdrv_pwrite(extent->file->bs, cluster_offset + skip_end_bytes,
+ whole_grain + skip_end_bytes,
+ cluster_bytes - skip_end_bytes);
if (ret < 0) {
ret = VMDK_ERROR;
goto exit;
}
}
+ ret = VMDK_OK;
exit:
qemu_vfree(whole_grain);
return ret;
@@ -1142,8 +1142,8 @@ static int get_cluster_offset(BlockDriverState *bs,
uint64_t offset,
bool allocate,
uint64_t *cluster_offset,
- uint64_t skip_start_sector,
- uint64_t skip_end_sector)
+ uint64_t skip_start_bytes,
+ uint64_t skip_end_bytes)
{
unsigned int l1_index, l2_offset, l2_index;
int min_index, i, j;
@@ -1230,10 +1230,8 @@ static int get_cluster_offset(BlockDriverState *bs,
* This problem may occur because of insufficient space on host disk
* or inappropriate VM shutdown.
*/
- ret = get_whole_cluster(bs, extent,
- cluster_sector,
- offset >> BDRV_SECTOR_BITS,
- skip_start_sector, skip_end_sector);
+ ret = get_whole_cluster(bs, extent, cluster_sector * BDRV_SECTOR_SIZE,
+ offset, skip_start_bytes, skip_end_bytes);
if (ret) {
return ret;
}
@@ -1330,38 +1328,57 @@ static int64_t coroutine_fn vmdk_co_get_block_status(BlockDriverState *bs,
}
static int vmdk_write_extent(VmdkExtent *extent, int64_t cluster_offset,
- int64_t offset_in_cluster, const uint8_t *buf,
- int nb_sectors, int64_t sector_num)
+ int64_t offset_in_cluster, QEMUIOVector *qiov,
+ uint64_t qiov_offset, uint64_t n_bytes,
+ uint64_t offset)
{
int ret;
VmdkGrainMarker *data = NULL;
uLongf buf_len;
- const uint8_t *write_buf = buf;
- int write_len = nb_sectors * 512;
+ QEMUIOVector local_qiov;
+ struct iovec iov;
int64_t write_offset;
int64_t write_end_sector;
if (extent->compressed) {
+ void *compressed_data;
+
if (!extent->has_marker) {
ret = -EINVAL;
goto out;
}
buf_len = (extent->cluster_sectors << 9) * 2;
data = g_malloc(buf_len + sizeof(VmdkGrainMarker));
- if (compress(data->data, &buf_len, buf, nb_sectors << 9) != Z_OK ||
- buf_len == 0) {
+
+ compressed_data = g_malloc(n_bytes);
+ qemu_iovec_to_buf(qiov, qiov_offset, compressed_data, n_bytes);
+ ret = compress(data->data, &buf_len, compressed_data, n_bytes);
+ free(compressed_data);
+
+ if (ret != Z_OK || buf_len == 0) {
ret = -EINVAL;
goto out;
}
- data->lba = sector_num;
+
+ data->lba = offset >> BDRV_SECTOR_BITS;
data->size = buf_len;
- write_buf = (uint8_t *)data;
- write_len = buf_len + sizeof(VmdkGrainMarker);
+
+ n_bytes = buf_len + sizeof(VmdkGrainMarker);
+ iov = (struct iovec) {
+ .iov_base = data,
+ .iov_len = n_bytes,
+ };
+ qemu_iovec_init_external(&local_qiov, &iov, 1);
+ } else {
+ qemu_iovec_init(&local_qiov, qiov->niov);
+ qemu_iovec_concat(&local_qiov, qiov, qiov_offset, n_bytes);
}
+
write_offset = cluster_offset + offset_in_cluster,
- ret = bdrv_pwrite(extent->file->bs, write_offset, write_buf, write_len);
+ ret = bdrv_co_pwritev(extent->file->bs, write_offset, n_bytes,
+ &local_qiov, 0);
- write_end_sector = DIV_ROUND_UP(write_offset + write_len, BDRV_SECTOR_SIZE);
+ write_end_sector = DIV_ROUND_UP(write_offset + n_bytes, BDRV_SECTOR_SIZE);
if (extent->compressed) {
extent->next_cluster_sector = write_end_sector;
@@ -1370,13 +1387,15 @@ static int vmdk_write_extent(VmdkExtent *extent, int64_t cluster_offset,
write_end_sector);
}
- if (ret != write_len) {
- ret = ret < 0 ? ret : -EIO;
+ if (ret < 0) {
goto out;
}
ret = 0;
out:
g_free(data);
+ if (!extent->compressed) {
+ qemu_iovec_destroy(&local_qiov);
+ }
return ret;
}
@@ -1521,38 +1540,38 @@ vmdk_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
*
* Returns: error code with 0 for success.
*/
-static int vmdk_write(BlockDriverState *bs, int64_t sector_num,
- const uint8_t *buf, int nb_sectors,
- bool zeroed, bool zero_dry_run)
+static int vmdk_pwritev(BlockDriverState *bs, uint64_t offset,
+ uint64_t bytes, QEMUIOVector *qiov,
+ bool zeroed, bool zero_dry_run)
{
BDRVVmdkState *s = bs->opaque;
VmdkExtent *extent = NULL;
int ret;
- int64_t index_in_cluster, n;
+ int64_t offset_in_cluster, n_bytes;
uint64_t cluster_offset;
+ uint64_t bytes_done = 0;
VmdkMetaData m_data;
- if (sector_num > bs->total_sectors) {
- error_report("Wrong offset: sector_num=0x%" PRIx64
+ if (DIV_ROUND_UP(offset, BDRV_SECTOR_SIZE) > bs->total_sectors) {
+ error_report("Wrong offset: offset=0x%" PRIx64
" total_sectors=0x%" PRIx64,
- sector_num, bs->total_sectors);
+ offset, bs->total_sectors);
return -EIO;
}
- while (nb_sectors > 0) {
- extent = find_extent(s, sector_num, extent);
+ while (bytes > 0) {
+ extent = find_extent(s, offset >> BDRV_SECTOR_BITS, extent);
if (!extent) {
return -EIO;
}
- index_in_cluster = vmdk_find_index_in_cluster(extent, sector_num);
- n = extent->cluster_sectors - index_in_cluster;
- if (n > nb_sectors) {
- n = nb_sectors;
- }
- ret = get_cluster_offset(bs, extent, &m_data, sector_num << 9,
+ offset_in_cluster = vmdk_find_offset_in_cluster(extent, offset);
+ n_bytes = MIN(bytes, extent->cluster_sectors * BDRV_SECTOR_SIZE
+ - offset_in_cluster);
+
+ ret = get_cluster_offset(bs, extent, &m_data, offset,
!(extent->compressed || zeroed),
- &cluster_offset,
- index_in_cluster, index_in_cluster + n);
+ &cluster_offset, offset_in_cluster,
+ offset_in_cluster + n_bytes);
if (extent->compressed) {
if (ret == VMDK_OK) {
/* Refuse write to allocated cluster for streamOptimized */
@@ -1561,7 +1580,7 @@ static int vmdk_write(BlockDriverState *bs, int64_t sector_num,
return -EIO;
} else {
/* allocate */
- ret = get_cluster_offset(bs, extent, &m_data, sector_num << 9,
+ ret = get_cluster_offset(bs, extent, &m_data, offset,
true, &cluster_offset, 0, 0);
}
}
@@ -1571,9 +1590,9 @@ static int vmdk_write(BlockDriverState *bs, int64_t sector_num,
if (zeroed) {
/* Do zeroed write, buf is ignored */
if (extent->has_zero_grain &&
- index_in_cluster == 0 &&
- n >= extent->cluster_sectors) {
- n = extent->cluster_sectors;
+ offset_in_cluster == 0 &&
+ n_bytes >= extent->cluster_sectors * BDRV_SECTOR_SIZE) {
+ n_bytes = extent->cluster_sectors * BDRV_SECTOR_SIZE;
if (!zero_dry_run) {
/* update L2 tables */
if (vmdk_L2update(extent, &m_data, VMDK_GTE_ZEROED)
@@ -1585,9 +1604,8 @@ static int vmdk_write(BlockDriverState *bs, int64_t sector_num,
return -ENOTSUP;
}
} else {
- ret = vmdk_write_extent(extent,
- cluster_offset, index_in_cluster * 512,
- buf, n, sector_num);
+ ret = vmdk_write_extent(extent, cluster_offset, offset_in_cluster,
+ qiov, bytes_done, n_bytes, offset);
if (ret) {
return ret;
}
@@ -1600,9 +1618,9 @@ static int vmdk_write(BlockDriverState *bs, int64_t sector_num,
}
}
}
- nb_sectors -= n;
- sector_num += n;
- buf += n * 512;
+ bytes -= n_bytes;
+ offset += n_bytes;
+ bytes_done += n_bytes;
/* update CID on the first write every time the virtual disk is
* opened */
@@ -1617,25 +1635,65 @@ static int vmdk_write(BlockDriverState *bs, int64_t sector_num,
return 0;
}
-static coroutine_fn int vmdk_co_write(BlockDriverState *bs, int64_t sector_num,
- const uint8_t *buf, int nb_sectors)
+static int coroutine_fn
+vmdk_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
+ QEMUIOVector *qiov, int flags)
{
int ret;
BDRVVmdkState *s = bs->opaque;
qemu_co_mutex_lock(&s->lock);
- ret = vmdk_write(bs, sector_num, buf, nb_sectors, false, false);
+ ret = vmdk_pwritev(bs, offset, bytes, qiov, false, false);
qemu_co_mutex_unlock(&s->lock);
return ret;
}
+typedef struct VmdkWriteCompressedCo {
+ BlockDriverState *bs;
+ int64_t sector_num;
+ const uint8_t *buf;
+ int nb_sectors;
+ int ret;
+} VmdkWriteCompressedCo;
+
+static void vmdk_co_write_compressed(void *opaque)
+{
+ VmdkWriteCompressedCo *co = opaque;
+ QEMUIOVector local_qiov;
+ uint64_t offset = co->sector_num * BDRV_SECTOR_SIZE;
+ uint64_t bytes = co->nb_sectors * BDRV_SECTOR_SIZE;
+
+ struct iovec iov = (struct iovec) {
+ .iov_base = (uint8_t*) co->buf,
+ .iov_len = bytes,
+ };
+ qemu_iovec_init_external(&local_qiov, &iov, 1);
+
+ co->ret = vmdk_pwritev(co->bs, offset, bytes, &local_qiov, false, false);
+}
+
static int vmdk_write_compressed(BlockDriverState *bs,
int64_t sector_num,
const uint8_t *buf,
int nb_sectors)
{
BDRVVmdkState *s = bs->opaque;
+
if (s->num_extents == 1 && s->extents[0].compressed) {
- return vmdk_write(bs, sector_num, buf, nb_sectors, false, false);
+ Coroutine *co;
+ AioContext *aio_context = bdrv_get_aio_context(bs);
+ VmdkWriteCompressedCo data = {
+ .bs = bs,
+ .sector_num = sector_num,
+ .buf = buf,
+ .nb_sectors = nb_sectors,
+ .ret = -EINPROGRESS,
+ };
+ co = qemu_coroutine_create(vmdk_co_write_compressed);
+ qemu_coroutine_enter(co, &data);
+ while (data.ret == -EINPROGRESS) {
+ aio_poll(aio_context, true);
+ }
+ return data.ret;
} else {
return -ENOTSUP;
}
@@ -1648,12 +1706,15 @@ static int coroutine_fn vmdk_co_write_zeroes(BlockDriverState *bs,
{
int ret;
BDRVVmdkState *s = bs->opaque;
+ uint64_t offset = sector_num * BDRV_SECTOR_SIZE;
+ uint64_t bytes = nb_sectors * BDRV_SECTOR_SIZE;
+
qemu_co_mutex_lock(&s->lock);
/* write zeroes could fail if sectors not aligned to cluster, test it with
* dry_run == true before really updating image */
- ret = vmdk_write(bs, sector_num, NULL, nb_sectors, true, true);
+ ret = vmdk_pwritev(bs, offset, bytes, NULL, true, true);
if (!ret) {
- ret = vmdk_write(bs, sector_num, NULL, nb_sectors, true, false);
+ ret = vmdk_pwritev(bs, offset, bytes, NULL, true, false);
}
qemu_co_mutex_unlock(&s->lock);
return ret;
@@ -2337,7 +2398,7 @@ static BlockDriver bdrv_vmdk = {
.bdrv_check = vmdk_check,
.bdrv_reopen_prepare = vmdk_reopen_prepare,
.bdrv_co_preadv = vmdk_co_preadv,
- .bdrv_write = vmdk_co_write,
+ .bdrv_co_pwritev = vmdk_co_pwritev,
.bdrv_write_compressed = vmdk_write_compressed,
.bdrv_co_write_zeroes = vmdk_co_write_zeroes,
.bdrv_close = vmdk_close,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH 14/17] vpc: Implement .bdrv_co_preadv() interface
2016-04-27 9:52 [Qemu-devel] [PATCH 00/17] block: Convert .bdrv_read/write to .bdrv_preadv/pwritev Kevin Wolf
` (12 preceding siblings ...)
2016-04-27 9:52 ` [Qemu-devel] [PATCH 13/17] vmdk: Implement .bdrv_co_pwritev() interface Kevin Wolf
@ 2016-04-27 9:52 ` Kevin Wolf
2016-04-27 9:52 ` [Qemu-devel] [PATCH 15/17] vpc: Implement .bdrv_co_pwritev() interface Kevin Wolf
` (3 subsequent siblings)
17 siblings, 0 replies; 34+ messages in thread
From: Kevin Wolf @ 2016-04-27 9:52 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, stefanha, famz, sw, qemu-devel
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/vpc.c | 79 ++++++++++++++++++++++++++++++++-----------------------------
1 file changed, 42 insertions(+), 37 deletions(-)
diff --git a/block/vpc.c b/block/vpc.c
index 3e2ea69..0099a80 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -454,22 +454,21 @@ static int vpc_reopen_prepare(BDRVReopenState *state,
* The parameter write must be 1 if the offset will be used for a write
* operation (the block bitmaps is updated then), 0 otherwise.
*/
-static inline int64_t get_sector_offset(BlockDriverState *bs,
- int64_t sector_num, int write)
+static inline int64_t get_image_offset(BlockDriverState *bs, uint64_t offset,
+ bool write)
{
BDRVVPCState *s = bs->opaque;
- uint64_t offset = sector_num * 512;
uint64_t bitmap_offset, block_offset;
- uint32_t pagetable_index, pageentry_index;
+ uint32_t pagetable_index, offset_in_block;
pagetable_index = offset / s->block_size;
- pageentry_index = (offset % s->block_size) / 512;
+ offset_in_block = offset % s->block_size;
if (pagetable_index >= s->max_table_entries || s->pagetable[pagetable_index] == 0xffffffff)
return -1; /* not allocated */
bitmap_offset = 512 * (uint64_t) s->pagetable[pagetable_index];
- block_offset = bitmap_offset + s->bitmap_size + (512 * pageentry_index);
+ block_offset = bitmap_offset + s->bitmap_size + offset_in_block;
/* We must ensure that we don't write to any sectors which are marked as
unused in the bitmap. We get away with setting all bits in the block
@@ -487,6 +486,12 @@ static inline int64_t get_sector_offset(BlockDriverState *bs,
return block_offset;
}
+static inline int64_t get_sector_offset(BlockDriverState *bs,
+ int64_t sector_num, bool write)
+{
+ return get_image_offset(bs, sector_num * BDRV_SECTOR_SIZE, write);
+}
+
/*
* Writes the footer to the end of the image file. This is needed when the
* file grows as it overwrites the old footer
@@ -573,52 +578,52 @@ static int vpc_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
return 0;
}
-static int vpc_read(BlockDriverState *bs, int64_t sector_num,
- uint8_t *buf, int nb_sectors)
+static int coroutine_fn
+vpc_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
+ QEMUIOVector *qiov, int flags)
{
BDRVVPCState *s = bs->opaque;
int ret;
- int64_t offset;
- int64_t sectors, sectors_per_block;
+ int64_t image_offset;
+ int64_t n_bytes;
+ int64_t bytes_done = 0;
VHDFooter *footer = (VHDFooter *) s->footer_buf;
+ QEMUIOVector local_qiov;
if (be32_to_cpu(footer->type) == VHD_FIXED) {
- return bdrv_read(bs->file->bs, sector_num, buf, nb_sectors);
+ return bdrv_co_preadv(bs->file->bs, offset, bytes, qiov, 0);
}
- while (nb_sectors > 0) {
- offset = get_sector_offset(bs, sector_num, 0);
- sectors_per_block = s->block_size >> BDRV_SECTOR_BITS;
- sectors = sectors_per_block - (sector_num % sectors_per_block);
- if (sectors > nb_sectors) {
- sectors = nb_sectors;
- }
+ qemu_co_mutex_lock(&s->lock);
+ qemu_iovec_init(&local_qiov, qiov->niov);
- if (offset == -1) {
- memset(buf, 0, sectors * BDRV_SECTOR_SIZE);
+ while (bytes > 0) {
+ image_offset = get_image_offset(bs, offset, false);
+ n_bytes = MIN(bytes, s->block_size - (offset % s->block_size));
+
+ if (image_offset == -1) {
+ qemu_iovec_memset(qiov, bytes_done, 0, n_bytes);
} else {
- ret = bdrv_pread(bs->file->bs, offset, buf,
- sectors * BDRV_SECTOR_SIZE);
- if (ret != sectors * BDRV_SECTOR_SIZE) {
- return -1;
+ qemu_iovec_reset(&local_qiov);
+ qemu_iovec_concat(&local_qiov, qiov, bytes_done, n_bytes);
+
+ ret = bdrv_co_preadv(bs->file->bs, image_offset, n_bytes,
+ &local_qiov, 0);
+ if (ret < 0) {
+ goto fail;
}
}
- nb_sectors -= sectors;
- sector_num += sectors;
- buf += sectors * BDRV_SECTOR_SIZE;
+ bytes -= n_bytes;
+ offset += n_bytes;
+ bytes_done += n_bytes;
}
- return 0;
-}
-static coroutine_fn int vpc_co_read(BlockDriverState *bs, int64_t sector_num,
- uint8_t *buf, int nb_sectors)
-{
- int ret;
- BDRVVPCState *s = bs->opaque;
- qemu_co_mutex_lock(&s->lock);
- ret = vpc_read(bs, sector_num, buf, nb_sectors);
+ ret = 0;
+fail:
+ qemu_iovec_destroy(&local_qiov);
qemu_co_mutex_unlock(&s->lock);
+
return ret;
}
@@ -1056,7 +1061,7 @@ static BlockDriver bdrv_vpc = {
.bdrv_reopen_prepare = vpc_reopen_prepare,
.bdrv_create = vpc_create,
- .bdrv_read = vpc_co_read,
+ .bdrv_co_preadv = vpc_co_preadv,
.bdrv_write = vpc_co_write,
.bdrv_co_get_block_status = vpc_co_get_block_status,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH 15/17] vpc: Implement .bdrv_co_pwritev() interface
2016-04-27 9:52 [Qemu-devel] [PATCH 00/17] block: Convert .bdrv_read/write to .bdrv_preadv/pwritev Kevin Wolf
` (13 preceding siblings ...)
2016-04-27 9:52 ` [Qemu-devel] [PATCH 14/17] vpc: Implement .bdrv_co_preadv() interface Kevin Wolf
@ 2016-04-27 9:52 ` Kevin Wolf
2016-04-27 9:52 ` [Qemu-devel] [PATCH 16/17] vvfat: Implement .bdrv_co_preadv/pwritev interfaces Kevin Wolf
` (2 subsequent siblings)
17 siblings, 0 replies; 34+ messages in thread
From: Kevin Wolf @ 2016-04-27 9:52 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, stefanha, famz, sw, qemu-devel
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/vpc.c | 86 ++++++++++++++++++++++++++++++-------------------------------
1 file changed, 43 insertions(+), 43 deletions(-)
diff --git a/block/vpc.c b/block/vpc.c
index 0099a80..61a8427 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -518,7 +518,7 @@ static int rewrite_footer(BlockDriverState* bs)
*
* Returns the sectors' offset in the image file on success and < 0 on error
*/
-static int64_t alloc_block(BlockDriverState* bs, int64_t sector_num)
+static int64_t alloc_block(BlockDriverState* bs, int64_t offset)
{
BDRVVPCState *s = bs->opaque;
int64_t bat_offset;
@@ -527,14 +527,13 @@ static int64_t alloc_block(BlockDriverState* bs, int64_t sector_num)
uint8_t bitmap[s->bitmap_size];
/* Check if sector_num is valid */
- if ((sector_num < 0) || (sector_num > bs->total_sectors))
- return -1;
+ if ((offset < 0) || (offset > bs->total_sectors * BDRV_SECTOR_SIZE)) {
+ return -EINVAL;
+ }
/* Write entry into in-memory BAT */
- index = (sector_num * 512) / s->block_size;
- if (s->pagetable[index] != 0xFFFFFFFF)
- return -1;
-
+ index = offset / s->block_size;
+ assert(s->pagetable[index] == 0xFFFFFFFF);
s->pagetable[index] = s->free_data_block_offset / 512;
/* Initialize the block's bitmap */
@@ -558,11 +557,11 @@ static int64_t alloc_block(BlockDriverState* bs, int64_t sector_num)
if (ret < 0)
goto fail;
- return get_sector_offset(bs, sector_num, 0);
+ return get_image_offset(bs, offset, false);
fail:
s->free_data_block_offset -= (s->block_size + s->bitmap_size);
- return -1;
+ return ret;
}
static int vpc_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
@@ -627,55 +626,56 @@ fail:
return ret;
}
-static int vpc_write(BlockDriverState *bs, int64_t sector_num,
- const uint8_t *buf, int nb_sectors)
+static int coroutine_fn
+vpc_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
+ QEMUIOVector *qiov, int flags)
{
BDRVVPCState *s = bs->opaque;
- int64_t offset;
- int64_t sectors, sectors_per_block;
+ int64_t image_offset;
+ int64_t n_bytes;
+ int64_t bytes_done = 0;
int ret;
VHDFooter *footer = (VHDFooter *) s->footer_buf;
+ QEMUIOVector local_qiov;
if (be32_to_cpu(footer->type) == VHD_FIXED) {
- return bdrv_write(bs->file->bs, sector_num, buf, nb_sectors);
+ return bdrv_co_pwritev(bs->file->bs, offset, bytes, qiov, 0);
}
- while (nb_sectors > 0) {
- offset = get_sector_offset(bs, sector_num, 1);
- sectors_per_block = s->block_size >> BDRV_SECTOR_BITS;
- sectors = sectors_per_block - (sector_num % sectors_per_block);
- if (sectors > nb_sectors) {
- sectors = nb_sectors;
- }
+ qemu_co_mutex_lock(&s->lock);
+ qemu_iovec_init(&local_qiov, qiov->niov);
+
+ while (bytes > 0) {
+ image_offset = get_image_offset(bs, offset, true);
+ n_bytes = MIN(bytes, s->block_size - (offset % s->block_size));
- if (offset == -1) {
- offset = alloc_block(bs, sector_num);
- if (offset < 0)
- return -1;
+ if (image_offset == -1) {
+ image_offset = alloc_block(bs, offset);
+ if (image_offset < 0) {
+ ret = image_offset;
+ goto fail;
+ }
}
- ret = bdrv_pwrite(bs->file->bs, offset, buf,
- sectors * BDRV_SECTOR_SIZE);
- if (ret != sectors * BDRV_SECTOR_SIZE) {
- return -1;
+ qemu_iovec_reset(&local_qiov);
+ qemu_iovec_concat(&local_qiov, qiov, bytes_done, n_bytes);
+
+ ret = bdrv_co_pwritev(bs->file->bs, image_offset, n_bytes,
+ &local_qiov, 0);
+ if (ret < 0) {
+ goto fail;
}
- nb_sectors -= sectors;
- sector_num += sectors;
- buf += sectors * BDRV_SECTOR_SIZE;
+ bytes -= n_bytes;
+ offset += n_bytes;
+ bytes_done += n_bytes;
}
- return 0;
-}
-
-static coroutine_fn int vpc_co_write(BlockDriverState *bs, int64_t sector_num,
- const uint8_t *buf, int nb_sectors)
-{
- int ret;
- BDRVVPCState *s = bs->opaque;
- qemu_co_mutex_lock(&s->lock);
- ret = vpc_write(bs, sector_num, buf, nb_sectors);
+ ret = 0;
+fail:
+ qemu_iovec_destroy(&local_qiov);
qemu_co_mutex_unlock(&s->lock);
+
return ret;
}
@@ -1062,7 +1062,7 @@ static BlockDriver bdrv_vpc = {
.bdrv_create = vpc_create,
.bdrv_co_preadv = vpc_co_preadv,
- .bdrv_write = vpc_co_write,
+ .bdrv_co_pwritev = vpc_co_pwritev,
.bdrv_co_get_block_status = vpc_co_get_block_status,
.bdrv_get_info = vpc_get_info,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH 16/17] vvfat: Implement .bdrv_co_preadv/pwritev interfaces
2016-04-27 9:52 [Qemu-devel] [PATCH 00/17] block: Convert .bdrv_read/write to .bdrv_preadv/pwritev Kevin Wolf
` (14 preceding siblings ...)
2016-04-27 9:52 ` [Qemu-devel] [PATCH 15/17] vpc: Implement .bdrv_co_pwritev() interface Kevin Wolf
@ 2016-04-27 9:52 ` Kevin Wolf
2016-04-27 9:52 ` [Qemu-devel] [PATCH 17/17] block: Remove BlockDriver.bdrv_read/write Kevin Wolf
2016-04-27 14:26 ` [Qemu-devel] [PATCH 00/17] block: Convert .bdrv_read/write to .bdrv_preadv/pwritev Stefan Hajnoczi
17 siblings, 0 replies; 34+ messages in thread
From: Kevin Wolf @ 2016-04-27 9:52 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, stefanha, famz, sw, qemu-devel
This doesn't really convert any of the actual vvfat logic to use
vectored I/O (and it's doubtful whether that would make sense), but
instead just adapts the wrappers to the modern interface.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/vvfat.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++---------
1 file changed, 46 insertions(+), 9 deletions(-)
diff --git a/block/vvfat.c b/block/vvfat.c
index 6b85314..ad7c22a 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1177,6 +1177,7 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
bs->read_only = 0;
}
+ bs->request_alignment = BDRV_SECTOR_SIZE; /* No sub-sector I/O supported */
bs->total_sectors = cyls * heads * secs;
if (init_directories(s, dirname, heads, secs, errp)) {
@@ -1419,14 +1420,31 @@ DLOG(fprintf(stderr, "sector %d not allocated\n", (int)sector_num));
return 0;
}
-static coroutine_fn int vvfat_co_read(BlockDriverState *bs, int64_t sector_num,
- uint8_t *buf, int nb_sectors)
+static int coroutine_fn
+vvfat_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
+ QEMUIOVector *qiov, int flags)
{
int ret;
BDRVVVFATState *s = bs->opaque;
+ uint64_t sector_num = offset >> BDRV_SECTOR_BITS;
+ int nb_sectors = bytes >> BDRV_SECTOR_BITS;
+ void *buf;
+
+ assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0);
+ assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0);
+
+ buf = g_try_malloc(bytes);
+ if (bytes && buf == NULL) {
+ return -ENOMEM;
+ }
+
qemu_co_mutex_lock(&s->lock);
ret = vvfat_read(bs, sector_num, buf, nb_sectors);
qemu_co_mutex_unlock(&s->lock);
+
+ qemu_iovec_from_buf(qiov, 0, buf, bytes);
+ g_free(buf);
+
return ret;
}
@@ -2873,14 +2891,31 @@ DLOG(checkpoint());
return 0;
}
-static coroutine_fn int vvfat_co_write(BlockDriverState *bs, int64_t sector_num,
- const uint8_t *buf, int nb_sectors)
+static int coroutine_fn
+vvfat_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
+ QEMUIOVector *qiov, int flags)
{
int ret;
BDRVVVFATState *s = bs->opaque;
+ uint64_t sector_num = offset >> BDRV_SECTOR_BITS;
+ int nb_sectors = bytes >> BDRV_SECTOR_BITS;
+ void *buf;
+
+ assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0);
+ assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0);
+
+ buf = g_try_malloc(bytes);
+ if (bytes && buf == NULL) {
+ return -ENOMEM;
+ }
+ qemu_iovec_to_buf(qiov, 0, buf, bytes);
+
qemu_co_mutex_lock(&s->lock);
ret = vvfat_write(bs, sector_num, buf, nb_sectors);
qemu_co_mutex_unlock(&s->lock);
+
+ g_free(buf);
+
return ret;
}
@@ -2897,8 +2932,10 @@ static int64_t coroutine_fn vvfat_co_get_block_status(BlockDriverState *bs,
return BDRV_BLOCK_DATA;
}
-static int write_target_commit(BlockDriverState *bs, int64_t sector_num,
- const uint8_t* buffer, int nb_sectors) {
+static int coroutine_fn
+write_target_commit(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
+ QEMUIOVector *qiov, int flags)
+{
BDRVVVFATState* s = *((BDRVVVFATState**) bs->opaque);
return try_commit(s);
}
@@ -2911,7 +2948,7 @@ static void write_target_close(BlockDriverState *bs) {
static BlockDriver vvfat_write_target = {
.format_name = "vvfat_write_target",
- .bdrv_write = write_target_commit,
+ .bdrv_co_pwritev = write_target_commit,
.bdrv_close = write_target_close,
};
@@ -3007,8 +3044,8 @@ static BlockDriver bdrv_vvfat = {
.bdrv_file_open = vvfat_open,
.bdrv_close = vvfat_close,
- .bdrv_read = vvfat_co_read,
- .bdrv_write = vvfat_co_write,
+ .bdrv_co_preadv = vvfat_co_preadv,
+ .bdrv_co_pwritev = vvfat_co_pwritev,
.bdrv_co_get_block_status = vvfat_co_get_block_status,
};
--
1.8.3.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH 17/17] block: Remove BlockDriver.bdrv_read/write
2016-04-27 9:52 [Qemu-devel] [PATCH 00/17] block: Convert .bdrv_read/write to .bdrv_preadv/pwritev Kevin Wolf
` (15 preceding siblings ...)
2016-04-27 9:52 ` [Qemu-devel] [PATCH 16/17] vvfat: Implement .bdrv_co_preadv/pwritev interfaces Kevin Wolf
@ 2016-04-27 9:52 ` Kevin Wolf
2016-04-27 14:26 ` [Qemu-devel] [PATCH 00/17] block: Convert .bdrv_read/write to .bdrv_preadv/pwritev Stefan Hajnoczi
17 siblings, 0 replies; 34+ messages in thread
From: Kevin Wolf @ 2016-04-27 9:52 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, stefanha, famz, sw, qemu-devel
There are no block drivers left that implement the old .bdrv_read/write
interface, so it can be removed now. This gets us rid of the
corresponding emulation functions, too.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block.c | 2 --
block/io.c | 92 -----------------------------------------------
include/block/block_int.h | 4 ---
3 files changed, 98 deletions(-)
diff --git a/block.c b/block.c
index d4939b4..78e1f15 100644
--- a/block.c
+++ b/block.c
@@ -218,8 +218,6 @@ void bdrv_get_full_backing_filename(BlockDriverState *bs, char *dest, size_t sz,
void bdrv_register(BlockDriver *bdrv)
{
- bdrv_setup_io_funcs(bdrv);
-
QLIST_INSERT_HEAD(&bdrv_drivers, bdrv, list);
}
diff --git a/block/io.c b/block/io.c
index 4aac4ab..ecd2acb 100644
--- a/block/io.c
+++ b/block/io.c
@@ -34,12 +34,6 @@
#define NOT_DONE 0x7fffffff /* used while emulated sync operation in progress */
-static BlockAIOCB *bdrv_aio_readv_em(BlockDriverState *bs,
- int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
- BlockCompletionFunc *cb, void *opaque);
-static BlockAIOCB *bdrv_aio_writev_em(BlockDriverState *bs,
- int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
- BlockCompletionFunc *cb, void *opaque);
static BlockAIOCB *bdrv_co_aio_rw_vector(BlockDriverState *bs,
int64_t sector_num,
QEMUIOVector *qiov,
@@ -117,18 +111,6 @@ void bdrv_io_limits_update_group(BlockDriverState *bs, const char *group)
bdrv_io_limits_enable(bs, group);
}
-void bdrv_setup_io_funcs(BlockDriver *bdrv)
-{
- /* bdrv_co_readv_em()/brdv_co_writev_em() work in terms of aio, so if
- * the block driver lacks aio we need to emulate that.
- */
- if (!bdrv->bdrv_aio_readv) {
- /* add AIO emulation layer */
- bdrv->bdrv_aio_readv = bdrv_aio_readv_em;
- bdrv->bdrv_aio_writev = bdrv_aio_writev_em;
- }
-}
-
void bdrv_refresh_limits(BlockDriverState *bs, Error **errp)
{
BlockDriver *drv = bs->drv;
@@ -2148,80 +2130,6 @@ void bdrv_aio_cancel_async(BlockAIOCB *acb)
/**************************************************************/
/* async block device emulation */
-typedef struct BlockAIOCBSync {
- BlockAIOCB common;
- QEMUBH *bh;
- int ret;
- /* vector translation state */
- QEMUIOVector *qiov;
- uint8_t *bounce;
- int is_write;
-} BlockAIOCBSync;
-
-static const AIOCBInfo bdrv_em_aiocb_info = {
- .aiocb_size = sizeof(BlockAIOCBSync),
-};
-
-static void bdrv_aio_bh_cb(void *opaque)
-{
- BlockAIOCBSync *acb = opaque;
-
- if (!acb->is_write && acb->ret >= 0) {
- qemu_iovec_from_buf(acb->qiov, 0, acb->bounce, acb->qiov->size);
- }
- qemu_vfree(acb->bounce);
- acb->common.cb(acb->common.opaque, acb->ret);
- qemu_bh_delete(acb->bh);
- acb->bh = NULL;
- qemu_aio_unref(acb);
-}
-
-static BlockAIOCB *bdrv_aio_rw_vector(BlockDriverState *bs,
- int64_t sector_num,
- QEMUIOVector *qiov,
- int nb_sectors,
- BlockCompletionFunc *cb,
- void *opaque,
- int is_write)
-
-{
- BlockAIOCBSync *acb;
-
- acb = qemu_aio_get(&bdrv_em_aiocb_info, bs, cb, opaque);
- acb->is_write = is_write;
- acb->qiov = qiov;
- acb->bounce = qemu_try_blockalign(bs, qiov->size);
- acb->bh = aio_bh_new(bdrv_get_aio_context(bs), bdrv_aio_bh_cb, acb);
-
- if (acb->bounce == NULL) {
- acb->ret = -ENOMEM;
- } else if (is_write) {
- qemu_iovec_to_buf(acb->qiov, 0, acb->bounce, qiov->size);
- acb->ret = bs->drv->bdrv_write(bs, sector_num, acb->bounce, nb_sectors);
- } else {
- acb->ret = bs->drv->bdrv_read(bs, sector_num, acb->bounce, nb_sectors);
- }
-
- qemu_bh_schedule(acb->bh);
-
- return &acb->common;
-}
-
-static BlockAIOCB *bdrv_aio_readv_em(BlockDriverState *bs,
- int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
- BlockCompletionFunc *cb, void *opaque)
-{
- return bdrv_aio_rw_vector(bs, sector_num, qiov, nb_sectors, cb, opaque, 0);
-}
-
-static BlockAIOCB *bdrv_aio_writev_em(BlockDriverState *bs,
- int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
- BlockCompletionFunc *cb, void *opaque)
-{
- return bdrv_aio_rw_vector(bs, sector_num, qiov, nb_sectors, cb, opaque, 1);
-}
-
-
typedef struct BlockAIOCBCoroutine {
BlockAIOCB common;
BlockRequest req;
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 0ef6f2c..22fbea4 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -127,10 +127,6 @@ struct BlockDriver {
Error **errp);
int (*bdrv_file_open)(BlockDriverState *bs, QDict *options, int flags,
Error **errp);
- int (*bdrv_read)(BlockDriverState *bs, int64_t sector_num,
- uint8_t *buf, int nb_sectors);
- int (*bdrv_write)(BlockDriverState *bs, int64_t sector_num,
- const uint8_t *buf, int nb_sectors);
void (*bdrv_close)(BlockDriverState *bs);
int (*bdrv_create)(const char *filename, QemuOpts *opts, Error **errp);
int (*bdrv_set_key)(BlockDriverState *bs, const char *key);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH 01/17] block: Introduce bdrv_driver_preadv()
2016-04-27 9:52 ` [Qemu-devel] [PATCH 01/17] block: Introduce bdrv_driver_preadv() Kevin Wolf
@ 2016-04-27 13:52 ` Eric Blake
0 siblings, 0 replies; 34+ messages in thread
From: Eric Blake @ 2016-04-27 13:52 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: famz, sw, qemu-devel, mreitz, stefanha
[-- Attachment #1: Type: text/plain, Size: 1572 bytes --]
On 04/27/2016 03:52 AM, Kevin Wolf wrote:
> This is a function that simply calls into the block driver for doing a
> read, providing the byte granularity interface we want to eventually
> have everywhere, and using whatever interface that driver supports.
>
> For now, this is just a wrapper for calling bs->drv->bdrv_co_readv().
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> block/io.c | 30 +++++++++++++++++++++++-------
> 1 file changed, 23 insertions(+), 7 deletions(-)
>
> diff --git a/block/io.c b/block/io.c
> index a7dbf85..586a46a 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -800,6 +800,21 @@ int bdrv_pwrite_sync(BlockDriverState *bs, int64_t offset,
> return 0;
> }
>
> +static int coroutine_fn bdrv_driver_preadv(BlockDriverState *bs,
> + uint64_t offset, uint64_t bytes,
> + QEMUIOVector *qiov, int flags)
> +{
> + BlockDriver *drv = bs->drv;
> + int64_t sector_num = offset >> BDRV_SECTOR_BITS;
> + unsigned int nb_sectors = bytes >> BDRV_SECTOR_BITS;
> +
> + assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0);
> + assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0);
> + assert((bytes >> BDRV_SECTOR_BITS) <= BDRV_REQUEST_MAX_SECTORS);
So for now it is still enforcing 512-byte alignment, but gives us the
opportunity to relax things later.
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH 02/17] block: Introduce bdrv_driver_pwritev()
2016-04-27 9:52 ` [Qemu-devel] [PATCH 02/17] block: Introduce bdrv_driver_pwritev() Kevin Wolf
@ 2016-04-27 14:03 ` Eric Blake
0 siblings, 0 replies; 34+ messages in thread
From: Eric Blake @ 2016-04-27 14:03 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: famz, sw, qemu-devel, mreitz, stefanha
[-- Attachment #1: Type: text/plain, Size: 3666 bytes --]
On 04/27/2016 03:52 AM, Kevin Wolf wrote:
> This is a function that simply calls into the block driver for doing a
> write, providing the byte granularity interface we want to eventually
> have everywhere, and using whatever interface that driver supports.
>
> This one is a bit more interesting that the version for reads: It adds
> support for .bdrv_co_writev_flags() everywhere, so that drivers
> implementing this function can drop .bdrv_co_writev() now.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> block/io.c | 51 ++++++++++++++++++++++++++++++++++++---------------
> block/iscsi.c | 8 --------
> block/nbd.c | 9 ---------
> block/raw_bsd.c | 8 --------
> 4 files changed, 36 insertions(+), 40 deletions(-)
>
>
> +static int coroutine_fn bdrv_driver_pwritev(BlockDriverState *bs,
> + uint64_t offset, uint64_t bytes,
> + QEMUIOVector *qiov, int flags)
> +{
> + BlockDriver *drv = bs->drv;
> + int64_t sector_num = offset >> BDRV_SECTOR_BITS;
> + unsigned int nb_sectors = bytes >> BDRV_SECTOR_BITS;
> + int ret;
> +
> + assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0);
> + 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);
Not for this patch, but should we be doing something like assert((flags
& ~drv->supported_write_flags) == 0)?
> + } else {
> + assert(drv->supported_write_flags == 0);
> + ret = drv->bdrv_co_writev(bs, sector_num, nb_sectors, qiov);
> + }
> +
> + if (ret == 0 && (flags & BDRV_REQ_FUA) &&
> + !(drv->supported_write_flags & BDRV_REQ_FUA))
> + {
> + ret = bdrv_co_flush(bs);
Unrelated to your patch here, but in my NBD work, I ran into the
situation where it would be nicer if drv->supported_write_flags were
dynamic. That is, an NBD client can tell on a per-connection basis
whether the server supports NBD_FLAG_FUA, but because
supported_write_flags is a property of the driver, rather than a
callback that is a function of the bs, NBD has to reflect it back to the
block layer by advertising supported_write_flags == BDRV_REQ_FUA always,
and when connecting to a less-capable server has to manually repeat the
bdrv_co_flush(bs) fallback dance itself:
http://git.qemu.org/?p=qemu.git;a=blob;f=block/nbd.c;h=f7ea3b3608;hb=3123bd8e#l358
Maybe we should do a patch series that converts supported_write_flags to
be a function call that can have per-bs configuration, so that the NBD
client can be simplified by letting the block layer take care of the FUA
fallback.
> @@ -1215,23 +1247,12 @@ static int coroutine_fn bdrv_aligned_pwritev(BlockDriverState *bs,
> } else if (flags & BDRV_REQ_ZERO_WRITE) {
> bdrv_debug_event(bs, BLKDBG_PWRITEV_ZERO);
> ret = bdrv_co_do_write_zeroes(bs, sector_num, nb_sectors, flags);
> - } else if (drv->bdrv_co_writev_flags) {
> - bdrv_debug_event(bs, BLKDBG_PWRITEV);
> - ret = drv->bdrv_co_writev_flags(bs, sector_num, nb_sectors, qiov,
> - flags);
Should bdrv_co_do_write_zeroes also be folded into bdrv_driver_pwritev()?
But what you have looks sane enough for:
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH 06/17] bochs: Implement .bdrv_co_preadv() interface
2016-04-27 9:52 ` [Qemu-devel] [PATCH 06/17] bochs: Implement .bdrv_co_preadv() interface Kevin Wolf
@ 2016-04-27 14:06 ` Stefan Hajnoczi
2016-04-27 14:33 ` Kevin Wolf
2016-04-27 15:51 ` Eric Blake
1 sibling, 1 reply; 34+ messages in thread
From: Stefan Hajnoczi @ 2016-04-27 14:06 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-block, mreitz, famz, sw, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 2513 bytes --]
On Wed, Apr 27, 2016 at 11:52:36AM +0200, Kevin Wolf wrote:
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> block/bochs.c | 46 +++++++++++++++++++++++++++++-----------------
> 1 file changed, 29 insertions(+), 17 deletions(-)
>
> diff --git a/block/bochs.c b/block/bochs.c
> index af8b7ab..d148454 100644
> --- a/block/bochs.c
> +++ b/block/bochs.c
> @@ -104,6 +104,7 @@ static int bochs_open(BlockDriverState *bs, QDict *options, int flags,
> int ret;
>
> bs->read_only = 1; // no write support yet
> + bs->request_alignment = BDRV_SECTOR_SIZE; /* No sub-sector I/O supported */
Can this be the default in block.c? Drivers that have other alignment
characteristics can set it explicitly but most drivers will want
BDRV_SECTOR_SIZE so it's nice to make it common code.
>
> ret = bdrv_pread(bs->file->bs, 0, &bochs, sizeof(bochs));
> if (ret < 0) {
> @@ -221,39 +222,50 @@ static int64_t seek_to_sector(BlockDriverState *bs, int64_t sector_num)
> return bitmap_offset + (512 * (s->bitmap_blocks + extent_offset));
> }
>
> -static int bochs_read(BlockDriverState *bs, int64_t sector_num,
> - uint8_t *buf, int nb_sectors)
> +static int coroutine_fn
> +bochs_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
> + QEMUIOVector *qiov, int flags)
> {
> + BDRVBochsState *s = bs->opaque;
> + uint64_t sector_num = offset >> BDRV_SECTOR_BITS;
> + int nb_sectors = bytes >> BDRV_SECTOR_BITS;
> + uint64_t bytes_done = 0;
> + QEMUIOVector local_qiov;
> int ret;
>
> + assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0);
> + assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0);
> +
> + qemu_iovec_init(&local_qiov, qiov->niov);
> + qemu_co_mutex_lock(&s->lock);
> +
> while (nb_sectors > 0) {
> int64_t block_offset = seek_to_sector(bs, sector_num);
> if (block_offset < 0) {
> return block_offset;
s->lock must be unlocked.
> - } else if (block_offset > 0) {
> - ret = bdrv_pread(bs->file->bs, block_offset, buf, 512);
> + }
> +
> + qemu_iovec_reset(&local_qiov);
> + qemu_iovec_concat(&local_qiov, qiov, bytes_done, 512);
> +
> + if (block_offset > 0) {
> + ret = bdrv_co_preadv(bs->file->bs, block_offset, 512,
> + &local_qiov, 0);
> if (ret < 0) {
> return ret;
Same here.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH 07/17] cloop: Implement .bdrv_co_preadv() interface
2016-04-27 9:52 ` [Qemu-devel] [PATCH 07/17] cloop: " Kevin Wolf
@ 2016-04-27 14:12 ` Stefan Hajnoczi
0 siblings, 0 replies; 34+ messages in thread
From: Stefan Hajnoczi @ 2016-04-27 14:12 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-block, mreitz, famz, sw, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1025 bytes --]
On Wed, Apr 27, 2016 at 11:52:37AM +0200, Kevin Wolf wrote:
> -static int cloop_read(BlockDriverState *bs, int64_t sector_num,
> - uint8_t *buf, int nb_sectors)
> +static int coroutine_fn
> +cloop_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
> + QEMUIOVector *qiov, int flags)
> {
> BDRVCloopState *s = bs->opaque;
> + uint64_t sector_num = offset >> BDRV_SECTOR_BITS;
> + int nb_sectors = bytes >> BDRV_SECTOR_BITS;
> int i;
>
> + assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0);
> + assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0);
> +
> + qemu_co_mutex_lock(&s->lock);
> +
> for (i = 0; i < nb_sectors; i++) {
> + void *data;
> uint32_t sector_offset_in_block =
> ((sector_num + i) % s->sectors_per_block),
> block_num = (sector_num + i) / s->sectors_per_block;
> if (cloop_read_block(bs, block_num) != 0) {
> return -1;
s->lock must be unlocked.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH 03/17] block: Support AIO drivers in bdrv_driver_preadv/pwritev()
2016-04-27 9:52 ` [Qemu-devel] [PATCH 03/17] block: Support AIO drivers in bdrv_driver_preadv/pwritev() Kevin Wolf
@ 2016-04-27 14:13 ` Eric Blake
0 siblings, 0 replies; 34+ messages in thread
From: Eric Blake @ 2016-04-27 14:13 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: famz, sw, qemu-devel, mreitz, stefanha
[-- Attachment #1: Type: text/plain, Size: 1603 bytes --]
On 04/27/2016 03:52 AM, Kevin Wolf wrote:
> Instead of registering emulation functions as .bdrv_co_writev, just
> directly check whether the function is there or not, and use the AIO
> interface if it isn't. This makes the read/write functions more
> consistent with how things are done in other places (flush, discard,
> etc.)
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> block/io.c | 126 +++++++++++++++++++++++++------------------------------------
> 1 file changed, 52 insertions(+), 74 deletions(-)
>
> -
> -static int coroutine_fn bdrv_co_io_em(BlockDriverState *bs, int64_t sector_num,
> - int nb_sectors, QEMUIOVector *iov,
> - bool is_write)
> -{
> - CoroutineIOCompletion co = {
> - .coroutine = qemu_coroutine_self(),
> - };
> - BlockAIOCB *acb;
> -
> - if (is_write) {
> - acb = bs->drv->bdrv_aio_writev(bs, sector_num, iov, nb_sectors,
> - bdrv_co_io_em_complete, &co);
> - } else {
> - acb = bs->drv->bdrv_aio_readv(bs, sector_num, iov, nb_sectors,
> - bdrv_co_io_em_complete, &co);
> - }
> -
> - trace_bdrv_co_io_em(bs, sector_num, nb_sectors, is_write, acb);
I don't see any more uses of this; do you need to tweak trace-events to
drop bdrv_co_io_em?
With that fixed (or explained),
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH 10/17] vdi: Implement .bdrv_co_pwritev() interface
2016-04-27 9:52 ` [Qemu-devel] [PATCH 10/17] vdi: Implement .bdrv_co_pwritev() interface Kevin Wolf
@ 2016-04-27 14:17 ` Stefan Hajnoczi
2016-04-27 14:36 ` Kevin Wolf
0 siblings, 1 reply; 34+ messages in thread
From: Stefan Hajnoczi @ 2016-04-27 14:17 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-block, mreitz, famz, sw, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 702 bytes --]
On Wed, Apr 27, 2016 at 11:52:40AM +0200, Kevin Wolf wrote:
> @@ -703,6 +712,7 @@ static int vdi_co_write(BlockDriverState *bs,
> VdiHeader *header = (VdiHeader *) block;
> uint8_t *base;
> uint64_t offset;
> + uint32_t n_sectors;
>
> logout("now writing modified header\n");
> assert(VDI_IS_ALLOCATED(bmap_first));
Unnecessary change?
> @@ -914,7 +924,7 @@ static BlockDriver bdrv_vdi = {
>
> .bdrv_co_preadv = vdi_co_preadv,
> #if defined(CONFIG_VDI_WRITE)
> - .bdrv_write = vdi_co_write,
> + .bdrv_co_pwritev = vdi_co_pwritev,
> #endif
>
> .bdrv_get_info = vdi_get_info,
> --
> 1.8.3.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH 13/17] vmdk: Implement .bdrv_co_pwritev() interface
2016-04-27 9:52 ` [Qemu-devel] [PATCH 13/17] vmdk: Implement .bdrv_co_pwritev() interface Kevin Wolf
@ 2016-04-27 14:21 ` Stefan Hajnoczi
0 siblings, 0 replies; 34+ messages in thread
From: Stefan Hajnoczi @ 2016-04-27 14:21 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-block, mreitz, famz, sw, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 308 bytes --]
On Wed, Apr 27, 2016 at 11:52:43AM +0200, Kevin Wolf wrote:
> + compressed_data = g_malloc(n_bytes);
> + qemu_iovec_to_buf(qiov, qiov_offset, compressed_data, n_bytes);
> + ret = compress(data->data, &buf_len, compressed_data, n_bytes);
> + free(compressed_data);
s/free/g_free/
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH 00/17] block: Convert .bdrv_read/write to .bdrv_preadv/pwritev
2016-04-27 9:52 [Qemu-devel] [PATCH 00/17] block: Convert .bdrv_read/write to .bdrv_preadv/pwritev Kevin Wolf
` (16 preceding siblings ...)
2016-04-27 9:52 ` [Qemu-devel] [PATCH 17/17] block: Remove BlockDriver.bdrv_read/write Kevin Wolf
@ 2016-04-27 14:26 ` Stefan Hajnoczi
17 siblings, 0 replies; 34+ messages in thread
From: Stefan Hajnoczi @ 2016-04-27 14:26 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-block, mreitz, famz, sw, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 3004 bytes --]
On Wed, Apr 27, 2016 at 11:52:30AM +0200, Kevin Wolf wrote:
> This series introduces a new BlockDriver interface, which will hopefully be the
> final one, or at least good enough for another few years: .bdrv_preadv/pwritev.
> It is based on coroutines, vectored, has flags and uses a byte granularity.
> This is now the preferred interface for new drivers.
>
> All drivers still using the legacy interface .bdrv_read/write are converted to
> the new interface and the emulation code we had for the old interface is
> removed. Most of the drivers become zero-copy with these patches as they are
> vectored now; only vvfat continues to allocate a bounce buffer.
>
> The less obscure formats (vmdk, vdi, vpc) also natively support byte-aligned
> requests now. The block layer is still enforcing a minimal alignment of 512, so
> this isn't actually used yet, but in a next step, we can lift this restriction
> for drivers that implement .bdrv_preadv/pwritev.
>
> Kevin Wolf (17):
> block: Introduce bdrv_driver_preadv()
> block: Introduce bdrv_driver_pwritev()
> block: Support AIO drivers in bdrv_driver_preadv/pwritev()
> block: Rename bdrv_co_do_preadv/writev to bdrv_co_preadv/writev
> block: Introduce .bdrv_co_preadv/pwritev BlockDriver function
> bochs: Implement .bdrv_co_preadv() interface
> cloop: Implement .bdrv_co_preadv() interface
> dmg: Implement .bdrv_co_preadv() interface
> vdi: Implement .bdrv_co_preadv() interface
> vdi: Implement .bdrv_co_pwritev() interface
> vmdk: Add vmdk_find_offset_in_cluster()
> vmdk: Implement .bdrv_co_preadv() interface
> vmdk: Implement .bdrv_co_pwritev() interface
> vpc: Implement .bdrv_co_preadv() interface
> vpc: Implement .bdrv_co_pwritev() interface
> vvfat: Implement .bdrv_co_preadv/pwritev interfaces
> block: Remove BlockDriver.bdrv_read/write
>
> block.c | 2 -
> block/block-backend.c | 4 +-
> block/bochs.c | 46 ++++---
> block/cloop.c | 33 ++---
> block/dmg.c | 36 +++---
> block/io.c | 317 +++++++++++++++++++--------------------------
> block/iscsi.c | 8 --
> block/nbd.c | 9 --
> block/raw_bsd.c | 12 +-
> block/vdi.c | 127 ++++++++++--------
> block/vmdk.c | 320 ++++++++++++++++++++++++++++------------------
> block/vpc.c | 165 ++++++++++++------------
> block/vvfat.c | 55 ++++++--
> hw/ide/macio.c | 4 +-
> include/block/block_int.h | 12 +-
> 15 files changed, 609 insertions(+), 541 deletions(-)
The idea to support byte-granularity requests is interesting. Seems
like there is not much stopping that anymore.
Please check all block driver changes to make sure the CoMutex is
unlocked in error return code paths. I only pointed it out the first
few times but there are more instances later in the series with missing
unlocks.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH 06/17] bochs: Implement .bdrv_co_preadv() interface
2016-04-27 14:06 ` Stefan Hajnoczi
@ 2016-04-27 14:33 ` Kevin Wolf
0 siblings, 0 replies; 34+ messages in thread
From: Kevin Wolf @ 2016-04-27 14:33 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: qemu-block, mreitz, famz, sw, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1620 bytes --]
Am 27.04.2016 um 16:06 hat Stefan Hajnoczi geschrieben:
> On Wed, Apr 27, 2016 at 11:52:36AM +0200, Kevin Wolf wrote:
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > ---
> > block/bochs.c | 46 +++++++++++++++++++++++++++++-----------------
> > 1 file changed, 29 insertions(+), 17 deletions(-)
> >
> > diff --git a/block/bochs.c b/block/bochs.c
> > index af8b7ab..d148454 100644
> > --- a/block/bochs.c
> > +++ b/block/bochs.c
> > @@ -104,6 +104,7 @@ static int bochs_open(BlockDriverState *bs, QDict *options, int flags,
> > int ret;
> >
> > bs->read_only = 1; // no write support yet
> > + bs->request_alignment = BDRV_SECTOR_SIZE; /* No sub-sector I/O supported */
>
> Can this be the default in block.c? Drivers that have other alignment
> characteristics can set it explicitly but most drivers will want
> BDRV_SECTOR_SIZE so it's nice to make it common code.
I absolutely don't expect BDRV_SECTOR_SIZE to be what most drivers will
want. It's basically a sign that a driver is either a protocol that
imposes restrictions (like O_DIRECT on raw-posix) or it is broken and we
don't care enough about the emulation overhead to fix it (like this one).
The expected value for most block drivers is 1.
> > while (nb_sectors > 0) {
> > int64_t block_offset = seek_to_sector(bs, sector_num);
> > if (block_offset < 0) {
> > return block_offset;
>
> s->lock must be unlocked.
I can't believe I messed this up in so many places. Thanks for catching
this, I'll have to go through all patches and check the locking.
Kevin
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH 04/17] block: Rename bdrv_co_do_preadv/writev to bdrv_co_preadv/writev
2016-04-27 9:52 ` [Qemu-devel] [PATCH 04/17] block: Rename bdrv_co_do_preadv/writev to bdrv_co_preadv/writev Kevin Wolf
@ 2016-04-27 14:34 ` Eric Blake
2016-04-27 14:40 ` Kevin Wolf
0 siblings, 1 reply; 34+ messages in thread
From: Eric Blake @ 2016-04-27 14:34 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: famz, sw, qemu-devel, mreitz, stefanha
[-- Attachment #1: Type: text/plain, Size: 2768 bytes --]
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>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH 10/17] vdi: Implement .bdrv_co_pwritev() interface
2016-04-27 14:17 ` Stefan Hajnoczi
@ 2016-04-27 14:36 ` Kevin Wolf
0 siblings, 0 replies; 34+ messages in thread
From: Kevin Wolf @ 2016-04-27 14:36 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: qemu-block, mreitz, famz, sw, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 763 bytes --]
Am 27.04.2016 um 16:17 hat Stefan Hajnoczi geschrieben:
> On Wed, Apr 27, 2016 at 11:52:40AM +0200, Kevin Wolf wrote:
> > @@ -703,6 +712,7 @@ static int vdi_co_write(BlockDriverState *bs,
> > VdiHeader *header = (VdiHeader *) block;
> > uint8_t *base;
> > uint64_t offset;
> > + uint32_t n_sectors;
> >
> > logout("now writing modified header\n");
> > assert(VDI_IS_ALLOCATED(bmap_first));
>
> Unnecessary change?
No, I removed a top-level n_sectors declaration because I wanted the
compiler to complain about any uses in the main part, which should be
byte-aligned now. This final block still uses some sector arithmetics,
though, and therefore needs to reintroduce the variable.
Kevin
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH 04/17] block: Rename bdrv_co_do_preadv/writev to bdrv_co_preadv/writev
2016-04-27 14:34 ` Eric Blake
@ 2016-04-27 14:40 ` Kevin Wolf
0 siblings, 0 replies; 34+ messages in thread
From: Kevin Wolf @ 2016-04-27 14:40 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-block, famz, sw, qemu-devel, mreitz, stefanha
[-- Attachment #1: Type: text/plain, Size: 1301 bytes --]
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
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH 05/17] block: Introduce .bdrv_co_preadv/pwritev BlockDriver function
2016-04-27 9:52 ` [Qemu-devel] [PATCH 05/17] block: Introduce .bdrv_co_preadv/pwritev BlockDriver function Kevin Wolf
@ 2016-04-27 15:44 ` Eric Blake
0 siblings, 0 replies; 34+ messages in thread
From: Eric Blake @ 2016-04-27 15:44 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: famz, sw, qemu-devel, mreitz, stefanha
[-- Attachment #1: Type: text/plain, Size: 780 bytes --]
On 04/27/2016 03:52 AM, Kevin Wolf wrote:
> Many parts of the block layer are already byte granularity. The block
> driver interface, however, was still missing an interface that allows
> making use of this. This patch introduces a new BlockDriver interface,
> which is based on coroutines, vectored, has flags and uses a byte
> granularity. This is now the preferred interface for new drivers.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> block/io.c | 28 ++++++++++++++++++++++------
> include/block/block_int.h | 4 ++++
> 2 files changed, 26 insertions(+), 6 deletions(-)
>
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH 06/17] bochs: Implement .bdrv_co_preadv() interface
2016-04-27 9:52 ` [Qemu-devel] [PATCH 06/17] bochs: Implement .bdrv_co_preadv() interface Kevin Wolf
2016-04-27 14:06 ` Stefan Hajnoczi
@ 2016-04-27 15:51 ` Eric Blake
2016-04-28 8:21 ` Kevin Wolf
1 sibling, 1 reply; 34+ messages in thread
From: Eric Blake @ 2016-04-27 15:51 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: famz, sw, qemu-devel, mreitz, stefanha
[-- Attachment #1: Type: text/plain, Size: 839 bytes --]
On 04/27/2016 03:52 AM, Kevin Wolf wrote:
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> block/bochs.c | 46 +++++++++++++++++++++++++++++-----------------
> 1 file changed, 29 insertions(+), 17 deletions(-)
>
> static void bochs_close(BlockDriverState *bs)
> @@ -267,7 +279,7 @@ static BlockDriver bdrv_bochs = {
> .instance_size = sizeof(BDRVBochsState),
> .bdrv_probe = bochs_probe,
> .bdrv_open = bochs_open,
> - .bdrv_read = bochs_co_read,
> + .bdrv_co_preadv = bochs_co_preadv,
> .bdrv_close = bochs_close,
> };
Alignment is funky here. I'd rather just get rid of all the extra
spaces, if that's easier than having half but not all of the = aligned.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH 06/17] bochs: Implement .bdrv_co_preadv() interface
2016-04-27 15:51 ` Eric Blake
@ 2016-04-28 8:21 ` Kevin Wolf
2016-04-28 8:42 ` Markus Armbruster
0 siblings, 1 reply; 34+ messages in thread
From: Kevin Wolf @ 2016-04-28 8:21 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-block, famz, sw, qemu-devel, mreitz, stefanha
[-- Attachment #1: Type: text/plain, Size: 1219 bytes --]
Am 27.04.2016 um 17:51 hat Eric Blake geschrieben:
> On 04/27/2016 03:52 AM, Kevin Wolf wrote:
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > ---
> > block/bochs.c | 46 +++++++++++++++++++++++++++++-----------------
> > 1 file changed, 29 insertions(+), 17 deletions(-)
> >
>
> > static void bochs_close(BlockDriverState *bs)
> > @@ -267,7 +279,7 @@ static BlockDriver bdrv_bochs = {
> > .instance_size = sizeof(BDRVBochsState),
> > .bdrv_probe = bochs_probe,
> > .bdrv_open = bochs_open,
> > - .bdrv_read = bochs_co_read,
> > + .bdrv_co_preadv = bochs_co_preadv,
> > .bdrv_close = bochs_close,
> > };
>
> Alignment is funky here. I'd rather just get rid of all the extra
> spaces, if that's easier than having half but not all of the = aligned.
Alignment is funky by definition when there are tabs involved and new
code follows the coding style and uses spaces instead. With a tab stop
of 4, this is aligned correctly.
The other option would be to just convert the whole struct to spaces,
but that's not really related to this patch and I think we're avoiding
pure style cleanup patches to keep things like 'git blame' useful.
Kevin
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH 06/17] bochs: Implement .bdrv_co_preadv() interface
2016-04-28 8:21 ` Kevin Wolf
@ 2016-04-28 8:42 ` Markus Armbruster
0 siblings, 0 replies; 34+ messages in thread
From: Markus Armbruster @ 2016-04-28 8:42 UTC (permalink / raw)
To: Kevin Wolf; +Cc: Eric Blake, famz, qemu-block, sw, qemu-devel, mreitz, stefanha
Kevin Wolf <kwolf@redhat.com> writes:
> Am 27.04.2016 um 17:51 hat Eric Blake geschrieben:
>> On 04/27/2016 03:52 AM, Kevin Wolf wrote:
>> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
>> > ---
>> > block/bochs.c | 46 +++++++++++++++++++++++++++++-----------------
>> > 1 file changed, 29 insertions(+), 17 deletions(-)
>> >
>>
>> > static void bochs_close(BlockDriverState *bs)
>> > @@ -267,7 +279,7 @@ static BlockDriver bdrv_bochs = {
>> > .instance_size = sizeof(BDRVBochsState),
>> > .bdrv_probe = bochs_probe,
>> > .bdrv_open = bochs_open,
>> > - .bdrv_read = bochs_co_read,
>> > + .bdrv_co_preadv = bochs_co_preadv,
>> > .bdrv_close = bochs_close,
>> > };
>>
>> Alignment is funky here. I'd rather just get rid of all the extra
>> spaces, if that's easier than having half but not all of the = aligned.
>
> Alignment is funky by definition when there are tabs involved and new
> code follows the coding style and uses spaces instead. With a tab stop
> of 4, this is aligned correctly.
Four? It comes out correctly with tab-width 8 for me.
You can have any tab stops you want as long as it's multiples of eight.
> The other option would be to just convert the whole struct to spaces,
> but that's not really related to this patch and I think we're avoiding
> pure style cleanup patches to keep things like 'git blame' useful.
We normally untabify the lines we touch anyway, and leave the rest
alone.
^ permalink raw reply [flat|nested] 34+ messages in thread
end of thread, other threads:[~2016-04-28 8:42 UTC | newest]
Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-27 9:52 [Qemu-devel] [PATCH 00/17] block: Convert .bdrv_read/write to .bdrv_preadv/pwritev Kevin Wolf
2016-04-27 9:52 ` [Qemu-devel] [PATCH 01/17] block: Introduce bdrv_driver_preadv() Kevin Wolf
2016-04-27 13:52 ` Eric Blake
2016-04-27 9:52 ` [Qemu-devel] [PATCH 02/17] block: Introduce bdrv_driver_pwritev() Kevin Wolf
2016-04-27 14:03 ` Eric Blake
2016-04-27 9:52 ` [Qemu-devel] [PATCH 03/17] block: Support AIO drivers in bdrv_driver_preadv/pwritev() Kevin Wolf
2016-04-27 14:13 ` Eric Blake
2016-04-27 9:52 ` [Qemu-devel] [PATCH 04/17] block: Rename bdrv_co_do_preadv/writev to bdrv_co_preadv/writev Kevin Wolf
2016-04-27 14:34 ` Eric Blake
2016-04-27 14:40 ` Kevin Wolf
2016-04-27 9:52 ` [Qemu-devel] [PATCH 05/17] block: Introduce .bdrv_co_preadv/pwritev BlockDriver function Kevin Wolf
2016-04-27 15:44 ` Eric Blake
2016-04-27 9:52 ` [Qemu-devel] [PATCH 06/17] bochs: Implement .bdrv_co_preadv() interface Kevin Wolf
2016-04-27 14:06 ` Stefan Hajnoczi
2016-04-27 14:33 ` Kevin Wolf
2016-04-27 15:51 ` Eric Blake
2016-04-28 8:21 ` Kevin Wolf
2016-04-28 8:42 ` Markus Armbruster
2016-04-27 9:52 ` [Qemu-devel] [PATCH 07/17] cloop: " Kevin Wolf
2016-04-27 14:12 ` Stefan Hajnoczi
2016-04-27 9:52 ` [Qemu-devel] [PATCH 08/17] dmg: " Kevin Wolf
2016-04-27 9:52 ` [Qemu-devel] [PATCH 09/17] vdi: " Kevin Wolf
2016-04-27 9:52 ` [Qemu-devel] [PATCH 10/17] vdi: Implement .bdrv_co_pwritev() interface Kevin Wolf
2016-04-27 14:17 ` Stefan Hajnoczi
2016-04-27 14:36 ` Kevin Wolf
2016-04-27 9:52 ` [Qemu-devel] [PATCH 11/17] vmdk: Add vmdk_find_offset_in_cluster() Kevin Wolf
2016-04-27 9:52 ` [Qemu-devel] [PATCH 12/17] vmdk: Implement .bdrv_co_preadv() interface Kevin Wolf
2016-04-27 9:52 ` [Qemu-devel] [PATCH 13/17] vmdk: Implement .bdrv_co_pwritev() interface Kevin Wolf
2016-04-27 14:21 ` Stefan Hajnoczi
2016-04-27 9:52 ` [Qemu-devel] [PATCH 14/17] vpc: Implement .bdrv_co_preadv() interface Kevin Wolf
2016-04-27 9:52 ` [Qemu-devel] [PATCH 15/17] vpc: Implement .bdrv_co_pwritev() interface Kevin Wolf
2016-04-27 9:52 ` [Qemu-devel] [PATCH 16/17] vvfat: Implement .bdrv_co_preadv/pwritev interfaces Kevin Wolf
2016-04-27 9:52 ` [Qemu-devel] [PATCH 17/17] block: Remove BlockDriver.bdrv_read/write Kevin Wolf
2016-04-27 14:26 ` [Qemu-devel] [PATCH 00/17] block: Convert .bdrv_read/write to .bdrv_preadv/pwritev Stefan Hajnoczi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.