On 17.09.2015 15:48, Kevin Wolf wrote: > Store the BdrvChild for bs->file. At this point, bs->file_child->bs just > duplicates the bs->file pointer. Later, it will completely replace it. > > Signed-off-by: Kevin Wolf <kw...@redhat.com> > --- > block.c | 12 +++++++++--- > include/block/block_int.h | 1 + > 2 files changed, 10 insertions(+), 3 deletions(-)
Reviewed-by: Max Reitz <mre...@redhat.com> Although I have a small comment below: > > diff --git a/block.c b/block.c > index 6268e37..2e9e5e2 100644 > --- a/block.c > +++ b/block.c > @@ -1487,11 +1487,17 @@ static int bdrv_open_inherit(BlockDriverState **pbs, > const char *filename, > > assert(file == NULL); This looks strange now, without the direct connection to bdrv_open_image(). I'd drop this... > bs->open_flags = flags; > - ret = bdrv_open_image(&file, filename, options, "file", > - bs, &child_file, true, &local_err); > - if (ret < 0) { > + > + bs->file_child = bdrv_open_child(filename, options, "file", bs, > + &child_file, true, &local_err); > + if (local_err) { > + ret = -EINVAL; > goto fail; > } > + > + if (bs->file_child) { > + file = bs->file_child->bs; > + } And make this file = bs->file_child ? bs->file_child->bs : NULL (or the respective long form, i.e. if (bs->file_child) { ... } else { file = NULL; }). We could even put this after this if block and drop the NULL initialization of file, but that might go overboard. Max > } > > /* Image format probing */ > diff --git a/include/block/block_int.h b/include/block/block_int.h > index 14ad4c3..d0dd93e 100644 > --- a/include/block/block_int.h > +++ b/include/block/block_int.h > @@ -381,6 +381,7 @@ struct BlockDriverState { > BlockDriverState *backing_hd; > BdrvChild *backing_child; > BlockDriverState *file; > + BdrvChild *file_child; > > NotifierList close_notifiers; > >
signature.asc
Description: OpenPGP digital signature