Re: [Qemu-devel] [PATCH v3 1/2] block: Do not prematurely remove "filename"

[Max Reitz <mreitz@redhat.com>]

On 30.06.2014 11:42, Kevin Wolf wrote:
> Am 26.06.2014 um 23:38 hat Max Reitz geschrieben:
>> If "filename" is removed from the options QDict before entering
>> bdrv_open_common(), it cannot be stored in the BDS.  Therefore, wait
>> until it has been copied there and remove it from the options only
>> afterwards.
>>
>> This fixes "filename" in the BDS being empty for block drivers which do
>> not need the filename because they have parsed it already (e.g. NBD).
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
> I can't say I like this approach. It looks a bit odd to pass a boolean
> variable to bdrv_open(), and in some other function called from there
> the cleanup is done that logically really belong to bdrv_fill_options().
>
> More importantly, the goal was to get rid of the filename and handle
> everything through the options so that we get a uniform state again.
> This would involve replacing bs->filename by a new callback function in
> BlockDriver that constructs a filename that describes the BDS. This way
> we would get useful output not only for "nbd:localhost:10809", but also
> for "driver=nbd,host=localhost".

Right. This "bug" isn't that severe either, so I guess it's fine to take 
more time to fix the general problem.

Max

> In hard cases, the callback might just use "json:{...}" syntax. This
> suggests that maybe in the end we'll want to have two different
> callbacks, one giving a short human-readable description
> ('localhost:10809') and another one giving something that can be used on
> the command line ('json:{"driver": "nbd", "host": "localhost", "ipv6":
> true}').
>
> Kevin