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

[Eric Blake]

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 <address@hidden>

...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 <address@hidden>

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

signature.asc
Description: OpenPGP digital signature