[Qemu-devel] [PATCH v4 0/3] qemu-img check: format allocation info

[Qemu-devel] [PATCH v4 0/3] qemu-img check: format allocation info

[Vladimir Sementsov-Ogievskiy]

Hi all.

See 01 patch for the doc.

Question to discuss.
If I understand correctly get_block_status flags allocated, zero, and data
actually provide 5 possible combinations, which I combine into three.

allocated data zero
1         1    1    \__ data
1         1    0    /
1         0    1    \__ zero
0         0    1    /
0         0    0    ___ discarded

This division looks not bad, but it is not the only one possible.
Separating data is really useful - it shows leaked clusters..
So the question is, don't we want to adjust the division?
I'm ok with the current one.

v4: - reword docs in 01
    - s/2.10/2.11/ for qapi

v3: - improve docs
    - rename fields
    - add 'zero' type of underlying file portions status. It as these
      areas cannot be presented as 'discarded', but they are not
      occupying real space, so we don't want to present them as
      'allocated data' too.
    - remove last patch. It is not needed after introducing new naming
      for the fields

v2: fix build error, gcc things that some variables may be used
    uninitialized (actually they didn't).

v1: 

These series is a replacement for "qemu-img check: unallocated size"
series.

There was a question, should we account allocated clusters in qcow2 but
actually holes in underalying file as allocated or not. Instead of
hiding this information in one-number statistic I've decided to print
the whole information, 5 numbers:

For allocated by top-level format driver (qcow2 for ex.) clusters, 3
numbers: number of bytes, which are:
 - allocated in underlying file
 - holes in underlying file
 - after end of underlying file

To account other areas of underlying file, 2 more numbers of bytes:
 - unallocated by top-level driver but allocated in underlying file
 - unallocated by top-level driver and holes in underlying file

Vladimir Sementsov-Ogievskiy (3):
  block: add bdrv_get_format_alloc_stat format interface
  qcow2: add .bdrv_get_format_alloc_stat
  qemu-img check: add format allocation info

 block.c                   |  16 +++++
 block/qcow2-refcount.c    | 152 ++++++++++++++++++++++++++++++++++++++++++++++
 block/qcow2.c             |   2 +
 block/qcow2.h             |   2 +
 include/block/block.h     |   3 +
 include/block/block_int.h |   2 +
 qapi/block-core.json      |  78 +++++++++++++++++++++++-
 qemu-img.c                |  42 +++++++++++++
 8 files changed, 296 insertions(+), 1 deletion(-)

-- 
2.11.1

[Qemu-devel] [PATCH v4 1/3] block: add bdrv_get_format_alloc_stat format interface

[Vladimir Sementsov-Ogievskiy]

The function should collect statistics, about used/unused by top-level
format driver space (in its .file) and allocation status
(data/zero/discarded/after-eof) of corresponding areas in this .file.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block.c                   | 16 +++++++++++
 include/block/block.h     |  3 ++
 include/block/block_int.h |  2 ++
 qapi/block-core.json      | 72 +++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 93 insertions(+)

diff --git a/block.c b/block.c
index 50ba264143..7d720ae0c2 100644
--- a/block.c
+++ b/block.c
@@ -3407,6 +3407,22 @@ int64_t bdrv_get_allocated_file_size(BlockDriverState *bs)
 }
 
 /**
+ * Collect format allocation info. See BlockFormatAllocInfo definition in
+ * qapi/block-core.json.
+ */
+int bdrv_get_format_alloc_stat(BlockDriverState *bs, BlockFormatAllocInfo *bfai)
+{
+    BlockDriver *drv = bs->drv;
+    if (!drv) {
+        return -ENOMEDIUM;
+    }
+    if (drv->bdrv_get_format_alloc_stat) {
+        return drv->bdrv_get_format_alloc_stat(bs, bfai);
+    }
+    return -ENOTSUP;
+}
+
+/**
  * Return number of sectors on success, -errno on error.
  */
 int64_t bdrv_nb_sectors(BlockDriverState *bs)
