Re: [Qemu-devel] [PATCH v14 00/14] qemu-img: Implement commit like QMP

[Stefan Hajnoczi]

On Fri, Oct 24, 2014 at 03:57:29PM +0200, Max Reitz wrote:
> qemu-img should use QMP commands whenever possible in order to ensure
> feature completeness of both online and offline image operations. For
> the "commit" command, this is relatively easy, so implement it first
> (in the hope that indeed others will follow).
> 
> As qemu-img does not have access to QMP (due to QMP being intertwined
> with basically everything in qemu), we cannot directly use QMP, but at
> least use the functions the corresponding QMP commands are using (which
> would be "block-commit", in this case).
> 
> 
> v14:
> - Patch 3:
>   - Pull the optimized code out into an own function [Kevin]
>   - Only run the optimized code if the image header, the reftable, a
>     single refblock and the L1 table can be described by that single
>     refblock [Kevin]
>   - Zero out the L1 table before modifying anything else [Kevin]
>   - Always set bs->drv to NULL if an error occured which broke the
>     in-memory refcount information and thus rendered the BDS unusable
>     (because all of the qcow2 code assumes that the in-memory
>     information is correct, obviously) [Kevin]
>   - Do truncation last, because while it is the eventual goal, it is
>     also the structurally least invasive and therefore least important
>     operation (we don't want it to fail and leave the BDS unusable)
>     [Kevin]
> - Patch 6: In the meantime, two more places which test data/len have
>   crept in, remove them [Kevin]
> - Patch 8:
>   - Drop the block_job_resume() call [Kevin, although he'd probably
>     prefer not to be mentioned for this]
>   - Improve error message when there is no backing file [Kevin]
> - Patch 10: Rebase onto patch 8 (dropped block_job_resume())
> - Patch 11:
>   - Rebase onto patch 8 (different error message if the base image has
>     not been explicitly specified)
>   - Explanation about what "For reasons of consistency" means (or at
>     least an example of inconsistency) [Kevin]
> - Patch 14:
>   - Write some data to the top image so it can be leaked
>   - Swap order of l1_update and empty_image_prepare (l1_update now
>     occurs before empty_image_prepare; the second l1_update occurs at
>     the same time as the reftable_update, so it's safe to omit that one)
>   - Output changed accordingly
> 
> 
> git-backport-diff against v13:
> 
> 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/14:[----] [--] 'qcow2: Allow "full" discard'
> 002/14:[----] [--] 'qcow2: Implement bdrv_make_empty()'
> 003/14:[0306] [FC] 'qcow2: Optimize bdrv_make_empty()'
> 004/14:[----] [--] 'blockjob: Introduce block_job_complete_sync()'
> 005/14:[----] [--] 'blockjob: Add "ready" field'
> 006/14:[0002] [FC] 'iotests: Omit length/offset test in 040 and 041'
> 007/14:[----] [--] 'block/mirror: Improve progress report'
> 008/14:[0009] [FC] 'qemu-img: Implement commit like QMP'
> 009/14:[----] [--] 'qemu-img: Empty image after commit'
> 010/14:[0001] [FC] 'qemu-img: Enable progress output for commit'
> 011/14:[0019] [FC] 'qemu-img: Specify backing file for commit'
> 012/14:[----] [--] 'iotests: Add _filter_qemu_img_map'
> 013/14:[----] [--] 'iotests: Add test for backing-chain commits'
> 014/14:[0026] [FC] 'iotests: Add test for qcow2's bdrv_make_empty'
> 
> 
> Max Reitz (14):
>   qcow2: Allow "full" discard
>   qcow2: Implement bdrv_make_empty()
>   qcow2: Optimize bdrv_make_empty()
>   blockjob: Introduce block_job_complete_sync()
>   blockjob: Add "ready" field
>   iotests: Omit length/offset test in 040 and 041
>   block/mirror: Improve progress report
>   qemu-img: Implement commit like QMP
>   qemu-img: Empty image after commit
>   qemu-img: Enable progress output for commit
>   qemu-img: Specify backing file for commit
>   iotests: Add _filter_qemu_img_map
>   iotests: Add test for backing-chain commits
>   iotests: Add test for qcow2's bdrv_make_empty
> 
>  block/Makefile.objs              |   3 +-
>  block/blkdebug.c                 |   2 +
>  block/mirror.c                   |  34 ++++---
>  block/qcow2-cluster.c            |  27 ++++--
>  block/qcow2-snapshot.c           |   2 +-
>  block/qcow2.c                    | 192 
> ++++++++++++++++++++++++++++++++++++++-
>  block/qcow2.h                    |   2 +-
>  blockjob.c                       |  42 +++++++--
>  include/block/block.h            |   2 +
>  include/block/blockjob.h         |  20 ++++
>  qapi/block-core.json             |   4 +-
>  qemu-img-cmds.hx                 |   4 +-
>  qemu-img.c                       | 147 +++++++++++++++++++++++++-----
>  qemu-img.texi                    |  16 +++-
>  tests/qemu-iotests/040           |   4 +-
>  tests/qemu-iotests/041           |   5 +-
>  tests/qemu-iotests/097           | 122 +++++++++++++++++++++++++
>  tests/qemu-iotests/097.out       | 119 ++++++++++++++++++++++++
>  tests/qemu-iotests/098           |  82 +++++++++++++++++
>  tests/qemu-iotests/098.out       |  52 +++++++++++
>  tests/qemu-iotests/common.filter |   7 ++
>  tests/qemu-iotests/group         |   2 +
>  tests/qemu-iotests/iotests.py    |   3 +-
>  23 files changed, 823 insertions(+), 70 deletions(-)
>  create mode 100755 tests/qemu-iotests/097
>  create mode 100644 tests/qemu-iotests/097.out
>  create mode 100755 tests/qemu-iotests/098
>  create mode 100644 tests/qemu-iotests/098.out
> 
> -- 
> 1.9.3
> 
> 

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan

pgpkeAUXRAPVX.pgp
Description: PGP signature