[PULL,12/21] block: bdrv_invalidate_cache: invalidate children first

Commit Message

Max Reitz Feb. 12, 2017, 1:38 a.m. UTC

From: Vladimir Sementsov-Ogievskiy <[e-mail]>

Current implementation invalidates firstly parent bds and then its
children. This leads to the following bug:

after incoming migration, in bdrv_invalidate_cache_all:
1. invalidate parent bds - reopen it with BDRV_O_INACTIVE cleared
2. child is not yet invalidated
3. parent check that its BDRV_O_INACTIVE is cleared
4. parent writes to child
5. assert in bdrv_co_pwritev, as BDRV_O_INACTIVE is set for child

This patch fixes it by just changing invalidate sequence: invalidate
children first.

Signed-off-by: Vladimir Sementsov-Ogievskiy <[e-mail]>
Message-id: [e-mail]
Reviewed-by: Max Reitz <[e-mail]>
Reviewed-by: Stefan Hajnoczi <[e-mail]>
Signed-off-by: Max Reitz <[e-mail]>
---
 block.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

Patch

diff --git a/block.c b/block.c
index b8bc2a1c68..743c349100 100644
--- a/block.c
+++ b/block.c
@@ -3248,19 +3248,18 @@  void bdrv_invalidate_cache(BlockDriverState *bs, Error **errp)
     if (!(bs->open_flags & BDRV_O_INACTIVE)) {
         return;
     }
-    bs->open_flags &= ~BDRV_O_INACTIVE;
 
-    if (bs->drv->bdrv_invalidate_cache) {
-        bs->drv->bdrv_invalidate_cache(bs, &local_err);
+    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;
         }
     }
 
-    QLIST_FOREACH(child, &bs->children, next) {
-        bdrv_invalidate_cache(child->bs, &local_err);
+    bs->open_flags &= ~BDRV_O_INACTIVE;
+    if (bs->drv->bdrv_invalidate_cache) {
+        bs->drv->bdrv_invalidate_cache(bs, &local_err);
         if (local_err) {
             bs->open_flags |= BDRV_O_INACTIVE;
             error_propagate(errp, local_err);