[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v5 14/45] block/snapshot: drop indirection around bdrv_snapsh
From: |
Hanna Reitz |
Subject: |
Re: [PATCH v5 14/45] block/snapshot: drop indirection around bdrv_snapshot_fallback_ptr |
Date: |
Tue, 7 Jun 2022 17:58:59 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.0 |
On 30.03.22 23:28, Vladimir Sementsov-Ogievskiy wrote:
Now the indirection is not actually used, we can safely reduce it to
simple pointer.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@openvz.org>
---
block/snapshot.c | 39 +++++++++++++++++----------------------
1 file changed, 17 insertions(+), 22 deletions(-)
Looks good, just wondering whether we should drop some of the "_ptr"
suffixes now.
diff --git a/block/snapshot.c b/block/snapshot.c
index 02a880911f..4eb9258de6 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -151,34 +151,29 @@ bool bdrv_snapshot_find_by_id_and_name(BlockDriverState
*bs,
}
/**
- * Return a pointer to the child BDS pointer to which we can fall
+ * Return a pointer to child of given BDS to which we can fall
* back if the given BDS does not support snapshots.
* Return NULL if there is no BDS to (safely) fall back to.
- *
- * We need to return an indirect pointer because bdrv_snapshot_goto()
- * has to modify the BdrvChild pointer.
*/
-static BdrvChild **bdrv_snapshot_fallback_ptr(BlockDriverState *bs)
+static BdrvChild *bdrv_snapshot_fallback_ptr(BlockDriverState *bs)
The _ptr part was meant to point out that it returns an indirect
pointer; maybe we should name it bdrv_snapshot_fallback_child() now?
{
- BdrvChild **fallback;
- BdrvChild *child = bdrv_primary_child(bs);
+ BdrvChild *fallback = bdrv_primary_child(bs);
+ BdrvChild *child;
/* We allow fallback only to primary child */
- if (!child) {
+ if (!fallback) {
return NULL;
}
- fallback = (child == bs->file ? &bs->file : &bs->backing);
- assert(*fallback == child);
/*
* Check that there are no other children that would need to be
* snapshotted. If there are, it is not safe to fall back to
- * *fallback.
+ * fallback.
*/
QLIST_FOREACH(child, &bs->children, next) {
if (child->role & (BDRV_CHILD_DATA | BDRV_CHILD_METADATA |
BDRV_CHILD_FILTERED) &&
- child != *fallback)
+ child != fallback)
{
return NULL;
}
@@ -189,8 +184,8 @@ static BdrvChild
**bdrv_snapshot_fallback_ptr(BlockDriverState *bs)
static BlockDriverState *bdrv_snapshot_fallback(BlockDriverState *bs)
{
- BdrvChild **child_ptr = bdrv_snapshot_fallback_ptr(bs);
Just "child" is enough (and better) now, I think.
- return child_ptr ? (*child_ptr)->bs : NULL;
+ BdrvChild *child_ptr = bdrv_snapshot_fallback_ptr(bs);
+ return child_ptr ? child_ptr->bs : NULL;
}
int bdrv_can_snapshot(BlockDriverState *bs)
- Re: [PATCH v5 14/45] block/snapshot: drop indirection around bdrv_snapshot_fallback_ptr,
Hanna Reitz <=