[PATCH v5 15/45] block: refactor bdrv_remove_file_or_backing_child to bdrv_remove_child

Vladimir Sementsov-Ogievskiy posted 45 patches 3 years, 8 months ago
Maintainers: Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, Ari Sundholm <ari@tuxera.com>, Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>, Paolo Bonzini <pbonzini@redhat.com>, Stefan Hajnoczi <stefanha@redhat.com>, John Snow <jsnow@redhat.com>, Vladimir Sementsov-Ogievskiy <v.sementsov-og@mail.ru>, "Denis V. Lunev" <den@openvz.org>, Wen Congyang <wencongyang2@huawei.com>, Xie Changlong <xiechanglong.d@gmail.com>, Stefan Weil <sw@weilnetz.de>, Jeff Cody <codyprime@gmail.com>, Fam Zheng <fam@euphon.net>, Markus Armbruster <armbru@redhat.com>, Eric Blake <eblake@redhat.com>
[PATCH v5 15/45] block: refactor bdrv_remove_file_or_backing_child to bdrv_remove_child
Posted by Vladimir Sementsov-Ogievskiy 3 years, 8 months ago
Now the function can remove any child, so give it more common name.
Drop assertions and drop bs argument which becomes unused. Function
would be reused in a further commit.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@openvz.org>
---
 block.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/block.c b/block.c
index 6b43e101a1..ea5687edc8 100644
--- a/block.c
+++ b/block.c
@@ -92,9 +92,7 @@ static bool bdrv_recurse_has_child(BlockDriverState *bs,
 
 static void bdrv_replace_child_noperm(BdrvChild *child,
                                       BlockDriverState *new_bs);
-static void bdrv_remove_file_or_backing_child(BlockDriverState *bs,
-                                              BdrvChild *child,
-                                              Transaction *tran);
+static void bdrv_remove_child(BdrvChild *child, Transaction *tran);
 static void bdrv_remove_filter_or_cow_child(BlockDriverState *bs,
                                             Transaction *tran);
 
@@ -3347,7 +3345,7 @@ static int bdrv_set_file_or_backing_noperm(BlockDriverState *parent_bs,
 
     if (child) {
         bdrv_unset_inherits_from(parent_bs, child, tran);
-        bdrv_remove_file_or_backing_child(parent_bs, child, tran);
+        bdrv_remove_child(child, tran);
     }
 
     if (!child_bs) {
@@ -5031,26 +5029,22 @@ static bool should_update_child(BdrvChild *c, BlockDriverState *to)
     return ret;
 }
 
-static void bdrv_remove_filter_or_cow_child_commit(void *opaque)
+static void bdrv_remove_child_commit(void *opaque)
 {
     GLOBAL_STATE_CODE();
     bdrv_child_free(opaque);
 }
 
-static TransactionActionDrv bdrv_remove_filter_or_cow_child_drv = {
-    .commit = bdrv_remove_filter_or_cow_child_commit,
+static TransactionActionDrv bdrv_remove_child_drv = {
+    .commit = bdrv_remove_child_commit,
 };
 
 /*
  * A function to remove backing or file child of @bs.
  * Function doesn't update permissions, caller is responsible for this.
  */
-static void bdrv_remove_file_or_backing_child(BlockDriverState *bs,
-                                              BdrvChild *child,
-                                              Transaction *tran)
+static void bdrv_remove_child(BdrvChild *child, Transaction *tran)
 {
-    assert(child == bs->backing || child == bs->file);
-
     if (!child) {
         return;
     }
@@ -5059,7 +5053,7 @@ static void bdrv_remove_file_or_backing_child(BlockDriverState *bs,
         bdrv_replace_child_tran(child, NULL, tran);
     }
 
-    tran_add(tran, &bdrv_remove_filter_or_cow_child_drv, child);
+    tran_add(tran, &bdrv_remove_child_drv, child);
 }
 
 /*
@@ -5070,7 +5064,7 @@ static void bdrv_remove_file_or_backing_child(BlockDriverState *bs,
 static void bdrv_remove_filter_or_cow_child(BlockDriverState *bs,
                                             Transaction *tran)
 {
-    bdrv_remove_file_or_backing_child(bs, bdrv_filter_or_cow_child(bs), tran);
+    bdrv_remove_child(bdrv_filter_or_cow_child(bs), tran);
 }
 
 static int bdrv_replace_node_noperm(BlockDriverState *from,
-- 
2.35.1
Re: [PATCH v5 15/45] block: refactor bdrv_remove_file_or_backing_child to bdrv_remove_child
Posted by Hanna Reitz 3 years, 6 months ago
On 30.03.22 23:28, Vladimir Sementsov-Ogievskiy wrote:
> Now the function can remove any child, so give it more common name.
> Drop assertions and drop bs argument which becomes unused. Function
> would be reused in a further commit.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@openvz.org>
> ---
>   block.c | 22 ++++++++--------------
>   1 file changed, 8 insertions(+), 14 deletions(-)

Good!

> diff --git a/block.c b/block.c
> index 6b43e101a1..ea5687edc8 100644
> --- a/block.c
> +++ b/block.c

[...]

> -static TransactionActionDrv bdrv_remove_filter_or_cow_child_drv = {
> -    .commit = bdrv_remove_filter_or_cow_child_commit,
> +static TransactionActionDrv bdrv_remove_child_drv = {
> +    .commit = bdrv_remove_child_commit,
>   };
>   
>   /*
>    * A function to remove backing or file child of @bs.

I think it’d make sense to update this description here.

>    * Function doesn't update permissions, caller is responsible for this.
>    */
> -static void bdrv_remove_file_or_backing_child(BlockDriverState *bs,
> -                                              BdrvChild *child,
> -                                              Transaction *tran)
> +static void bdrv_remove_child(BdrvChild *child, Transaction *tran)
>   {
> -    assert(child == bs->backing || child == bs->file);
> -
>       if (!child) {
>           return;
>       }