[PULL 10/24] block: bdrv_reopen_parse_backing(): simplify handling implicit filters

Kevin Wolf posted 24 patches 3 years, 10 months ago
Maintainers: Kevin Wolf <kwolf@redhat.com>, John Snow <jsnow@redhat.com>, Raphael Norwitz <raphael.norwitz@nutanix.com>, Fam Zheng <fam@euphon.net>, Gerd Hoffmann <kraxel@redhat.com>, Jason Wang <jasowang@redhat.com>, Stefan Hajnoczi <stefanha@redhat.com>, "Gonglei (Arei)" <arei.gonglei@huawei.com>, Paolo Bonzini <pbonzini@redhat.com>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>, Max Reitz <mreitz@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>, "Michael S. Tsirkin" <mst@redhat.com>
There is a newer version of this series
[PULL 10/24] block: bdrv_reopen_parse_backing(): simplify handling implicit filters
Posted by Kevin Wolf 3 years, 10 months ago
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

The logic around finding overlay here is not obvious. Actually it does
two simple things:

1. If new bs is already in backing chain, split from parent bs by
   several implicit filters we are done, do nothing.

2. Otherwise, don't try to replace implicit filter.

Let's rewrite this in more obvious way.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20210610120537.196183-6-vsementsov@virtuozzo.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c | 53 ++++++++++++++++-------------------------------------
 1 file changed, 16 insertions(+), 37 deletions(-)

diff --git a/block.c b/block.c
index 6472866283..d047cae43c 100644
--- a/block.c
+++ b/block.c
@@ -4237,7 +4237,7 @@ static int bdrv_reopen_parse_backing(BDRVReopenState *reopen_state,
                                      Error **errp)
 {
     BlockDriverState *bs = reopen_state->bs;
-    BlockDriverState *overlay_bs, *below_bs, *new_backing_bs;
+    BlockDriverState *new_backing_bs;
     QObject *value;
     const char *str;
 
@@ -4266,6 +4266,18 @@ static int bdrv_reopen_parse_backing(BDRVReopenState *reopen_state,
         g_assert_not_reached();
     }
 
+    if (bs->backing) {
+        if (bdrv_skip_implicit_filters(bs->backing->bs) == new_backing_bs) {
+            return 0;
+        }
+
+        if (bs->backing->bs->implicit) {
+            error_setg(errp, "Cannot change backing link if '%s' has "
+                       "an implicit backing file", bs->node_name);
+            return -EPERM;
+        }
+    }
+
     /*
      * Ensure that @bs can really handle backing files, because we are
      * about to give it one (or swap the existing one)
@@ -4283,42 +4295,9 @@ static int bdrv_reopen_parse_backing(BDRVReopenState *reopen_state,
         return -EINVAL;
     }
 
-    /*
-     * Find the "actual" backing file by skipping all links that point
-     * to an implicit node, if any (e.g. a commit filter node).
-     * We cannot use any of the bdrv_skip_*() functions here because
-     * those return the first explicit node, while we are looking for
-     * its overlay here.
-     */
-    overlay_bs = bs;
-    for (below_bs = bdrv_filter_or_cow_bs(overlay_bs);
-         below_bs && below_bs->implicit;
-         below_bs = bdrv_filter_or_cow_bs(overlay_bs))
-    {
-        overlay_bs = below_bs;
-    }
-
-    /* If we want to replace the backing file we need some extra checks */
-    if (new_backing_bs != bdrv_filter_or_cow_bs(overlay_bs)) {
-        int ret;
-
-        /* Check for implicit nodes between bs and its backing file */
-        if (bs != overlay_bs) {
-            error_setg(errp, "Cannot change backing link if '%s' has "
-                       "an implicit backing file", bs->node_name);
-            return -EPERM;
-        }
-
-        reopen_state->replace_backing_bs = true;
-        reopen_state->old_backing_bs = bs->backing ? bs->backing->bs : NULL;
-        ret = bdrv_set_backing_noperm(bs, new_backing_bs, set_backings_tran,
-                                      errp);
-        if (ret < 0) {
-            return ret;
-        }
-    }
-
-    return 0;
+    reopen_state->replace_backing_bs = true;
+    reopen_state->old_backing_bs = bs->backing ? bs->backing->bs : NULL;
+    return bdrv_set_backing_noperm(bs, new_backing_bs, set_backings_tran, errp);
 }
 
 /*
-- 
2.31.1