Similarly to the previous commit, we need to also keep a ledger of the
additional directory size burden that we've not yet committed so we can
reject new additions sooner instead of later.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 block/qcow2.h        |  1 +
 block/qcow2-bitmap.c | 13 ++++++++++++-
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/block/qcow2.h b/block/qcow2.h
index ebf60ac236..5aff97eb9c 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -318,6 +318,7 @@ typedef struct BDRVQcow2State {
 
     uint32_t nb_bitmaps;
     uint32_t nb_queued_bitmaps;
+    uint32_t queued_directory_size;
     uint64_t bitmap_directory_size;
     uint64_t bitmap_directory_offset;
 
diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
index 7193c66787..b103fab362 100644
--- a/block/qcow2-bitmap.c
+++ b/block/qcow2-bitmap.c
@@ -1405,16 +1405,23 @@ static Qcow2Bitmap *find_bitmap_by_name(Qcow2BitmapList *bm_list,
 static int qcow2_remove_queued_dirty_bitmap(
     BlockDriverState *bs, const char *name, Error **errp)
 {
+    uint32_t size_delta;
     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;
     }
+
+    size_delta = calc_dir_entry_size(strlen(name), 0);
     assert(s->nb_queued_bitmaps > 0);
     assert(bdrv_dirty_bitmap_get_persistence(bitmap));
+    assert(s->queued_directory_size >= size_delta);
+
     s->nb_queued_bitmaps -= 1;
+    s->queued_directory_size -= size_delta;
 
     return 0;
 }
@@ -1561,6 +1568,7 @@ void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp)
         goto fail;
     }
     s->nb_queued_bitmaps = 0;
+    s->queued_directory_size = 0;
 
     /* Bitmap directory was successfully updated, so, old data can be dropped.
      * TODO it is better to reuse these clusters */
@@ -1636,6 +1644,7 @@ int qcow2_add_persistent_dirty_bitmap(BlockDriverState *bs,
     const char *name = bdrv_dirty_bitmap_name(bitmap);
     uint32_t granularity = bdrv_dirty_bitmap_granularity(bitmap);
     uint32_t nb_bitmaps;
+    uint32_t size_delta;
     int ret = 0;
 
     if (s->qcow_version < 3) {
@@ -1666,7 +1675,8 @@ int qcow2_add_persistent_dirty_bitmap(BlockDriverState *bs,
         goto fail;
     }
 
-    if (s->bitmap_directory_size + calc_dir_entry_size(strlen(name), 0) >
+    size_delta = calc_dir_entry_size(strlen(name), 0);
+    if (s->bitmap_directory_size + s->queued_directory_size + size_delta >
         QCOW2_MAX_BITMAP_DIRECTORY_SIZE)
     {
         error_setg(errp, "Not enough space in the bitmap directory");
@@ -1687,6 +1697,7 @@ int qcow2_add_persistent_dirty_bitmap(BlockDriverState *bs,
     }
 
     s->nb_queued_bitmaps += 1;
+    s->queued_directory_size += size_delta;
 
     return 0;
 fail:
-- 
2.20.1

On 6/6/19 1:41 PM, John Snow wrote:
> Similarly to the previous commit, we need to also keep a ledger of the
> additional directory size burden that we've not yet committed so we can
> reject new additions sooner instead of later.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  block/qcow2.h        |  1 +
>  block/qcow2-bitmap.c | 13 ++++++++++++-
>  2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/block/qcow2.h b/block/qcow2.h
> index ebf60ac236..5aff97eb9c 100644
> --- a/block/qcow2.h
> +++ b/block/qcow2.h
> @@ -318,6 +318,7 @@ typedef struct BDRVQcow2State {
>  
>      uint32_t nb_bitmaps;
>      uint32_t nb_queued_bitmaps;
> +    uint32_t queued_directory_size;
>      uint64_t bitmap_directory_size;

Why can we get away with uint32_t for the queue size, but uint64_t for
the stored size? Something feels fishy.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

On 6/6/19 10:30 PM, Eric Blake wrote:
> On 6/6/19 1:41 PM, John Snow wrote:
>> Similarly to the previous commit, we need to also keep a ledger of the
>> additional directory size burden that we've not yet committed so we can
>> reject new additions sooner instead of later.
>>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>>  block/qcow2.h        |  1 +
>>  block/qcow2-bitmap.c | 13 ++++++++++++-
>>  2 files changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/block/qcow2.h b/block/qcow2.h
>> index ebf60ac236..5aff97eb9c 100644
>> --- a/block/qcow2.h
>> +++ b/block/qcow2.h
>> @@ -318,6 +318,7 @@ typedef struct BDRVQcow2State {
>>  
>>      uint32_t nb_bitmaps;
>>      uint32_t nb_queued_bitmaps;
>> +    uint32_t queued_directory_size;
>>      uint64_t bitmap_directory_size;
> 
> Why can we get away with uint32_t for the queue size, but uint64_t for
> the stored size? Something feels fishy.
> 

That would be me using the wrong type.

--js