Re: [Qemu-devel] [PATCH 02/14] block: Add blk_new_open()

[Max Reitz <mreitz@redhat.com>]

On 2014-12-18 at 16:19, Kevin Wolf wrote:
> Am 11.12.2014 um 14:20 hat Max Reitz geschrieben:
>> blk_new_with_bs() creates a BlockBackend with an empty BlockDriverState
>> attached to it. Empty BDSs are not nice, therefore add an alternative
>> function which combines blk_new_with_bs() with bdrv_open().
>>
>> Note: In contrast to bdrv_open() which takes a BlockDriver parameter,
>> blk_new_open() does not take such a parameter. This is because
>> bdrv_open() opens a BlockDriverState, therefore it is naturally to be
>> able to set the BlockDriver for that BDS. The fact that bdrv_open() can
>> open more than a single BDS is merely some form of a byproduct.
>>
>> blk_new_open() on the other hand is intended to be used to create a
>> whole tree of BlockDriverStates. Therefore, setting a single BlockDriver
>> does not make much sense. Instead, the drivers to be used for each of
>> the nodes must be configured through the "options" QDict; including the
>> driver of the root BDS.
> This is an interesting point. I generally agree with your reasoning, but
> if we did things right, the same would apply to filename and flags as
> well. But we don't, so we can't remove them now.
>
> I'm actually surprised that leaving out the driver option seem to work
> well enough.

Well, just putting the filename into the QDict won't work thanks to the 
additional parsing that is done on the non-QDict filename; and most 
flags don't have any correspondence in the QDict.

On the other hand, putting the driver name into the QDict is easy enough 
and will do exactly the same as specifying the BlockDriver directly, 
except for some different error messages.

> Should at least a TODO be left here for removing filename and flags?

Yes, will do.

Max