diff --git a/include/block/block.h b/include/block/block.h
index 9b355e92d8..646376a772 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -335,6 +335,9 @@ typedef enum {
 
 int bdrv_check(BlockDriverState *bs, BdrvCheckResult *res, BdrvCheckMode fix);
 
+int bdrv_get_format_alloc_stat(BlockDriverState *bs,
+                               BlockFormatAllocInfo *bfai);
+
 /* The units of offset and total_work_size may be chosen arbitrarily by the
  * block driver; total_work_size may change during the course of the amendment
  * operation */
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 8d3724cce6..458c715e99 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -208,6 +208,8 @@ struct BlockDriver {
     int64_t (*bdrv_getlength)(BlockDriverState *bs);
     bool has_variable_length;
     int64_t (*bdrv_get_allocated_file_size)(BlockDriverState *bs);
+    int (*bdrv_get_format_alloc_stat)(BlockDriverState *bs,
+                                      BlockFormatAllocInfo *bfai);
 
     int coroutine_fn (*bdrv_co_pwritev_compressed)(BlockDriverState *bs,
         uint64_t offset, uint64_t bytes, QEMUIOVector *qiov);
diff --git a/qapi/block-core.json b/qapi/block-core.json
index ea0b3e8b13..93f6995381 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -139,6 +139,78 @@
            '*format-specific': 'ImageInfoSpecific' } }
 
 ##
+# @BlockFormatAllocInfo:
+#
+#
+# Allocation information of an underlying protocol file, as partitioned by a
+# format driver's utilization of said allocations.
+# All fields are in bytes.
+#
+# Regions of the underlying protocol file may be considered used or unused by
+# the format driver interpreting these regions. It is at the discretion of the
+# format driver (e.g. qcow2) which regions of its backing storage are considered
+# in-use or not.
+#
+# For now, the only format driver supporting this feature is Qcow2 which is a
+# cluster based format. Clusters considered in-use by qcow2 are those with a
+# non-zero refcount in the format metadata. All other clusters, if present, are
+# considered unused. Examples of unused allocations for the Qcow2 format are
+# leaked clusters, pre-allocated clusters, and recently freed clusters.
+#
+# Note: the whole underlying protocol file is described as well as all format
+# file allocations, not only virtual disk data (metadata, internal snapshots,
+# etc. are included).
+#
+# For the underlying protocol file there are native block-status types of the
+# regions:
+#  - data: allocated data
+#  - zero: reported as zero (for example, this type corresponds to holes for
+#          POSIX files on sparce file-system)
+#  - discarded: not allocated
+# 4th additional type is 'overrun', is data referenced by the format driver
+# located beyond EOF of the underlying protocol file. For example, a partially
+# allocated cluster at the end of a QCOW2 file, where Qcow2 generally operates
+# on complete clusters.
+#
+# So, the fields are:
+#
+# @used-data: used by the format file and backed by data in the underlying
+#             protocol file
+#
+# @used-zero: used by the format file and backed by zeroes in the underlying
+#             protocol file; which may be a filesystem hole for POSIX files.
+#
+# @used-discarded: used by the format file but actually unallocated in the
+#                  underlying protocol file
+#
+# @used-overrun: used by the format file beyond the end of the underlying
+#                protocol file
+#
+# @unused-data: allocated data in the underlying protocol file not used by the
+#               format file
+#
+# @unused-zero: reported-as-zero regions in the underlying protocol file not
+#               used by the format file
+#
+# @unused-discarded: unallocated areas in the underlying protocol file not used
+#                    by the format file
+#
+# Note: sum of 6 fields {used,unused}-{data,zero,discarded} is equal to the
+#       length of the underlying protocol file.
+#
+# Since: 2.11
+#
+##
+{ 'struct': 'BlockFormatAllocInfo',
+  'data': {'used-data':        'uint64',
+           'used-zero':        'uint64',
+           'used-discarded':   'uint64',
+           'used-overrun':     'uint64',
+           'unused-data':      'uint64',
+           'unused-zero':      'uint64',
+           'unused-discarded': 'uint64' } }
+
+##
 # @ImageCheck:
 #
 # Information about a QEMU image file check
