Re: [Qemu-devel] [PATCH 06/10] block: Remove bdrv_new() from bdrv_file_open()

[Max Reitz]

On 29.01.2014 14:35, Kevin Wolf wrote:
> Am 26.01.2014 um 20:02 hat Max Reitz geschrieben:
> > Change bdrv_file_open() to take a simple pointer to an already existing
> > BDS instead of an indirect one. The BDS will be created in bdrv_open()
> > if necessary.
> >
> > Signed-off-by: Max Reitz <address@hidden>
> > ---
> >   block.c | 29 ++++++++++++++++++-----------
> >   1 file changed, 18 insertions(+), 11 deletions(-)
> >
> > diff --git a/block.c b/block.c
> > index 6c29115..72eddd5 100644
> > --- a/block.c
> > +++ b/block.c
> > @@ -947,17 +947,15 @@ free_and_fail:
> >    * after the call (even on failure), so if the caller intends to reuse the
> >    * dictionary, it needs to use QINCREF() before calling bdrv_file_open.
> >    */
> > -static int bdrv_file_open(BlockDriverState **pbs, const char *filename,
> > +static int bdrv_file_open(BlockDriverState *bs, const char *filename,
> >                             QDict *options, int flags, Error **errp)
> >   {
> > -    BlockDriverState *bs = NULL;
> >       BlockDriver *drv;
> >       const char *drvname;
> >       bool allow_protocol_prefix = false;
> >       Error *local_err = NULL;
> >       int ret;
> >   
> > -    bs = bdrv_new("");
> >       bs->options = options;
> >       options = qdict_clone_shallow(options);
> >   
> > @@ -1036,7 +1034,6 @@ static int bdrv_file_open(BlockDriverState **pbs, const char *filename,
> >       QDECREF(options);
> >   
> >       bs->growable = 1;
> > -    *pbs = bs;
> >       return 0;
> >   
> >   fail:
> > @@ -1044,7 +1041,6 @@ fail:
> >       if (!bs->drv) {
> >           QDECREF(bs->options);
> >       }
> > -    bdrv_unref(bs);
> >       return ret;
> >   }
> >   
> > @@ -1235,17 +1231,28 @@ int bdrv_open(BlockDriverState **pbs, const char *filename,
> >           return 0;
> >       }
> >   
> > -    if (flags & BDRV_O_PROTOCOL) {
> > -        assert(!drv);
> > -        return bdrv_file_open(pbs, filename, options, flags & ~BDRV_O_PROTOCOL,
> > -                              errp);
> > -    }
> > -
> >       if (*pbs) {
> >           bs = *pbs;
> >       } else {
> >           bs = bdrv_new("");
> >       }
> > +
> > +    if (flags & BDRV_O_PROTOCOL) {
> > +        assert(!drv);
> > +        ret = bdrv_file_open(bs, filename, options, flags & ~BDRV_O_PROTOCOL,
> > +                             errp);
> > +        if (ret) {
> > +            if (*pbs) {
> > +                bdrv_close(bs);
> Why would bs be open when bdrv_file_open() just failed?

Good question. *g*

I'll remove this. ;-)

Max

> > +            } else {
> > +                bdrv_unref(bs);
> > +            }
> > +        } else {
> > +            *pbs = bs;
> > +        }
> > +        return ret;
> > +    }
> > +
> >       bs->options = options;
> >       options = qdict_clone_shallow(options);
> Kevin