Re: [Qemu-devel] [PATCHv7 00/17] block: logical block provisioning enhancements

[Paolo Bonzini <pbonzini@redhat.com>]

Il 24/10/2013 11:06, Peter Lieven ha scritto:
> this patch adds the ability for targets to stay sparse during
> block migration (if the zero_blocks capability is set) and qemu-img convert
> even if the target does not have has_zero_init = 1.
> 
> the series was especially developed for iSCSI, but it should also work
> with other drivers with little or no adjustments. these adjustments
> should be limited to providing block provisioning information through
> get_block_info and/or honouring BDRV_REQ_MAY_UNMAP on writing zeroes.
> 
> v6->v7:
>  - switched position of "iscsi: set limits in BlockDriverState" and
>    "iscsi: simplify iscsi_co_discard". (Paolo)
>  - fixed commit message of 
>    "block/get_block_status: fix BDRV_BLOCK_ZERO for unallocated blocks".
>    (Paolo)
>  - moved "block/raw: copy BlockLimits on raw_open" right after
>    "block: add BlockLimits structure to BlockDriverState". (Paolo)
>  - Reworded desciption for -S 0 in
>    "qemu-img: add support for fully allocated images" as suggested
>    by Paolo.
>  - Reworded commit message of:
>    "qemu-img: conditionally zero out target on convert".
>    regarding iscsi (Paolo)
>  
> v5->v6:
>  - protected iscsi_co_write_zeroes by the existence of the
>    SCSI_SENSE_ASCQ_CAPACITY_DATA_HAS_CHANGED macro. This is ugly
>    but necessary because the semantic of iscsi_writesame16_task
>    silently changed between libiscsi 1.8.0 and 1.9.0. The above
>    macro was the first added after the change. I already contacted
>    Ronnie to introduce an API version macro which has to be bumped
>    on each new function that will be added. Changes to the parameters
>    should not happen at all of course.
> 
> v4->v5:
>  - new patches 4-6 to move the block provisioning information
>    to the BlockDriverInfo.
>  - kept 2 wrappers to read the information from the BDI and
>    renamed them to make more clear what they do:
> 
>  bdrv_has_discard_zeroes -> bdrv_unallocated_blocks_are_zero
>  bdrv_has_discard_write_zeroes -> bdrv_can_write_zeroes_with_unmap
> 
>  - added additional information about the 2 flags in the
>    BDI struct in block.h
> 
> v3->v4:
>  - changed BlockLimits struct to typedef (Stefan, Eric)
>  - renamed bdrv_zeroize to bdrv_make_zero (Stefan)
>  - added comment about the -S flag of qemu-img convert in
>    qemu-img.texi (Eric)
>  - used struct assignment for bs->bl in raw_open (Stefan, Eric)
>  - dropped 3 get_block_status fixes that are independent of
>    this series and already partly merged.
> 
> v2->v3:
>  - fix merge conflict in block/qcow2_cluster.c
>  - changed return type of bdrv_has_discard_zeroes and
>    bdrv_has_discard_write_zeroes to bool.
>  - moved alignment and limits info to a BlockLimits struct (Paolo).
>  - added magic constanst for default maximum in bdrv_co_do_write_zeroes
>    and bdrv_co_discard (Eric).
>  - bdrv_co_do_write_zeroes: allocating the bounce buffer only once (Eric),
>    fixed bounce iov_len in the fall back path.
>  - bdrv_zeroize: added inline docu (Eric) and do not mask flags passed
>    to bdrv_write_zeroes (Eric).
>  - qemu-img: changed the default hint for -S (min_sparse) in the usage
>    help to 4k. not changing the default as it is unclear why this default
>    was set. size suffixes are already supported (Eric).
> 
> v1->v2:
>  - moved block max_discard and max_write_zeroes to BlockDriverState
>  - added discard_alignment and write_zeroes_alignment to BlockDriverState
>  - added bdrv_has_discard_zeroes() and bdrv_has_discard_write_zeroes()
>  - added logic to bdrv_co_discard and bdrv_co_do_write_zeroes to honour
>    limit and alignment info.
>  - added support for -S 0 in qemu-img convert.
> 
> Peter Lieven (17):
>   block: make BdrvRequestFlags public
>   block: add flags to bdrv_*_write_zeroes
>   block: introduce BDRV_REQ_MAY_UNMAP request flag
>   block: add logical block provisioning info to BlockDriverInfo
>   block: add wrappers for logical block provisioning information
>   block/iscsi: add .bdrv_get_info
>   block: add BlockLimits structure to BlockDriverState
>   block/raw: copy BlockLimits on raw_open
>   block: honour BlockLimits in bdrv_co_do_write_zeroes
>   block: honour BlockLimits in bdrv_co_discard
>   iscsi: set limits in BlockDriverState
>   iscsi: simplify iscsi_co_discard
>   iscsi: add bdrv_co_write_zeroes
>   block: introduce bdrv_make_zero
>   block/get_block_status: fix BDRV_BLOCK_ZERO for unallocated blocks
>   qemu-img: add support for fully allocated images
>   qemu-img: conditionally zero out target on convert
> 
>  block-migration.c         |    3 +-
>  block.c                   |  200 +++++++++++++++++++++++++++++++++++++--------
>  block/backup.c            |    3 +-
>  block/iscsi.c             |  150 +++++++++++++++++++++++++---------
>  block/qcow2-cluster.c     |    2 +-
>  block/qcow2.c             |    2 +-
>  block/qed.c               |    3 +-
>  block/raw_bsd.c           |    6 +-
>  block/vmdk.c              |    3 +-
>  include/block/block.h     |   35 +++++++-
>  include/block/block_int.h |   19 ++++-
>  qemu-img.c                |   20 ++++-
>  qemu-img.texi             |    6 ++
>  qemu-io-cmds.c            |    2 +-
>  14 files changed, 366 insertions(+), 88 deletions(-)
> 

This looks good to me.

Paolo