-- 
2.11.1

[Qemu-devel] [PATCH v4 2/3] qcow2: add .bdrv_get_format_alloc_stat

[Vladimir Sementsov-Ogievskiy]

Realize .bdrv_get_format_alloc_stat interface.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block/qcow2-refcount.c | 152 +++++++++++++++++++++++++++++++++++++++++++++++++
 block/qcow2.c          |   2 +
 block/qcow2.h          |   2 +
 3 files changed, 156 insertions(+)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 7c06061aae..fd6027f0c2 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -2931,3 +2931,155 @@ done:
     qemu_vfree(new_refblock);
     return ret;
 }
+
+typedef struct FormatAllocStatCo {
+    BlockDriverState *bs;
+    BlockFormatAllocInfo *bfai;
+    int64_t ret;
+} FormatAllocStatCo;
+
+static void coroutine_fn qcow2_get_format_alloc_stat_entry(void *opaque)
+{
+    int ret = 0;
+    FormatAllocStatCo *nbco = opaque;
+    BlockDriverState *bs = nbco->bs;
+    BDRVQcow2State *s = bs->opaque;
+    BlockFormatAllocInfo *bfai = nbco->bfai;
+    int64_t cluster, file_sectors, sector;
+    int refcount_block_offset;
+    uint32_t i;
+    bool used = false, f_data = false, f_zero = false;
+    int dif, num = 0, f_num = 0;
+
+    memset(bfai, 0, sizeof(*bfai));
+
+    file_sectors = bdrv_nb_sectors(bs->file->bs);
+    if (file_sectors < 0) {
+        nbco->ret = file_sectors;
+        return;
+    }
+
+    qemu_co_mutex_lock(&s->lock);
+
+    for (sector = 0; sector < file_sectors; sector += dif) {
+        if (f_num == 0) {
+            BlockDriverState *file;
+            ret = bdrv_get_block_status(bs->file->bs, sector,
+                                        file_sectors - sector, &f_num, &file);
+            if (ret < 0) {
+                goto fail;
+            }
+            f_data = ret & BDRV_BLOCK_DATA;
+            f_zero = ret & BDRV_BLOCK_ZERO;
+        }
+
+        if (num == 0) {
+            uint64_t refcount;
+            assert(((sector << BDRV_SECTOR_BITS) & (s->cluster_size - 1)) == 0);
+            ret = qcow2_get_refcount(
+                bs, (sector << BDRV_SECTOR_BITS) >> s->cluster_bits, &refcount);
+            if (ret < 0) {
+                goto fail;
+            }
+            used = refcount > 0;
+            num = s->cluster_size >> BDRV_SECTOR_BITS;
+        }
+
+        dif = MIN(f_num, MIN(num, file_sectors - sector));
+        if (used) {
+            if (f_data) {
+                bfai->used_data += dif;
+            } else if (f_zero) {
+                bfai->used_zero += dif;
+            } else {
+                bfai->used_discarded += dif;
+            }
+        } else {
+            if (f_data) {
+                bfai->unused_data += dif;
+            } else if (f_zero) {
+                bfai->unused_zero += dif;
+            } else {
+                bfai->unused_discarded += dif;
+            }
+        }
+        f_num -= dif;
+        num -= dif;
+    }
+
+    assert(f_num == 0);
+
+    if (used) {
+        bfai->used_overrun += num;
+    }
+
+    cluster = size_to_clusters(s, sector << BDRV_SECTOR_BITS);
+    refcount_block_offset = cluster & (s->refcount_block_size - 1);
+    for (i = cluster >> s->refcount_block_bits;
+         i <= s->max_refcount_table_index; i++)
+    {
+        int j;
+
+        if (!(s->refcount_table[i] & REFT_OFFSET_MASK)) {
+            refcount_block_offset = 0;
+            continue;
+        }
+
+        for (j = refcount_block_offset; j < s->refcount_block_size; j++) {
+            uint64_t refcount;
+            cluster = (i << s->refcount_block_bits) + j;
+
+            ret = qcow2_get_refcount(bs, cluster, &refcount);
+            if (ret < 0) {
+                goto fail;
+            }
+            if (refcount > 0) {
+                bfai->used_overrun++;
+            }
+        }
+
+        refcount_block_offset = 0;
+    }
+
+    qemu_co_mutex_unlock(&s->lock);
+
+    bfai->used_data = bfai->used_data << BDRV_SECTOR_BITS;
+    bfai->used_zero = bfai->used_zero << BDRV_SECTOR_BITS;
+    bfai->used_discarded = bfai->used_discarded << BDRV_SECTOR_BITS;
+    bfai->used_overrun = bfai->used_overrun << BDRV_SECTOR_BITS;
+
+    bfai->unused_data = bfai->unused_data << BDRV_SECTOR_BITS;
+    bfai->unused_zero = bfai->unused_zero << BDRV_SECTOR_BITS;
+    bfai->unused_discarded = bfai->unused_discarded << BDRV_SECTOR_BITS;
+
+    nbco->ret = 0;
+    return;
+
+fail:
+    nbco->ret = ret;
+    qemu_co_mutex_unlock(&s->lock);
+}
+
+/* qcow2_get_format_alloc_stat()
+ * Fills @bfai struct. In case of failure @bfai content is unpredicted.
+ */
+int qcow2_get_format_alloc_stat(BlockDriverState *bs,
+                                BlockFormatAllocInfo *bfai)
+{
+    FormatAllocStatCo nbco = {
+        .bs = bs,
+        .bfai = bfai,
+        .ret = -EINPROGRESS
+    };
+
+    if (qemu_in_coroutine()) {
+        qcow2_get_format_alloc_stat_entry(&nbco);
+    } else {
+        Coroutine *co =
+            qemu_coroutine_create(qcow2_get_format_alloc_stat_entry, &nbco);
+        qemu_coroutine_enter(co);
+        BDRV_POLL_WHILE(bs, nbco.ret == -EINPROGRESS);
+    }
+
+    return nbco.ret;
+}
diff --git a/block/qcow2.c b/block/qcow2.c
index a8d61f0981..0e26b548fd 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3469,6 +3469,8 @@ BlockDriver bdrv_qcow2 = {
 
     .bdrv_detach_aio_context  = qcow2_detach_aio_context,
     .bdrv_attach_aio_context  = qcow2_attach_aio_context,
+
+    .bdrv_get_format_alloc_stat = qcow2_get_format_alloc_stat,
 };
 
 static void bdrv_qcow2_init(void)
