qemu-block
[Top][All Lists]
Advanced

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

Re: [PATCH v2 2/3] qemu-img: map: report compressed data blocks


From: Hanna Czenczek
Subject: Re: [PATCH v2 2/3] qemu-img: map: report compressed data blocks
Date: Tue, 29 Aug 2023 10:46:17 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.13.0

On 29.08.23 08:44, Andrey Drobyshev wrote:
On 8/25/23 17:14, Hanna Czenczek wrote:
On 06.07.23 18:30, Andrey Drobyshev wrote:
Right now "qemu-img map" reports compressed blocks as containing data
but having no host offset.  This is not very informative.  Instead,
let's add another boolean field named "compressed" in case JSON output
mode is specified.  This is achieved by utilizing new allocation status
flag BDRV_BLOCK_COMPRESSED for bdrv_block_status().

Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
   qapi/block-core.json |  7 +++++--
   qemu-img.c           | 16 +++++++++++++---
   2 files changed, 18 insertions(+), 5 deletions(-)
Patch 3 must be merged into this patch.  Every test must pass on every
commit so we don’t break bisecting.
Agreed, should've figured that myself.

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 5dd5f7e4b0..b263d2cd30 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -409,6 +409,9 @@
   #
   # @zero: whether the virtual blocks read as zeroes
   #
+# @compressed: true indicates that data is stored compressed.  Optional,
+#     only valid for the formats whith support compression
This is missing information for when this field was introduced (i.e. a
“(since 8.2)”).
Noted.

I also wonder why this field is optional.  We have compression
information even for formats that don’t support compression,
specifically, nothing is compressed.  I would just make this field
mandatory and print it always.  (A technical reason to do so is that
this patch uses block_driver_can_compress() to figure out whether there
is compression support; but that function only tells whether the driver
can write compressed data.  Even if it cannot do that, the format may
still support compression, and the driver may be able to read compressed
data, just not write it.)

I figured that for the formats which surely can't support compression,
such as "raw", this information would simply be redundant.  AFAICT right
now the only drivers which can read compressed data are exactly the ones
which can write it: vmdk, qcow and qcow2.  But if we assume that this
might change, and that we'd better show the field value no matter what
(as we do with "zero" field) -- I'm OK with it.

It is indeed redundant, but as this is intended to be the machine-readable output, I don’t mind the output becoming arguably needlessly verbose.




reply via email to

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