Re: [Qemu-devel] [PATCH v3 7/7] block/dirty-bitmaps: implement inconsistent bit

[John Snow <jsnow@redhat.com>]

On 3/7/19 11:37 AM, Eric Blake wrote:
> On 3/6/19 3:46 PM, John Snow wrote:
> 
>>> I think, inconsistent bitmaps should have readonly flag always set or always unset, independently on can_write,
>>> as we are not going to write something related to inconsistent bitmaps.
>>>
>>
>> We'll never change metadata for inconsistent bitmaps, no. We might
>> delete them which causes a metadata change, though -- and right now the
>> readonly check stops that in blockdev.c before we attempt (and fail) the
>> operation. It does have at least a light usage.
>>
>> It's not a crucial feature, but I think the error message is nicer.
>>
>>> And I think, better is always unset, to have inconsistent bitmaps as something unrelated to readonly.
>>>
>>
>> The way I think of it is as unrelated, which is why I set readonly and
>> inconsistent orthogonally.
>>
>>> Readonly are bitmaps, which are not marked IN_USE on open for some reason..
>>>
>>
>> I tend to think of readonly bitmaps as persistent bitmaps attached to
>> storage we are unable to write back to, so we must prevent their
>> modification.
> 
> I like that wording (keeping read-only and inconsistent orthogonal),
> 
>>
>> I suppose the IN_USE viewpoint works too; "This is a bitmap that we have
>> not marked as IN_USE so it must not be used."
>>
> 
> Yes, it does work, but it is a bit more of a stretch, so I'm not quite
> as sure about using it.
> 
>>> I understand, that inconsistent bitmaps are some kind of readonly too, as we can't write to them..
>>> But we can't read too anyway, and we don't interfere with reopen logic at all. So again, better is
>>> don't mark inconsistent bitmaps as readonly.
>>>
>>
>> Hm, so in your model; we just NEVER set readonly for persistent bitmaps,
>> which incurs some changes at load time, but allows the reload_rw
>> function to go completely unmodified.
>>
>> That's not unreasonable in terms of SLOC, but semantically I am not sure
>> which approach is better. I'm leaning towards keeping this as written
>> for now, but... well. Any thoughts, Eric? Would you like to flip a coin?
> 
> No strong preferences, other than let's get it in before soft freeze, to
> make sure we don't miss out on it entirely due to waiting for the coin
> flip :)
> 
> 

OK. I have a slight preference for keeping the flag meanings orthogonal
and logically consistent, because I believe it keeps the code less
surprising to anyone who isn't Eric, Vlad, or myself.

So I think I will stage this as-is, and if it poses a problem I will
accept counter-proposals during freeze to shore it up if necessary.

--js