diff --git a/block/qcow2.h b/block/qcow2.h
index 1801dc30dc..16d31a8624 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -598,4 +598,6 @@ int qcow2_cache_get_empty(BlockDriverState *bs, Qcow2Cache *c, uint64_t offset,
     void **table);
 void qcow2_cache_put(BlockDriverState *bs, Qcow2Cache *c, void **table);
 
+int qcow2_get_format_alloc_stat(BlockDriverState *bs,
+                                BlockFormatAllocInfo *bfai);
 #endif
-- 
2.11.1

[Qemu-devel] [PATCH v4 3/3] qemu-img check: add format allocation info

[Vladimir Sementsov-Ogievskiy]

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 qapi/block-core.json |  6 +++++-
 qemu-img.c           | 42 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 93f6995381..d662786261 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -249,6 +249,9 @@
 #                       field is present if the driver for the image format
 #                       supports it
 #
+# @format-alloc-info: Format-allocation information, see
+#                     BlockFormatAllocInfo description. (Since: 2.11)
+#
 # Since: 1.4
 #
 ##
@@ -257,7 +260,8 @@
            '*image-end-offset': 'int', '*corruptions': 'int', '*leaks': 'int',
            '*corruptions-fixed': 'int', '*leaks-fixed': 'int',
            '*total-clusters': 'int', '*allocated-clusters': 'int',
