* [Qemu-devel] [PATCH v2 1/3] block: Invalidate all children
2016-05-11 2:45 [Qemu-devel] [PATCH v2 0/3] block: More complete inactivate/invalidate of graph nodes Fam Zheng
@ 2016-05-11 2:45 ` Fam Zheng
2016-05-11 10:37 ` Alberto Garcia
2016-05-11 2:45 ` [Qemu-devel] [PATCH v2 2/3] block: Drop superfluous invalidating bs->file from drivers Fam Zheng
` (2 subsequent siblings)
3 siblings, 1 reply; 7+ messages in thread
From: Fam Zheng @ 2016-05-11 2:45 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Max Reitz, Stefan Hajnoczi, Alberto Garcia, qemu-block
Currently we only recurse to bs->file, which will miss the children in quorum
and VMDK.
Recurse into the whole subtree to avoid that.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
block.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/block.c b/block.c
index d4939b4..fa8b38f 100644
--- a/block.c
+++ b/block.c
@@ -3201,6 +3201,7 @@ void bdrv_init_with_whitelist(void)
void bdrv_invalidate_cache(BlockDriverState *bs, Error **errp)
{
+ BdrvChild *child;
Error *local_err = NULL;
int ret;
@@ -3215,13 +3216,20 @@ void bdrv_invalidate_cache(BlockDriverState *bs, Error **errp)
if (bs->drv->bdrv_invalidate_cache) {
bs->drv->bdrv_invalidate_cache(bs, &local_err);
- } else if (bs->file) {
- bdrv_invalidate_cache(bs->file->bs, &local_err);
+ if (local_err) {
+ bs->open_flags |= BDRV_O_INACTIVE;
+ error_propagate(errp, local_err);
+ return;
+ }
}
- if (local_err) {
- bs->open_flags |= BDRV_O_INACTIVE;
- error_propagate(errp, local_err);
- return;
+
+ QLIST_FOREACH(child, &bs->children, next) {
+ bdrv_invalidate_cache(child->bs, &local_err);
+ if (local_err) {
+ bs->open_flags |= BDRV_O_INACTIVE;
+ error_propagate(errp, local_err);
+ return;
+ }
}
ret = refresh_total_sectors(bs, bs->total_sectors);
--
2.8.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH v2 2/3] block: Drop superfluous invalidating bs->file from drivers
2016-05-11 2:45 [Qemu-devel] [PATCH v2 0/3] block: More complete inactivate/invalidate of graph nodes Fam Zheng
2016-05-11 2:45 ` [Qemu-devel] [PATCH v2 1/3] block: Invalidate all children Fam Zheng
@ 2016-05-11 2:45 ` Fam Zheng
2016-05-11 10:43 ` Alberto Garcia
2016-05-11 2:45 ` [Qemu-devel] [PATCH v2 3/3] block: Inactivate all children Fam Zheng
2016-05-11 12:03 ` [Qemu-devel] [PATCH v2 0/3] block: More complete inactivate/invalidate of graph nodes Kevin Wolf
3 siblings, 1 reply; 7+ messages in thread
From: Fam Zheng @ 2016-05-11 2:45 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Max Reitz, Stefan Hajnoczi, Alberto Garcia, qemu-block
Now they are invalidated by the block layer, so it's not necessary to
do this in block drivers' implementations of .bdrv_invalidate_cache.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
block/qcow2.c | 7 -------
block/qed.c | 6 ------
block/quorum.c | 16 ----------------
3 files changed, 29 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index 470734b..d4431e0 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1757,13 +1757,6 @@ static void qcow2_invalidate_cache(BlockDriverState *bs, Error **errp)
qcow2_close(bs);
- bdrv_invalidate_cache(bs->file->bs, &local_err);
- if (local_err) {
- error_propagate(errp, local_err);
- bs->drv = NULL;
- return;
- }
-
memset(s, 0, sizeof(BDRVQcow2State));
options = qdict_clone_shallow(bs->options);
diff --git a/block/qed.c b/block/qed.c
index 0af5274..9081941 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -1594,12 +1594,6 @@ static void bdrv_qed_invalidate_cache(BlockDriverState *bs, Error **errp)
bdrv_qed_close(bs);
- bdrv_invalidate_cache(bs->file->bs, &local_err);
- if (local_err) {
- error_propagate(errp, local_err);
- return;
- }
-
memset(s, 0, sizeof(BDRVQEDState));
ret = bdrv_qed_open(bs, NULL, bs->open_flags, &local_err);
if (local_err) {
diff --git a/block/quorum.c b/block/quorum.c
index da15465..8f7c099 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -747,21 +747,6 @@ static int64_t quorum_getlength(BlockDriverState *bs)
return result;
}
-static void quorum_invalidate_cache(BlockDriverState *bs, Error **errp)
-{
- BDRVQuorumState *s = bs->opaque;
- Error *local_err = NULL;
- int i;
-
- for (i = 0; i < s->num_children; i++) {
- bdrv_invalidate_cache(s->children[i]->bs, &local_err);
- if (local_err) {
- error_propagate(errp, local_err);
- return;
- }
- }
-}
-
static coroutine_fn int quorum_co_flush(BlockDriverState *bs)
{
BDRVQuorumState *s = bs->opaque;
@@ -1070,7 +1055,6 @@ static BlockDriver bdrv_quorum = {
.bdrv_aio_readv = quorum_aio_readv,
.bdrv_aio_writev = quorum_aio_writev,
- .bdrv_invalidate_cache = quorum_invalidate_cache,
.bdrv_detach_aio_context = quorum_detach_aio_context,
.bdrv_attach_aio_context = quorum_attach_aio_context,
--
2.8.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH v2 3/3] block: Inactivate all children
2016-05-11 2:45 [Qemu-devel] [PATCH v2 0/3] block: More complete inactivate/invalidate of graph nodes Fam Zheng
2016-05-11 2:45 ` [Qemu-devel] [PATCH v2 1/3] block: Invalidate all children Fam Zheng
2016-05-11 2:45 ` [Qemu-devel] [PATCH v2 2/3] block: Drop superfluous invalidating bs->file from drivers Fam Zheng
@ 2016-05-11 2:45 ` Fam Zheng
2016-05-11 12:03 ` [Qemu-devel] [PATCH v2 0/3] block: More complete inactivate/invalidate of graph nodes Kevin Wolf
3 siblings, 0 replies; 7+ messages in thread
From: Fam Zheng @ 2016-05-11 2:45 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Max Reitz, Stefan Hajnoczi, Alberto Garcia, qemu-block
Currently we only inactivate the top BDS. Actually bdrv_inactivate
should be the opposite of bdrv_invalidate_cache.
Recurse into the whole subtree instead.
Because a node may have multiple parents, and because once
BDRV_O_INACTIVE is set for a node, further writes are not allowed, we
cannot interleave flag settings and .bdrv_inactivate calls (that may
submit write to other nodes in a graph) within a single pass. Therefore
two passes are used here.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
block.c | 47 ++++++++++++++++++++++++++++++++++++-----------
1 file changed, 36 insertions(+), 11 deletions(-)
diff --git a/block.c b/block.c
index fa8b38f..93481c4 100644
--- a/block.c
+++ b/block.c
@@ -3258,38 +3258,63 @@ void bdrv_invalidate_cache_all(Error **errp)
}
}
-static int bdrv_inactivate(BlockDriverState *bs)
+static int bdrv_inactivate_recurse(BlockDriverState *bs,
+ bool setting_flag)
{
+ BdrvChild *child;
int ret;
- if (bs->drv->bdrv_inactivate) {
+ if (!setting_flag && bs->drv->bdrv_inactivate) {
ret = bs->drv->bdrv_inactivate(bs);
if (ret < 0) {
return ret;
}
}
- bs->open_flags |= BDRV_O_INACTIVE;
+ QLIST_FOREACH(child, &bs->children, next) {
+ ret = bdrv_inactivate_recurse(child->bs, setting_flag);
+ if (ret < 0) {
+ return ret;
+ }
+ }
+
+ if (setting_flag) {
+ bs->open_flags |= BDRV_O_INACTIVE;
+ }
return 0;
}
int bdrv_inactivate_all(void)
{
BlockDriverState *bs = NULL;
- int ret;
+ int ret = 0;
+ int pass;
while ((bs = bdrv_next(bs)) != NULL) {
- AioContext *aio_context = bdrv_get_aio_context(bs);
+ aio_context_acquire(bdrv_get_aio_context(bs));
+ }
- aio_context_acquire(aio_context);
- ret = bdrv_inactivate(bs);
- aio_context_release(aio_context);
- if (ret < 0) {
- return ret;
+ /* We do two passes of inactivation. The first pass calls to drivers'
+ * .bdrv_inactivate callbacks recursively so all cache is flushed to disk;
+ * the second pass sets the BDRV_O_INACTIVE flag so that no further write
+ * is allowed. */
+ for (pass = 0; pass < 2; pass++) {
+ bs = NULL;
+ while ((bs = bdrv_next(bs)) != NULL) {
+ ret = bdrv_inactivate_recurse(bs, pass);
+ if (ret < 0) {
+ goto out;
+ }
}
}
- return 0;
+out:
+ bs = NULL;
+ while ((bs = bdrv_next(bs)) != NULL) {
+ aio_context_release(bdrv_get_aio_context(bs));
+ }
+
+ return ret;
}
/**************************************************************/
--
2.8.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/3] block: More complete inactivate/invalidate of graph nodes
2016-05-11 2:45 [Qemu-devel] [PATCH v2 0/3] block: More complete inactivate/invalidate of graph nodes Fam Zheng
` (2 preceding siblings ...)
2016-05-11 2:45 ` [Qemu-devel] [PATCH v2 3/3] block: Inactivate all children Fam Zheng
@ 2016-05-11 12:03 ` Kevin Wolf
3 siblings, 0 replies; 7+ messages in thread
From: Kevin Wolf @ 2016-05-11 12:03 UTC (permalink / raw)
To: Fam Zheng
Cc: qemu-devel, Max Reitz, Stefan Hajnoczi, Alberto Garcia, qemu-block
Am 11.05.2016 um 04:45 hat Fam Zheng geschrieben:
> v2: Two passes inactivation. [Kevin]
>
> For now we only consider bs->file chain. In fact this is incomplete. For
> example, if qcow2 is a quorum child, we don't properly invalidate or inactivate
> it. This series recurses into the subtrees in both bdrv_invalidate_cache_all
> and bdrv_inactivate_all. This is also necessary to make the image locking
> cooperate with migration.
Thanks, applied to block-next.
Kevin
^ permalink raw reply [flat|nested] 7+ messages in thread