Re: [Qemu-devel] [v9 Patch 5/6]Qemu: Framework for reopening images safely

[Supriya Kannery <supriyak@linux.vnet.ibm.com>]

On 11/22/2011 05:19 PM, Stefan Hajnoczi wrote:
> On Tue, Nov 22, 2011 at 11:16 AM, supriya kannery<supriyak@in.ibm.com>  wrote:
>> Kevin Wolf wrote:
>>>
>>> Am 22.11.2011 11:24, schrieb supriya kannery:
>>>
>>>>
>>>>>
>>>>> How does VMDK implement its prepare/commit/abort?  It needs to use the
>>>>> "public" bdrv_reopen_prepare() function on its image files.
>>>>>
>>>>
>>>> bdrv_reopen() is the public interface which gets called by any of the
>>>> image formats.
>>>> So VMDK or any image format has to call bdrv_reopen which decides to call
>>>> driver specific prepare/commit/abort or simply close and reopen the file.
>>>>
>>>
>>> No, that doesn't work. In order to get all-or-nothing semantics, you
>>> need to explicitly prepare all child images and only when you know the
>>> results of all preparations, you can decide whether to commit or abort
>>> all.
>>>
>>
>> bdrv_reopen_prepare/commit/abort will be implemented specific to VMDK in
>> vmdk.c. Then for vmdk,
>> drv->bdrv_reopen_prepare() will handle  preparing child images and return
>> success to bdrv_reopen ()
>> only if all of them get prepared successfully.  The prepare/commit/abort
>> concept we took up considering
>> vmdk's special case of multiple files.
>>
>> So it is bdrv_reopen() which is public and called by hostcache change
>> request for any of the image formats.
>> It then routes the processing to respective prepare/commit/abort implemented
>> by the drivers, including VMDK.
>> In cases where drivers don't have their own implementation, default route is
>> taken which is simply
>> closing and opening the file.
>
> VMDK must call bdrv_reopen_prepare()/bdrv_reopen_commit()/bdrv_reopen_abort()
> on its child images in order to support aborting when there is a
> failure half-way through.  If it used bdrv_reopen() on its child
> images then it could not roll back later when there is a failure on
> the next child.
>

I got both your's and Kevin's point now, after looking into vmdk.c code
related to extents.
My initial understanding was bdrv_reopen_prepare() implemented inside 
vmdk.c can be called for handling reopening of multiple child
images. But observing how vmdk_open() is implemented for child images, I 
should follow that method. So will make 
bdrv_reopen_prepare()/commit/abort in block.c to be used by vmdk as well 
as other required image formats (like how raw.c is using them).

- thanks, Supriya