-           '*fragmented-clusters': 'int', '*compressed-clusters': 'int' } }
+           '*fragmented-clusters': 'int', '*compressed-clusters': 'int',
+           '*format-alloc-info': 'BlockFormatAllocInfo' } }
 
 ##
 # @MapEntry:
diff --git a/qemu-img.c b/qemu-img.c
index b506839ef0..b3adf9a1a2 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -560,6 +560,35 @@ static void dump_json_image_check(ImageCheck *check, bool quiet)
     QDECREF(str);
 }
 
+static void dump_human_format_alloc_info(BlockFormatAllocInfo *bfai, bool quiet)
+{
+    char *used_data = size_to_str(bfai->used_data);
+    char *used_zero = size_to_str(bfai->used_zero);
+    char *used_discarded = size_to_str(bfai->used_discarded);
+    char *used_overrun = size_to_str(bfai->used_overrun);
+
+    char *unused_data = size_to_str(bfai->unused_data);
+    char *unused_zero = size_to_str(bfai->unused_zero);
+    char *unused_discarded = size_to_str(bfai->unused_discarded);
+
+    qprintf(quiet,
+            "Format allocation info (including metadata):\n"
+            "               data        zero   discarded   after-eof\n"
+            "used     %10s  %10s  %10s  %10s\n"
+            "unused   %10s  %10s  %10s\n",
+            used_data, used_zero, used_discarded, used_overrun,
+            unused_data, unused_zero, unused_discarded);
+
+    g_free(used_data);
+    g_free(used_zero);
+    g_free(used_discarded);
+    g_free(used_overrun);
+
+    g_free(unused_data);
+    g_free(unused_zero);
+    g_free(unused_discarded);
+}
+
 static void dump_human_image_check(ImageCheck *check, bool quiet)
 {
     if (!(check->corruptions || check->leaks || check->check_errors)) {
@@ -601,6 +630,10 @@ static void dump_human_image_check(ImageCheck *check, bool quiet)
         qprintf(quiet,
                 "Image end offset: %" PRId64 "\n", check->image_end_offset);
     }
+
+    if (check->has_format_alloc_info) {
+        dump_human_format_alloc_info(check->format_alloc_info, quiet);
+    }
 }
 
 static int collect_image_check(BlockDriverState *bs,
@@ -611,6 +644,7 @@ static int collect_image_check(BlockDriverState *bs,
 {
     int ret;
     BdrvCheckResult result;
+    BlockFormatAllocInfo *bfai = g_new0(BlockFormatAllocInfo, 1);
 
     ret = bdrv_check(bs, &result, fix);
     if (ret < 0) {
@@ -639,6 +673,14 @@ static int collect_image_check(BlockDriverState *bs,
     check->compressed_clusters      = result.bfi.compressed_clusters;
     check->has_compressed_clusters  = result.bfi.compressed_clusters != 0;
 
+    ret = bdrv_get_format_alloc_stat(bs, bfai);
+    if (ret < 0) {
+        qapi_free_BlockFormatAllocInfo(bfai);
+    } else {
+        check->has_format_alloc_info = true;
+        check->format_alloc_info = bfai;
+    }
+
     return 0;
 }
 
-- 
2.11.1

Re: [Qemu-devel] [PATCH v4 0/3] qemu-img check: format allocation info

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 2008 bytes --]

On 07/29/2017 11:41 AM, Vladimir Sementsov-Ogievskiy wrote:
> Hi all.
> 
> See 01 patch for the doc.
> 
> Question to discuss.
> If I understand correctly get_block_status flags allocated, zero, and data
> actually provide 5 possible combinations, which I combine into three.

There are actually 8 possible bit combinations, but you are right that
some of them are in practice impossible (since the allocated bit can
only be set in cases where the underlying driver set the data or zero bit).

> 
> allocated data zero
> 1         1    1    \__ data

This one is interesting - it means we know the contents read as zero,
but that it occupies space on the disk instead of being a hole;
reporting it as zero may make it easier to punch a hole.

> 1         1    0    /

Yes, definitely data, and no clue if it can be turned into a hole.

> 1         0    1    \__ zero
> 0         0    1    /

Yes, definitely zero.  (The former happens when a format layer directly
reports that the current layer reads as zero; the latter is possible
when a format layer doesn't have an allocation, but where we know
unallocated clusters read as zero, perhaps because there is no backing
file to further fall back to).

> 0         0    0    ___ discarded

Could also mean hasn't been touched yet (discarded sort of implies that
it has been touched at some point in the past)

The other bit patterns:

  0         1    0    - not possible: if a driver sets data, then the
block layer sets allocated

  0         1    1    - ditto
  1         0    0    - not possible: nothing sets the allocated bit in
isolation

> 
> This division looks not bad, but it is not the only one possible.
> Separating data is really useful - it shows leaked clusters..
> So the question is, don't we want to adjust the division?
> I'm ok with the current one.
> 
-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: [Qemu-devel] [PATCH v4 0/3] qemu-img check: format allocation info

[Vladimir Sementsov-Ogievskiy]

31.07.2017 18:14, Eric Blake wrote:
> On 07/29/2017 11:41 AM, Vladimir Sementsov-Ogievskiy wrote:
>> Hi all.
>>
>> See 01 patch for the doc.
>>
>> Question to discuss.
>> If I understand correctly get_block_status flags allocated, zero, and data
>> actually provide 5 possible combinations, which I combine into three.
> There are actually 8 possible bit combinations, but you are right that
> some of them are in practice impossible (since the allocated bit can
> only be set in cases where the underlying driver set the data or zero bit).
>
>> allocated data zero
>> 1         1    1    \__ data
> This one is interesting - it means we know the contents read as zero,
> but that it occupies space on the disk instead of being a hole;
> reporting it as zero may make it easier to punch a hole.
>
>> 1         1    0    /
> Yes, definitely data, and no clue if it can be turned into a hole.
>
>> 1         0    1    \__ zero
>> 0         0    1    /
> Yes, definitely zero.  (The former happens when a format layer directly
> reports that the current layer reads as zero; the latter is possible
> when a format layer doesn't have an allocation, but where we know
> unallocated clusters read as zero, perhaps because there is no backing
> file to further fall back to).
>
>> 0         0    0    ___ discarded
> Could also mean hasn't been touched yet (discarded sort of implies that
> it has been touched at some point in the past)

last time I don't like it too. What about renaming it to just 
'unallocated'?

>
> The other bit patterns:
>
>    0         1    0    - not possible: if a driver sets data, then the
> block layer sets allocated
>
>    0         1    1    - ditto
>    1         0    0    - not possible: nothing sets the allocated bit in
> isolation
>
>> This division looks not bad, but it is not the only one possible.
>> Separating data is really useful - it shows leaked clusters..
>> So the question is, don't we want to adjust the division?
>> I'm ok with the current one.
>>

-- 
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH v4 1/3] block: add bdrv_get_format_alloc_stat format interface

