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

[Max Reitz]

On 2017-06-02 00:29, John Snow wrote:
> 
> 
> 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?)

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...)

I don't mind either way. I like a dirty flag because this is a common
concept (we basically cache the persistent bitmaps in RAM, so it's
natural for them to have a dirty/clean state); but I also like keeping
the read-only flag because it's probably simpler to implement (and makes
just as much sense).

Max

signature.asc
Description: OpenPGP digital signature