Re: [Qemu-devel] [PATCH v7] qemu-img info lists bitmap directory entries

[Andrey Shinkevich]

Ping / Note

Dear colleagues,
Kindly remind you to complete reviewing of this simple series.

Happy New Year!


On 13/12/2018 13:59, Andrey Shinkevich wrote:
> In the 'Format specific information' section of the 'qemu-img info'
> command output, the supplemental information about existing QCOW2
> bitmaps will be shown, such as a bitmap name, flags and granularity:
> 
> image: /vz/vmprivate/VM1/harddisk.hdd
> file format: qcow2
> virtual size: 64G (68719476736 bytes)
> disk size: 3.0M
> cluster_size: 1048576
> Format specific information:
>      compat: 1.1
>      lazy refcounts: true
>      bitmaps:
>          [0]:
>              flags:
>                  [0]: in-use
>                  [1]: auto
>              name: back-up1
>              unknown flags: 4
>              granularity: 65536
>          [1]:
>              flags:
>                  [0]: in-use
>                  [1]: auto
>              name: back-up2
>              unknown flags: 8
>              granularity: 65536
>      refcount bits: 16
>      corrupt: false
> 
> Signed-off-by: Andrey Shinkevich <address@hidden>
> Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> Reviewed-by: Eric Blake <address@hidden>
> ---
> v7:
> A description was added to the function qcow2_get_bitmap_info_list().
> In the function qcow2_get_specific_info(), the comment was modified
> so that we ignore any error in obtaining the list of bitmaps to
> pass the rest of QCOW2 specific information to a caller.
> 
> v6:
> '[PATCH v6] qemu-img info lists bitmap directory entries'.
> The error handling logic for the bitmaps empty list was reversed.
> 
> v5:
> '[PATCH v5] qemu-img info lists bitmap directory entries'.
> The error handling logic for the bitmaps empty list was fixed and documented.
> 
> v4:
> '[PATCH v4] qemu-img info lists bitmap directory entries'.
> Unknown flags are checked with the mask BME_RESERVED_FLAGS.
> The code minor refactoring was made.
> 
> v3:
> '[PATCH v3] qemu-img info lists bitmap directory entries'.
> Now, qcow2_get_bitmap_info_list() is invoked under the condition of QCOW
> version #3 to avoid memory leaks in case of QCOW version #2.
> Furthermore, qcow2_get_bitmap_info_list() checks the number of existing 
> bitmaps.
> So, if no bitmap exists, no bitmap error message is printed in the output.
> The data type of the bitmap 'granularity' parameter was left as 'uint32'
> because bitmap_list_load() returns error if granularity_bits is grater than 
> 31.
> 
> v2:
> '[PATCH v2] qemu-img info lists bitmap directory entries'.
> The targeted version of the release at 'Since' word of the comments to the new
> structures changed to 4.0 in the file qapi/block-core.json.
> A comment to the 'bitmaps' new member was supplied.
> The 'unknown flags' parameter was introduced to indicate presence of QCOW2
> bitmap unknown flags, if any.
> The word 'dirty' was removed from the code and from the comments as we list 
> all
> the bitmaps.
> The 'bitmaps' printed parameter was removed for the release versions earlier
> than 3.x.
> The example of the output was moved above the 'Signed-off-by' line.
> 
> The first version was '[PATCH] qemu-img info lists bitmap directory entries'.
> 
>   block/qcow2-bitmap.c | 63 
> ++++++++++++++++++++++++++++++++++++++++++++++++++++
>   block/qcow2.c        |  9 ++++++++
>   block/qcow2.h        |  2 ++
>   qapi/block-core.json | 42 ++++++++++++++++++++++++++++++++++-
>   qemu-img.c           |  5 +++++
>   5 files changed, 120 insertions(+), 1 deletion(-)
> 
> diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
> index accebef..8aedddc 100644
> --- a/block/qcow2-bitmap.c
> +++ b/block/qcow2-bitmap.c
> @@ -1008,6 +1008,69 @@ fail:
>       return false;
>   }
>   
> +static Qcow2BitmapInfoFlagsList *get_bitmap_info_flags(uint32_t flags)
> +{
> +    Qcow2BitmapInfoFlagsList *list = NULL;
> +    Qcow2BitmapInfoFlagsList **plist = &list;
> +
> +    if (flags & BME_FLAG_IN_USE) {
> +        Qcow2BitmapInfoFlagsList *entry = g_new0(Qcow2BitmapInfoFlagsList, 
> 1);
> +        entry->value = QCOW2_BITMAP_INFO_FLAGS_IN_USE;
> +        *plist = entry;
> +        plist = &entry->next;
> +    }
> +    if (flags & BME_FLAG_AUTO) {
> +        Qcow2BitmapInfoFlagsList *entry = g_new0(Qcow2BitmapInfoFlagsList, 
> 1);
> +        entry->value = QCOW2_BITMAP_INFO_FLAGS_AUTO;
> +        *plist = entry;
> +    }
> +    return list;
> +}
> +
> +/* qcow2_get_bitmap_info_list()
> + * Returns a list of QCOW2 bitmap details.
> + * In case of no bitmaps, the function returns NULL and
> + * the @errp parameter is not set (for a 0-length list in the QMP).
> + * When bitmap information can not be obtained, the function returns
> + * NULL and the @errp parameter is set (for omitting the list in QMP).
> + */
> +Qcow2BitmapInfoList *qcow2_get_bitmap_info_list(BlockDriverState *bs,
> +                                                Error **errp)
> +{
> +    BDRVQcow2State *s = bs->opaque;
> +    Qcow2BitmapList *bm_list;
> +    Qcow2Bitmap *bm;
> +    Qcow2BitmapInfoList *list = NULL;
> +    Qcow2BitmapInfoList **plist = &list;
> +
> +    if (s->nb_bitmaps == 0) {
> +        return NULL;
> +    }
> +
> +    bm_list = bitmap_list_load(bs, s->bitmap_directory_offset,
> +                               s->bitmap_directory_size, errp);
> +    if (bm_list == NULL) {
> +        return NULL;
> +    }
> +
> +    QSIMPLEQ_FOREACH(bm, bm_list, entry) {
> +        Qcow2BitmapInfo *info = g_new0(Qcow2BitmapInfo, 1);
> +        Qcow2BitmapInfoList *obj = g_new0(Qcow2BitmapInfoList, 1);
> +        info->granularity = 1U << bm->granularity_bits;
> +        info->name = g_strdup(bm->name);
> +        info->flags = get_bitmap_info_flags(bm->flags);
> +        info->unknown_flags = bm->flags & BME_RESERVED_FLAGS;
> +        info->has_unknown_flags = !!info->unknown_flags;
> +        obj->value = info;
> +        *plist = obj;
> +        plist = &obj->next;
> +    }
> +
> +    bitmap_list_free(bm_list);
> +
> +    return list;
> +}
> +
>   int qcow2_reopen_bitmaps_rw_hint(BlockDriverState *bs, bool *header_updated,
>                                    Error **errp)
>   {
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 991d6ac..d79f667 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -4270,6 +4270,10 @@ static ImageInfoSpecific 
> *qcow2_get_specific_info(BlockDriverState *bs)
>               .refcount_bits      = s->refcount_bits,
>           };
>       } else if (s->qcow_version == 3) {
> +        Qcow2BitmapInfoList *bitmaps;
> +        Error *local_err = NULL;
> +
> +        bitmaps = qcow2_get_bitmap_info_list(bs, &local_err);
>           *spec_info->u.qcow2.data = (ImageInfoSpecificQCow2){
>               .compat             = g_strdup("1.1"),
>               .lazy_refcounts     = s->compatible_features &
> @@ -4279,7 +4283,12 @@ static ImageInfoSpecific 
> *qcow2_get_specific_info(BlockDriverState *bs)
>                                     QCOW2_INCOMPAT_CORRUPT,
>               .has_corrupt        = true,
>               .refcount_bits      = s->refcount_bits,
> +            .has_bitmaps        = !local_err,
> +            .bitmaps            = bitmaps,
>           };
> +        /* If an error occurs in obtaining bitmaps, ignore
> +         * it to show other QCOW2 specific information. */
> +        error_free(local_err);
>       } else {
>           /* if this assertion fails, this probably means a new version was
>            * added without having it covered here */
> diff --git a/block/qcow2.h b/block/qcow2.h
> index 8662b68..0ec2b3d 100644
> --- a/block/qcow2.h
> +++ b/block/qcow2.h
> @@ -685,6 +685,8 @@ int qcow2_check_bitmaps_refcounts(BlockDriverState *bs, 
> BdrvCheckResult *res,
>                                     void **refcount_table,
>                                     int64_t *refcount_table_size);
>   bool qcow2_load_dirty_bitmaps(BlockDriverState *bs, Error **errp);
> +Qcow2BitmapInfoList *qcow2_get_bitmap_info_list(BlockDriverState *bs,
> +                                                Error **errp);
>   int qcow2_reopen_bitmaps_rw_hint(BlockDriverState *bs, bool *header_updated,
>                                    Error **errp);
>   int qcow2_reopen_bitmaps_rw(BlockDriverState *bs, Error **errp);
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index d4fe710..9898baa 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -69,6 +69,10 @@
>   # @encrypt: details about encryption parameters; only set if image
>   #           is encrypted (since 2.10)
>   #
> +# @bitmaps: A list of qcow2 bitmap details (possibly empty, such as for
> +#           v2 images which do not support bitmaps); absent if bitmap
> +#           information could not be obtained (since 4.0)
> +#
>   # Since: 1.7
>   ##
>   { 'struct': 'ImageInfoSpecificQCow2',
> @@ -77,7 +81,8 @@
>         '*lazy-refcounts': 'bool',
>         '*corrupt': 'bool',
>         'refcount-bits': 'int',
> -      '*encrypt': 'ImageInfoSpecificQCow2Encryption'
> +      '*encrypt': 'ImageInfoSpecificQCow2Encryption',
> +      '*bitmaps': ['Qcow2BitmapInfo']
>     } }
>   
>   ##
> @@ -454,6 +459,41 @@
>              'status': 'DirtyBitmapStatus'} }
>   
>   ##
> +# @Qcow2BitmapInfoFlags:
> +#
> +# An enumeration of flags that a bitmap can report to the user.
> +#
> +# @in-use: The bitmap was not saved correctly and may be inconsistent.
> +#
> +# @auto: The bitmap must reflect all changes of the virtual disk by any
> +#        application that would write to this qcow2 file.
> +#
> +# Since: 4.0
> +##
> +{ 'enum': 'Qcow2BitmapInfoFlags',
> +  'data': ['in-use', 'auto'] }
> +
> +##
> +# @Qcow2BitmapInfo:
> +#
> +# Qcow2 bitmap information.
> +#
> +# @name: the name of the bitmap
> +#
> +# @granularity: granularity of the bitmap in bytes
> +#
> +# @flags: flags of the bitmap
> +#
> +# @unknown-flags: unspecified flags if detected
> +#
> +# Since: 4.0
> +##
> +{ 'struct': 'Qcow2BitmapInfo',
> +  'data': {'name': 'str', 'granularity': 'uint32',
> +           'flags': ['Qcow2BitmapInfoFlags'],
> +           '*unknown-flags': 'uint32' } }
> +
> +##
>   # @BlockLatencyHistogramInfo:
>   #
>   # Block latency histogram.
> diff --git a/qemu-img.c b/qemu-img.c
> index ad04f59..144d7b8 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -2601,6 +2601,11 @@ static ImageInfoList *collect_image_info_list(bool 
> image_opts,
>               blk_unref(blk);
>               goto err;
>           }
> +        if (info->format_specific &&
> +            info->format_specific->type == IMAGE_INFO_SPECIFIC_KIND_QCOW2 &&
> +            info->format_specific->u.qcow2.data->has_bitmaps == false) {
> +            warn_report("Failed to load bitmap list");
> +        }
>   
>           elem = g_new0(ImageInfoList, 1);
>           elem->value = info;
> 

-- 
With the best regards,
Andrey Shinkevich