[Markus Armbruster]

Cc: Eric Blake for additional schema review expertise.

Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes:

> The function should collect statistics, about used/unused by top-level
> format driver space (in its .file) and allocation status
> (data/zero/discarded/after-eof) of corresponding areas in this .file.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  block.c                   | 16 +++++++++++
>  include/block/block.h     |  3 ++
>  include/block/block_int.h |  2 ++
>  qapi/block-core.json      | 72 +++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 93 insertions(+)
>
> diff --git a/block.c b/block.c
> index 50ba264143..7d720ae0c2 100644
> --- a/block.c
> +++ b/block.c
> @@ -3407,6 +3407,22 @@ int64_t bdrv_get_allocated_file_size(BlockDriverState *bs)
>  }
>  
>  /**
> + * Collect format allocation info. See BlockFormatAllocInfo definition in
> + * qapi/block-core.json.
> + */

I'd prefer 

   /**
    * Collect format allocation info.
    * See BlockFormatAllocInfo definition in qapi/block-core.json.
    */

Admittedly a matter of taste.

> +int bdrv_get_format_alloc_stat(BlockDriverState *bs, BlockFormatAllocInfo *bfai)

bdrv_get_format_alloc_info(), please, for symmetry with
BlockFormatAllocInfo.

> +{
> +    BlockDriver *drv = bs->drv;
> +    if (!drv) {
> +        return -ENOMEDIUM;
> +    }
> +    if (drv->bdrv_get_format_alloc_stat) {
> +        return drv->bdrv_get_format_alloc_stat(bs, bfai);
> +    }
> +    return -ENOTSUP;
> +}
> +
> +/**
>   * Return number of sectors on success, -errno on error.
>   */
>  int64_t bdrv_nb_sectors(BlockDriverState *bs)
> diff --git a/include/block/block.h b/include/block/block.h
> index 9b355e92d8..646376a772 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -335,6 +335,9 @@ typedef enum {
>  
>  int bdrv_check(BlockDriverState *bs, BdrvCheckResult *res, BdrvCheckMode fix);
>  
> +int bdrv_get_format_alloc_stat(BlockDriverState *bs,
> +                               BlockFormatAllocInfo *bfai);
> +
>  /* The units of offset and total_work_size may be chosen arbitrarily by the
>   * block driver; total_work_size may change during the course of the amendment
>   * operation */
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index 8d3724cce6..458c715e99 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -208,6 +208,8 @@ struct BlockDriver {
>      int64_t (*bdrv_getlength)(BlockDriverState *bs);
>      bool has_variable_length;
>      int64_t (*bdrv_get_allocated_file_size)(BlockDriverState *bs);
> +    int (*bdrv_get_format_alloc_stat)(BlockDriverState *bs,
> +                                      BlockFormatAllocInfo *bfai);
>  
>      int coroutine_fn (*bdrv_co_pwritev_compressed)(BlockDriverState *bs,
>          uint64_t offset, uint64_t bytes, QEMUIOVector *qiov);
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index ea0b3e8b13..93f6995381 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -139,6 +139,78 @@
>             '*format-specific': 'ImageInfoSpecific' } }
>  
>  ##
> +# @BlockFormatAllocInfo:
> +#
> +#

