diff mbox

[12/16] block: Introduce parents list

Message ID 1442497700-2536-13-git-send-email-kwolf@redhat.com
State New
Headers show

Commit Message

Kevin Wolf Sept. 17, 2015, 1:48 p.m. UTC
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c                   | 3 +++
 include/block/block_int.h | 2 ++
 2 files changed, 5 insertions(+)

Comments

Max Reitz Sept. 23, 2015, 4:39 p.m. UTC | #1
On 17.09.2015 15:48, Kevin Wolf wrote:
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block.c                   | 3 +++
>  include/block/block_int.h | 2 ++
>  2 files changed, 5 insertions(+)

Reviewed-by: Max Reitz <mreitz@redhat.com>
Alberto Garcia Sept. 28, 2015, 1:09 p.m. UTC | #2
On Thu 17 Sep 2015 03:48:16 PM CEST, Kevin Wolf wrote:

> @@ -1090,6 +1090,7 @@ static BdrvChild *bdrv_attach_child(BlockDriverState *parent_bs,
>      };
>  
>      QLIST_INSERT_HEAD(&parent_bs->children, child, next);
> +    QLIST_INSERT_HEAD(&child_bs->parents, child, next_parent);
>  
>      return child;
>  }

Ok, I'm probably slow today, but what is this used for? :-? And why is
it called 'parents'? The list doesn't contain pointers to the parents of
child_bs, but to child_bs itself...

I would expect a BdrvChild *parent, with parent->bs = parent_bs.

Berto
Kevin Wolf Sept. 29, 2015, 12:21 p.m. UTC | #3
Am 28.09.2015 um 15:09 hat Alberto Garcia geschrieben:
> On Thu 17 Sep 2015 03:48:16 PM CEST, Kevin Wolf wrote:
> 
> > @@ -1090,6 +1090,7 @@ static BdrvChild *bdrv_attach_child(BlockDriverState *parent_bs,
> >      };
> >  
> >      QLIST_INSERT_HEAD(&parent_bs->children, child, next);
> > +    QLIST_INSERT_HEAD(&child_bs->parents, child, next_parent);
> >  
> >      return child;
> >  }
> 
> Ok, I'm probably slow today, but what is this used for? :-? And why is
> it called 'parents'? The list doesn't contain pointers to the parents of
> child_bs, but to child_bs itself...
> 
> I would expect a BdrvChild *parent, with parent->bs = parent_bs.

It's the list of BdrvChild objects that point to the given
BlockDriverState. This is good enough for updating the pointers. If we
ever need the actual parent BDS, we can add a pointer to BdrvChild.

Kevin
Alberto Garcia Sept. 29, 2015, 2 p.m. UTC | #4
On Tue 29 Sep 2015 02:21:37 PM CEST, Kevin Wolf wrote:
>> > @@ -1090,6 +1090,7 @@ static BdrvChild *bdrv_attach_child(BlockDriverState *parent_bs,
>> >      };
>> >  
>> >      QLIST_INSERT_HEAD(&parent_bs->children, child, next);
>> > +    QLIST_INSERT_HEAD(&child_bs->parents, child, next_parent);
>> >  
>> >      return child;
>> >  }
>> 
>> Ok, I'm probably slow today, but what is this used for? :-? And why
>> is it called 'parents'? The list doesn't contain pointers to the
>> parents of child_bs, but to child_bs itself...
>
> It's the list of BdrvChild objects that point to the given
> BlockDriverState.

Ok I see now. I think the names here are misleading, 'parent' means the
BdrvChild object, not parent_bs.

The name child_bs->parents suggests that you can actually iterate over
the BDSs that have child_bs as a child, but that's not the case, it's a
mechanism that allows children to change the pointers that are pointing
at them.

I'm not sure what a better name would be ("reverse refs"?), but at least
I think this deserves a comment.

Berto
diff mbox

Patch

diff --git a/block.c b/block.c
index 7930f3c..c196f83 100644
--- a/block.c
+++ b/block.c
@@ -1090,6 +1090,7 @@  static BdrvChild *bdrv_attach_child(BlockDriverState *parent_bs,
     };
 
     QLIST_INSERT_HEAD(&parent_bs->children, child, next);
+    QLIST_INSERT_HEAD(&child_bs->parents, child, next_parent);
 
     return child;
 }
@@ -1097,6 +1098,7 @@  static BdrvChild *bdrv_attach_child(BlockDriverState *parent_bs,
 void bdrv_detach_child(BdrvChild *child)
 {
     QLIST_REMOVE(child, next);
+    QLIST_REMOVE(child, next_parent);
     g_free(child);
 }
 
@@ -2037,6 +2039,7 @@  static void bdrv_move_reference_fields(BlockDriverState *bs_dest,
     /* keep the same entry in bdrv_states */
     bs_dest->device_list = bs_src->device_list;
     bs_dest->blk = bs_src->blk;
+    bs_dest->parents = bs_src->parents;
 
     memcpy(bs_dest->op_blockers, bs_src->op_blockers,
            sizeof(bs_dest->op_blockers));
diff --git a/include/block/block_int.h b/include/block/block_int.h
index cfcae52..52ea7c0 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -339,6 +339,7 @@  struct BdrvChild {
     BlockDriverState *bs;
     const BdrvChildRole *role;
     QLIST_ENTRY(BdrvChild) next;
+    QLIST_ENTRY(BdrvChild) next_parent;
 };
 
 /*
@@ -445,6 +446,7 @@  struct BlockDriverState {
      * parent node of this node. */
     BlockDriverState *inherits_from;
     QLIST_HEAD(, BdrvChild) children;
+    QLIST_HEAD(, BdrvChild) parents;
 
     QDict *options;
     BlockdevDetectZeroesOptions detect_zeroes;