(first, but not the least sorry for late reply. It was really strange that I have come back here several times to write it correctly. Hope this will answer to all remaining questions.)
On 9/4/26 10:49, Markus Armbruster wrote: > "Denis V. Lunev" <[email protected]> writes: > >> From: Denis V. Lunev <[email protected]> >> >> Nothing tells which persistent bitmaps an image carries. qemu-img info >> says nothing about them, and the only other way to see one is to export >> the image over NBD and ask for a bitmap by name, which needs the name >> beforehand. >> >> Add ImageInfoSpecificParallels with the bitmaps and their granularity, >> in the same way as qcow2 reports the contents of its bitmap directory: >> >> Format specific information: >> bitmaps: >> [0]: >> name: b2c9e1a4-5d3f-4e8b-9a7c-6f0d1e2b3a45 >> granularity: 65536 >> >> For running VM process the list is built on the fly from in-memory >> list. This is the best we can do. FormatExtension could be dead in >> the image while VMs are running as all bitmaps are cleared on >> non-clean VM stop. > What's "FormatExtension"? Format Extension is the area through which dirty bitmap is accessed. For now this is catalog of extensions with the only implemented extension - dirty bitmap extension. This is specified in details in docs/interop/parallels.rst, section "Format Extension". >> A bitmap of an image which was not closed correctly is inconsistent >> and can not be used. Report that as well, the way qcow2 reports its >> in-use flag, so such a bitmap is not listed as a valid one. > This is helpful, thanks! > >> The section is left empty for an image with no persistent dirty >> bitmaps, and an empty section is not printed, so the human readable >> output of "qemu-img info" and "info block" is unchanged. > Perfectly clear now. > >> Cc: Stefan Hajnoczi <[email protected]> >> Cc: Eric Blake <[email protected]> >> Cc: Markus Armbruster <[email protected]> >> Signed-off-by: Denis V. Lunev <[email protected]> >> --- >> block/parallels-ext.c | 26 +++++++++++ >> block/parallels.c | 20 ++++++++ >> block/parallels.h | 3 ++ >> qapi/block-core.json | 46 ++++++++++++++++++- >> tests/qemu-iotests/tests/parallels-checks | 6 +++ >> tests/qemu-iotests/tests/parallels-checks.out | 19 ++++++++ >> 6 files changed, 118 insertions(+), 2 deletions(-) >> >> diff --git a/block/parallels-ext.c b/block/parallels-ext.c >> index f687f2da7c..8b8035af91 100644 >> --- a/block/parallels-ext.c >> +++ b/block/parallels-ext.c >> @@ -736,3 +736,29 @@ >> parallels_co_remove_persistent_dirty_bitmap(BlockDriverState *bs, >> >> return ret; >> } >> + >> +void parallels_get_bitmap_info_list(BlockDriverState *bs, >> + ParallelsBitmapInfoList **info_list) >> +{ >> + BdrvDirtyBitmap *bitmap; >> + ParallelsBitmapInfoList **tail = info_list; >> + >> + *info_list = NULL; >> + >> + FOR_EACH_DIRTY_BITMAP(bs, bitmap) { >> + ParallelsBitmapInfo *info; >> + >> + if (!bdrv_dirty_bitmap_get_persistence(bitmap)) { >> + continue; >> + } >> + >> + info = g_new0(ParallelsBitmapInfo, 1); >> + info->name = g_strdup(bdrv_dirty_bitmap_name(bitmap)); >> + info->granularity = bdrv_dirty_bitmap_granularity(bitmap); >> + if (bdrv_dirty_bitmap_inconsistent(bitmap)) { >> + info->has_inconsistent = true; >> + info->inconsistent = true; >> + } >> + QAPI_LIST_APPEND(tail, info); >> + } >> +} >> diff --git a/block/parallels.c b/block/parallels.c >> index 2a5ceb8978..e7d65d0458 100644 >> --- a/block/parallels.c >> +++ b/block/parallels.c >> @@ -1624,6 +1624,25 @@ static bool >> parallels_is_support_dirty_bitmaps(BlockDriverState *bs) >> return 1; >> } >> >> +static ImageInfoSpecific * GRAPH_RDLOCK >> +parallels_get_specific_info(BlockDriverState *bs, Error **errp) >> +{ >> + ImageInfoSpecificParallels *parallels_info; >> + ImageInfoSpecific *spec_info; >> + >> + parallels_info = g_new0(ImageInfoSpecificParallels, 1); >> + parallels_get_bitmap_info_list(bs, ¶llels_info->bitmaps); >> + parallels_info->has_bitmaps = !!parallels_info->bitmaps; >> + >> + spec_info = g_new(ImageInfoSpecific, 1); >> + *spec_info = (ImageInfoSpecific){ >> + .type = IMAGE_INFO_SPECIFIC_KIND_PARALLELS, >> + .u.parallels.data = parallels_info, >> + }; >> + >> + return spec_info; >> +} >> + >> static BlockDriver bdrv_parallels = { >> .format_name = "parallels", >> .instance_size = sizeof(BDRVParallelsState), >> @@ -1653,6 +1672,7 @@ static BlockDriver bdrv_parallels = { >> parallels_co_can_store_new_dirty_bitmap, >> .bdrv_co_remove_persistent_dirty_bitmap = >> >> parallels_co_remove_persistent_dirty_bitmap, >> + .bdrv_get_specific_info = parallels_get_specific_info, >> }; >> >> static void bdrv_parallels_init(void) >> diff --git a/block/parallels.h b/block/parallels.h >> index 27d8c3ac83..012f47320b 100644 >> --- a/block/parallels.h >> +++ b/block/parallels.h >> @@ -32,6 +32,7 @@ >> #ifndef BLOCK_PARALLELS_H >> #define BLOCK_PARALLELS_H >> #include "qemu/coroutine.h" >> +#include "qapi/qapi-types-block-core.h" >> >> #define HEADS_NUMBER 16 >> #define SEC_IN_CYL 32 >> @@ -111,5 +112,7 @@ parallels_co_can_store_new_dirty_bitmap(BlockDriverState >> *bs, const char *name, >> int coroutine_fn GRAPH_RDLOCK >> parallels_co_remove_persistent_dirty_bitmap(BlockDriverState *bs, >> const char *name, Error **errp); >> +void parallels_get_bitmap_info_list(BlockDriverState *bs, >> + ParallelsBitmapInfoList **info_list); >> >> #endif >> diff --git a/qapi/block-core.json b/qapi/block-core.json >> index 199efc1e00..7499aea641 100644 >> --- a/qapi/block-core.json >> +++ b/qapi/block-core.json >> @@ -188,6 +188,35 @@ >> '*extent-size-hint': 'size' >> } } >> >> +## >> +# @ParallelsBitmapInfo: >> +# >> +# Parallels dirty bitmap information. >> +# >> +# @name: the name of the bitmap >> +# >> +# @granularity: granularity of the bitmap in bytes >> +# >> +# @inconsistent: true if the bitmap was improperly stored and cannot >> +# be used > I'd recommend "if the bitmap is in an inconsistent state and". Ok >> +# >> +# Since: 11.2 >> +## >> +{ 'struct': 'ParallelsBitmapInfo', >> + 'data': { 'name': 'str', 'granularity': 'uint32', >> + '*inconsistent': 'bool' } } >> + >> +## >> +# @ImageInfoSpecificParallels: >> +# >> +# @bitmaps: A list of the persistent dirty bitmaps of the image, >> +# including the ones which are not written out yet > In review of v6, I asked whether "not written out yet" relevant to a > user / management application. You explained what it means, but didn't > actually answer my question. I failed to point that out then, sorry. > Can you answer it now? No. Dropped in v8. Right now the main pattern of usage of Parallels image driver is working with existing images, which are still available in some places. In theory people could start VM directly from these drivers but this is would be very strange story even for me. Thus the following patterns are to be supported as the first priority: * qemu-img check * qemu-img convert * qemu-img info All these commands opens images by itself and handles things by themselves. That is why QAPI change here is collateral. It is not possible to show ImageSpecificInfo without this change. Nothing here could observe the distinction. > I also inquired about the relation to ImageInfoSpecificQCow2's @bitmaps, > and you told me they're basically the same (correct me if I > misunderstood). I then replied that the documentation should be > basically the same, and you agreed. Did you forget? This is my fault and I have missed this is the v7. # @bitmaps: A list of parallels bitmap details without the "(since ...)": qcow2 needs it because @bitmaps arrived in 4.0 while ImageInfoSpecificQCow2 is Since: 1.7, and ImageInfoSpecificParallels is itself Since: 11.2. Also in the diff: the struct description lost its stray "dirty", and ParallelsBitmapInfo moved next to Qcow2BitmapInfo, out of the ImageInfoSpecificFOO run - the v6 ordering you asked for. Thank you for your time, Den P.S. Can you pls drop 2 words into thread for https://lore.kernel.org/qemu-devel/[email protected]/ If you believe that proposed API change is no go, I'd go rewrite code with online 'check' command. This problem is real and the damage is very big, which is not just single cluster lost.
