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

[Jeff Cody <jcody@redhat.com>]

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.