Re: [Qemu-devel] [PATCH 4/5] block/qcow2-bitmap: Count queued bitmaps towards nb_bitmaps

[John Snow <jsnow@redhat.com>]

On 6/6/19 10:27 PM, Eric Blake wrote:
> On 6/6/19 1:41 PM, John Snow wrote:
>> When we check to see if we can store a bitmap, we don't check how many
>> we've queued up. This can cause a problem saving bitmaps on close
>> instead of when we request them to be added. With the stricter add
>> interface, prohibit these bitmaps specifically.
>>
>> To match, make the remove interface more strict as well; now rejecting
>> any requests to remove bitmaps that were never queued for storage.
>>
>> We don't need to "find" the bitmap when the interface has been given the
>> bitmap explicitly, but this is done to make sure that the bitmap given
>> actually does belong to the bs we were passed as a paranoia check to
>> enforce consistency.
>>
>> ---
> 
> Oops - that marker...
> 
>>
>> "What about directory size?" Please see the following patch.
>>
>> Signed-off-by: John Snow <jsnow@redhat.com>
> 
> ...renders the S-o-b invisible.
> 
>> +++ b/block/qcow2-bitmap.c
>> @@ -1402,6 +1402,23 @@ static Qcow2Bitmap *find_bitmap_by_name(Qcow2BitmapList *bm_list,
>>      return NULL;
>>  }
>>  
>> +static int qcow2_remove_queued_dirty_bitmap(
>> +    BlockDriverState *bs, const char *name, Error **errp)
>> +{
>> +    BDRVQcow2State *s = bs->opaque;
>> +    BdrvDirtyBitmap *bitmap = bdrv_find_dirty_bitmap(bs, name);
>> +    if (!bitmap) {
>> +        error_setg(errp, "Node '%s' has no stored or enqueued bitmap '%s'",
>> +                   bdrv_get_node_name(bs), name);
>> +        return -ENOENT;
>> +    }
>> +    assert(s->nb_queued_bitmaps > 0);
>> +    assert(bdrv_dirty_bitmap_get_persistence(bitmap));
>> +    s->nb_queued_bitmaps -= 1;
> 
> I tend to use -- over -= 1.
> 
>> @@ -1667,6 +1686,8 @@ int qcow2_add_persistent_dirty_bitmap(BlockDriverState *bs,
>>          goto fail;
>>      }
>>  
>> +    s->nb_queued_bitmaps += 1;
> 
> And again, for ++
> 
> Reviewed-by: Eric Blake <eblake@redhat.com>
> 

Wow, sorry, lots of Python lately!