Just one blank like, please.

> +# Allocation information of an underlying protocol file, as partitioned by a
> +# format driver's utilization of said allocations.
> +# All fields are in bytes.
> +#
> +# Regions of the underlying protocol file may be considered used or unused by
> +# the format driver interpreting these regions. It is at the discretion of the
> +# format driver (e.g. qcow2) which regions of its backing storage are considered

Long line.  Please wrap your comment lines around column 70 for legibility.

> +# in-use or not.
> +#
> +# For now, the only format driver supporting this feature is Qcow2 which is a

"is QCow2, which"

> +# cluster based format. Clusters considered in-use by qcow2 are those with a
> +# non-zero refcount in the format metadata. All other clusters, if present, are
> +# considered unused. Examples of unused allocations for the Qcow2 format are
> +# leaked clusters, pre-allocated clusters, and recently freed clusters.
> +#
> +# Note: the whole underlying protocol file is described as well as all format
> +# file allocations, not only virtual disk data (metadata, internal snapshots,
> +# etc. are included).

I'm not sure I get this note.

> +#
> +# For the underlying protocol file there are native block-status types of the
> +# regions:

Comma after "protocol file"?  The sentence doesn't feel right to me even
with the comma, though.  What's a "native block-status type"?

> +#  - data: allocated data
> +#  - zero: reported as zero (for example, this type corresponds to holes for
> +#          POSIX files on sparce file-system)
> +#  - discarded: not allocated
> +# 4th additional type is 'overrun', is data referenced by the format driver
> +# located beyond EOF of the underlying protocol file. For example, a partially
> +# allocated cluster at the end of a QCOW2 file, where Qcow2 generally operates
> +# on complete clusters.

