Re: [Qemu-devel] [PATCH v2 09/18] block/dirty-bitmap: add bdrv_dirty_bitmap_merge_internal

[Max Reitz]

On 05.07.19 18:45, John Snow wrote:
> 
> 
> On 7/4/19 12:49 PM, Max Reitz wrote:
>> On 03.07.19 23:55, John Snow wrote:

[...]

>>> +
>>> +/**
>>> + * bdrv_dirty_bitmap_merge_internal: merge src into dest.
>>> + * Does NOT check bitmap permissions; not suitable for use as public API.
>>> + *
>>> + * @backup: If provided, make a copy of dest here prior to merge.
>>> + * @lock: If true, lock and unlock bitmaps on the way in/out.
>>> + * returns true if the merge succeeded; false if unattempted.
>>> + */
>>> +bool bdrv_dirty_bitmap_merge_internal(BdrvDirtyBitmap *dest,
>>> +                                      const BdrvDirtyBitmap *src,
>>> +                                      HBitmap **backup,
>>> +                                      bool lock)
>>> +{
>>> +    bool ret;
>>> +
>>> +    if (lock) {
>>> +        qemu_mutex_lock(dest->mutex);
>>> +        if (src->mutex != dest->mutex) {
>>> +            qemu_mutex_lock(src->mutex);
>>> +        }
>>> +    }
>>> +
>>
>> Why not check for INCONSISTENT and RO still?
>>
>> Max
>>
> 
> Well. "why", I guess. The user-facing API will always use the
> non-internal version. This is the scary dangerous version that you don't
> call unless you are Very Sure You Know What You're Doing.
> 
> I guess I just intended for the suitability checking to happen in
> bdrv_dirty_bitmap_merge, and this is the implementation that you can
> shoot yourself in the foot with if you'd like.

I’m asking because the reasoning behind being allowed to call this
function is that BUSY means someone who is not the monitor has exclusive
access to the bitmap, and that someone is the caller of this function.

There is no justification for why it should be allowed to call this
function for bitmaps that are inconsistent or read-only.  If someone
needs that, they should justify it with a patch, I think.

Max

signature.asc
Description: OpenPGP digital signature