Re: [Qemu-devel] [PATCH 09/25] block/dirty-bitmap: add readonly field to BdrvDirtyBitmap

[John Snow]

On 05/31/2017 09:43 AM, Max Reitz wrote:
> On 2017-05-30 08:50, Vladimir Sementsov-Ogievskiy wrote:
>> Thank you for this scenario. Hmm.
>>
>> So, as I need guarantee that image and bitmap are unchanged,
>> bdrv_set_dirty should return error and fail the whole write. Ok?
> 
> I don't know. That would mean that you couldn't commit to an image that
> has a persistent auto-loading bitmap, which doesn't seem very nice to me.
> 
> I'm not quite sure what to do myself. So first I'd definitely want the
> commit operation to succeed. That means we'd have to automatically make
> the bitmap non-readonly once we write to it. The "readonly" flag would
> then be an "unchanged" flag, rather, to signify that the bitmap has not
> been changed since it was loaded, which means that it does not need to
> be written back to the image file.
> 
> Now the issue remains that if you modify a persistent bitmap that is
> stored in an image file that is opened RO when it's closed, you won't be
> able to write the modifications back.
> > So in addition, I guess we'd need to "flush" all persistent bitmaps
> (that is, write all modifications back to the file and set the
> "unchanged" flag (you could also call it "dirty" and then mean the
> opposite) for each bitmap) not only when the image is closed or
> invalidated, but also when it is reopened read-only.
>

Makes sense.

> (block-commit reopens the backing BDS R/W, then writes to them, thus
> modifying the dirty bitmaps, and finally reopens the BDS as read-only;
> before that happens, we will have to flush the modified bitmap data.)
> 

OK, so it would perhaps be enough to toggle the RO flag on/off when
nodes get reopened. When they get reopened RO, we'd need to flush at
that point.

(Right?)

Of course, a changed flag makes this a little moot as it is probably
more flexible; but there is something slightly attractive about the more
rigid form.

(Hmm, for the purposes of periodic flushing, we may want a changed flag
anyway...)

> Max
> 
Thanks for the scenario and the explainer.