qemu-block
[Top][All Lists]
Advanced

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

[PATCH v2 03/12] block/vmdk: Change extent info type


From: Hanna Reitz
Subject: [PATCH v2 03/12] block/vmdk: Change extent info type
Date: Mon, 20 Jun 2022 18:26:55 +0200

VMDK's implementation of .bdrv_get_specific_info() returns information
about its extent files, ostensibly in the form of ImageInfo objects.
However, it does not get this information through
bdrv_query_image_info(), but fills only a select few fields with custom
information that does not always match the fields' purposes.

For example, @format, which is supposed to be a block driver name, is
filled with the extent type, e.g. SPARSE or FLAT.

In ImageInfo, @compressed shows whether the data that can be seen in the
image is stored in compressed form or not.  For example, a compressed
qcow2 image will store compressed data in its data file, but when
accessing the qcow2 node, you will see normal data.  This is not how
VMDK uses the @compressed field for its extent files: Instead, it
signifies whether accessing the extent file will yield compressed data
(which the VMDK driver then (de-)compresses).

Create a new structure to represent the extent information.  This allows
us to clarify the fields' meanings, and it clearly shows that these are
not complete ImageInfo objects.  (That is, if a user wants an extent
file's ImageInfo object, they will need to query it separately, and will
not get it from ImageInfoSpecificVmdk.extents.)

Note that this removes the last use of ['ImageInfo'] (i.e. an array of
ImageInfo objects), so the QAPI generator will no longer generate
ImageInfoList by default.  However, we use it in qemu-img.c, so we need
to create a dummy object to force the generate to create that type,
similarly to DummyForceArrays in machine.json (introduced in commit
9f08c8ec73878122ad4b061ed334f0437afaaa32 ("qapi: Lazy creation of array
types")).

Signed-off-by: Hanna Reitz <hreitz@redhat.com>
---
I'm not sure whether this is an incompatible change.  I'm also not sure
if it even matters whether it's an incompatible change (i.e. whether
anyone cares).

I believe we can get away without this change.  I find it useful to make
it clear that (A) this extent information is not what you would find in
other ImageInfo objects (i.e., I consider this a fix for
ImageInfoSpecificVmdk's @extents field), and (B) that the upcoming
BlockGraphInfo type will not duplicate the extent nodes' ImageInfo
information, because those are actual ImageInfo objects.

We can probably replace this patch by clarifying all of this in
documentation, but if possible I would prefer a syntactic clarification
(as done here).
---
 qapi/block-core.json | 38 +++++++++++++++++++++++++++++++++++++-
 block/vmdk.c         |  8 ++++----
 2 files changed, 41 insertions(+), 5 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 40fb307e2d..e0c8f07932 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -124,7 +124,33 @@
       'create-type': 'str',
       'cid': 'int',
       'parent-cid': 'int',
-      'extents': ['ImageInfo']
+      'extents': ['VmdkExtentInfo']
+  } }
+
+##
+# @VmdkExtentInfo:
+#
+# Information about a VMDK extent file
+#
+# @filename: Name of the extent file
+#
+# @format: Extent type (e.g. FLAT or SPARSE)
+#
+# @virtual-size: Number of bytes covered by this extent
+#
+# @cluster-size: Cluster size in bytes (for non-flat extents)
+#
+# @compressed: Whether this extent contains compressed data
+#
+# Since: 7.1
+##
+{ 'struct': 'VmdkExtentInfo',
+  'data': {
+      'filename': 'str',
+      'format': 'str',
+      'virtual-size': 'int',
+      '*cluster-size': 'int',
+      '*compressed': 'bool'
   } }
 
 ##
@@ -5638,3 +5664,13 @@
   'data': { 'device': 'str', '*id': 'str', '*name': 'str'},
   'returns': 'SnapshotInfo',
   'allow-preconfig': true }
+
+##
+# @DummyBlockCoreForceArrays:
+#
+# Not used by QMP; hack to let us use ImageInfoList internally
+#
+# Since: 7.1
+##
+{ 'struct': 'DummyBlockCoreForceArrays',
+  'data': { 'unused-image-info': ['ImageInfo'] } }
diff --git a/block/vmdk.c b/block/vmdk.c
index 38e5ab3806..35131a916e 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -2908,12 +2908,12 @@ static int vmdk_has_zero_init(BlockDriverState *bs)
     return 1;
 }
 
-static ImageInfo *vmdk_get_extent_info(VmdkExtent *extent)
+static VmdkExtentInfo *vmdk_get_extent_info(VmdkExtent *extent)
 {
-    ImageInfo *info = g_new0(ImageInfo, 1);
+    VmdkExtentInfo *info = g_new0(VmdkExtentInfo, 1);
 
     bdrv_refresh_filename(extent->file->bs);
-    *info = (ImageInfo){
+    *info = (VmdkExtentInfo){
         .filename         = g_strdup(extent->file->bs->filename),
         .format           = g_strdup(extent->type),
         .virtual_size     = extent->sectors * BDRV_SECTOR_SIZE,
@@ -2992,7 +2992,7 @@ static ImageInfoSpecific 
*vmdk_get_specific_info(BlockDriverState *bs,
     int i;
     BDRVVmdkState *s = bs->opaque;
     ImageInfoSpecific *spec_info = g_new0(ImageInfoSpecific, 1);
-    ImageInfoList **tail;
+    VmdkExtentInfoList **tail;
 
     *spec_info = (ImageInfoSpecific){
         .type = IMAGE_INFO_SPECIFIC_KIND_VMDK,
-- 
2.35.3




reply via email to

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