Re: [Qemu-devel] [PATCH v3 07/16] block: Convert bs->backing_hd to BdrvChild

[Kevin Wolf <kwolf@redhat.com>]

Am 13.10.2015 um 03:53 hat Jeff Cody geschrieben:
> On Fri, Oct 09, 2015 at 02:15:32PM +0200, Kevin Wolf wrote:
> > This is the final step in converting all of the BlockDriverState
> > pointers that block drivers use to BdrvChild.
> > 
> > After this patch, bs->children contains the full list of child nodes
> > that are referenced by a given BDS, and these children are only
> > referenced through BdrvChild, so that updating the pointer in there is
> > enough for changing edges in the graph.
> > 
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > ---
> >  block.c                   | 105 +++++++++++++++++++++++-----------------------
> >  block/io.c                |  24 +++++------
> >  block/mirror.c            |   6 +--
> >  block/qapi.c              |   8 ++--
> >  block/qcow.c              |   4 +-
> >  block/qcow2-cluster.c     |   4 +-
> >  block/qcow2.c             |   6 +--
> >  block/qed.c               |  12 +++---
> >  block/stream.c            |   8 ++--
> >  block/vmdk.c              |  21 +++++-----
> >  block/vvfat.c             |   6 +--
> >  blockdev.c                |   4 +-
> >  include/block/block_int.h |  12 ++++--
> >  qemu-img.c                |   4 +-
> >  14 files changed, 115 insertions(+), 109 deletions(-)
> > 
> 
> [...]
> 
> > diff --git a/include/block/block_int.h b/include/block/block_int.h
> > index 98936c9..90971c0 100644
> > --- a/include/block/block_int.h
> > +++ b/include/block/block_int.h
> > @@ -378,8 +378,7 @@ struct BlockDriverState {
> >      QDict *full_open_options;
> >      char exact_filename[PATH_MAX];
> >  
> > -    BlockDriverState *backing_hd;
> > -    BdrvChild *backing_child;
> > +    BdrvChild *backing;
> >      BdrvChild *file;
> >  
> >      NotifierList close_notifiers;
> > @@ -458,6 +457,11 @@ struct BlockDriverState {
> >      NotifierWithReturn write_threshold_notifier;
> >  };
> >  
> > +static inline BlockDriverState *backing_bs(BlockDriverState *bs)
> > +{
> > +    return bs->backing ? bs->backing->bs : NULL;
> > +}
> > +
> 
> Is there a good guideline regarding when you prefer backing_bs() to be
> used, over accessing bs->backing->bs directly?  There seems to be a
> lot of mixed usage left in this patch, and I'm not sure if I am just
> missing the intended distinction.

Essentially, I'm using backing_bs() whenever bs->backing could be NULL,
because bs->backing->bs would segfault then. When I know that it's not
NULL, I prefer the direct access.

Kevin