Re: [Qemu-block] [RFC PATCH 07/12] qcow2-bitmap: add basic bitmaps info

[Vladimir Sementsov-Ogievskiy]

12.05.2018 04:25, John Snow wrote:
> Add functions for querying the basic information inside of bitmaps.
> Restructure the bitmaps flags masks to facilitate providing a list of
> flags belonging to the bitmap(s) being queried.
>
> Signed-off-by: John Snow <address@hidden>
> ---
>   block/qcow2-bitmap.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++--
>   block/qcow2.c        |  7 +++++
>   block/qcow2.h        |  1 +
>   3 files changed, 87 insertions(+), 2 deletions(-)
>
> diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
> index 60e01abfd7..811b82743a 100644
> --- a/block/qcow2-bitmap.c
> +++ b/block/qcow2-bitmap.c
> @@ -49,8 +49,28 @@
>   
>   /* Bitmap directory entry flags */
>   #define BME_RESERVED_FLAGS 0xfffffffcU
> -#define BME_FLAG_IN_USE (1U << 0)
> -#define BME_FLAG_AUTO   (1U << 1)
> +
> +enum BME_FLAG_BITS {
> +    BME_FLAG_BIT__BEGIN = 0,
> +    BME_FLAG_BIT_IN_USE = BME_FLAG_BIT__BEGIN,
> +    BME_FLAG_BIT_AUTO   = 1,
> +    BME_FLAG_BIT_EXTRA  = 2,
> +    BME_FLAG_BIT__MAX,
> +};
> +
> +#define BME_FLAG_IN_USE (1U << BME_FLAG_BIT_IN_USE)
> +#define BME_FLAG_AUTO   (1U << BME_FLAG_BIT_AUTO)
> +#define BME_FLAG_EXTRA  (1U << BME_FLAG_BIT_EXTRA)
> +
> +/* Correlate canonical spec values to autogenerated QAPI values */
> +struct {
> +    uint32_t mask;

mask is unused in this patch

> +    int qapi_value;
> +} BMEFlagMap[BME_FLAG_BIT__MAX] = {
> +    [BME_FLAG_BIT_IN_USE] = { BME_FLAG_IN_USE, BITMAP_FLAG_ENUM_IN_USE },
> +    [BME_FLAG_BIT_AUTO]   = { BME_FLAG_AUTO,   BITMAP_FLAG_ENUM_AUTO },
> +    [BME_FLAG_BIT_EXTRA]  = { BME_FLAG_EXTRA,  
> BITMAP_FLAG_ENUM_EXTRA_DATA_COMPATIBLE },
> +};
>   
>   /* bits [1, 8] U [56, 63] are reserved */
>   #define BME_TABLE_ENTRY_RESERVED_MASK 0xff000000000001feULL
> @@ -663,6 +683,63 @@ static void del_bitmap_list(BlockDriverState *bs)
>       }
>   }
>   
> +static BitmapFlagEnumList *get_bitmap_flags(uint32_t flags)
> +{
> +    int i;
> +    BitmapFlagEnumList *flist = NULL;
> +    BitmapFlagEnumList *ftmp;
> +
> +    while (flags) {
> +        i = ctz32(flags);
> +        ftmp = g_new0(BitmapFlagEnumList, 1);
> +        if (i >= BME_FLAG_BIT__BEGIN && i < BME_FLAG_BIT__MAX) {
> +            ftmp->value = BMEFlagMap[i].qapi_value;
> +        } else {
> +            ftmp->value = BITMAP_FLAG_ENUM_UNKNOWN;

so, there may be several "unknown" entries. It's inconsistent with 
"@unknown: This bitmap has unknown or reserved properties.".

Finally, can we export values for unknown flags? It should be more 
informative.

> +        }
> +        ftmp->next = flist;
> +        flist = ftmp;
> +        flags &= ~(1 << i);
> +    }
> +
> +    return flist;
> +}
> +
> +BitmapInfoList *qcow2_get_bitmap_info(BlockDriverState *bs)
> +{
> +    Qcow2BitmapList *bm_list;
> +    Qcow2Bitmap *bm;
> +    BitmapInfoList *info_list = NULL;
> +    BitmapInfoList *tmp;
> +    BitmapInfo *bitmap_info;
> +
> +    bm_list = get_bitmap_list(bs, NULL);
> +    if (!bm_list) {
> +        return info_list;
> +    }
> +
> +    QSIMPLEQ_FOREACH(bm, bm_list, entry) {
> +        bitmap_info = g_new0(BitmapInfo, 1);
> +        bitmap_info->name = g_strdup(bm->name);
> +        if (bm->type == BT_DIRTY_TRACKING_BITMAP) {
> +            bitmap_info->type = BITMAP_TYPE_ENUM_DIRTY_TRACKING;
> +        } else {
> +            bitmap_info->type = BITMAP_TYPE_ENUM_UNKNOWN;
> +        }
> +        bitmap_info->granularity = 1 << bm->granularity_bits;
> +        bitmap_info->count = bdrv_get_dirty_count(bm->dirty_bitmap);
> +        bitmap_info->flags = get_bitmap_flags(bm->flags);
> +        bitmap_info->has_flags = !!bitmap_info->flags;
> +
> +        tmp = g_new0(BitmapInfoList, 1);
> +        tmp->value = bitmap_info;
> +        tmp->next = info_list;
> +        info_list = tmp;
> +    }
> +
> +    return info_list;
> +}
> +
>   int qcow2_check_bitmaps_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
>                                     void **refcount_table,
>                                     int64_t *refcount_table_size)
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 7ae9000656..0b24ecb6b2 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -3960,6 +3960,7 @@ static ImageInfoSpecific 
> *qcow2_get_specific_info(BlockDriverState *bs)
>       BDRVQcow2State *s = bs->opaque;
>       ImageInfoSpecific *spec_info;
>       QCryptoBlockInfo *encrypt_info = NULL;
> +    BitmapInfoList *bitmap_list = NULL;
>   
>       if (s->crypto != NULL) {
>           encrypt_info = qcrypto_block_get_info(s->crypto, &error_abort);
> @@ -4016,6 +4017,12 @@ static ImageInfoSpecific 
> *qcow2_get_specific_info(BlockDriverState *bs)
>           spec_info->u.qcow2.data->encrypt = qencrypt;
>       }
>   
> +    bitmap_list = qcow2_get_bitmap_info(bs);
> +    if (bitmap_list) {
> +        spec_info->u.qcow2.data->has_bitmaps = true;
> +        spec_info->u.qcow2.data->bitmaps = bitmap_list;
> +    }
> +
>       return spec_info;
>   }
>   
> diff --git a/block/qcow2.h b/block/qcow2.h
> index 796a8c914b..76bf5fa55d 100644
> --- a/block/qcow2.h
> +++ b/block/qcow2.h
> @@ -686,5 +686,6 @@ bool qcow2_can_store_new_dirty_bitmap(BlockDriverState *bs,
>   void qcow2_remove_persistent_dirty_bitmap(BlockDriverState *bs,
>                                             const char *name,
>                                             Error **errp);
> +BitmapInfoList *qcow2_get_bitmap_info(BlockDriverState *bs);
>   
>   #endif


--
Best regards,
Vladimir