qemu-devel
[Top][All Lists]
Advanced

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

[PATCH v2 00/12] qemu-img info: Show protocol-level information


From: Hanna Reitz
Subject: [PATCH v2 00/12] qemu-img info: Show protocol-level information
Date: Mon, 20 Jun 2022 18:26:52 +0200

Hi,

This series is a v2 to:

https://lists.nongnu.org/archive/html/qemu-block/2022-05/msg00042.html

Like v1, the purpose is to have qemu-img info print the extent-size-hint
for images on filesystems that support it.  In contrast to v1, it does
so in a more complicated way.

v1 printed this information by adding protocol-specific information to
ImageInfo, which Kevin disliked and instead proposed to have ImageInfo
point to full ImageInfo objects for the respective node’s children.

I considered two possible solutions:
(A) Ignore the proposal, but not add protocol-specific information to
    ImageInfo.  Instead, have img_info() itself try to find a clearly
    identifiable protocol node and print its driver-specific information
    alongside the rest of the information.
    I discarded that idea because --output=json is supposed to produce a
    QAPI type, and so this additional information wouldn’t be there.  It
    wouldn’t be great to print more information with --output=human than
    with --output=json.

(B) Somehow let qemu-img info print the block graph.

This implements (B).  I decided against simply putting recursive
ImageInfo objects into ImageInfo, for three reasons:
1. Extremely personal and unsubstantiated opinion: I don’t like it for
   block query functions to return a whole graph.  I prefer query
   functions to work on single nodes and users to manually walk the
   graph if they need information about multiple nodes.

2. ImageInfo already has a link to the backing node’s ImageInfo.  It
   would be strange to link the backing node’s ImageInfo twice (once in
   backing-image, once in the list of all child nodes).

3. query-named-block-nodes returns a list of BlockDeviceInfo objects,
   and every such object has an ImageInfo field.  I think it would be a
   mistake for these ImageInfo fields to be full block subgraphs.
   Now, query-named-block-nodes already has a @flat parameter that can
   be used to suppress the backing-image information that is in
   ImageInfo.  Still, it would be a bad idea to surprise users that
   don’t set it to true (it defaults to false) by blowing up the data
   they get back.  We could add another parameter @nested that needs to
   be explicitly set to true or users will not get the child
   information; but having both @flat and @nested is kind of a bad
   interface.

So I decided to split a new structure BlockNodeInfo off of ImageInfo,
where BlockNodeInfo contains everything that ImageInfo did except for
the backing-image link.  We can then create another structure
BlockGraphInfo that builds on BlockNodeInfo, and in contrast to
ImageInfo has link to all children instead of just backing-image.  We
then let qemu-img info output BlockGraphInfo objects, which works well
because qemu-img info has always ensured the backing-image field would
not be used (so the ImageInfo objects it emitted effectively always were
what are now BlockNodeInfo objects).

(I hope this reorganization isn’t an incompatible change, but I’m not
sure, I have to admit...)

This gets around having to deal with QMP changes relating to ImageInfo
or BlockDeviceInfo (e.g. with query-named-block-nodes), and we don’t
have to worry about the fact that the backing node’s ImageInfo were
duplicated.


There is another potential duplication problem, though: VMDK’s
format-specific info (ImageInfoSpecificVmdk) contains an array of
ImageInfo objects for its extent files.  Just like with backing-image,
it seems not great to duplicate these links.

On closer inspection, however, it turns out that these objects aren’t
actually the extent nodes’ ImageInfo data at all.  These objects are
filled by the VMDK driver with custom information that actually does not
fit the ImageInfo structure, for example, the @format field is not a
block driver type, but a VMDK format, like "SPARSE" or "FLAT".
Therefore, the child links in the new BlockGraphInfo object actually
would not link to duplicate information.

I decided to make that explicit by changing the extent information type
from ImageInfo to a new VmdkExtentInfo type.  I don’t know whether that
is technically an incompatible change, and I don’t know whether it even
matters.  We can skip that type change, and this series should still
work, but I feel like it would have been better for these objects to
have had their own type from the start.


