Re: [Qemu-devel] [PATCH 2/7] block: Framework for reopening files safely

[Kevin Wolf <kwolf@redhat.com>]

Am 11.09.2012 17:36, schrieb Jeff Cody:
> On 09/11/2012 11:14 AM, Kevin Wolf wrote:
>> Am 11.09.2012 16:57, schrieb Jeff Cody:
>>> On 08/30/2012 02:47 PM, Jeff Cody wrote:
>>>> This is based heavily on Supriya Kannery's bdrv_reopen()
>>>> patch series.
>>>>
>>>> This provides a transactional method to reopen multiple
>>>> images files safely.
>>>>
>>>> Image files are queue for reopen via bdrv_reopen_queue(), and the
>>>> reopen occurs when bdrv_reopen_multiple() is called.  Changes are
>>>> staged in bdrv_reopen_prepare() and in the equivalent driver level
>>>> functions.  If any of the staged images fails a prepare, then all
>>>> of the images left untouched, and the staged changes for each image
>>>> abandoned.
>>>>
>>>
>>> Open question (my assumption is yes):
>>>
>>> Is it safe to assume that reopen() should always enable BDRV_O_CACHE_WB
>>> (without affecting enable_write_cache), so as to not undo what was done
>>> by Paolo's commit e1e9b0ac?
>>
>> I think it makes sense to behave the same as bdrv_open_common(), so I
>> guess yes. But now I'm wondering if we also need other code from there,
>> like filtering out BDRV_O_SNAPSHOT/NO_BACKING etc.
>>
> 
> I was wondering the same thing w/regards to BDRV_O_SNAPSHOT/NO_BACKING,
> but I fell on the side of 'no'. Mainly because the raw drivers (raw-posix,
> raw-win32) actively parse the passed flags to determine the actual open
> flags, and so spurious flags such as those are ignored.  However,
> BDRV_O_CACHE_WB is used in their flag parsing logic, so I think it needs
> to be preserved.

That's probably logic that should be removed in raw-posix/win32.c as it
is unused now.

Kevin