[PATCH v2 01/33] block: rename bdrv_replace_child to bdrv_replace_child_tran

Vladimir Sementsov-Ogievskiy posted 33 patches 3 years, 5 months ago
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Max Reitz <mreitz@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Cleber Rosa <crosa@redhat.com>, Eric Blake <eblake@redhat.com>, John Snow <jsnow@redhat.com>, Kevin Wolf <kwolf@redhat.com>, Eduardo Habkost <ehabkost@redhat.com>, Markus Armbruster <armbru@redhat.com>, Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
There is a newer version of this series
[PATCH v2 01/33] block: rename bdrv_replace_child to bdrv_replace_child_tran
Posted by Vladimir Sementsov-Ogievskiy 3 years, 5 months ago
We have bdrv_replace_child() wrapper on bdrv_replace_child_noperm().
But bdrv_replace_child() doesn't update permissions. It's rather
strange, as normally it's expected that foo() should call foo_noperm()
and update permissions.

Let's rename and add comment.

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

diff --git a/block.c b/block.c
index 75a82af641..384413c578 100644
--- a/block.c
+++ b/block.c
@@ -2247,12 +2247,14 @@ static TransactionActionDrv bdrv_replace_child_drv = {
 };
 
 /*
- * bdrv_replace_child
+ * bdrv_replace_child_tran
  *
  * Note: real unref of old_bs is done only on commit.
+ *
+ * The function doesn't update permissions, caller is responsible for this.
  */
-static void bdrv_replace_child(BdrvChild *child, BlockDriverState *new_bs,
-                               Transaction *tran)
+static void bdrv_replace_child_tran(BdrvChild *child, BlockDriverState *new_bs,
+                                    Transaction *tran)
 {
     BdrvReplaceChildState *s = g_new(BdrvReplaceChildState, 1);
     *s = (BdrvReplaceChildState) {
@@ -4749,7 +4751,7 @@ static void bdrv_remove_filter_or_cow_child_abort(void *opaque)
     }
 
     /*
-     * We don't have to restore child->bs here to undo bdrv_replace_child()
+     * We don't have to restore child->bs here to undo bdrv_replace_child_tran()
      * because that function is transactionable and it registered own completion
      * entries in @tran, so .abort() for bdrv_replace_child_safe() will be
      * called automatically.
@@ -4785,7 +4787,7 @@ static void bdrv_remove_filter_or_cow_child(BlockDriverState *bs,
     }
 
     if (child->bs) {
-        bdrv_replace_child(child, NULL, tran);
+        bdrv_replace_child_tran(child, NULL, tran);
     }
 
     s = g_new(BdrvRemoveFilterOrCowChild, 1);
@@ -4825,7 +4827,7 @@ static int bdrv_replace_node_noperm(BlockDriverState *from,
                        c->name, from->node_name);
             return -EPERM;
         }
-        bdrv_replace_child(c, to, tran);
+        bdrv_replace_child_tran(c, to, tran);
     }
 
     return 0;
-- 
2.29.2


Re: [PATCH v2 01/33] block: rename bdrv_replace_child to bdrv_replace_child_tran
Posted by Max Reitz 3 years, 5 months ago
On 20.05.21 16:21, Vladimir Sementsov-Ogievskiy wrote:
> We have bdrv_replace_child() wrapper on bdrv_replace_child_noperm().
> But bdrv_replace_child() doesn't update permissions. It's rather
> strange, as normally it's expected that foo() should call foo_noperm()
> and update permissions.
>
> Let's rename and add comment.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   block.c | 14 ++++++++------
>   1 file changed, 8 insertions(+), 6 deletions(-)


Reviewed-by: Max Reitz <mreitz@redhat.com>