So the final state is that despite the QAPI changes, hopefully only the
qemu-img info output changes, and it now prints every image node’s
subgraph.  This results in an output like the following (for a qcow2
image):

image: test.qcow2
file format: qcow2
virtual size: 64 MiB (67108864 bytes)
disk size: 196 KiB
cluster_size: 65536
Format specific information:
    compat: 1.1
    compression type: zlib
    lazy refcounts: false
    refcount bits: 16
    corrupt: false
    extended l2: false
Child node '/file':
    filename: test.qcow2
    protocol type: file
    file length: 192 KiB (197120 bytes)
    disk size: 196 KiB
    Format specific information:
        extent size hint: 1048576

For reference, the output from v1 was this (with “extent size” corrected
to “extent size hint”):

image: test.qcow2
file format: qcow2
virtual size: 64 MiB (67108864 bytes)
disk size: 196 KiB
cluster_size: 65536
Format specific information:
    compat: 1.1
    compression type: zlib
    lazy refcounts: false
    refcount bits: 16
    corrupt: false
    extended l2: false
Protocol specific information:
    extent size hint: 1048576


I think I still slightly prefer the output from v1, because the
additional information is mostly just duplicated information (and thus
clutters the output), but I can see that hard-adding protocol-specific
information to ImageInfo wasn’t a good way to go about it (and I can’t
find any better reasonable way to only print that driver-specific
information (and nothing else) from the protocol node).


For completeness’s sake, git-backport-diff to v1:

Key:
[----] : patches are identical
[####] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively

001/12:[----] [--] 'block: Improve empty format-specific info dump'
002/12:[0008] [FC] 'block/file: Add file-specific image info'
003/12:[down] 'block/vmdk: Change extent info type'
004/12:[down] 'block: Split BlockNodeInfo off of ImageInfo'
005/12:[down] 'qemu-img: Use BlockNodeInfo'
006/12:[down] 'block/qapi: Let bdrv_query_image_info() recurse'
007/12:[down] 'block/qapi: Introduce BlockGraphInfo'
008/12:[down] 'block/qapi: Add indentation to bdrv_node_info_dump()'
009/12:[down] 'iotests: Filter child node information'
010/12:[down] 'iotests/106, 214, 308: Read only one size line'
011/12:[down] 'qemu-img: Let info print block graph'
012/12:[down] 'qemu-img: Change info key names for protocol nodes'


Hanna Reitz (12):
  block: Improve empty format-specific info dump
  block/file: Add file-specific image info
  block/vmdk: Change extent info type
  block: Split BlockNodeInfo off of ImageInfo
  qemu-img: Use BlockNodeInfo
  block/qapi: Let bdrv_query_image_info() recurse
  block/qapi: Introduce BlockGraphInfo
  block/qapi: Add indentation to bdrv_node_info_dump()
  iotests: Filter child node information
  iotests/106, 214, 308: Read only one size line
  qemu-img: Let info print block graph
  qemu-img: Change info key names for protocol nodes

 qapi/block-core.json             | 121 +++++++++++-
 include/block/qapi.h             |  14 +-
 block/file-posix.c               |  30 +++
 block/monitor/block-hmp-cmds.c   |   2 +-
 block/qapi.c                     | 319 ++++++++++++++++++++++++-------
 block/vmdk.c                     |   8 +-
 qemu-img.c                       |  76 +++++---
 qemu-io-cmds.c                   |   5 +-
 tests/qemu-iotests/065           |   2 +-
 tests/qemu-iotests/106           |   4 +-
 tests/qemu-iotests/214           |   6 +-
 tests/qemu-iotests/302.out       |   5 +
 tests/qemu-iotests/308           |   4 +-
 tests/qemu-iotests/common.filter |  22 ++-
 tests/qemu-iotests/common.rc     |  22 ++-
 tests/qemu-iotests/iotests.py    |  18 +-
 16 files changed, 519 insertions(+), 139 deletions(-)

-- 
2.35.3




reply via email to

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