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 <js...@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 <ebl...@redhat.com>
>
Wow, sorry, lots of Python lately!