Re: [Qemu-block] [PATCH 7/8] block: Make bdrv_open() return a BDS

[Max Reitz]

On 01.12.2015 15:44, Alberto Garcia wrote:
> On Tue 10 Nov 2015 04:44:22 AM CET, Max Reitz wrote:
>> @@ -1398,32 +1397,21 @@ static int bdrv_open_inherit(BlockDriverState **pbs, 
>> const char *filename,
>>          bool options_non_empty = options ? qdict_size(options) : false;
>>          QDECREF(options);
>>  
>> -        if (*pbs) {
>> -            error_setg(errp, "Cannot reuse an existing BDS when referencing 
>> "
>> -                       "another block device");
>> -            return -EINVAL;
>> -        }
>> -
>>          if (filename || options_non_empty) {
>>              error_setg(errp, "Cannot reference an existing block device 
>> with "
>>                         "additional options or a new filename");
>> -            return -EINVAL;
>> +            return NULL;
>>          }
>>  
>>          bs = bdrv_lookup_bs(reference, reference, errp);
>>          if (!bs) {
>> -            return -ENODEV;
>> +            return NULL;
>>          }
>>          bdrv_ref(bs);
>> -        *pbs = bs;
>> -        return 0;
>> +        return bs;
>>      }
> 
> This last part is probably a bit simpler with just one return statement:
> 
>            bs = bdrv_lookup_bs(reference, reference, errp);
>            if (bs) {
>                bdrv_ref(bs);
>            }
>            return bs;
> 
> but I'm fine either way.

Yes, thanks for the suggestion; but I think I like it to be explicit
("ret = do_something(); if (ret indicates failure) { fail; } go_on;").
Maybe I'm just not flexible enough to discard my precious patterns. :-)

> Reviewed-by: Alberto Garcia <address@hidden>

Thanks!

Max

signature.asc
Description: OpenPGP digital signature