[Qemu-devel] Re: [PATCH] block: Split bdrv_open

[Kevin Wolf <kwolf@redhat.com>]

Am 13.04.2010 20:20, schrieb Christoph Hellwig:
> On Mon, Apr 12, 2010 at 04:49:16PM +0200, Kevin Wolf wrote:
>> bdrv_open contains quite some code that is only useful for opening images (as
>> opposed to opening files by a protocol), for example snapshots.
>>
>> This patch splits the code so that we have bdrv_open_file() for files (uses
>> protocols), bdrv_open() for images (uses format drivers) and bdrv_do_open() for
>> the code common for opening both images and files.
>>
>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
>> ---
>> This patch applies on top of Christoph's RFC for the format/protocol split
> 
> I like this a lot.  A few comments:
> 
>  - why is bdrv_do_open below it's users in the code?  I really hate
>    forward declarations of functions and they can usually be easily
>    avoided.

Ok, I'll move it.

>  - a "do" a function name is not very meaningfull - what about
>    bdrv_open_common instead?

Heh, did exactly this yesterday because I felt the same. It's just not
pushed yet.

>  - doesn't the backing device handling only apply to image formats, too?

Hm, probably yes. At least currently no protocol uses it. We could add
an assert(bs->backing_hd == NULL) to bdrv_file_open to make clear that
backing files are not wanted for protocols.

Kevin