Explaining three of four types in a list, and the fourth in the
paragraph following the list feels awkward.  Turn it into a fourth list
item?

> +#
> +# So, the fields are:
> +#
> +# @used-data: used by the format file and backed by data in the underlying
> +#             protocol file
> +#
> +# @used-zero: used by the format file and backed by zeroes in the underlying
> +#             protocol file; which may be a filesystem hole for POSIX files.
> +#
> +# @used-discarded: used by the format file but actually unallocated in the
> +#                  underlying protocol file

Are discarded clusters normal or some kind of problem?

> +#
> +# @used-overrun: used by the format file beyond the end of the underlying
> +#                protocol file

Are overruns normal or some kind of problem?

> +#
> +# @unused-data: allocated data in the underlying protocol file not used by the
> +#               format file
> +#
> +# @unused-zero: reported-as-zero regions in the underlying protocol file not
> +#               used by the format file
> +#
> +# @unused-discarded: unallocated areas in the underlying protocol file not used
> +#                    by the format file
> +#
> +# Note: sum of 6 fields {used,unused}-{data,zero,discarded} is equal to the
> +#       length of the underlying protocol file.
> +#
> +# Since: 2.11
> +#
> +##
> +{ 'struct': 'BlockFormatAllocInfo',
> +  'data': {'used-data':        'uint64',
> +           'used-zero':        'uint64',
> +           'used-discarded':   'uint64',
> +           'used-overrun':     'uint64',
> +           'unused-data':      'uint64',
> +           'unused-zero':      'uint64',
> +           'unused-discarded': 'uint64' } }

Please use 'size' for byte counts.  See also "[RFC PATCH 00/56] qapi:
Use 'size' for byte counts & offsets".

> +
> +##
>  # @ImageCheck:
>  #
>  # Information about a QEMU image file check

Re: [Qemu-devel] [PATCH v4 3/3] qemu-img check: add format allocation info

[Markus Armbruster]

Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes:

> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  qapi/block-core.json |  6 +++++-
>  qemu-img.c           | 42 ++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 47 insertions(+), 1 deletion(-)
>
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 93f6995381..d662786261 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -249,6 +249,9 @@
>  #                       field is present if the driver for the image format
>  #                       supports it
>  #
> +# @format-alloc-info: Format-allocation information, see
> +#                     BlockFormatAllocInfo description. (Since: 2.11)

We don't usually add 'see the type'.  Would just

   # @format-alloc-info: Format allocation information

suffice?

> +#
>  # Since: 1.4
>  #
>  ##
> @@ -257,7 +260,8 @@
>             '*image-end-offset': 'int', '*corruptions': 'int', '*leaks': 'int',
>             '*corruptions-fixed': 'int', '*leaks-fixed': 'int',
>             '*total-clusters': 'int', '*allocated-clusters': 'int',
> -           '*fragmented-clusters': 'int', '*compressed-clusters': 'int' } }
> +           '*fragmented-clusters': 'int', '*compressed-clusters': 'int',
> +           '*format-alloc-info': 'BlockFormatAllocInfo' } }
>  
>  ##
>  # @MapEntry:
[...]