Re: [Qemu-devel] [PATCH 00/17] Add qemu-img subcommand to dump file metadata

[Peter Lieven <pl@kamp.de>]

On 16.07.2013 05:41, Stefan Hajnoczi wrote:
> On Wed, Jul 03, 2013 at 04:34:14PM +0200, Paolo Bonzini wrote:
>> This series adds a subcommand to "map" that can dump file metadata.
>> Metadata that is dumped includes:
>>
>> - whether blocks are allocated in bs->file and, if so, where
>>
>> - whether blocks are zero
>>
>> - whether data is read from bs or bs->backing_hd
>>
>> Metadata is dumped for an entire chain of images.  One example usage is
>> (for non-compressed, non-encrypted images) to transform the metadata
>> into a Linux device-mapper setup, and make a qcow2 image available (for
>> read only) as a block device.  Another possible usage is to determine
>> the used areas of a file, and convert it in place to another format.
>> Alternatively, the richer data can be used by block jobs to quickly
>> determine if a block is zero without reading it.
>>
>> The patches implement a new operation, bdrv_co_get_block_status, that
>> supersedes bdrv_co_is_allocated.  The bdrv_is_allocated API is still
>> available of course, and implemented on top of the new operation.
>>
>> Patches 1-3 touch cow, which uses bdrv_co_is_allocated during its reads
>> and writes.  I optimized it a bit because it was unbearably slow during
>> testing.  With these patches (also tested with blkverify), booting of
>> a cow guest image is not particularly slow.
>>
>> Patches 4 to 6 clean up the bdrv_is_allocated and bdrv_is_allocated_above
>> implementation, eliminating some code duplication.
>>
>> Patches 7 and 8 tweak bdrv_has_zero_init and its usage in qemu-img in
>> a way that helps when implementing the new API.
>>
>> Patches 9 to 11 implement bdrv_get_block_status and change the formats
>> to report all the available information.
>>
>> Patch 12 adds the "map" subcommand to qemu-img.
>>
>> Finally, patches 13 to 17 tweak the implementation to extract more
>> information from protocols, and combine this information with format
>> metadata whenever possible.
>>
>> Paolo
>>
>> Paolo Bonzini (17):
>>    cow: make reads go at a decent speed
>>    cow: make writes go at a less indecent speed
>>    cow: do not call bdrv_co_is_allocated
>>    block: make bdrv_co_is_allocated static
>>    block: remove bdrv_is_allocated_above/bdrv_co_is_allocated_above
>>      distinction
>>    block: expect errors from bdrv_co_is_allocated
>>    qemu-img: always probe the input image for allocated sectors
>>    block: make bdrv_has_zero_init return false for copy-on-write-images
>>    block: introduce bdrv_get_block_status API
>>    block: define get_block_status return value
>>    block: return get_block_status data and flags for formats
>>    qemu-img: add a "map" subcommand
>>    block: use bdrv_has_zero_init to return BDRV_BLOCK_ZERO
>>    raw-posix: return get_block_status data and flags
>>    raw-posix: detect XFS unwritten extents
>>    block: add default get_block_status implementation for protocols
>>    block: look for zero blocks in bs->file
>>
>>   block.c                   | 134 +++++++++++++--------------
>>   block/commit.c            |   6 +-
>>   block/cow.c               |  90 +++++++++++++------
>>   block/mirror.c            |   4 +-
>>   block/qcow.c              |  13 ++-
>>   block/qcow2.c             |  24 +++--
>>   block/qed.c               |  39 ++++++--
>>   block/raw-posix.c         |  24 +++--
>>   block/raw.c               |   6 +-
>>   block/sheepdog.c          |  14 +--
>>   block/stream.c            |  10 +--
>>   block/vdi.c               |  17 +++-
>>   block/vmdk.c              |  20 ++++-
>>   block/vvfat.c             |  15 ++--
>>   include/block/block.h     |  31 +++++--
>>   include/block/block_int.h |   2 +-
>>   qemu-img-cmds.hx          |   6 ++
>>   qemu-img.c                | 225 +++++++++++++++++++++++++++++++++++++++++-----
>>   18 files changed, 497 insertions(+), 183 deletions(-)
> Makes sense to me.  I have left a few small comments but nothing major.
>
> Stefan
Paolo, could you respin it once with all the small fixes. I would go over it again
after that if you like.

Peter