[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [PATCH] block: Split bdrv_open
From: |
Kevin Wolf |
Subject: |
[Qemu-devel] Re: [PATCH] block: Split bdrv_open |
Date: |
Wed, 14 Apr 2010 09:22:19 +0200 |
User-agent: |
Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.9) Gecko/20100330 Fedora/3.0.4-1.fc12 Thunderbird/3.0.4 |
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 <address@hidden>
>> ---
>> 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