[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 3/4] block/parallels: Don't update header until
From: |
Jeff Cody |
Subject: |
Re: [Qemu-devel] [PATCH 3/4] block/parallels: Don't update header until the first actual write |
Date: |
Fri, 27 Oct 2017 06:18:30 -0400 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
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 <address@hidden>
>
> 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