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

[Peter Lieven]

Am 06.11.2013 um 14:08 schrieb Stefan Hajnoczi <address@hidden>:

> On Thu, Oct 24, 2013 at 12:06:49PM +0200, Peter Lieven wrote:
>> 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(-)
> 
> Thanks, applied to my block-next tree:
> https://github.com/stefanha/qemu/commits/block-next
> 
> This will go into QEMU 1.8.  Since it's a new feature that touches core
> block layer code and several block drivers, I can't merge it into
> 1.7-rc.

Thats clear. Thank you for applying. As soon as this is the the official
git I will reach out for the block driver maintainers and ask them to
supply information for discard and write zero alignment + maxima.
They might also want to extend their BlockDriverInfo to reflect
their drivers abilities regarding unallocated_blocks_are_zero
and can_write_zeroes_with_unmap. Hopefully we get this
completed until 1.8 comes out.

Thanks
Peter