| Message ID | 20220620162704.80987-1-hreitz@redhat.com |
|---|---|
| Headers | show |
| Series | qemu-img info: Show protocol-level information | expand |
On 20.06.22 18:26, Hanna Reitz wrote: > Hi, > > This series is a v2 to: > > https://lists.nongnu.org/archive/html/qemu-block/2022-05/msg00042.html Ping, it looks like this still applies (to the master branch and kevin’s block-next branch at least). Hanna > 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
Am 08.12.2022 um 13:24 hat Hanna Reitz geschrieben: > On 20.06.22 18:26, Hanna Reitz wrote: > > Hi, > > > > This series is a v2 to: > > > > https://lists.nongnu.org/archive/html/qemu-block/2022-05/msg00042.html > > Ping, it looks like this still applies (to the master branch and kevin’s > block-next branch at least). Not any more. :-) But the conflicts seemed obvious enough, so I rebased it (including changing the "Since: 7.1" occurrences to 8.0) and applied it to my block branch. Thanks! Kevin
On 19.01.23 21:12, Kevin Wolf wrote: > Am 08.12.2022 um 13:24 hat Hanna Reitz geschrieben: >> On 20.06.22 18:26, Hanna Reitz wrote: >>> Hi, >>> >>> This series is a v2 to: >>> >>> https://lists.nongnu.org/archive/html/qemu-block/2022-05/msg00042.html >> Ping, it looks like this still applies (to the master branch and kevin’s >> block-next branch at least). > Not any more. :-) > > But the conflicts seemed obvious enough, so I rebased it (including > changing the "Since: 7.1" occurrences to 8.0) and applied it to my block > branch. Ah, yes. That I should have fixed. > Thanks! Thank you! :) Hanna
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(-)