[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [PATCH v3 1/3] block: add bdrv_get_format_alloc_stat fo

From: Vladimir Sementsov-Ogievskiy
Subject: Re: [Qemu-block] [PATCH v3 1/3] block: add bdrv_get_format_alloc_stat format interface
Date: Fri, 28 Jul 2017 11:25:18 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1

28.07.2017 00:23, John Snow wrote:

On 07/12/2017 11:18 AM, Vladimir Sementsov-Ogievskiy wrote:
30.06.2017 03:27, John Snow wrote:

On 06/06/2017 12:26 PM, Vladimir Sementsov-Ogievskiy wrote:
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 <address@hidden>
  block.c                   | 16 ++++++++++++++
  include/block/block.h     |  3 +++
  include/block/block_int.h |  2 ++
qapi/block-core.json | 55 +++++++++++++++++++++++++++++++++++++++++++++++
  4 files changed, 76 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..fd7b52bd69 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -139,6 +139,61 @@
             '*format-specific': 'ImageInfoSpecific' } }
+# @BlockFormatAllocInfo:
+# Allocation relations between format file and underlying protocol file.
+# All fields are in bytes.
I guess this is a relation in the sense that the format differentiates
between used-unused and the protocol differentiates between
data-zero-trim which gives us the 2D matrix, showing a relation between
"two" files.

I find the use of the word "file" here a bit of a misdirection, though,
as qcow2 (or any other format) in this sense is not a file but rather a
schema used for interpreting data, and the file itself belongs solely to
the protocol.

I might suggest phrasing this as ...

"Allocation information of an underlying protocol file, as partitioned
by a format driver's utilization of said allocations."

Maybe that's too wordy. Eric?

+# There are two types of the format file portions: 'used' and 'unused'. It's up +# to the format how to interpret these types. For now the only format supporting
"format file" seems misleading for similar reasons to my objection
above, which is that the format doesn't have a file, exactly. It's an
abstract schema.

"It's up to the format how to interpret these types" is also a bit too
vague to help inform readers what the types mean, IMO.

Here's an attempt:

"Allocations may be considered either used or unused by the format
driver interpreting those allocations. It is at the discretion of the
format driver (e.g. qcow2) which regions of its backing storage are
considered in-use or not."

So we are saying about "allocations". But unallocated data may be used/unused too, so,
can we call unallocated areas "allocations"?

You're right, "allocation" may imply something that is extant, but these are more like regions ...


"Regions" may be considered either used or unused?


"Regions of the underlying protocol file may be considered used or unused by the format driver interpreting these regions."

Something along these lines, perhaps?

This is OK I think.

Best regards,

reply via email to

[Prev in Thread] Current Thread [Next in Thread]