On Wed, 2019-11-13 at 20:46 -0600, Eric Blake wrote:
> We document that for qcow2 persistent bitmaps, the name cannot exceed
> 1023 bytes. It is inconsistent if transient bitmaps do not have to
> abide by the same limit, and it is unlikely that any existing client
> even cares about using bitmap names this long. It's time to codify
> that ALL bitmaps managed by qemu (whether persistent in qcow2 or not)
> have a documented maximum length.
Strange a bit that 1023 was choosen, I guess implementation
uses a 1024 zero terminated string for storing the names
in memory.
>
> Signed-off-by: Eric Blake <ebl...@redhat.com>
> ---
> qapi/block-core.json | 2 +-
> include/block/dirty-bitmap.h | 2 ++
> block/dirty-bitmap.c | 12 +++++++++---
> block/qcow2-bitmap.c | 2 ++
> 4 files changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index aa97ee264112..0cf68fea1450 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -2042,7 +2042,7 @@
> #
> # @node: name of device/node which the bitmap is tracking
> #
> -# @name: name of the dirty bitmap
> +# @name: name of the dirty bitmap (must be less than 1024 bytes)
> #
> # @granularity: the bitmap granularity, default is 64k for
> # block-dirty-bitmap-add
> diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
> index 958e7474fb51..e2b20ecab9a3 100644
> --- a/include/block/dirty-bitmap.h
> +++ b/include/block/dirty-bitmap.h
> @@ -14,6 +14,8 @@ typedef enum BitmapCheckFlags {
> BDRV_BITMAP_INCONSISTENT)
> #define BDRV_BITMAP_ALLOW_RO (BDRV_BITMAP_BUSY | BDRV_BITMAP_INCONSISTENT)
>
> +#define BDRV_BITMAP_MAX_NAME_SIZE 1023
> +
> BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
> uint32_t granularity,
> const char *name,
> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
> index 4bbb251b2c9c..7039e8252009 100644
> --- a/block/dirty-bitmap.c
> +++ b/block/dirty-bitmap.c
> @@ -104,9 +104,15 @@ BdrvDirtyBitmap
> *bdrv_create_dirty_bitmap(BlockDriverState *bs,
>
> assert(is_power_of_2(granularity) && granularity >= BDRV_SECTOR_SIZE);
>
> - if (name && bdrv_find_dirty_bitmap(bs, name)) {
> - error_setg(errp, "Bitmap already exists: %s", name);
> - return NULL;
> + if (name) {
> + if (bdrv_find_dirty_bitmap(bs, name)) {
> + error_setg(errp, "Bitmap already exists: %s", name);
> + return NULL;
> + }
> + if (strlen(name) > BDRV_BITMAP_MAX_NAME_SIZE) {
> + error_setg(errp, "Bitmap name too long: %s", name);
> + return NULL;
> + }
> }
> bitmap_size = bdrv_getlength(bs);
> if (bitmap_size < 0) {
> diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
> index ef9ef628a0d0..809bbc5d20c8 100644
> --- a/block/qcow2-bitmap.c
> +++ b/block/qcow2-bitmap.c
> @@ -42,6 +42,8 @@
> #define BME_MIN_GRANULARITY_BITS 9
> #define BME_MAX_NAME_SIZE 1023
>
> +QEMU_BUILD_BUG_ON(BME_MAX_NAME_SIZE != BDRV_BITMAP_MAX_NAME_SIZE);
> +
> #if BME_MAX_TABLE_SIZE * 8ULL > INT_MAX
> #error In the code bitmap table physical size assumed to fit into int
> #endif
Reviewed-by: Maxim Levitsky <mlevi...@redhat.com>
Best regards,
Maxim Levitsky
