On Fri, Oct 27, 2017 at 11:09:19AM +0200, Kevin Wolf wrote: > Am 27.10.2017 um 10:57 hat Jeff Cody geschrieben: > > The on disk image format 'inuse' header field is updated blindly if the > > image is opened RDWR. This can cause problems if the QEMU runstate is > > set to INMIGRATE, at which point the underlying file is set to INACTIVE. > > This causes an assert in bdrv_co_pwritev(). > > > > Do something similar to what is done in VHDX; latch the first write, and > > update the header the first time we modify the file. > > > > Signed-off-by: Jeff Cody <jc...@redhat.com> > > For VHDX, it seems that we have to have the header update in the write > path anyway, so it might be justifiable, but I think for parallels, it's > just ugly. >
A bit ugly. I think we could get around VHDX needing to do it as well; it does it in the write path for two scenarios: * First normal write, or * Journal log replay, if dirty, on open (if r/w) The log check happens early in vhdx_open(). If it does not write anything, then we can just write the headers during the open like normal, if we are R/W (and !BDRV_O_INACTIVE, of course). > The conservative approach to this would be doing the header write in > .bdrv_open() only if BDRV_O_INACTIVE is cleared, and otherwise do it > during .bdrv_invalidate_cache(). What scenarios cause BDRV_O_INACTIVE to not be set on bs, but set on bs->file-bs? > By the way, random design thought: It might make sense to change > .bdrv_open() so that it always opens inactive images and then call > .bdrv_invalidate_cache() (possibly renamed to .bdrv_activate()) > unconditionally even without migration. > > Kevin