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.