* [PATCH for-5.0? 0/9] block/io: safer inc/dec in_flight sections
@ 2020-04-08 9:30 Vladimir Sementsov-Ogievskiy
2020-04-08 9:30 ` [PATCH 1/9] block/io: refactor bdrv_is_allocated_above to run only one coroutine Vladimir Sementsov-Ogievskiy
` (12 more replies)
0 siblings, 13 replies; 27+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-04-08 9:30 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, fam, vsementsov, qemu-devel, mreitz, stefanha, den
Hi all!
This is inspired by Kevin's
"block: Fix blk->in_flight during blk_wait_while_drained()" series.
So, like it's now done for block-backends, let's expand
in_flight-protected sections for bdrv_ interfaces, including
coroutine_enter and BDRV_POLL_WHILE loop into these sections.
Vladimir Sementsov-Ogievskiy (9):
block/io: refactor bdrv_is_allocated_above to run only one coroutine
block/io: refactor bdrv_co_ioctl: move aio stuff to corresponding
block
block/io: move flush and pdiscard stuff down
block/io: move bdrv_rw_co_entry and friends down
block/io: expand in_flight inc/dec section: simple cases
block/io: expand in_flight inc/dec section: block-status
block/io: add bdrv_do_pwrite_zeroes
block/io: move bdrv_make_zero under block-status
block/io: expand in_flight inc/dec section: bdrv_make_zero
block/io.c | 780 +++++++++++++++++++++++++++++++++++------------------
1 file changed, 516 insertions(+), 264 deletions(-)
--
2.21.0
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 1/9] block/io: refactor bdrv_is_allocated_above to run only one coroutine
2020-04-08 9:30 [PATCH for-5.0? 0/9] block/io: safer inc/dec in_flight sections Vladimir Sementsov-Ogievskiy
@ 2020-04-08 9:30 ` Vladimir Sementsov-Ogievskiy
2020-04-20 15:56 ` Stefan Hajnoczi
2020-04-08 9:30 ` [PATCH 2/9] block/io: refactor bdrv_co_ioctl: move aio stuff to corresponding block Vladimir Sementsov-Ogievskiy
` (11 subsequent siblings)
12 siblings, 1 reply; 27+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-04-08 9:30 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, fam, vsementsov, qemu-devel, mreitz, stefanha, den
bdrv_is_allocated_above creates new coroutine on each iteration if
called from non-coroutine context. To simplify expansion of in_flight
inc/dec sections in further patch let's refactor it.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block/io.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 72 insertions(+), 5 deletions(-)
diff --git a/block/io.c b/block/io.c
index aba67f66b9..a9f1a3e93e 100644
--- a/block/io.c
+++ b/block/io.c
@@ -2482,6 +2482,22 @@ static int coroutine_fn bdrv_co_block_status_above(BlockDriverState *bs,
return ret;
}
+static int coroutine_fn bdrv_co_is_allocated(BlockDriverState *bs,
+ int64_t offset, int64_t bytes,
+ int64_t *pnum)
+{
+ int ret;
+ int64_t dummy;
+
+ ret = bdrv_co_block_status_above(bs, backing_bs(bs), false, offset,
+ bytes, pnum ? pnum : &dummy, NULL,
+ NULL);
+ if (ret < 0) {
+ return ret;
+ }
+ return !!(ret & BDRV_BLOCK_ALLOCATED);
+}
+
/* Coroutine wrapper for bdrv_block_status_above() */
static void coroutine_fn bdrv_block_status_above_co_entry(void *opaque)
{
@@ -2578,10 +2594,10 @@ int coroutine_fn bdrv_is_allocated(BlockDriverState *bs, int64_t offset,
* but 'pnum' will only be 0 when end of file is reached.
*
*/
-int bdrv_is_allocated_above(BlockDriverState *top,
- BlockDriverState *base,
- bool include_base, int64_t offset,
- int64_t bytes, int64_t *pnum)
+static int coroutine_fn
+bdrv_co_is_allocated_above(BlockDriverState *top, BlockDriverState *base,
+ bool include_base, int64_t offset, int64_t bytes,
+ int64_t *pnum)
{
BlockDriverState *intermediate;
int ret;
@@ -2595,7 +2611,7 @@ int bdrv_is_allocated_above(BlockDriverState *top,
int64_t size_inter;
assert(intermediate);
- ret = bdrv_is_allocated(intermediate, offset, bytes, &pnum_inter);
+ ret = bdrv_co_is_allocated(intermediate, offset, bytes, &pnum_inter);
if (ret < 0) {
return ret;
}
@@ -2624,6 +2640,57 @@ int bdrv_is_allocated_above(BlockDriverState *top,
return 0;
}
+typedef struct BdrvCoIsAllocatedAboveData {
+ BlockDriverState *top;
+ BlockDriverState *base;
+ bool include_base;
+ int64_t offset;
+ int64_t bytes;
+ int64_t *pnum;
+ int ret;
+ bool done;
+} BdrvCoIsAllocatedAboveData;
+
+static void coroutine_fn bdrv_is_allocated_above_co_entry(void *opaque)
+{
+ BdrvCoIsAllocatedAboveData *data = opaque;
+
+ data->ret = bdrv_co_is_allocated_above(data->top, data->base,
+ data->include_base,
+ data->offset, data->bytes,
+ data->pnum);
+ data->done = true;
+ aio_wait_kick();
+}
+
+int coroutine_fn
+bdrv_is_allocated_above(BlockDriverState *top, BlockDriverState *base,
+ bool include_base, int64_t offset, int64_t bytes,
+ int64_t *pnum)
+{
+ Coroutine *co;
+ BdrvCoIsAllocatedAboveData data = {
+ .top = top,
+ .base = base,
+ .include_base = include_base,
+ .offset = offset,
+ .bytes = bytes,
+ .pnum = pnum,
+ .done = false,
+ };
+
+ if (qemu_in_coroutine()) {
+ /* Fast-path if already in coroutine context */
+ bdrv_is_allocated_above_co_entry(&data);
+ } else {
+ co = qemu_coroutine_create(bdrv_is_allocated_above_co_entry, &data);
+ bdrv_coroutine_enter(top, co);
+ BDRV_POLL_WHILE(top, !data.done);
+ }
+
+ return data.ret;
+}
+
typedef struct BdrvVmstateCo {
BlockDriverState *bs;
QEMUIOVector *qiov;
--
2.21.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 2/9] block/io: refactor bdrv_co_ioctl: move aio stuff to corresponding block
2020-04-08 9:30 [PATCH for-5.0? 0/9] block/io: safer inc/dec in_flight sections Vladimir Sementsov-Ogievskiy
2020-04-08 9:30 ` [PATCH 1/9] block/io: refactor bdrv_is_allocated_above to run only one coroutine Vladimir Sementsov-Ogievskiy
@ 2020-04-08 9:30 ` Vladimir Sementsov-Ogievskiy
2020-04-20 15:57 ` Stefan Hajnoczi
2020-04-08 9:30 ` [PATCH 3/9] block/io: move flush and pdiscard stuff down Vladimir Sementsov-Ogievskiy
` (10 subsequent siblings)
12 siblings, 1 reply; 27+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-04-08 9:30 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, fam, vsementsov, qemu-devel, mreitz, stefanha, den
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block/io.c | 23 +++++++++++++++--------
1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/block/io.c b/block/io.c
index a9f1a3e93e..29e53271b6 100644
--- a/block/io.c
+++ b/block/io.c
@@ -3126,31 +3126,38 @@ int bdrv_pdiscard(BdrvChild *child, int64_t offset, int64_t bytes)
int bdrv_co_ioctl(BlockDriverState *bs, int req, void *buf)
{
+ int ret;
BlockDriver *drv = bs->drv;
- CoroutineIOCompletion co = {
- .coroutine = qemu_coroutine_self(),
- };
- BlockAIOCB *acb;
bdrv_inc_in_flight(bs);
+
if (!drv || (!drv->bdrv_aio_ioctl && !drv->bdrv_co_ioctl)) {
- co.ret = -ENOTSUP;
+ ret = -ENOTSUP;
goto out;
}
if (drv->bdrv_co_ioctl) {
- co.ret = drv->bdrv_co_ioctl(bs, req, buf);
+ ret = drv->bdrv_co_ioctl(bs, req, buf);
} else {
+ CoroutineIOCompletion co = {
+ .coroutine = qemu_coroutine_self(),
+ };
+ BlockAIOCB *acb;
+
acb = drv->bdrv_aio_ioctl(bs, req, buf, bdrv_co_io_em_complete, &co);
if (!acb) {
- co.ret = -ENOTSUP;
+ ret = -ENOTSUP;
goto out;
}
+
qemu_coroutine_yield();
+ ret = co.ret;
}
+
out:
bdrv_dec_in_flight(bs);
- return co.ret;
+
+ return ret;
}
void *qemu_blockalign(BlockDriverState *bs, size_t size)
--
2.21.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 3/9] block/io: move flush and pdiscard stuff down
2020-04-08 9:30 [PATCH for-5.0? 0/9] block/io: safer inc/dec in_flight sections Vladimir Sementsov-Ogievskiy
2020-04-08 9:30 ` [PATCH 1/9] block/io: refactor bdrv_is_allocated_above to run only one coroutine Vladimir Sementsov-Ogievskiy
2020-04-08 9:30 ` [PATCH 2/9] block/io: refactor bdrv_co_ioctl: move aio stuff to corresponding block Vladimir Sementsov-Ogievskiy
@ 2020-04-08 9:30 ` Vladimir Sementsov-Ogievskiy
2020-04-20 16:01 ` Stefan Hajnoczi
2020-04-08 9:30 ` [PATCH 4/9] block/io: move bdrv_rw_co_entry and friends down Vladimir Sementsov-Ogievskiy
` (9 subsequent siblings)
12 siblings, 1 reply; 27+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-04-08 9:30 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, fam, vsementsov, qemu-devel, mreitz, stefanha, den
bdrv_co_flush and bdrv_co_pdiscard will become static in further patch,
move their usage down.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block/io.c | 56 +++++++++++++++++++++++++++---------------------------
1 file changed, 28 insertions(+), 28 deletions(-)
diff --git a/block/io.c b/block/io.c
index 29e53271b6..379e86eeb5 100644
--- a/block/io.c
+++ b/block/io.c
@@ -2828,20 +2828,6 @@ void bdrv_aio_cancel_async(BlockAIOCB *acb)
/**************************************************************/
/* Coroutine block device emulation */
-typedef struct FlushCo {
- BlockDriverState *bs;
- int ret;
-} FlushCo;
-
-
-static void coroutine_fn bdrv_flush_co_entry(void *opaque)
-{
- FlushCo *rwco = opaque;
-
- rwco->ret = bdrv_co_flush(rwco->bs);
- aio_wait_kick();
-}
-
int coroutine_fn bdrv_co_flush(BlockDriverState *bs)
{
int current_gen;
@@ -2954,6 +2940,19 @@ early_exit:
return ret;
}
+typedef struct FlushCo {
+ BlockDriverState *bs;
+ int ret;
+} FlushCo;
+
+static void coroutine_fn bdrv_flush_co_entry(void *opaque)
+{
+ FlushCo *rwco = opaque;
+
+ rwco->ret = bdrv_co_flush(rwco->bs);
+ aio_wait_kick();
+}
+
int bdrv_flush(BlockDriverState *bs)
{
Coroutine *co;
@@ -2974,20 +2973,6 @@ int bdrv_flush(BlockDriverState *bs)
return flush_co.ret;
}
-typedef struct DiscardCo {
- BdrvChild *child;
- int64_t offset;
- int64_t bytes;
- int ret;
-} DiscardCo;
-static void coroutine_fn bdrv_pdiscard_co_entry(void *opaque)
-{
- DiscardCo *rwco = opaque;
-
- rwco->ret = bdrv_co_pdiscard(rwco->child, rwco->offset, rwco->bytes);
- aio_wait_kick();
-}
-
int coroutine_fn bdrv_co_pdiscard(BdrvChild *child, int64_t offset,
int64_t bytes)
{
@@ -3102,6 +3087,21 @@ out:
return ret;
}
+typedef struct DiscardCo {
+ BdrvChild *child;
+ int64_t offset;
+ int64_t bytes;
+ int ret;
+} DiscardCo;
+
+static void coroutine_fn bdrv_pdiscard_co_entry(void *opaque)
+{
+ DiscardCo *rwco = opaque;
+
+ rwco->ret = bdrv_co_pdiscard(rwco->child, rwco->offset, rwco->bytes);
+ aio_wait_kick();
+}
+
int bdrv_pdiscard(BdrvChild *child, int64_t offset, int64_t bytes)
{
Coroutine *co;
--
2.21.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 4/9] block/io: move bdrv_rw_co_entry and friends down
2020-04-08 9:30 [PATCH for-5.0? 0/9] block/io: safer inc/dec in_flight sections Vladimir Sementsov-Ogievskiy
` (2 preceding siblings ...)
2020-04-08 9:30 ` [PATCH 3/9] block/io: move flush and pdiscard stuff down Vladimir Sementsov-Ogievskiy
@ 2020-04-08 9:30 ` Vladimir Sementsov-Ogievskiy
2020-04-20 16:04 ` Stefan Hajnoczi
2020-04-08 9:30 ` [PATCH 5/9] block/io: expand in_flight inc/dec section: simple cases Vladimir Sementsov-Ogievskiy
` (8 subsequent siblings)
12 siblings, 1 reply; 27+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-04-08 9:30 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, fam, vsementsov, qemu-devel, mreitz, stefanha, den
We are going to use bdrv_co_pwritev_part and bdrv_co_preadv_part in
bdrv_rw_co_entry, so move it down.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block/io.c | 361 +++++++++++++++++++++++++++--------------------------
1 file changed, 181 insertions(+), 180 deletions(-)
diff --git a/block/io.c b/block/io.c
index 379e86eeb5..dfbe68f428 100644
--- a/block/io.c
+++ b/block/io.c
@@ -891,186 +891,6 @@ static int bdrv_check_byte_request(BlockDriverState *bs, int64_t offset,
return 0;
}
-typedef struct RwCo {
- BdrvChild *child;
- int64_t offset;
- QEMUIOVector *qiov;
- bool is_write;
- int ret;
- BdrvRequestFlags flags;
-} RwCo;
-
-static void coroutine_fn bdrv_rw_co_entry(void *opaque)
-{
- RwCo *rwco = opaque;
-
- if (!rwco->is_write) {
- rwco->ret = bdrv_co_preadv(rwco->child, rwco->offset,
- rwco->qiov->size, rwco->qiov,
- rwco->flags);
- } else {
- rwco->ret = bdrv_co_pwritev(rwco->child, rwco->offset,
- rwco->qiov->size, rwco->qiov,
- rwco->flags);
- }
- aio_wait_kick();
-}
-
-/*
- * Process a vectored synchronous request using coroutines
- */
-static int bdrv_prwv_co(BdrvChild *child, int64_t offset,
- QEMUIOVector *qiov, bool is_write,
- BdrvRequestFlags flags)
-{
- Coroutine *co;
- RwCo rwco = {
- .child = child,
- .offset = offset,
- .qiov = qiov,
- .is_write = is_write,
- .ret = NOT_DONE,
- .flags = flags,
- };
-
- if (qemu_in_coroutine()) {
- /* Fast-path if already in coroutine context */
- bdrv_rw_co_entry(&rwco);
- } else {
- co = qemu_coroutine_create(bdrv_rw_co_entry, &rwco);
- bdrv_coroutine_enter(child->bs, co);
- BDRV_POLL_WHILE(child->bs, rwco.ret == NOT_DONE);
- }
- return rwco.ret;
-}
-
-int bdrv_pwrite_zeroes(BdrvChild *child, int64_t offset,
- int bytes, BdrvRequestFlags flags)
-{
- QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, NULL, bytes);
-
- return bdrv_prwv_co(child, offset, &qiov, true,
- BDRV_REQ_ZERO_WRITE | flags);
-}
-
-/*
- * Completely zero out a block device with the help of bdrv_pwrite_zeroes.
- * The operation is sped up by checking the block status and only writing
- * zeroes to the device if they currently do not return zeroes. Optional
- * flags are passed through to bdrv_pwrite_zeroes (e.g. BDRV_REQ_MAY_UNMAP,
- * BDRV_REQ_FUA).
- *
- * Returns < 0 on error, 0 on success. For error codes see bdrv_write().
- */
-int bdrv_make_zero(BdrvChild *child, BdrvRequestFlags flags)
-{
- int ret;
- int64_t target_size, bytes, offset = 0;
- BlockDriverState *bs = child->bs;
-
- target_size = bdrv_getlength(bs);
- if (target_size < 0) {
- return target_size;
- }
-
- for (;;) {
- bytes = MIN(target_size - offset, BDRV_REQUEST_MAX_BYTES);
- if (bytes <= 0) {
- return 0;
- }
- ret = bdrv_block_status(bs, offset, bytes, &bytes, NULL, NULL);
- if (ret < 0) {
- return ret;
- }
- if (ret & BDRV_BLOCK_ZERO) {
- offset += bytes;
- continue;
- }
- ret = bdrv_pwrite_zeroes(child, offset, bytes, flags);
- if (ret < 0) {
- return ret;
- }
- offset += bytes;
- }
-}
-
-int bdrv_preadv(BdrvChild *child, int64_t offset, QEMUIOVector *qiov)
-{
- int ret;
-
- ret = bdrv_prwv_co(child, offset, qiov, false, 0);
- if (ret < 0) {
- return ret;
- }
-
- return qiov->size;
-}
-
-/* See bdrv_pwrite() for the return codes */
-int bdrv_pread(BdrvChild *child, int64_t offset, void *buf, int bytes)
-{
- QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, buf, bytes);
-
- if (bytes < 0) {
- return -EINVAL;
- }
-
- return bdrv_preadv(child, offset, &qiov);
-}
-
-int bdrv_pwritev(BdrvChild *child, int64_t offset, QEMUIOVector *qiov)
-{
- int ret;
-
- ret = bdrv_prwv_co(child, offset, qiov, true, 0);
- if (ret < 0) {
- return ret;
- }
-
- return qiov->size;
-}
-
-/* Return no. of bytes on success or < 0 on error. Important errors are:
- -EIO generic I/O error (may happen for all errors)
- -ENOMEDIUM No media inserted.
- -EINVAL Invalid offset or number of bytes
- -EACCES Trying to write a read-only device
-*/
-int bdrv_pwrite(BdrvChild *child, int64_t offset, const void *buf, int bytes)
-{
- QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, buf, bytes);
-
- if (bytes < 0) {
- return -EINVAL;
- }
-
- return bdrv_pwritev(child, offset, &qiov);
-}
-
-/*
- * Writes to the file and ensures that no writes are reordered across this
- * request (acts as a barrier)
- *
- * Returns 0 on success, -errno in error cases.
- */
-int bdrv_pwrite_sync(BdrvChild *child, int64_t offset,
- const void *buf, int count)
-{
- int ret;
-
- ret = bdrv_pwrite(child, offset, buf, count);
- if (ret < 0) {
- return ret;
- }
-
- ret = bdrv_flush(child->bs);
- if (ret < 0) {
- return ret;
- }
-
- return 0;
-}
-
typedef struct CoroutineIOCompletion {
Coroutine *coroutine;
int ret;
@@ -2185,6 +2005,187 @@ int coroutine_fn bdrv_co_pwrite_zeroes(BdrvChild *child, int64_t offset,
BDRV_REQ_ZERO_WRITE | flags);
}
+typedef struct RwCo {
+ BdrvChild *child;
+ int64_t offset;
+ QEMUIOVector *qiov;
+ bool is_write;
+ int ret;
+ BdrvRequestFlags flags;
+} RwCo;
+
+static void coroutine_fn bdrv_rw_co_entry(void *opaque)
+{
+ RwCo *rwco = opaque;
+
+ if (!rwco->is_write) {
+ rwco->ret = bdrv_co_preadv(rwco->child, rwco->offset,
+ rwco->qiov->size, rwco->qiov,
+ rwco->flags);
+ } else {
+ rwco->ret = bdrv_co_pwritev(rwco->child, rwco->offset,
+ rwco->qiov->size, rwco->qiov,
+ rwco->flags);
+ }
+ aio_wait_kick();
+}
+
+/*
+ * Process a vectored synchronous request using coroutines
+ */
+static int bdrv_prwv_co(BdrvChild *child, int64_t offset,
+ QEMUIOVector *qiov, bool is_write,
+ BdrvRequestFlags flags)
+{
+ Coroutine *co;
+ RwCo rwco = {
+ .child = child,
+ .offset = offset,
+ .qiov = qiov,
+ .is_write = is_write,
+ .ret = NOT_DONE,
+ .flags = flags,
+ };
+
+ if (qemu_in_coroutine()) {
+ /* Fast-path if already in coroutine context */
+ bdrv_rw_co_entry(&rwco);
+ } else {
+ co = qemu_coroutine_create(bdrv_rw_co_entry, &rwco);
+ bdrv_coroutine_enter(child->bs, co);
+ BDRV_POLL_WHILE(child->bs, rwco.ret == NOT_DONE);
+ }
+ return rwco.ret;
+}
+
+int bdrv_pwrite_zeroes(BdrvChild *child, int64_t offset,
+ int bytes, BdrvRequestFlags flags)
+{
+ QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, NULL, bytes);
+
+ return bdrv_prwv_co(child, offset, &qiov, true,
+ BDRV_REQ_ZERO_WRITE | flags);
+}
+
+/* See bdrv_pwrite() for the return codes */
+int bdrv_pread(BdrvChild *child, int64_t offset, void *buf, int bytes)
+{
+ QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, buf, bytes);
+
+ if (bytes < 0) {
+ return -EINVAL;
+ }
+
+ return bdrv_preadv(child, offset, &qiov);
+}
+
+int bdrv_pwritev(BdrvChild *child, int64_t offset, QEMUIOVector *qiov)
+{
+ int ret;
+
+ ret = bdrv_prwv_co(child, offset, qiov, true, 0);
+ if (ret < 0) {
+ return ret;
+ }
+
+ return qiov->size;
+}
+
+/*
+ * Return no. of bytes on success or < 0 on error. Important errors are:
+ * -EIO generic I/O error (may happen for all errors)
+ * -ENOMEDIUM No media inserted.
+ * -EINVAL Invalid offset or number of bytes
+ * -EACCES Trying to write a read-only device
+ */
+int bdrv_pwrite(BdrvChild *child, int64_t offset, const void *buf, int bytes)
+{
+ QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, buf, bytes);
+
+ if (bytes < 0) {
+ return -EINVAL;
+ }
+
+ return bdrv_pwritev(child, offset, &qiov);
+}
+
+/*
+ * Writes to the file and ensures that no writes are reordered across this
+ * request (acts as a barrier)
+ *
+ * Returns 0 on success, -errno in error cases.
+ */
+int bdrv_pwrite_sync(BdrvChild *child, int64_t offset,
+ const void *buf, int count)
+{
+ int ret;
+
+ ret = bdrv_pwrite(child, offset, buf, count);
+ if (ret < 0) {
+ return ret;
+ }
+
+ ret = bdrv_flush(child->bs);
+ if (ret < 0) {
+ return ret;
+ }
+
+ return 0;
+}
+
+/*
+ * Completely zero out a block device with the help of bdrv_pwrite_zeroes.
+ * The operation is sped up by checking the block status and only writing
+ * zeroes to the device if they currently do not return zeroes. Optional
+ * flags are passed through to bdrv_pwrite_zeroes (e.g. BDRV_REQ_MAY_UNMAP,
+ * BDRV_REQ_FUA).
+ *
+ * Returns < 0 on error, 0 on success. For error codes see bdrv_write().
+ */
+int bdrv_make_zero(BdrvChild *child, BdrvRequestFlags flags)
+{
+ int ret;
+ int64_t target_size, bytes, offset = 0;
+ BlockDriverState *bs = child->bs;
+
+ target_size = bdrv_getlength(bs);
+ if (target_size < 0) {
+ return target_size;
+ }
+
+ for (;;) {
+ bytes = MIN(target_size - offset, BDRV_REQUEST_MAX_BYTES);
+ if (bytes <= 0) {
+ return 0;
+ }
+ ret = bdrv_block_status(bs, offset, bytes, &bytes, NULL, NULL);
+ if (ret < 0) {
+ return ret;
+ }
+ if (ret & BDRV_BLOCK_ZERO) {
+ offset += bytes;
+ continue;
+ }
+ ret = bdrv_pwrite_zeroes(child, offset, bytes, flags);
+ if (ret < 0) {
+ return ret;
+ }
+ offset += bytes;
+ }
+}
+
+int bdrv_preadv(BdrvChild *child, int64_t offset, QEMUIOVector *qiov)
+{
+ int ret;
+
+ ret = bdrv_prwv_co(child, offset, qiov, false, 0);
+ if (ret < 0) {
+ return ret;
+ }
+
+ return qiov->size;
+}
+
/*
* Flush ALL BDSes regardless of if they are reachable via a BlkBackend or not.
*/
--
2.21.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 5/9] block/io: expand in_flight inc/dec section: simple cases
2020-04-08 9:30 [PATCH for-5.0? 0/9] block/io: safer inc/dec in_flight sections Vladimir Sementsov-Ogievskiy
` (3 preceding siblings ...)
2020-04-08 9:30 ` [PATCH 4/9] block/io: move bdrv_rw_co_entry and friends down Vladimir Sementsov-Ogievskiy
@ 2020-04-08 9:30 ` Vladimir Sementsov-Ogievskiy
2020-04-20 16:22 ` Stefan Hajnoczi
2020-04-08 9:30 ` [PATCH 6/9] block/io: expand in_flight inc/dec section: block-status Vladimir Sementsov-Ogievskiy
` (7 subsequent siblings)
12 siblings, 1 reply; 27+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-04-08 9:30 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, fam, vsementsov, qemu-devel, mreitz, stefanha, den
It's safer to expand in_flight request to start before enter to
coroutine in synchronous wrappers and end after BDRV_POLL_WHILE loop.
Note that qemu_coroutine_enter may only schedule the coroutine in some
circumstances.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block/io.c | 155 ++++++++++++++++++++++++++++++++++++++++-------------
1 file changed, 119 insertions(+), 36 deletions(-)
diff --git a/block/io.c b/block/io.c
index dfbe68f428..9b57c7e422 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1511,7 +1511,8 @@ int coroutine_fn bdrv_co_preadv(BdrvChild *child,
return bdrv_co_preadv_part(child, offset, bytes, qiov, 0, flags);
}
-int coroutine_fn bdrv_co_preadv_part(BdrvChild *child,
+/* To be called between exactly one pair of bdrv_inc/dec_in_flight() */
+static int coroutine_fn bdrv_do_preadv_part(BdrvChild *child,
int64_t offset, unsigned int bytes,
QEMUIOVector *qiov, size_t qiov_offset,
BdrvRequestFlags flags)
@@ -1540,8 +1541,6 @@ int coroutine_fn bdrv_co_preadv_part(BdrvChild *child,
return 0;
}
- bdrv_inc_in_flight(bs);
-
/* Don't do copy-on-read if we read data before write operation */
if (atomic_read(&bs->copy_on_read)) {
flags |= BDRV_REQ_COPY_ON_READ;
@@ -1554,13 +1553,26 @@ int coroutine_fn bdrv_co_preadv_part(BdrvChild *child,
bs->bl.request_alignment,
qiov, qiov_offset, flags);
tracked_request_end(&req);
- bdrv_dec_in_flight(bs);
bdrv_padding_destroy(&pad);
return ret;
}
+int coroutine_fn bdrv_co_preadv_part(BdrvChild *child,
+ int64_t offset, unsigned int bytes,
+ QEMUIOVector *qiov, size_t qiov_offset,
+ BdrvRequestFlags flags)
+{
+ int ret;
+
+ bdrv_inc_in_flight(child->bs);
+ ret = bdrv_do_preadv_part(child, offset, bytes, qiov, qiov_offset, flags);
+ bdrv_dec_in_flight(child->bs);
+
+ return ret;
+}
+
static int coroutine_fn bdrv_co_do_pwrite_zeroes(BlockDriverState *bs,
int64_t offset, int bytes, BdrvRequestFlags flags)
{
@@ -1922,7 +1934,8 @@ int coroutine_fn bdrv_co_pwritev(BdrvChild *child,
return bdrv_co_pwritev_part(child, offset, bytes, qiov, 0, flags);
}
-int coroutine_fn bdrv_co_pwritev_part(BdrvChild *child,
+/* To be called between exactly one pair of bdrv_inc/dec_in_flight() */
+static int coroutine_fn bdrv_do_pwritev_part(BdrvChild *child,
int64_t offset, unsigned int bytes, QEMUIOVector *qiov, size_t qiov_offset,
BdrvRequestFlags flags)
{
@@ -1962,7 +1975,6 @@ int coroutine_fn bdrv_co_pwritev_part(BdrvChild *child,
return 0;
}
- bdrv_inc_in_flight(bs);
/*
* Align write if necessary by performing a read-modify-write cycle.
* Pad qiov with the read parts and be sure to have a tracked request not
@@ -1987,7 +1999,19 @@ int coroutine_fn bdrv_co_pwritev_part(BdrvChild *child,
out:
tracked_request_end(&req);
- bdrv_dec_in_flight(bs);
+
+ return ret;
+}
+
+int coroutine_fn bdrv_co_pwritev_part(BdrvChild *child,
+ int64_t offset, unsigned int bytes, QEMUIOVector *qiov, size_t qiov_offset,
+ BdrvRequestFlags flags)
+{
+ int ret;
+
+ bdrv_inc_in_flight(child->bs);
+ ret = bdrv_do_pwritev_part(child, offset, bytes, qiov, qiov_offset, flags);
+ bdrv_dec_in_flight(child->bs);
return ret;
}
@@ -2019,12 +2043,12 @@ static void coroutine_fn bdrv_rw_co_entry(void *opaque)
RwCo *rwco = opaque;
if (!rwco->is_write) {
- rwco->ret = bdrv_co_preadv(rwco->child, rwco->offset,
- rwco->qiov->size, rwco->qiov,
+ rwco->ret = bdrv_do_preadv_part(rwco->child, rwco->offset,
+ rwco->qiov->size, rwco->qiov, 0,
rwco->flags);
} else {
- rwco->ret = bdrv_co_pwritev(rwco->child, rwco->offset,
- rwco->qiov->size, rwco->qiov,
+ rwco->ret = bdrv_do_pwritev_part(rwco->child, rwco->offset,
+ rwco->qiov->size, rwco->qiov, 0,
rwco->flags);
}
aio_wait_kick();
@@ -2047,6 +2071,8 @@ static int bdrv_prwv_co(BdrvChild *child, int64_t offset,
.flags = flags,
};
+ bdrv_inc_in_flight(child->bs);
+
if (qemu_in_coroutine()) {
/* Fast-path if already in coroutine context */
bdrv_rw_co_entry(&rwco);
@@ -2055,6 +2081,9 @@ static int bdrv_prwv_co(BdrvChild *child, int64_t offset,
bdrv_coroutine_enter(child->bs, co);
BDRV_POLL_WHILE(child->bs, rwco.ret == NOT_DONE);
}
+
+ bdrv_dec_in_flight(child->bs);
+
return rwco.ret;
}
@@ -2700,15 +2729,14 @@ typedef struct BdrvVmstateCo {
int ret;
} BdrvVmstateCo;
+/* To be called between exactly one pair of bdrv_inc/dec_in_flight() */
static int coroutine_fn
-bdrv_co_rw_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos,
+bdrv_do_rw_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos,
bool is_read)
{
BlockDriver *drv = bs->drv;
int ret = -ENOTSUP;
- bdrv_inc_in_flight(bs);
-
if (!drv) {
ret = -ENOMEDIUM;
} else if (drv->bdrv_load_vmstate) {
@@ -2718,17 +2746,18 @@ bdrv_co_rw_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos,
ret = drv->bdrv_save_vmstate(bs, qiov, pos);
}
} else if (bs->file) {
- ret = bdrv_co_rw_vmstate(bs->file->bs, qiov, pos, is_read);
+ bdrv_inc_in_flight(bs->file->bs);
+ ret = bdrv_do_rw_vmstate(bs->file->bs, qiov, pos, is_read);
+ bdrv_dec_in_flight(bs->file->bs);
}
- bdrv_dec_in_flight(bs);
return ret;
}
static void coroutine_fn bdrv_co_rw_vmstate_entry(void *opaque)
{
BdrvVmstateCo *co = opaque;
- co->ret = bdrv_co_rw_vmstate(co->bs, co->qiov, co->pos, co->is_read);
+ co->ret = bdrv_do_rw_vmstate(co->bs, co->qiov, co->pos, co->is_read);
aio_wait_kick();
}
@@ -2736,8 +2765,12 @@ static inline int
bdrv_rw_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos,
bool is_read)
{
+ int ret;
+
+ bdrv_inc_in_flight(bs);
+
if (qemu_in_coroutine()) {
- return bdrv_co_rw_vmstate(bs, qiov, pos, is_read);
+ ret = bdrv_do_rw_vmstate(bs, qiov, pos, is_read);
} else {
BdrvVmstateCo data = {
.bs = bs,
@@ -2750,8 +2783,12 @@ bdrv_rw_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos,
bdrv_coroutine_enter(bs, co);
BDRV_POLL_WHILE(bs, data.ret == -EINPROGRESS);
- return data.ret;
+ ret = data.ret;
}
+
+ bdrv_dec_in_flight(bs);
+
+ return ret;
}
int bdrv_save_vmstate(BlockDriverState *bs, const uint8_t *buf,
@@ -2829,16 +2866,14 @@ void bdrv_aio_cancel_async(BlockAIOCB *acb)
/**************************************************************/
/* Coroutine block device emulation */
-int coroutine_fn bdrv_co_flush(BlockDriverState *bs)
+/* To be called between exactly one pair of bdrv_inc/dec_in_flight() */
+static int coroutine_fn bdrv_do_flush(BlockDriverState *bs)
{
int current_gen;
- int ret = 0;
-
- bdrv_inc_in_flight(bs);
+ int ret;
- if (!bdrv_is_inserted(bs) || bdrv_is_read_only(bs) ||
- bdrv_is_sg(bs)) {
- goto early_exit;
+ if (!bdrv_is_inserted(bs) || bdrv_is_read_only(bs) || bdrv_is_sg(bs)) {
+ return 0;
}
qemu_co_mutex_lock(&bs->reqs_lock);
@@ -2936,8 +2971,17 @@ out:
qemu_co_queue_next(&bs->flush_queue);
qemu_co_mutex_unlock(&bs->reqs_lock);
-early_exit:
+ return ret;
+}
+
+int coroutine_fn bdrv_co_flush(BlockDriverState *bs)
+{
+ int ret;
+
+ bdrv_inc_in_flight(bs);
+ ret = bdrv_do_flush(bs);
bdrv_dec_in_flight(bs);
+
return ret;
}
@@ -2950,7 +2994,7 @@ static void coroutine_fn bdrv_flush_co_entry(void *opaque)
{
FlushCo *rwco = opaque;
- rwco->ret = bdrv_co_flush(rwco->bs);
+ rwco->ret = bdrv_do_flush(rwco->bs);
aio_wait_kick();
}
@@ -2962,6 +3006,8 @@ int bdrv_flush(BlockDriverState *bs)
.ret = NOT_DONE,
};
+ bdrv_inc_in_flight(bs);
+
if (qemu_in_coroutine()) {
/* Fast-path if already in coroutine context */
bdrv_flush_co_entry(&flush_co);
@@ -2971,11 +3017,14 @@ int bdrv_flush(BlockDriverState *bs)
BDRV_POLL_WHILE(bs, flush_co.ret == NOT_DONE);
}
+ bdrv_dec_in_flight(bs);
+
return flush_co.ret;
}
-int coroutine_fn bdrv_co_pdiscard(BdrvChild *child, int64_t offset,
- int64_t bytes)
+/* To be called between exactly one pair of bdrv_inc/dec_in_flight() */
+static int coroutine_fn bdrv_do_pdiscahd(BdrvChild *child, int64_t offset,
+ int64_t bytes)
{
BdrvTrackedRequest req;
int max_pdiscard, ret;
@@ -3013,7 +3062,6 @@ int coroutine_fn bdrv_co_pdiscard(BdrvChild *child, int64_t offset,
head = offset % align;
tail = (offset + bytes) % align;
- bdrv_inc_in_flight(bs);
tracked_request_begin(&req, bs, offset, bytes, BDRV_TRACKED_DISCARD);
ret = bdrv_co_write_req_prepare(child, offset, bytes, &req, 0);
@@ -3084,7 +3132,19 @@ int coroutine_fn bdrv_co_pdiscard(BdrvChild *child, int64_t offset,
out:
bdrv_co_write_req_finish(child, req.offset, req.bytes, &req, ret);
tracked_request_end(&req);
- bdrv_dec_in_flight(bs);
+ return ret;
+}
+
+int coroutine_fn bdrv_co_pdiscard(BdrvChild *child,
+ int64_t offset, int64_t bytes)
+{
+ int ret;
+ BlockDriverState *bs = child->bs;
+
+ bdrv_inc_in_flight(child->bs);
+ ret = bdrv_do_pdiscahd(child, offset, bytes);
+ bdrv_dec_in_flight(child->bs);
+
return ret;
}
@@ -3113,6 +3173,8 @@ int bdrv_pdiscard(BdrvChild *child, int64_t offset, int64_t bytes)
.ret = NOT_DONE,
};
+ bdrv_inc_in_flight(child->bs);
+
if (qemu_in_coroutine()) {
/* Fast-path if already in coroutine context */
bdrv_pdiscard_co_entry(&rwco);
@@ -3122,6 +3184,8 @@ int bdrv_pdiscard(BdrvChild *child, int64_t offset, int64_t bytes)
BDRV_POLL_WHILE(child->bs, rwco.ret == NOT_DONE);
}
+ bdrv_dec_in_flight(child->bs);
+
return rwco.ret;
}
@@ -3412,9 +3476,12 @@ static void bdrv_parent_cb_resize(BlockDriverState *bs)
* If 'exact' is true, the file must be resized to exactly the given
* 'offset'. Otherwise, it is sufficient for the node to be at least
* 'offset' bytes in length.
+ *
+ * To be called between exactly one pair of bdrv_inc/dec_in_flight()
*/
-int coroutine_fn bdrv_co_truncate(BdrvChild *child, int64_t offset, bool exact,
- PreallocMode prealloc, Error **errp)
+static int coroutine_fn bdrv_do_truncate(BdrvChild *child,
+ int64_t offset, bool exact,
+ PreallocMode prealloc, Error **errp)
{
BlockDriverState *bs = child->bs;
BlockDriver *drv = bs->drv;
@@ -3445,7 +3512,6 @@ int coroutine_fn bdrv_co_truncate(BdrvChild *child, int64_t offset, bool exact,
new_bytes = 0;
}
- bdrv_inc_in_flight(bs);
tracked_request_begin(&req, bs, offset - new_bytes, new_bytes,
BDRV_TRACKED_TRUNCATE);
@@ -3494,6 +3560,19 @@ int coroutine_fn bdrv_co_truncate(BdrvChild *child, int64_t offset, bool exact,
out:
tracked_request_end(&req);
+
+ return ret;
+}
+
+int coroutine_fn bdrv_co_truncate(BdrvChild *child,
+ int64_t offset, bool exact,
+ PreallocMode prealloc, Error **errp)
+{
+ int ret;
+ BlockDriverState *bs = child->bs;
+
+ bdrv_inc_in_flight(bs);
+ ret = bdrv_do_truncate(child, offset, exact, prealloc, errp);
bdrv_dec_in_flight(bs);
return ret;
@@ -3511,7 +3590,7 @@ typedef struct TruncateCo {
static void coroutine_fn bdrv_truncate_co_entry(void *opaque)
{
TruncateCo *tco = opaque;
- tco->ret = bdrv_co_truncate(tco->child, tco->offset, tco->exact,
+ tco->ret = bdrv_do_truncate(tco->child, tco->offset, tco->exact,
tco->prealloc, tco->errp);
aio_wait_kick();
}
@@ -3529,6 +3608,8 @@ int bdrv_truncate(BdrvChild *child, int64_t offset, bool exact,
.ret = NOT_DONE,
};
+ bdrv_inc_in_flight(child->bs);
+
if (qemu_in_coroutine()) {
/* Fast-path if already in coroutine context */
bdrv_truncate_co_entry(&tco);
@@ -3538,5 +3619,7 @@ int bdrv_truncate(BdrvChild *child, int64_t offset, bool exact,
BDRV_POLL_WHILE(child->bs, tco.ret == NOT_DONE);
}
+ bdrv_dec_in_flight(child->bs);
+
return tco.ret;
}
--
2.21.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 6/9] block/io: expand in_flight inc/dec section: block-status
2020-04-08 9:30 [PATCH for-5.0? 0/9] block/io: safer inc/dec in_flight sections Vladimir Sementsov-Ogievskiy
` (4 preceding siblings ...)
2020-04-08 9:30 ` [PATCH 5/9] block/io: expand in_flight inc/dec section: simple cases Vladimir Sementsov-Ogievskiy
@ 2020-04-08 9:30 ` Vladimir Sementsov-Ogievskiy
2020-04-20 16:44 ` Stefan Hajnoczi
2020-04-08 9:30 ` [PATCH 7/9] block/io: add bdrv_do_pwrite_zeroes Vladimir Sementsov-Ogievskiy
` (6 subsequent siblings)
12 siblings, 1 reply; 27+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-04-08 9:30 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, fam, vsementsov, qemu-devel, mreitz, stefanha, den
It's safer to expand in_flight request to start before enter to
coroutine in synchronous wrappers and end after BDRV_POLL_WHILE loop.
Note that qemu_coroutine_enter may only schedule the coroutine in some
circumstances.
block-status requests are complex, they involve querying different
block driver states across backing chain. Let's expand only in_flight
section for the top bs, keeping other sections as is.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block/io.c | 60 +++++++++++++++++++++++++++++++++++++++++-------------
1 file changed, 46 insertions(+), 14 deletions(-)
diff --git a/block/io.c b/block/io.c
index 9b57c7e422..760170731f 100644
--- a/block/io.c
+++ b/block/io.c
@@ -2302,6 +2302,10 @@ int coroutine_fn bdrv_co_block_status_from_backing(BlockDriverState *bs,
* _ZERO where possible; otherwise, the result favors larger 'pnum',
* with a focus on accurate BDRV_BLOCK_ALLOCATED.
*
+ * If 'inc_in_flight' is true, in_flight counter will be increased for bs during
+ * the operation. All nested block_status calls will increase the counter for
+ * corresponding bs anyway.
+ *
* If 'offset' is beyond the end of the disk image the return value is
* BDRV_BLOCK_EOF and 'pnum' is set to 0.
*
@@ -2320,7 +2324,7 @@ int coroutine_fn bdrv_co_block_status_from_backing(BlockDriverState *bs,
* set to the host mapping and BDS corresponding to the guest offset.
*/
static int coroutine_fn bdrv_co_block_status(BlockDriverState *bs,
- bool want_zero,
+ bool want_zero, bool inc_in_flight,
int64_t offset, int64_t bytes,
int64_t *pnum, int64_t *map,
BlockDriverState **file)
@@ -2371,7 +2375,9 @@ static int coroutine_fn bdrv_co_block_status(BlockDriverState *bs,
goto early_out;
}
- bdrv_inc_in_flight(bs);
+ if (inc_in_flight) {
+ bdrv_inc_in_flight(bs);
+ }
/* Round out to request_alignment boundaries */
align = bs->bl.request_alignment;
@@ -2408,7 +2414,7 @@ static int coroutine_fn bdrv_co_block_status(BlockDriverState *bs,
if (ret & BDRV_BLOCK_RAW) {
assert(ret & BDRV_BLOCK_OFFSET_VALID && local_file);
- ret = bdrv_co_block_status(local_file, want_zero, local_map,
+ ret = bdrv_co_block_status(local_file, want_zero, true, local_map,
*pnum, pnum, &local_map, &local_file);
goto out;
}
@@ -2435,7 +2441,7 @@ static int coroutine_fn bdrv_co_block_status(BlockDriverState *bs,
int64_t file_pnum;
int ret2;
- ret2 = bdrv_co_block_status(local_file, want_zero, local_map,
+ ret2 = bdrv_co_block_status(local_file, want_zero, true, local_map,
*pnum, &file_pnum, NULL, NULL);
if (ret2 >= 0) {
/* Ignore errors. This is just providing extra information, it
@@ -2458,7 +2464,9 @@ static int coroutine_fn bdrv_co_block_status(BlockDriverState *bs,
}
out:
- bdrv_dec_in_flight(bs);
+ if (inc_in_flight) {
+ bdrv_dec_in_flight(bs);
+ }
if (ret >= 0 && offset + *pnum == total_size) {
ret |= BDRV_BLOCK_EOF;
}
@@ -2472,9 +2480,15 @@ early_out:
return ret;
}
+/*
+ * If 'inc_in_flight' is true, in_flight counter will be increased for bs during
+ * the operation. All block_status calls to the backing chain of bs will
+ * increase the counter for corresponding bs anyway.
+ */
static int coroutine_fn bdrv_co_block_status_above(BlockDriverState *bs,
BlockDriverState *base,
bool want_zero,
+ bool inc_in_flight,
int64_t offset,
int64_t bytes,
int64_t *pnum,
@@ -2487,11 +2501,13 @@ static int coroutine_fn bdrv_co_block_status_above(BlockDriverState *bs,
assert(bs != base);
for (p = bs; p != base; p = backing_bs(p)) {
- ret = bdrv_co_block_status(p, want_zero, offset, bytes, pnum, map,
- file);
+ ret = bdrv_co_block_status(p, want_zero, inc_in_flight,
+ offset, bytes, pnum, map, file);
if (ret < 0) {
break;
}
+ inc_in_flight = true;
+
if (ret & BDRV_BLOCK_ZERO && ret & BDRV_BLOCK_EOF && !first) {
/*
* Reading beyond the end of the file continues to read
@@ -2513,15 +2529,16 @@ static int coroutine_fn bdrv_co_block_status_above(BlockDriverState *bs,
}
static int coroutine_fn bdrv_co_is_allocated(BlockDriverState *bs,
+ bool inc_in_flight,
int64_t offset, int64_t bytes,
int64_t *pnum)
{
int ret;
int64_t dummy;
- ret = bdrv_co_block_status_above(bs, backing_bs(bs), false, offset,
- bytes, pnum ? pnum : &dummy, NULL,
- NULL);
+ ret = bdrv_co_block_status_above(bs, backing_bs(bs), false, inc_in_flight,
+ offset, bytes, pnum ? pnum : &dummy,
+ NULL, NULL);
if (ret < 0) {
return ret;
}
@@ -2534,7 +2551,7 @@ static void coroutine_fn bdrv_block_status_above_co_entry(void *opaque)
BdrvCoBlockStatusData *data = opaque;
data->ret = bdrv_co_block_status_above(data->bs, data->base,
- data->want_zero,
+ data->want_zero, false,
data->offset, data->bytes,
data->pnum, data->map, data->file);
data->done = true;
@@ -2566,6 +2583,8 @@ static int bdrv_common_block_status_above(BlockDriverState *bs,
.done = false,
};
+ bdrv_inc_in_flight(bs);
+
if (qemu_in_coroutine()) {
/* Fast-path if already in coroutine context */
bdrv_block_status_above_co_entry(&data);
@@ -2574,6 +2593,9 @@ static int bdrv_common_block_status_above(BlockDriverState *bs,
bdrv_coroutine_enter(bs, co);
BDRV_POLL_WHILE(bs, !data.done);
}
+
+ bdrv_dec_in_flight(bs);
+
return data.ret;
}
@@ -2623,15 +2645,19 @@ int coroutine_fn bdrv_is_allocated(BlockDriverState *bs, int64_t offset,
* words, the result is not necessarily the maximum possible range);
* but 'pnum' will only be 0 when end of file is reached.
*
+ * To be called between exactly one pair of bdrv_inc/dec_in_flight() for top bs.
+ * bdrv_do_is_allocated_above takes care of increasing in_fligth for other block
+ * driver states from bs backing chain.
*/
static int coroutine_fn
-bdrv_co_is_allocated_above(BlockDriverState *top, BlockDriverState *base,
+bdrv_do_is_allocated_above(BlockDriverState *top, BlockDriverState *base,
bool include_base, int64_t offset, int64_t bytes,
int64_t *pnum)
{
BlockDriverState *intermediate;
int ret;
int64_t n = bytes;
+ bool inc_in_flight = false;
assert(base || !include_base);
@@ -2641,10 +2667,12 @@ bdrv_co_is_allocated_above(BlockDriverState *top, BlockDriverState *base,
int64_t size_inter;
assert(intermediate);
- ret = bdrv_co_is_allocated(intermediate, offset, bytes, &pnum_inter);
+ ret = bdrv_co_is_allocated(intermediate, inc_in_flight, offset, bytes,
+ &pnum_inter);
if (ret < 0) {
return ret;
}
+ inc_in_flight = true;
if (ret) {
*pnum = pnum_inter;
return 1;
@@ -2685,7 +2713,7 @@ static void coroutine_fn bdrv_is_allocated_above_co_entry(void *opaque)
{
BdrvCoIsAllocatedAboveData *data = opaque;
- data->ret = bdrv_co_is_allocated_above(data->top, data->base,
+ data->ret = bdrv_do_is_allocated_above(data->top, data->base,
data->include_base,
data->offset, data->bytes,
data->pnum);
@@ -2709,6 +2737,8 @@ bdrv_is_allocated_above(BlockDriverState *top, BlockDriverState *base,
.done = false,
};
+ bdrv_inc_in_flight(top);
+
if (qemu_in_coroutine()) {
/* Fast-path if already in coroutine context */
bdrv_is_allocated_above_co_entry(&data);
@@ -2718,6 +2748,8 @@ bdrv_is_allocated_above(BlockDriverState *top, BlockDriverState *base,
BDRV_POLL_WHILE(top, !data.done);
}
+ bdrv_inc_in_flight(top);
+
return data.ret;
}
--
2.21.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 7/9] block/io: add bdrv_do_pwrite_zeroes
2020-04-08 9:30 [PATCH for-5.0? 0/9] block/io: safer inc/dec in_flight sections Vladimir Sementsov-Ogievskiy
` (5 preceding siblings ...)
2020-04-08 9:30 ` [PATCH 6/9] block/io: expand in_flight inc/dec section: block-status Vladimir Sementsov-Ogievskiy
@ 2020-04-08 9:30 ` Vladimir Sementsov-Ogievskiy
2020-04-20 16:38 ` Stefan Hajnoczi
2020-04-08 9:30 ` [PATCH 8/9] block/io: move bdrv_make_zero under block-status Vladimir Sementsov-Ogievskiy
` (5 subsequent siblings)
12 siblings, 1 reply; 27+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-04-08 9:30 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, fam, vsementsov, qemu-devel, mreitz, stefanha, den
We'll need a bdrv_co_pwrite_zeroes version without inc/dec in_flight to
be used in further implementation of bdrv_make_zero.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block/io.c | 23 +++++++++++++++++++----
1 file changed, 19 insertions(+), 4 deletions(-)
diff --git a/block/io.c b/block/io.c
index 760170731f..b1fc8b2367 100644
--- a/block/io.c
+++ b/block/io.c
@@ -2016,8 +2016,10 @@ int coroutine_fn bdrv_co_pwritev_part(BdrvChild *child,
return ret;
}
-int coroutine_fn bdrv_co_pwrite_zeroes(BdrvChild *child, int64_t offset,
- int bytes, BdrvRequestFlags flags)
+/* To be called between exactly one pair of bdrv_inc/dec_in_flight() */
+static int coroutine_fn
+bdrv_do_pwrite_zeroes(BdrvChild *child, int64_t offset, int bytes,
+ BdrvRequestFlags flags)
{
trace_bdrv_co_pwrite_zeroes(child->bs, offset, bytes, flags);
@@ -2025,8 +2027,21 @@ int coroutine_fn bdrv_co_pwrite_zeroes(BdrvChild *child, int64_t offset,
flags &= ~BDRV_REQ_MAY_UNMAP;
}
- return bdrv_co_pwritev(child, offset, bytes, NULL,
- BDRV_REQ_ZERO_WRITE | flags);
+ return bdrv_do_pwritev_part(child, offset, bytes, NULL, 0,
+ BDRV_REQ_ZERO_WRITE | flags);
+}
+
+int coroutine_fn
+bdrv_co_pwrite_zeroes(BdrvChild *child, int64_t offset, int bytes,
+ BdrvRequestFlags flags)
+{
+ int ret;
+
+ bdrv_inc_in_flight(child->bs);
+ ret = bdrv_do_pwrite_zeroes(child, offset, bytes, flags);
+ bdrv_dec_in_flight(child->bs);
+
+ return ret;
}
typedef struct RwCo {
--
2.21.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 8/9] block/io: move bdrv_make_zero under block-status
2020-04-08 9:30 [PATCH for-5.0? 0/9] block/io: safer inc/dec in_flight sections Vladimir Sementsov-Ogievskiy
` (6 preceding siblings ...)
2020-04-08 9:30 ` [PATCH 7/9] block/io: add bdrv_do_pwrite_zeroes Vladimir Sementsov-Ogievskiy
@ 2020-04-08 9:30 ` Vladimir Sementsov-Ogievskiy
2020-04-20 16:40 ` Stefan Hajnoczi
2020-04-08 9:30 ` [PATCH 9/9] block/io: expand in_flight inc/dec section: bdrv_make_zero Vladimir Sementsov-Ogievskiy
` (4 subsequent siblings)
12 siblings, 1 reply; 27+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-04-08 9:30 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, fam, vsementsov, qemu-devel, mreitz, stefanha, den
We are going to use bdrv_co_block_status in bdrv_make_zero, so move it
now down.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block/io.c | 82 +++++++++++++++++++++++++++---------------------------
1 file changed, 41 insertions(+), 41 deletions(-)
diff --git a/block/io.c b/block/io.c
index b1fc8b2367..d2ac9ac7b5 100644
--- a/block/io.c
+++ b/block/io.c
@@ -2177,47 +2177,6 @@ int bdrv_pwrite_sync(BdrvChild *child, int64_t offset,
return 0;
}
-/*
- * Completely zero out a block device with the help of bdrv_pwrite_zeroes.
- * The operation is sped up by checking the block status and only writing
- * zeroes to the device if they currently do not return zeroes. Optional
- * flags are passed through to bdrv_pwrite_zeroes (e.g. BDRV_REQ_MAY_UNMAP,
- * BDRV_REQ_FUA).
- *
- * Returns < 0 on error, 0 on success. For error codes see bdrv_write().
- */
-int bdrv_make_zero(BdrvChild *child, BdrvRequestFlags flags)
-{
- int ret;
- int64_t target_size, bytes, offset = 0;
- BlockDriverState *bs = child->bs;
-
- target_size = bdrv_getlength(bs);
- if (target_size < 0) {
- return target_size;
- }
-
- for (;;) {
- bytes = MIN(target_size - offset, BDRV_REQUEST_MAX_BYTES);
- if (bytes <= 0) {
- return 0;
- }
- ret = bdrv_block_status(bs, offset, bytes, &bytes, NULL, NULL);
- if (ret < 0) {
- return ret;
- }
- if (ret & BDRV_BLOCK_ZERO) {
- offset += bytes;
- continue;
- }
- ret = bdrv_pwrite_zeroes(child, offset, bytes, flags);
- if (ret < 0) {
- return ret;
- }
- offset += bytes;
- }
-}
-
int bdrv_preadv(BdrvChild *child, int64_t offset, QEMUIOVector *qiov)
{
int ret;
@@ -2768,6 +2727,47 @@ bdrv_is_allocated_above(BlockDriverState *top, BlockDriverState *base,
return data.ret;
}
+/*
+ * Completely zero out a block device with the help of bdrv_pwrite_zeroes.
+ * The operation is sped up by checking the block status and only writing
+ * zeroes to the device if they currently do not return zeroes. Optional
+ * flags are passed through to bdrv_pwrite_zeroes (e.g. BDRV_REQ_MAY_UNMAP,
+ * BDRV_REQ_FUA).
+ *
+ * Returns < 0 on error, 0 on success. For error codes see bdrv_write().
+ */
+int bdrv_make_zero(BdrvChild *child, BdrvRequestFlags flags)
+{
+ int ret;
+ int64_t target_size, bytes, offset = 0;
+ BlockDriverState *bs = child->bs;
+
+ target_size = bdrv_getlength(bs);
+ if (target_size < 0) {
+ return target_size;
+ }
+
+ for (;;) {
+ bytes = MIN(target_size - offset, BDRV_REQUEST_MAX_BYTES);
+ if (bytes <= 0) {
+ return 0;
+ }
+ ret = bdrv_block_status(bs, offset, bytes, &bytes, NULL, NULL);
+ if (ret < 0) {
+ return ret;
+ }
+ if (ret & BDRV_BLOCK_ZERO) {
+ offset += bytes;
+ continue;
+ }
+ ret = bdrv_pwrite_zeroes(child, offset, bytes, flags);
+ if (ret < 0) {
+ return ret;
+ }
+ offset += bytes;
+ }
+}
+
typedef struct BdrvVmstateCo {
BlockDriverState *bs;
QEMUIOVector *qiov;
--
2.21.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 9/9] block/io: expand in_flight inc/dec section: bdrv_make_zero
2020-04-08 9:30 [PATCH for-5.0? 0/9] block/io: safer inc/dec in_flight sections Vladimir Sementsov-Ogievskiy
` (7 preceding siblings ...)
2020-04-08 9:30 ` [PATCH 8/9] block/io: move bdrv_make_zero under block-status Vladimir Sementsov-Ogievskiy
@ 2020-04-08 9:30 ` Vladimir Sementsov-Ogievskiy
2020-04-20 16:43 ` Stefan Hajnoczi
2020-04-08 10:15 ` [PATCH for-5.0? 0/9] block/io: safer inc/dec in_flight sections no-reply
` (3 subsequent siblings)
12 siblings, 1 reply; 27+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-04-08 9:30 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, fam, vsementsov, qemu-devel, mreitz, stefanha, den
It's safer to expand in_flight request to start before enter to
coroutine in synchronous wrappers and end after BDRV_POLL_WHILE loop.
Note that qemu_coroutine_enter may only schedule the coroutine in some
circumstances.
bdrv_make_zero update includes refactoring: move the whole loop into
coroutine, which has additional benefit of not create/enter new
coroutine on each iteration.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block/io.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 50 insertions(+), 3 deletions(-)
diff --git a/block/io.c b/block/io.c
index d2ac9ac7b5..220b5c04d7 100644
--- a/block/io.c
+++ b/block/io.c
@@ -2735,8 +2735,11 @@ bdrv_is_allocated_above(BlockDriverState *top, BlockDriverState *base,
* BDRV_REQ_FUA).
*
* Returns < 0 on error, 0 on success. For error codes see bdrv_write().
+ *
+ * To be called between exactly one pair of bdrv_inc/dec_in_flight()
*/
-int bdrv_make_zero(BdrvChild *child, BdrvRequestFlags flags)
+static int coroutine_fn
+bdrv_do_make_zero(BdrvChild *child, BdrvRequestFlags flags)
{
int ret;
int64_t target_size, bytes, offset = 0;
@@ -2752,7 +2755,8 @@ int bdrv_make_zero(BdrvChild *child, BdrvRequestFlags flags)
if (bytes <= 0) {
return 0;
}
- ret = bdrv_block_status(bs, offset, bytes, &bytes, NULL, NULL);
+ ret = bdrv_co_block_status(bs, true, false,
+ offset, bytes, &bytes, NULL, NULL);
if (ret < 0) {
return ret;
}
@@ -2760,7 +2764,7 @@ int bdrv_make_zero(BdrvChild *child, BdrvRequestFlags flags)
offset += bytes;
continue;
}
- ret = bdrv_pwrite_zeroes(child, offset, bytes, flags);
+ ret = bdrv_do_pwrite_zeroes(child, offset, bytes, flags);
if (ret < 0) {
return ret;
}
@@ -2768,6 +2772,49 @@ int bdrv_make_zero(BdrvChild *child, BdrvRequestFlags flags)
}
}
+typedef struct BdrvDoMakeZeroData {
+ BdrvChild *child;
+ BdrvRequestFlags flags;
+ int ret;
+ bool done;
+} BdrvDoMakeZeroData;
+
+static void coroutine_fn bdrv_make_zero_co_entry(void *opaque)
+{
+ BdrvDoMakeZeroData *data = opaque;
+
+ data->ret = bdrv_do_make_zero(data->child, data->flags);
+ data->done = true;
+ aio_wait_kick();
+}
+
+int bdrv_make_zero(BdrvChild *child, BdrvRequestFlags flags)
+{
+ int ret;
+
+ bdrv_inc_in_flight(child->bs);
+
+ if (qemu_in_coroutine()) {
+ /* Fast-path if already in coroutine context */
+ ret = bdrv_do_make_zero(child, flags);
+ } else {
+ BdrvDoMakeZeroData data = {
+ .child = child,
+ .flags = flags,
+ .done = false,
+ };
+ Coroutine *co = qemu_coroutine_create(bdrv_make_zero_co_entry, &data);
+
+ bdrv_coroutine_enter(child->bs, co);
+ BDRV_POLL_WHILE(child->bs, !data.done);
+ ret = data.ret;
+ }
+
+ bdrv_dec_in_flight(child->bs);
+
+ return ret;
+}
+
typedef struct BdrvVmstateCo {
BlockDriverState *bs;
QEMUIOVector *qiov;
--
2.21.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH for-5.0? 0/9] block/io: safer inc/dec in_flight sections
2020-04-08 9:30 [PATCH for-5.0? 0/9] block/io: safer inc/dec in_flight sections Vladimir Sementsov-Ogievskiy
` (8 preceding siblings ...)
2020-04-08 9:30 ` [PATCH 9/9] block/io: expand in_flight inc/dec section: bdrv_make_zero Vladimir Sementsov-Ogievskiy
@ 2020-04-08 10:15 ` no-reply
2020-04-08 10:22 ` no-reply
` (2 subsequent siblings)
12 siblings, 0 replies; 27+ messages in thread
From: no-reply @ 2020-04-08 10:15 UTC (permalink / raw)
To: vsementsov
Cc: kwolf, fam, vsementsov, qemu-block, qemu-devel, mreitz, stefanha, den
Patchew URL: https://patchew.org/QEMU/20200408093051.9893-1-vsementsov@virtuozzo.com/
Hi,
This series failed the asan build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.
=== TEST SCRIPT BEGIN ===
#!/bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===
CC qemu-storage-daemon.o
CC chardev/char.o
CC chardev/char-fd.o
/tmp/qemu-test/src/block/io.c:3236:23: error: unused variable 'bs' [-Werror,-Wunused-variable]
BlockDriverState *bs = child->bs;
^
1 error generated.
make: *** [/tmp/qemu-test/src/rules.mak:69: block/io.o] Error 1
make: *** Waiting for unfinished jobs....
Traceback (most recent call last):
File "./tests/docker/docker.py", line 664, in <module>
---
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=fe09506e16304737ad88a479330eedd0', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=x86_64-softmmu', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-p88wn5d0/src/docker-src.2020-04-08-06.11.00.19577:/var/tmp/qemu:z,ro', 'qemu:fedora', '/var/tmp/qemu/run', 'test-debug']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=fe09506e16304737ad88a479330eedd0
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-p88wn5d0/src'
make: *** [docker-run-test-debug@fedora] Error 2
real 3m59.109s
user 0m8.928s
The full log is available at
http://patchew.org/logs/20200408093051.9893-1-vsementsov@virtuozzo.com/testing.asan/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH for-5.0? 0/9] block/io: safer inc/dec in_flight sections
2020-04-08 9:30 [PATCH for-5.0? 0/9] block/io: safer inc/dec in_flight sections Vladimir Sementsov-Ogievskiy
` (9 preceding siblings ...)
2020-04-08 10:15 ` [PATCH for-5.0? 0/9] block/io: safer inc/dec in_flight sections no-reply
@ 2020-04-08 10:22 ` no-reply
2020-04-08 10:25 ` no-reply
2020-04-09 17:00 ` Stefan Hajnoczi
12 siblings, 0 replies; 27+ messages in thread
From: no-reply @ 2020-04-08 10:22 UTC (permalink / raw)
To: vsementsov
Cc: kwolf, fam, vsementsov, qemu-block, qemu-devel, mreitz, stefanha, den
Patchew URL: https://patchew.org/QEMU/20200408093051.9893-1-vsementsov@virtuozzo.com/
Hi,
This series failed the docker-quick@centos7 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.
=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===
CC crypto/tlssession.o
CC crypto/secret.o
/tmp/qemu-test/src/block/io.c: In function 'bdrv_co_pdiscard':
/tmp/qemu-test/src/block/io.c:3236:23: error: unused variable 'bs' [-Werror=unused-variable]
BlockDriverState *bs = child->bs;
^
cc1: all warnings being treated as errors
make: *** [block/io.o] Error 1
make: *** Waiting for unfinished jobs....
Traceback (most recent call last):
File "./tests/docker/docker.py", line 664, in <module>
---
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=a0f1e41cc0a74bd7a341e17a6e374b91', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-jzfd32f9/src/docker-src.2020-04-08-06.20.30.5705:/var/tmp/qemu:z,ro', 'qemu:centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=a0f1e41cc0a74bd7a341e17a6e374b91
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-jzfd32f9/src'
make: *** [docker-run-test-quick@centos7] Error 2
real 2m13.993s
user 0m8.229s
The full log is available at
http://patchew.org/logs/20200408093051.9893-1-vsementsov@virtuozzo.com/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH for-5.0? 0/9] block/io: safer inc/dec in_flight sections
2020-04-08 9:30 [PATCH for-5.0? 0/9] block/io: safer inc/dec in_flight sections Vladimir Sementsov-Ogievskiy
` (10 preceding siblings ...)
2020-04-08 10:22 ` no-reply
@ 2020-04-08 10:25 ` no-reply
2020-04-09 17:00 ` Stefan Hajnoczi
12 siblings, 0 replies; 27+ messages in thread
From: no-reply @ 2020-04-08 10:25 UTC (permalink / raw)
To: vsementsov
Cc: kwolf, fam, vsementsov, qemu-block, qemu-devel, mreitz, stefanha, den
Patchew URL: https://patchew.org/QEMU/20200408093051.9893-1-vsementsov@virtuozzo.com/
Hi,
This series failed the docker-mingw@fedora build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.
=== TEST SCRIPT BEGIN ===
#! /bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-mingw@fedora J=14 NETWORK=1
=== TEST SCRIPT END ===
CC hw/acpi/cpu_hotplug.o
CC hw/acpi/memory_hotplug.o
/tmp/qemu-test/src/block/io.c: In function 'bdrv_co_pdiscard':
/tmp/qemu-test/src/block/io.c:3236:23: error: unused variable 'bs' [-Werror=unused-variable]
BlockDriverState *bs = child->bs;
^~
cc1: all warnings being treated as errors
make: *** [/tmp/qemu-test/src/rules.mak:69: block/io.o] Error 1
make: *** Waiting for unfinished jobs....
Traceback (most recent call last):
File "./tests/docker/docker.py", line 664, in <module>
---
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=0a3c46e828f54c7eacc85733f086e093', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-9znq69ii/src/docker-src.2020-04-08-06.23.18.12334:/var/tmp/qemu:z,ro', 'qemu:fedora', '/var/tmp/qemu/run', 'test-mingw']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=0a3c46e828f54c7eacc85733f086e093
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-9znq69ii/src'
make: *** [docker-run-test-mingw@fedora] Error 2
real 2m12.399s
user 0m8.326s
The full log is available at
http://patchew.org/logs/20200408093051.9893-1-vsementsov@virtuozzo.com/testing.docker-mingw@fedora/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH for-5.0? 0/9] block/io: safer inc/dec in_flight sections
2020-04-08 9:30 [PATCH for-5.0? 0/9] block/io: safer inc/dec in_flight sections Vladimir Sementsov-Ogievskiy
` (11 preceding siblings ...)
2020-04-08 10:25 ` no-reply
@ 2020-04-09 17:00 ` Stefan Hajnoczi
2020-04-09 18:49 ` Vladimir Sementsov-Ogievskiy
12 siblings, 1 reply; 27+ messages in thread
From: Stefan Hajnoczi @ 2020-04-09 17:00 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy
Cc: kwolf, fam, qemu-block, qemu-devel, mreitz, den
[-- Attachment #1: Type: text/plain, Size: 621 bytes --]
On Wed, Apr 08, 2020 at 12:30:42PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Hi all!
>
> This is inspired by Kevin's
> "block: Fix blk->in_flight during blk_wait_while_drained()" series.
>
> So, like it's now done for block-backends, let's expand
> in_flight-protected sections for bdrv_ interfaces, including
> coroutine_enter and BDRV_POLL_WHILE loop into these sections.
This looks like a code improvement but let's leave it for the next
release since QEMU 5.0 is in freeze and this patch series does not fix a
specific user-visible bug.
I will review this in depth next week. Thanks!
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH for-5.0? 0/9] block/io: safer inc/dec in_flight sections
2020-04-09 17:00 ` Stefan Hajnoczi
@ 2020-04-09 18:49 ` Vladimir Sementsov-Ogievskiy
0 siblings, 0 replies; 27+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-04-09 18:49 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: kwolf, fam, qemu-block, qemu-devel, mreitz, den
09.04.2020 20:00, Stefan Hajnoczi wrote:
> On Wed, Apr 08, 2020 at 12:30:42PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>> Hi all!
>>
>> This is inspired by Kevin's
>> "block: Fix blk->in_flight during blk_wait_while_drained()" series.
>>
>> So, like it's now done for block-backends, let's expand
>> in_flight-protected sections for bdrv_ interfaces, including
>> coroutine_enter and BDRV_POLL_WHILE loop into these sections.
>
> This looks like a code improvement but let's leave it for the next
> release since QEMU 5.0 is in freeze and this patch series does not fix a
> specific user-visible bug.
>
> I will review this in depth next week. Thanks!
>
Hmm, it possibly fixes some bugs, but at least I didn't see them :) Anyway, it shouldn't be a degradation.
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/9] block/io: refactor bdrv_is_allocated_above to run only one coroutine
2020-04-08 9:30 ` [PATCH 1/9] block/io: refactor bdrv_is_allocated_above to run only one coroutine Vladimir Sementsov-Ogievskiy
@ 2020-04-20 15:56 ` Stefan Hajnoczi
0 siblings, 0 replies; 27+ messages in thread
From: Stefan Hajnoczi @ 2020-04-20 15:56 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy
Cc: kwolf, fam, qemu-block, qemu-devel, mreitz, den
[-- Attachment #1: Type: text/plain, Size: 473 bytes --]
On Wed, Apr 08, 2020 at 12:30:43PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> +int coroutine_fn
> +bdrv_is_allocated_above(BlockDriverState *top, BlockDriverState *base,
> + bool include_base, int64_t offset, int64_t bytes,
> + int64_t *pnum)
s/coroutine_fn//
Only functions that must be called from coroutine context should be
marked as coroutine_fn. Since this function supports both contexts, so
it doesn't apply here.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/9] block/io: refactor bdrv_co_ioctl: move aio stuff to corresponding block
2020-04-08 9:30 ` [PATCH 2/9] block/io: refactor bdrv_co_ioctl: move aio stuff to corresponding block Vladimir Sementsov-Ogievskiy
@ 2020-04-20 15:57 ` Stefan Hajnoczi
0 siblings, 0 replies; 27+ messages in thread
From: Stefan Hajnoczi @ 2020-04-20 15:57 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy
Cc: kwolf, fam, qemu-block, qemu-devel, mreitz, den
[-- Attachment #1: Type: text/plain, Size: 304 bytes --]
On Wed, Apr 08, 2020 at 12:30:44PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> block/io.c | 23 +++++++++++++++--------
> 1 file changed, 15 insertions(+), 8 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/9] block/io: move flush and pdiscard stuff down
2020-04-08 9:30 ` [PATCH 3/9] block/io: move flush and pdiscard stuff down Vladimir Sementsov-Ogievskiy
@ 2020-04-20 16:01 ` Stefan Hajnoczi
0 siblings, 0 replies; 27+ messages in thread
From: Stefan Hajnoczi @ 2020-04-20 16:01 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy
Cc: kwolf, fam, qemu-block, qemu-devel, mreitz, den
[-- Attachment #1: Type: text/plain, Size: 448 bytes --]
On Wed, Apr 08, 2020 at 12:30:45PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> bdrv_co_flush and bdrv_co_pdiscard will become static in further patch,
> move their usage down.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> block/io.c | 56 +++++++++++++++++++++++++++---------------------------
> 1 file changed, 28 insertions(+), 28 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 4/9] block/io: move bdrv_rw_co_entry and friends down
2020-04-08 9:30 ` [PATCH 4/9] block/io: move bdrv_rw_co_entry and friends down Vladimir Sementsov-Ogievskiy
@ 2020-04-20 16:04 ` Stefan Hajnoczi
2020-04-27 14:11 ` Vladimir Sementsov-Ogievskiy
0 siblings, 1 reply; 27+ messages in thread
From: Stefan Hajnoczi @ 2020-04-20 16:04 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy
Cc: kwolf, fam, qemu-block, qemu-devel, mreitz, den
[-- Attachment #1: Type: text/plain, Size: 613 bytes --]
On Wed, Apr 08, 2020 at 12:30:46PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> We are going to use bdrv_co_pwritev_part and bdrv_co_preadv_part in
> bdrv_rw_co_entry, so move it down.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> block/io.c | 361 +++++++++++++++++++++++++++--------------------------
> 1 file changed, 181 insertions(+), 180 deletions(-)
Note to other reviewers: Comment formatting was changed to conform to
coding style and function order was changed. Otherwise the code is
unmodified.
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 5/9] block/io: expand in_flight inc/dec section: simple cases
2020-04-08 9:30 ` [PATCH 5/9] block/io: expand in_flight inc/dec section: simple cases Vladimir Sementsov-Ogievskiy
@ 2020-04-20 16:22 ` Stefan Hajnoczi
2020-04-22 13:47 ` Vladimir Sementsov-Ogievskiy
0 siblings, 1 reply; 27+ messages in thread
From: Stefan Hajnoczi @ 2020-04-20 16:22 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy
Cc: kwolf, fam, qemu-block, qemu-devel, mreitz, den
[-- Attachment #1: Type: text/plain, Size: 1710 bytes --]
On Wed, Apr 08, 2020 at 12:30:47PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> It's safer to expand in_flight request to start before enter to
Please explain what exeactly "safer" means. If I understand correctly
this is just a refactoring and does not fix bugs that have been hit in
the real world.
Is this just a generate attempt to avoid accidentally performing
operations that need to happen as part of the request after the dec
call?
> @@ -2718,17 +2746,18 @@ bdrv_co_rw_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos,
> ret = drv->bdrv_save_vmstate(bs, qiov, pos);
> }
> } else if (bs->file) {
> - ret = bdrv_co_rw_vmstate(bs->file->bs, qiov, pos, is_read);
> + bdrv_inc_in_flight(bs->file->bs);
> + ret = bdrv_do_rw_vmstate(bs->file->bs, qiov, pos, is_read);
> + bdrv_dec_in_flight(bs->file->bs);
Here we inc/dec...
> }
>
> - bdrv_dec_in_flight(bs);
> return ret;
> }
>
> static void coroutine_fn bdrv_co_rw_vmstate_entry(void *opaque)
> {
> BdrvVmstateCo *co = opaque;
> - co->ret = bdrv_co_rw_vmstate(co->bs, co->qiov, co->pos, co->is_read);
> + co->ret = bdrv_do_rw_vmstate(co->bs, co->qiov, co->pos, co->is_read);
...here we don't. The code is correct, but bdrv_co_rw_vmstate_entry()
should also document that its caller must inc/dec.
> @@ -2950,7 +2994,7 @@ static void coroutine_fn bdrv_flush_co_entry(void *opaque)
> {
> FlushCo *rwco = opaque;
>
> - rwco->ret = bdrv_co_flush(rwco->bs);
> + rwco->ret = bdrv_do_flush(rwco->bs);
> aio_wait_kick();
> }
This function should also document that the caller must inc/dec.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 7/9] block/io: add bdrv_do_pwrite_zeroes
2020-04-08 9:30 ` [PATCH 7/9] block/io: add bdrv_do_pwrite_zeroes Vladimir Sementsov-Ogievskiy
@ 2020-04-20 16:38 ` Stefan Hajnoczi
0 siblings, 0 replies; 27+ messages in thread
From: Stefan Hajnoczi @ 2020-04-20 16:38 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy
Cc: kwolf, fam, qemu-block, qemu-devel, mreitz, den
[-- Attachment #1: Type: text/plain, Size: 446 bytes --]
On Wed, Apr 08, 2020 at 12:30:49PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> We'll need a bdrv_co_pwrite_zeroes version without inc/dec in_flight to
> be used in further implementation of bdrv_make_zero.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> block/io.c | 23 +++++++++++++++++++----
> 1 file changed, 19 insertions(+), 4 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 8/9] block/io: move bdrv_make_zero under block-status
2020-04-08 9:30 ` [PATCH 8/9] block/io: move bdrv_make_zero under block-status Vladimir Sementsov-Ogievskiy
@ 2020-04-20 16:40 ` Stefan Hajnoczi
0 siblings, 0 replies; 27+ messages in thread
From: Stefan Hajnoczi @ 2020-04-20 16:40 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy
Cc: kwolf, fam, qemu-block, qemu-devel, mreitz, den
[-- Attachment #1: Type: text/plain, Size: 434 bytes --]
On Wed, Apr 08, 2020 at 12:30:50PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> We are going to use bdrv_co_block_status in bdrv_make_zero, so move it
> now down.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> block/io.c | 82 +++++++++++++++++++++++++++---------------------------
> 1 file changed, 41 insertions(+), 41 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 9/9] block/io: expand in_flight inc/dec section: bdrv_make_zero
2020-04-08 9:30 ` [PATCH 9/9] block/io: expand in_flight inc/dec section: bdrv_make_zero Vladimir Sementsov-Ogievskiy
@ 2020-04-20 16:43 ` Stefan Hajnoczi
0 siblings, 0 replies; 27+ messages in thread
From: Stefan Hajnoczi @ 2020-04-20 16:43 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy
Cc: kwolf, fam, qemu-block, qemu-devel, mreitz, den
[-- Attachment #1: Type: text/plain, Size: 468 bytes --]
On Wed, Apr 08, 2020 at 12:30:51PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> @@ -2768,6 +2772,49 @@ int bdrv_make_zero(BdrvChild *child, BdrvRequestFlags flags)
> }
> }
>
> +typedef struct BdrvDoMakeZeroData {
> + BdrvChild *child;
> + BdrvRequestFlags flags;
> + int ret;
> + bool done;
> +} BdrvDoMakeZeroData;
> +
> +static void coroutine_fn bdrv_make_zero_co_entry(void *opaque)
Please document that the caller must inc/dec.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 6/9] block/io: expand in_flight inc/dec section: block-status
2020-04-08 9:30 ` [PATCH 6/9] block/io: expand in_flight inc/dec section: block-status Vladimir Sementsov-Ogievskiy
@ 2020-04-20 16:44 ` Stefan Hajnoczi
0 siblings, 0 replies; 27+ messages in thread
From: Stefan Hajnoczi @ 2020-04-20 16:44 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy
Cc: kwolf, fam, qemu-block, qemu-devel, mreitz, den
[-- Attachment #1: Type: text/plain, Size: 783 bytes --]
On Wed, Apr 08, 2020 at 12:30:48PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> It's safer to expand in_flight request to start before enter to
> coroutine in synchronous wrappers and end after BDRV_POLL_WHILE loop.
> Note that qemu_coroutine_enter may only schedule the coroutine in some
> circumstances.
>
> block-status requests are complex, they involve querying different
> block driver states across backing chain. Let's expand only in_flight
> section for the top bs, keeping other sections as is.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> block/io.c | 60 +++++++++++++++++++++++++++++++++++++++++-------------
> 1 file changed, 46 insertions(+), 14 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 5/9] block/io: expand in_flight inc/dec section: simple cases
2020-04-20 16:22 ` Stefan Hajnoczi
@ 2020-04-22 13:47 ` Vladimir Sementsov-Ogievskiy
2020-04-22 16:06 ` Stefan Hajnoczi
0 siblings, 1 reply; 27+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-04-22 13:47 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: kwolf, fam, qemu-block, qemu-devel, mreitz, den
20.04.2020 19:22, Stefan Hajnoczi wrote:
> On Wed, Apr 08, 2020 at 12:30:47PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>> It's safer to expand in_flight request to start before enter to
>
> Please explain what exeactly "safer" means. If I understand correctly
> this is just a refactoring and does not fix bugs that have been hit in
> the real world.
>
> Is this just a generate attempt to avoid accidentally performing
> operations that need to happen as part of the request after the dec
> call?
Consider write.
It's possible, that qemu_coroutine_enter only schedules execution, assume such case.
Then we may possibly have the following:
1. Somehow check that we are not in drained section in outer code
2. call bdrv_pwritev(), assuming that it will increse in_flight, which will protect us from starting drained section
3. it calls bdrv_prwv_co -> bdrv_coroutine_enter (not yet increased in_flight)
4. assume coroutine not yet actually entered, only scheduled, and we go to some code, which starts drained section (as in_flight is zero)
5. scheduled coroutine starts, and blindly increases in_flight, and we are in drained section with in_flight request.
The series does the same thing for block/io.c like Kevin's "block: Fix blk->in_flight during blk_wait_while_drained()" for blk layer.
>
>> @@ -2718,17 +2746,18 @@ bdrv_co_rw_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos,
>> ret = drv->bdrv_save_vmstate(bs, qiov, pos);
>> }
>> } else if (bs->file) {
>> - ret = bdrv_co_rw_vmstate(bs->file->bs, qiov, pos, is_read);
>> + bdrv_inc_in_flight(bs->file->bs);
>> + ret = bdrv_do_rw_vmstate(bs->file->bs, qiov, pos, is_read);
>> + bdrv_dec_in_flight(bs->file->bs);
>
> Here we inc/dec...
>
>> }
>>
>> - bdrv_dec_in_flight(bs);
>> return ret;
>> }
>>
>> static void coroutine_fn bdrv_co_rw_vmstate_entry(void *opaque)
>> {
>> BdrvVmstateCo *co = opaque;
>> - co->ret = bdrv_co_rw_vmstate(co->bs, co->qiov, co->pos, co->is_read);
>> + co->ret = bdrv_do_rw_vmstate(co->bs, co->qiov, co->pos, co->is_read);
>
> ...here we don't. The code is correct, but bdrv_co_rw_vmstate_entry()
> should also document that its caller must inc/dec.
>
>> @@ -2950,7 +2994,7 @@ static void coroutine_fn bdrv_flush_co_entry(void *opaque)
>> {
>> FlushCo *rwco = opaque;
>>
>> - rwco->ret = bdrv_co_flush(rwco->bs);
>> + rwco->ret = bdrv_do_flush(rwco->bs);
>> aio_wait_kick();
>> }
>
> This function should also document that the caller must inc/dec.
>
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 5/9] block/io: expand in_flight inc/dec section: simple cases
2020-04-22 13:47 ` Vladimir Sementsov-Ogievskiy
@ 2020-04-22 16:06 ` Stefan Hajnoczi
0 siblings, 0 replies; 27+ messages in thread
From: Stefan Hajnoczi @ 2020-04-22 16:06 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy
Cc: kwolf, fam, qemu-block, qemu-devel, mreitz, Stefan Hajnoczi, den
[-- Attachment #1: Type: text/plain, Size: 1538 bytes --]
On Wed, Apr 22, 2020 at 04:47:07PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> 20.04.2020 19:22, Stefan Hajnoczi wrote:
> > On Wed, Apr 08, 2020 at 12:30:47PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> > > It's safer to expand in_flight request to start before enter to
> >
> > Please explain what exeactly "safer" means. If I understand correctly
> > this is just a refactoring and does not fix bugs that have been hit in
> > the real world.
> >
> > Is this just a generate attempt to avoid accidentally performing
> > operations that need to happen as part of the request after the dec
> > call?
>
> Consider write.
>
> It's possible, that qemu_coroutine_enter only schedules execution, assume such case.
> Then we may possibly have the following:
>
> 1. Somehow check that we are not in drained section in outer code
>
> 2. call bdrv_pwritev(), assuming that it will increse in_flight, which will protect us from starting drained section
>
> 3. it calls bdrv_prwv_co -> bdrv_coroutine_enter (not yet increased in_flight)
>
> 4. assume coroutine not yet actually entered, only scheduled, and we go to some code, which starts drained section (as in_flight is zero)
>
> 5. scheduled coroutine starts, and blindly increases in_flight, and we are in drained section with in_flight request.
>
> The series does the same thing for block/io.c like Kevin's "block: Fix blk->in_flight during blk_wait_while_drained()" for blk layer.
Please include this in the commit description. Thanks!
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 4/9] block/io: move bdrv_rw_co_entry and friends down
2020-04-20 16:04 ` Stefan Hajnoczi
@ 2020-04-27 14:11 ` Vladimir Sementsov-Ogievskiy
0 siblings, 0 replies; 27+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-04-27 14:11 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: kwolf, fam, qemu-block, qemu-devel, mreitz, den
20.04.2020 19:04, Stefan Hajnoczi wrote:
> On Wed, Apr 08, 2020 at 12:30:46PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>> We are going to use bdrv_co_pwritev_part and bdrv_co_preadv_part in
>> bdrv_rw_co_entry, so move it down.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>> block/io.c | 361 +++++++++++++++++++++++++++--------------------------
>> 1 file changed, 181 insertions(+), 180 deletions(-)
>
> Note to other reviewers: Comment formatting was changed to conform to
> coding style and function order was changed. Otherwise the code is
> unmodified.
I think, I'll put it directly to commit message, hope you don't mind.
>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
>
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2020-04-27 14:13 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-08 9:30 [PATCH for-5.0? 0/9] block/io: safer inc/dec in_flight sections Vladimir Sementsov-Ogievskiy
2020-04-08 9:30 ` [PATCH 1/9] block/io: refactor bdrv_is_allocated_above to run only one coroutine Vladimir Sementsov-Ogievskiy
2020-04-20 15:56 ` Stefan Hajnoczi
2020-04-08 9:30 ` [PATCH 2/9] block/io: refactor bdrv_co_ioctl: move aio stuff to corresponding block Vladimir Sementsov-Ogievskiy
2020-04-20 15:57 ` Stefan Hajnoczi
2020-04-08 9:30 ` [PATCH 3/9] block/io: move flush and pdiscard stuff down Vladimir Sementsov-Ogievskiy
2020-04-20 16:01 ` Stefan Hajnoczi
2020-04-08 9:30 ` [PATCH 4/9] block/io: move bdrv_rw_co_entry and friends down Vladimir Sementsov-Ogievskiy
2020-04-20 16:04 ` Stefan Hajnoczi
2020-04-27 14:11 ` Vladimir Sementsov-Ogievskiy
2020-04-08 9:30 ` [PATCH 5/9] block/io: expand in_flight inc/dec section: simple cases Vladimir Sementsov-Ogievskiy
2020-04-20 16:22 ` Stefan Hajnoczi
2020-04-22 13:47 ` Vladimir Sementsov-Ogievskiy
2020-04-22 16:06 ` Stefan Hajnoczi
2020-04-08 9:30 ` [PATCH 6/9] block/io: expand in_flight inc/dec section: block-status Vladimir Sementsov-Ogievskiy
2020-04-20 16:44 ` Stefan Hajnoczi
2020-04-08 9:30 ` [PATCH 7/9] block/io: add bdrv_do_pwrite_zeroes Vladimir Sementsov-Ogievskiy
2020-04-20 16:38 ` Stefan Hajnoczi
2020-04-08 9:30 ` [PATCH 8/9] block/io: move bdrv_make_zero under block-status Vladimir Sementsov-Ogievskiy
2020-04-20 16:40 ` Stefan Hajnoczi
2020-04-08 9:30 ` [PATCH 9/9] block/io: expand in_flight inc/dec section: bdrv_make_zero Vladimir Sementsov-Ogievskiy
2020-04-20 16:43 ` Stefan Hajnoczi
2020-04-08 10:15 ` [PATCH for-5.0? 0/9] block/io: safer inc/dec in_flight sections no-reply
2020-04-08 10:22 ` no-reply
2020-04-08 10:25 ` no-reply
2020-04-09 17:00 ` Stefan Hajnoczi
2020-04-09 18:49 ` Vladimir Sementsov-Ogievskiy
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.