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

[Max Reitz <mreitz@redhat.com>]

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 <mreitz@redhat.com>
>> ---
>>   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