* [Qemu-devel] [PATCH v3 0/3] add bdrv_co_drain_begin/end BlockDriver callbacks
@ 2017-09-23 11:14 Manos Pitsidianakis
2017-09-23 11:14 ` [Qemu-devel] [PATCH v3 1/3] block: add bdrv_co_drain_end callback Manos Pitsidianakis
` (5 more replies)
0 siblings, 6 replies; 12+ messages in thread
From: Manos Pitsidianakis @ 2017-09-23 11:14 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, Kevin Wolf, Fam Zheng, Stefan Hajnoczi, Max Reitz
This patch series renames bdrv_co_drain to bdrv_co_drain_begin and adds a new
bdrv_co_drain_end callback to match bdrv_drained_begin/end and
drained_begin/end of BdrvChild. This is needed because the throttle driver
(block/throttle.c) needs a way to mark the end of the drain in order to toggle
io_limits_disabled correctly.
Based-on: <20170918202529.28379-1-el13635@mail.ntua.gr>
"block/throttle-groups.c: allocate RestartData on the heap"
Which fixes a coroutine crash in block/throttle-groups.c
v3:
fixed commit message typo in first patch [Fam]
rephrased doc comment based on mailing discussion
v2:
add doc for callbacks and change order of request polling for completion
[Stefan]
Manos Pitsidianakis (3):
block: add bdrv_co_drain_end callback
block: rename bdrv_co_drain to bdrv_co_drain_begin
block/throttle.c: add bdrv_co_drain_begin/end callbacks
include/block/block_int.h | 13 ++++++++++---
block/io.c | 48 +++++++++++++++++++++++++++++++++--------------
block/qed.c | 6 +++---
block/throttle.c | 18 ++++++++++++++++++
4 files changed, 65 insertions(+), 20 deletions(-)
--
2.11.0
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v3 1/3] block: add bdrv_co_drain_end callback
2017-09-23 11:14 [Qemu-devel] [PATCH v3 0/3] add bdrv_co_drain_begin/end BlockDriver callbacks Manos Pitsidianakis
@ 2017-09-23 11:14 ` Manos Pitsidianakis
2017-09-25 2:27 ` Fam Zheng
2017-09-25 10:06 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2017-09-23 11:14 ` [Qemu-devel] [PATCH v3 2/3] block: rename bdrv_co_drain to bdrv_co_drain_begin Manos Pitsidianakis
` (4 subsequent siblings)
5 siblings, 2 replies; 12+ messages in thread
From: Manos Pitsidianakis @ 2017-09-23 11:14 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, Kevin Wolf, Fam Zheng, Stefan Hajnoczi, Max Reitz
BlockDriverState has a bdrv_co_drain() callback but no equivalent for
the end of the drain. The throttle driver (block/throttle.c) needs a way
to mark the end of the drain in order to toggle io_limits_disabled
correctly, thus bdrv_co_drain_end is needed.
Signed-off-by: Manos Pitsidianakis <el13635@mail.ntua.gr>
---
include/block/block_int.h | 11 +++++++++--
block/io.c | 48 +++++++++++++++++++++++++++++++++--------------
2 files changed, 43 insertions(+), 16 deletions(-)
diff --git a/include/block/block_int.h b/include/block/block_int.h
index ba4c383393..9ebdeb6db0 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -354,10 +354,17 @@ struct BlockDriver {
int (*bdrv_probe_geometry)(BlockDriverState *bs, HDGeometry *geo);
/**
- * Drain and stop any internal sources of requests in the driver, and
- * remain so until next I/O callback (e.g. bdrv_co_writev) is called.
+ * bdrv_co_drain is called if implemented in the beginning of a
+ * drain operation to drain and stop any internal sources of requests in
+ * the driver.
+ * bdrv_co_drain_end is called if implemented at the end of the drain.
+ *
+ * They should be used by the driver to e.g. manage scheduled I/O
+ * requests, or toggle an internal state. After the end of the drain new
+ * requests will continue normally.
*/
void coroutine_fn (*bdrv_co_drain)(BlockDriverState *bs);
+ void coroutine_fn (*bdrv_co_drain_end)(BlockDriverState *bs);
void (*bdrv_add_child)(BlockDriverState *parent, BlockDriverState *child,
Error **errp);
diff --git a/block/io.c b/block/io.c
index 4378ae4c7d..b0a10ad3ef 100644
--- a/block/io.c
+++ b/block/io.c
@@ -153,6 +153,7 @@ typedef struct {
Coroutine *co;
BlockDriverState *bs;
bool done;
+ bool begin;
} BdrvCoDrainData;
static void coroutine_fn bdrv_drain_invoke_entry(void *opaque)
@@ -160,18 +161,23 @@ static void coroutine_fn bdrv_drain_invoke_entry(void *opaque)
BdrvCoDrainData *data = opaque;
BlockDriverState *bs = data->bs;
- bs->drv->bdrv_co_drain(bs);
+ if (data->begin) {
+ bs->drv->bdrv_co_drain(bs);
+ } else {
+ bs->drv->bdrv_co_drain_end(bs);
+ }
/* Set data->done before reading bs->wakeup. */
atomic_mb_set(&data->done, true);
bdrv_wakeup(bs);
}
-static void bdrv_drain_invoke(BlockDriverState *bs)
+static void bdrv_drain_invoke(BlockDriverState *bs, bool begin)
{
- BdrvCoDrainData data = { .bs = bs, .done = false };
+ BdrvCoDrainData data = { .bs = bs, .done = false, .begin = begin};
- if (!bs->drv || !bs->drv->bdrv_co_drain) {
+ if (!bs->drv || (begin && !bs->drv->bdrv_co_drain) ||
+ (!begin && !bs->drv->bdrv_co_drain_end)) {
return;
}
@@ -180,15 +186,16 @@ static void bdrv_drain_invoke(BlockDriverState *bs)
BDRV_POLL_WHILE(bs, !data.done);
}
-static bool bdrv_drain_recurse(BlockDriverState *bs)
+static bool bdrv_drain_recurse(BlockDriverState *bs, bool begin)
{
BdrvChild *child, *tmp;
bool waited;
- waited = BDRV_POLL_WHILE(bs, atomic_read(&bs->in_flight) > 0);
-
/* Ensure any pending metadata writes are submitted to bs->file. */
- bdrv_drain_invoke(bs);
+ bdrv_drain_invoke(bs, begin);
+
+ /* Wait for drained requests to finish */
+ waited = BDRV_POLL_WHILE(bs, atomic_read(&bs->in_flight) > 0);
QLIST_FOREACH_SAFE(child, &bs->children, next, tmp) {
BlockDriverState *bs = child->bs;
@@ -205,7 +212,7 @@ static bool bdrv_drain_recurse(BlockDriverState *bs)
*/
bdrv_ref(bs);
}
- waited |= bdrv_drain_recurse(bs);
+ waited |= bdrv_drain_recurse(bs, begin);
if (in_main_loop) {
bdrv_unref(bs);
}
@@ -221,12 +228,18 @@ static void bdrv_co_drain_bh_cb(void *opaque)
BlockDriverState *bs = data->bs;
bdrv_dec_in_flight(bs);
- bdrv_drained_begin(bs);
+ if (data->begin) {
+ bdrv_drained_begin(bs);
+ } else {
+ bdrv_drained_end(bs);
+ }
+
data->done = true;
aio_co_wake(co);
}
-static void coroutine_fn bdrv_co_yield_to_drain(BlockDriverState *bs)
+static void coroutine_fn bdrv_co_yield_to_drain(BlockDriverState *bs,
+ bool begin)
{
BdrvCoDrainData data;
@@ -239,6 +252,7 @@ static void coroutine_fn bdrv_co_yield_to_drain(BlockDriverState *bs)
.co = qemu_coroutine_self(),
.bs = bs,
.done = false,
+ .begin = begin,
};
bdrv_inc_in_flight(bs);
aio_bh_schedule_oneshot(bdrv_get_aio_context(bs),
@@ -253,7 +267,7 @@ static void coroutine_fn bdrv_co_yield_to_drain(BlockDriverState *bs)
void bdrv_drained_begin(BlockDriverState *bs)
{
if (qemu_in_coroutine()) {
- bdrv_co_yield_to_drain(bs);
+ bdrv_co_yield_to_drain(bs, true);
return;
}
@@ -262,17 +276,22 @@ void bdrv_drained_begin(BlockDriverState *bs)
bdrv_parent_drained_begin(bs);
}
- bdrv_drain_recurse(bs);
+ bdrv_drain_recurse(bs, true);
}
void bdrv_drained_end(BlockDriverState *bs)
{
+ if (qemu_in_coroutine()) {
+ bdrv_co_yield_to_drain(bs, false);
+ return;
+ }
assert(bs->quiesce_counter > 0);
if (atomic_fetch_dec(&bs->quiesce_counter) > 1) {
return;
}
bdrv_parent_drained_end(bs);
+ bdrv_drain_recurse(bs, false);
aio_enable_external(bdrv_get_aio_context(bs));
}
@@ -350,7 +369,7 @@ void bdrv_drain_all_begin(void)
aio_context_acquire(aio_context);
for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
if (aio_context == bdrv_get_aio_context(bs)) {
- waited |= bdrv_drain_recurse(bs);
+ waited |= bdrv_drain_recurse(bs, true);
}
}
aio_context_release(aio_context);
@@ -371,6 +390,7 @@ void bdrv_drain_all_end(void)
aio_context_acquire(aio_context);
aio_enable_external(aio_context);
bdrv_parent_drained_end(bs);
+ bdrv_drain_recurse(bs, false);
aio_context_release(aio_context);
}
--
2.11.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v3 2/3] block: rename bdrv_co_drain to bdrv_co_drain_begin
2017-09-23 11:14 [Qemu-devel] [PATCH v3 0/3] add bdrv_co_drain_begin/end BlockDriver callbacks Manos Pitsidianakis
2017-09-23 11:14 ` [Qemu-devel] [PATCH v3 1/3] block: add bdrv_co_drain_end callback Manos Pitsidianakis
@ 2017-09-23 11:14 ` Manos Pitsidianakis
2017-09-25 10:06 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2017-09-23 11:14 ` [Qemu-devel] [PATCH v3 3/3] block/throttle.c: add bdrv_co_drain_begin/end callbacks Manos Pitsidianakis
` (3 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Manos Pitsidianakis @ 2017-09-23 11:14 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, Kevin Wolf, Fam Zheng, Stefan Hajnoczi, Max Reitz
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Manos Pitsidianakis <el13635@mail.ntua.gr>
---
include/block/block_int.h | 4 ++--
block/io.c | 4 ++--
block/qed.c | 6 +++---
3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 9ebdeb6db0..51575d22b6 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -354,7 +354,7 @@ struct BlockDriver {
int (*bdrv_probe_geometry)(BlockDriverState *bs, HDGeometry *geo);
/**
- * bdrv_co_drain is called if implemented in the beginning of a
+ * bdrv_co_drain_begin is called if implemented in the beginning of a
* drain operation to drain and stop any internal sources of requests in
* the driver.
* bdrv_co_drain_end is called if implemented at the end of the drain.
@@ -363,7 +363,7 @@ struct BlockDriver {
* requests, or toggle an internal state. After the end of the drain new
* requests will continue normally.
*/
- void coroutine_fn (*bdrv_co_drain)(BlockDriverState *bs);
+ void coroutine_fn (*bdrv_co_drain_begin)(BlockDriverState *bs);
void coroutine_fn (*bdrv_co_drain_end)(BlockDriverState *bs);
void (*bdrv_add_child)(BlockDriverState *parent, BlockDriverState *child,
diff --git a/block/io.c b/block/io.c
index b0a10ad3ef..65e8094d7c 100644
--- a/block/io.c
+++ b/block/io.c
@@ -162,7 +162,7 @@ static void coroutine_fn bdrv_drain_invoke_entry(void *opaque)
BlockDriverState *bs = data->bs;
if (data->begin) {
- bs->drv->bdrv_co_drain(bs);
+ bs->drv->bdrv_co_drain_begin(bs);
} else {
bs->drv->bdrv_co_drain_end(bs);
}
@@ -176,7 +176,7 @@ static void bdrv_drain_invoke(BlockDriverState *bs, bool begin)
{
BdrvCoDrainData data = { .bs = bs, .done = false, .begin = begin};
- if (!bs->drv || (begin && !bs->drv->bdrv_co_drain) ||
+ if (!bs->drv || (begin && !bs->drv->bdrv_co_drain_begin) ||
(!begin && !bs->drv->bdrv_co_drain_end)) {
return;
}
diff --git a/block/qed.c b/block/qed.c
index 28e2ec89e8..821dcaa055 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -265,7 +265,7 @@ static bool qed_plug_allocating_write_reqs(BDRVQEDState *s)
assert(!s->allocating_write_reqs_plugged);
if (s->allocating_acb != NULL) {
/* Another allocating write came concurrently. This cannot happen
- * from bdrv_qed_co_drain, but it can happen when the timer runs.
+ * from bdrv_qed_co_drain_begin, but it can happen when the timer runs.
*/
qemu_co_mutex_unlock(&s->table_lock);
return false;
@@ -358,7 +358,7 @@ static void bdrv_qed_attach_aio_context(BlockDriverState *bs,
}
}
-static void coroutine_fn bdrv_qed_co_drain(BlockDriverState *bs)
+static void coroutine_fn bdrv_qed_co_drain_begin(BlockDriverState *bs)
{
BDRVQEDState *s = bs->opaque;
@@ -1608,7 +1608,7 @@ static BlockDriver bdrv_qed = {
.bdrv_check = bdrv_qed_check,
.bdrv_detach_aio_context = bdrv_qed_detach_aio_context,
.bdrv_attach_aio_context = bdrv_qed_attach_aio_context,
- .bdrv_co_drain = bdrv_qed_co_drain,
+ .bdrv_co_drain_begin = bdrv_qed_co_drain_begin,
};
static void bdrv_qed_init(void)
--
2.11.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v3 3/3] block/throttle.c: add bdrv_co_drain_begin/end callbacks
2017-09-23 11:14 [Qemu-devel] [PATCH v3 0/3] add bdrv_co_drain_begin/end BlockDriver callbacks Manos Pitsidianakis
2017-09-23 11:14 ` [Qemu-devel] [PATCH v3 1/3] block: add bdrv_co_drain_end callback Manos Pitsidianakis
2017-09-23 11:14 ` [Qemu-devel] [PATCH v3 2/3] block: rename bdrv_co_drain to bdrv_co_drain_begin Manos Pitsidianakis
@ 2017-09-23 11:14 ` Manos Pitsidianakis
2017-09-26 10:02 ` [Qemu-devel] [PATCH v3 0/3] add bdrv_co_drain_begin/end BlockDriver callbacks Stefan Hajnoczi
` (2 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Manos Pitsidianakis @ 2017-09-23 11:14 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, Kevin Wolf, Fam Zheng, Stefan Hajnoczi, Max Reitz
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Manos Pitsidianakis <el13635@mail.ntua.gr>
---
block/throttle.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/block/throttle.c b/block/throttle.c
index 5bca76300f..833175ac77 100644
--- a/block/throttle.c
+++ b/block/throttle.c
@@ -197,6 +197,21 @@ static bool throttle_recurse_is_first_non_filter(BlockDriverState *bs,
return bdrv_recurse_is_first_non_filter(bs->file->bs, candidate);
}
+static void coroutine_fn throttle_co_drain_begin(BlockDriverState *bs)
+{
+ ThrottleGroupMember *tgm = bs->opaque;
+ if (atomic_fetch_inc(&tgm->io_limits_disabled) == 0) {
+ throttle_group_restart_tgm(tgm);
+ }
+}
+
+static void coroutine_fn throttle_co_drain_end(BlockDriverState *bs)
+{
+ ThrottleGroupMember *tgm = bs->opaque;
+ assert(tgm->io_limits_disabled);
+ atomic_dec(&tgm->io_limits_disabled);
+}
+
static BlockDriver bdrv_throttle = {
.format_name = "throttle",
.protocol_name = "throttle",
@@ -226,6 +241,9 @@ static BlockDriver bdrv_throttle = {
.bdrv_reopen_abort = throttle_reopen_abort,
.bdrv_co_get_block_status = bdrv_co_get_block_status_from_file,
+ .bdrv_co_drain_begin = throttle_co_drain_begin,
+ .bdrv_co_drain_end = throttle_co_drain_end,
+
.is_filter = true,
};
--
2.11.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/3] block: add bdrv_co_drain_end callback
2017-09-23 11:14 ` [Qemu-devel] [PATCH v3 1/3] block: add bdrv_co_drain_end callback Manos Pitsidianakis
@ 2017-09-25 2:27 ` Fam Zheng
2017-09-25 10:06 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
1 sibling, 0 replies; 12+ messages in thread
From: Fam Zheng @ 2017-09-25 2:27 UTC (permalink / raw)
To: Manos Pitsidianakis
Cc: qemu-devel, qemu-block, Kevin Wolf, Stefan Hajnoczi, Max Reitz
On Sat, 09/23 14:14, Manos Pitsidianakis wrote:
> BlockDriverState has a bdrv_co_drain() callback but no equivalent for
> the end of the drain. The throttle driver (block/throttle.c) needs a way
> to mark the end of the drain in order to toggle io_limits_disabled
> correctly, thus bdrv_co_drain_end is needed.
>
> Signed-off-by: Manos Pitsidianakis <el13635@mail.ntua.gr>
Reviewed-by: Fam Zheng <famz@redhat.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH v3 1/3] block: add bdrv_co_drain_end callback
2017-09-23 11:14 ` [Qemu-devel] [PATCH v3 1/3] block: add bdrv_co_drain_end callback Manos Pitsidianakis
2017-09-25 2:27 ` Fam Zheng
@ 2017-09-25 10:06 ` Stefan Hajnoczi
1 sibling, 0 replies; 12+ messages in thread
From: Stefan Hajnoczi @ 2017-09-25 10:06 UTC (permalink / raw)
To: Manos Pitsidianakis
Cc: qemu-devel, Kevin Wolf, Fam Zheng, Stefan Hajnoczi, qemu-block,
Max Reitz
On Sat, Sep 23, 2017 at 02:14:09PM +0300, Manos Pitsidianakis wrote:
> BlockDriverState has a bdrv_co_drain() callback but no equivalent for
> the end of the drain. The throttle driver (block/throttle.c) needs a way
> to mark the end of the drain in order to toggle io_limits_disabled
> correctly, thus bdrv_co_drain_end is needed.
>
> Signed-off-by: Manos Pitsidianakis <el13635@mail.ntua.gr>
> ---
> include/block/block_int.h | 11 +++++++++--
> block/io.c | 48 +++++++++++++++++++++++++++++++++--------------
> 2 files changed, 43 insertions(+), 16 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH v3 2/3] block: rename bdrv_co_drain to bdrv_co_drain_begin
2017-09-23 11:14 ` [Qemu-devel] [PATCH v3 2/3] block: rename bdrv_co_drain to bdrv_co_drain_begin Manos Pitsidianakis
@ 2017-09-25 10:06 ` Stefan Hajnoczi
0 siblings, 0 replies; 12+ messages in thread
From: Stefan Hajnoczi @ 2017-09-25 10:06 UTC (permalink / raw)
To: Manos Pitsidianakis
Cc: qemu-devel, Kevin Wolf, Fam Zheng, Stefan Hajnoczi, qemu-block,
Max Reitz
On Sat, Sep 23, 2017 at 02:14:10PM +0300, Manos Pitsidianakis wrote:
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> Reviewed-by: Fam Zheng <famz@redhat.com>
> Signed-off-by: Manos Pitsidianakis <el13635@mail.ntua.gr>
> ---
> include/block/block_int.h | 4 ++--
> block/io.c | 4 ++--
> block/qed.c | 6 +++---
> 3 files changed, 7 insertions(+), 7 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v3 0/3] add bdrv_co_drain_begin/end BlockDriver callbacks
2017-09-23 11:14 [Qemu-devel] [PATCH v3 0/3] add bdrv_co_drain_begin/end BlockDriver callbacks Manos Pitsidianakis
` (2 preceding siblings ...)
2017-09-23 11:14 ` [Qemu-devel] [PATCH v3 3/3] block/throttle.c: add bdrv_co_drain_begin/end callbacks Manos Pitsidianakis
@ 2017-09-26 10:02 ` Stefan Hajnoczi
2017-09-26 11:00 ` Stefan Hajnoczi
2017-10-13 12:27 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
5 siblings, 0 replies; 12+ messages in thread
From: Stefan Hajnoczi @ 2017-09-26 10:02 UTC (permalink / raw)
To: Manos Pitsidianakis
Cc: qemu-devel, qemu-block, Kevin Wolf, Fam Zheng, Max Reitz
On Sat, Sep 23, 2017 at 02:14:08PM +0300, Manos Pitsidianakis wrote:
> This patch series renames bdrv_co_drain to bdrv_co_drain_begin and adds a new
> bdrv_co_drain_end callback to match bdrv_drained_begin/end and
> drained_begin/end of BdrvChild. This is needed because the throttle driver
> (block/throttle.c) needs a way to mark the end of the drain in order to toggle
> io_limits_disabled correctly.
>
> Based-on: <20170918202529.28379-1-el13635@mail.ntua.gr>
> "block/throttle-groups.c: allocate RestartData on the heap"
> Which fixes a coroutine crash in block/throttle-groups.c
>
> v3:
> fixed commit message typo in first patch [Fam]
> rephrased doc comment based on mailing discussion
> v2:
> add doc for callbacks and change order of request polling for completion
> [Stefan]
>
> Manos Pitsidianakis (3):
> block: add bdrv_co_drain_end callback
> block: rename bdrv_co_drain to bdrv_co_drain_begin
> block/throttle.c: add bdrv_co_drain_begin/end callbacks
>
> include/block/block_int.h | 13 ++++++++++---
> block/io.c | 48 +++++++++++++++++++++++++++++++++--------------
> block/qed.c | 6 +++---
> block/throttle.c | 18 ++++++++++++++++++
> 4 files changed, 65 insertions(+), 20 deletions(-)
>
> --
> 2.11.0
>
Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block
Stefan
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v3 0/3] add bdrv_co_drain_begin/end BlockDriver callbacks
2017-09-23 11:14 [Qemu-devel] [PATCH v3 0/3] add bdrv_co_drain_begin/end BlockDriver callbacks Manos Pitsidianakis
` (3 preceding siblings ...)
2017-09-26 10:02 ` [Qemu-devel] [PATCH v3 0/3] add bdrv_co_drain_begin/end BlockDriver callbacks Stefan Hajnoczi
@ 2017-09-26 11:00 ` Stefan Hajnoczi
2017-09-26 12:44 ` Manos Pitsidianakis
2017-10-13 12:27 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
5 siblings, 1 reply; 12+ messages in thread
From: Stefan Hajnoczi @ 2017-09-26 11:00 UTC (permalink / raw)
To: Manos Pitsidianakis
Cc: qemu-devel, qemu-block, Kevin Wolf, Fam Zheng, Max Reitz
On Sat, Sep 23, 2017 at 02:14:08PM +0300, Manos Pitsidianakis wrote:
> This patch series renames bdrv_co_drain to bdrv_co_drain_begin and adds a new
> bdrv_co_drain_end callback to match bdrv_drained_begin/end and
> drained_begin/end of BdrvChild. This is needed because the throttle driver
> (block/throttle.c) needs a way to mark the end of the drain in order to toggle
> io_limits_disabled correctly.
>
> Based-on: <20170918202529.28379-1-el13635@mail.ntua.gr>
> "block/throttle-groups.c: allocate RestartData on the heap"
> Which fixes a coroutine crash in block/throttle-groups.c
>
> v3:
> fixed commit message typo in first patch [Fam]
> rephrased doc comment based on mailing discussion
> v2:
> add doc for callbacks and change order of request polling for completion
> [Stefan]
>
> Manos Pitsidianakis (3):
> block: add bdrv_co_drain_end callback
> block: rename bdrv_co_drain to bdrv_co_drain_begin
> block/throttle.c: add bdrv_co_drain_begin/end callbacks
>
> include/block/block_int.h | 13 ++++++++++---
> block/io.c | 48 +++++++++++++++++++++++++++++++++--------------
> block/qed.c | 6 +++---
> block/throttle.c | 18 ++++++++++++++++++
> 4 files changed, 65 insertions(+), 20 deletions(-)
Oops, this seems to cause a qemu-iotests failure. Please take a look:
$ ./check -qcow2 184
184 0s ... - output mismatch (see 184.out.bad)
--- /home/stefanha/qemu/tests/qemu-iotests/184.out 2017-09-19 14:51:46.673854437 +0100
+++ 184.out.bad 2017-09-26 11:13:06.946610239 +0100
@@ -142,6 +142,9 @@
"guest": false
}
}
+./common.config: line 118: 9196 Segmentation fault (core dumped) ( if [ -n "${QEMU_NEED_PID}" ]; then
+ echo $BASHPID > "${QEMU_TEST_DIR}/qemu-${_QEMU_HANDLE}.pid";
+fi; exec "$QEMU_PROG" $QEMU_OPTIONS "$@" )
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v3 0/3] add bdrv_co_drain_begin/end BlockDriver callbacks
2017-09-26 11:00 ` Stefan Hajnoczi
@ 2017-09-26 12:44 ` Manos Pitsidianakis
2017-09-29 12:25 ` Kevin Wolf
0 siblings, 1 reply; 12+ messages in thread
From: Manos Pitsidianakis @ 2017-09-26 12:44 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: qemu-devel, qemu-block, Kevin Wolf, Fam Zheng, Max Reitz
[-- Attachment #1: Type: text/plain, Size: 2264 bytes --]
On Tue, Sep 26, 2017 at 12:00:24PM +0100, Stefan Hajnoczi wrote:
>On Sat, Sep 23, 2017 at 02:14:08PM +0300, Manos Pitsidianakis wrote:
>> This patch series renames bdrv_co_drain to bdrv_co_drain_begin and adds a new
>> bdrv_co_drain_end callback to match bdrv_drained_begin/end and
>> drained_begin/end of BdrvChild. This is needed because the throttle driver
>> (block/throttle.c) needs a way to mark the end of the drain in order to toggle
>> io_limits_disabled correctly.
>>
>> Based-on: <20170918202529.28379-1-el13635@mail.ntua.gr>
>> "block/throttle-groups.c: allocate RestartData on the heap"
>> Which fixes a coroutine crash in block/throttle-groups.c
>>
>> v3:
>> fixed commit message typo in first patch [Fam]
>> rephrased doc comment based on mailing discussion
>> v2:
>> add doc for callbacks and change order of request polling for completion
>> [Stefan]
>>
>> Manos Pitsidianakis (3):
>> block: add bdrv_co_drain_end callback
>> block: rename bdrv_co_drain to bdrv_co_drain_begin
>> block/throttle.c: add bdrv_co_drain_begin/end callbacks
>>
>> include/block/block_int.h | 13 ++++++++++---
>> block/io.c | 48 +++++++++++++++++++++++++++++++++--------------
>> block/qed.c | 6 +++---
>> block/throttle.c | 18 ++++++++++++++++++
>> 4 files changed, 65 insertions(+), 20 deletions(-)
>
>Oops, this seems to cause a qemu-iotests failure. Please take a look:
>
>$ ./check -qcow2 184
>184 0s ... - output mismatch (see 184.out.bad)
>--- /home/stefanha/qemu/tests/qemu-iotests/184.out 2017-09-19 14:51:46.673854437 +0100
>+++ 184.out.bad 2017-09-26 11:13:06.946610239 +0100
>@@ -142,6 +142,9 @@
> "guest": false
> }
> }
>+./common.config: line 118: 9196 Segmentation fault (core dumped) ( if [ -n "${QEMU_NEED_PID}" ]; then
>+ echo $BASHPID > "${QEMU_TEST_DIR}/qemu-${_QEMU_HANDLE}.pid";
>+fi; exec "$QEMU_PROG" $QEMU_OPTIONS "$@" )
>
Hey Stefan,
This is fixed in
>> Based-on: <20170918202529.28379-1-el13635@mail.ntua.gr>
>> "block/throttle-groups.c: allocate RestartData on the heap"
>> Which fixes a coroutine crash in block/throttle-groups.c
Which I sent before this series and is in Kevin's branch.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v3 0/3] add bdrv_co_drain_begin/end BlockDriver callbacks
2017-09-26 12:44 ` Manos Pitsidianakis
@ 2017-09-29 12:25 ` Kevin Wolf
0 siblings, 0 replies; 12+ messages in thread
From: Kevin Wolf @ 2017-09-29 12:25 UTC (permalink / raw)
To: Manos Pitsidianakis, Stefan Hajnoczi, qemu-devel, qemu-block,
Fam Zheng, Max Reitz
[-- Attachment #1: Type: text/plain, Size: 2598 bytes --]
Am 26.09.2017 um 14:44 hat Manos Pitsidianakis geschrieben:
> On Tue, Sep 26, 2017 at 12:00:24PM +0100, Stefan Hajnoczi wrote:
> > On Sat, Sep 23, 2017 at 02:14:08PM +0300, Manos Pitsidianakis wrote:
> > > This patch series renames bdrv_co_drain to bdrv_co_drain_begin and adds a new
> > > bdrv_co_drain_end callback to match bdrv_drained_begin/end and
> > > drained_begin/end of BdrvChild. This is needed because the throttle driver
> > > (block/throttle.c) needs a way to mark the end of the drain in order to toggle
> > > io_limits_disabled correctly.
> > >
> > > Based-on: <20170918202529.28379-1-el13635@mail.ntua.gr>
> > > "block/throttle-groups.c: allocate RestartData on the heap"
> > > Which fixes a coroutine crash in block/throttle-groups.c
> > >
> > > v3:
> > > fixed commit message typo in first patch [Fam]
> > > rephrased doc comment based on mailing discussion
> > > v2:
> > > add doc for callbacks and change order of request polling for completion
> > > [Stefan]
> > >
> > > Manos Pitsidianakis (3):
> > > block: add bdrv_co_drain_end callback
> > > block: rename bdrv_co_drain to bdrv_co_drain_begin
> > > block/throttle.c: add bdrv_co_drain_begin/end callbacks
> > >
> > > include/block/block_int.h | 13 ++++++++++---
> > > block/io.c | 48 +++++++++++++++++++++++++++++++++--------------
> > > block/qed.c | 6 +++---
> > > block/throttle.c | 18 ++++++++++++++++++
> > > 4 files changed, 65 insertions(+), 20 deletions(-)
> >
> > Oops, this seems to cause a qemu-iotests failure. Please take a look:
> >
> > $ ./check -qcow2 184
> > 184 0s ... - output mismatch (see 184.out.bad)
> > --- /home/stefanha/qemu/tests/qemu-iotests/184.out 2017-09-19 14:51:46.673854437 +0100
> > +++ 184.out.bad 2017-09-26 11:13:06.946610239 +0100
> > @@ -142,6 +142,9 @@
> > "guest": false
> > }
> > }
> > +./common.config: line 118: 9196 Segmentation fault (core dumped) ( if [ -n "${QEMU_NEED_PID}" ]; then
> > + echo $BASHPID > "${QEMU_TEST_DIR}/qemu-${_QEMU_HANDLE}.pid";
> > +fi; exec "$QEMU_PROG" $QEMU_OPTIONS "$@" )
> >
>
> Hey Stefan,
>
> This is fixed in
> > > Based-on: <20170918202529.28379-1-el13635@mail.ntua.gr>
> > > "block/throttle-groups.c: allocate RestartData on the heap"
> > > Which fixes a coroutine crash in block/throttle-groups.c
>
> Which I sent before this series and is in Kevin's branch.
Stefan, the fix is in master by now, so you can stage this series again.
(Or I can take it, if you prefer.)
Kevin
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH v3 0/3] add bdrv_co_drain_begin/end BlockDriver callbacks
2017-09-23 11:14 [Qemu-devel] [PATCH v3 0/3] add bdrv_co_drain_begin/end BlockDriver callbacks Manos Pitsidianakis
` (4 preceding siblings ...)
2017-09-26 11:00 ` Stefan Hajnoczi
@ 2017-10-13 12:27 ` Stefan Hajnoczi
5 siblings, 0 replies; 12+ messages in thread
From: Stefan Hajnoczi @ 2017-10-13 12:27 UTC (permalink / raw)
To: Manos Pitsidianakis
Cc: qemu-devel, Kevin Wolf, Fam Zheng, Stefan Hajnoczi, qemu-block,
Max Reitz
On Sat, Sep 23, 2017 at 02:14:08PM +0300, Manos Pitsidianakis wrote:
> This patch series renames bdrv_co_drain to bdrv_co_drain_begin and adds a new
> bdrv_co_drain_end callback to match bdrv_drained_begin/end and
> drained_begin/end of BdrvChild. This is needed because the throttle driver
> (block/throttle.c) needs a way to mark the end of the drain in order to toggle
> io_limits_disabled correctly.
>
> Based-on: <20170918202529.28379-1-el13635@mail.ntua.gr>
> "block/throttle-groups.c: allocate RestartData on the heap"
> Which fixes a coroutine crash in block/throttle-groups.c
>
> v3:
> fixed commit message typo in first patch [Fam]
> rephrased doc comment based on mailing discussion
> v2:
> add doc for callbacks and change order of request polling for completion
> [Stefan]
>
> Manos Pitsidianakis (3):
> block: add bdrv_co_drain_end callback
> block: rename bdrv_co_drain to bdrv_co_drain_begin
> block/throttle.c: add bdrv_co_drain_begin/end callbacks
>
> include/block/block_int.h | 13 ++++++++++---
> block/io.c | 48 +++++++++++++++++++++++++++++++++--------------
> block/qed.c | 6 +++---
> block/throttle.c | 18 ++++++++++++++++++
> 4 files changed, 65 insertions(+), 20 deletions(-)
>
> --
> 2.11.0
>
>
Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block
Stefan
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2017-10-13 12:27 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-23 11:14 [Qemu-devel] [PATCH v3 0/3] add bdrv_co_drain_begin/end BlockDriver callbacks Manos Pitsidianakis
2017-09-23 11:14 ` [Qemu-devel] [PATCH v3 1/3] block: add bdrv_co_drain_end callback Manos Pitsidianakis
2017-09-25 2:27 ` Fam Zheng
2017-09-25 10:06 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2017-09-23 11:14 ` [Qemu-devel] [PATCH v3 2/3] block: rename bdrv_co_drain to bdrv_co_drain_begin Manos Pitsidianakis
2017-09-25 10:06 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2017-09-23 11:14 ` [Qemu-devel] [PATCH v3 3/3] block/throttle.c: add bdrv_co_drain_begin/end callbacks Manos Pitsidianakis
2017-09-26 10:02 ` [Qemu-devel] [PATCH v3 0/3] add bdrv_co_drain_begin/end BlockDriver callbacks Stefan Hajnoczi
2017-09-26 11:00 ` Stefan Hajnoczi
2017-09-26 12:44 ` Manos Pitsidianakis
2017-09-29 12:25 ` Kevin Wolf
2017-10-13 12:27 ` [Qemu-devel] [Qemu-block] " 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.