Re: [Qemu-devel] [PATCH] block: fix the use of protocols in backing files

[Kevin Wolf <kwolf@redhat.com>]

Am 04.11.2010 14:14, schrieb Anthony Liguori:
> On 11/04/2010 07:54 AM, Kevin Wolf wrote:
>> Am 27.10.2010 20:19, schrieb Anthony Liguori:
>>    
>>> Signed-off-by: Anthony Liguori<aliguori@us.ibm.com>
>>>
>>> diff --git a/block.c b/block.c
>>> index 1a965b2..00b6f21 100644
>>> --- a/block.c
>>> +++ b/block.c
>>> @@ -603,10 +603,16 @@ int bdrv_open(BlockDriverState *bs, const char *filename, int flags,
>>>           BlockDriver *back_drv = NULL;
>>>
>>>           bs->backing_hd = bdrv_new("");
>>> -        path_combine(backing_filename, sizeof(backing_filename),
>>> -                     filename, bs->backing_file);
>>> -        if (bs->backing_format[0] != '\0')
>>> -            back_drv = bdrv_find_format(bs->backing_format);
>>> +        back_drv = bdrv_find_protocol(bs->backing_file);
>>> +        if (!back_drv) {
>>>      
>> If no protocol is specified, bdrv_find_protocol doesn't return NULL but
>> the file: driver.
>>    
> 
> An ugly way to handle this would be to do if (strstr(bs->backing_file, 
> ":") == NULL) instead.

Ugly indeed.

> A deeper refactoring could return NULL in bdrv_find_protocol and fixup 
> the callers to default to file: if none are specified.

NULL is already used for errors, so we'd have to have something like

  int bdrv_find_protocol(const char *filename, BlockDriver **drv)

in order to be able to distinguish "invalid protocol" from "no explicit
protocol". You could rename this function and retain a
bdrv_find_protocol with the old prototype as a wrapper that returns file
instead of NULL (there are several callers that expect this behaviour,
so probably it makes sense to have it in a central place).

Does that sound reasonable?

Kevin