Re: [Qemu-devel] [PATCH v3 00/14] block: Remove "growable", add blk_new_open()

[Stefano Stabellini]

On Mon, 26 Jan 2015, Max Reitz wrote:
> This series removes the "growable" field from the BlockDriverState
> object. Its use was to clamp guest requests against the limits of the
> BDS; however, this can now be done more easily by moving those checks
> into the BlockBackend functions.
> 
> In a future series, "growable" may be reintroduced (maybe with a
> different name); it will then signify whether a BDS is able to grow (in
> contrast to the current "growable", which signifies whether it is
> allowed to). Maybe I will add it to the BlockDriver instead of the BDS,
> though.
> 
> To be able to remove that field, qemu-io needs to be converted to
> BlockBackend, which is done by this series as well. While working on
> that I decided to convert blk_new_with_bs()+bdrv_open() to
> blk_new_open(). I was skeptical about that decision at first, but it
> seems good now that I was able to replace nearly every blk_new_with_bs()
> call by blk_new_open(). In a future series I may try to convert some
> remaining bdrv_open() calls to blk_new_open() as well. (And, in fact, in
> a future series I *will* replace the last remaining blk_new_with_bs()
> outside of blk_new_open() by blk_new_open().)
> 
> Finally, the question needs to be asked: If, after this series, every
> BDS is allowed to grow, are there any users which do not use BB, but
> should still be disallowed from reading/writing beyond a BDS's limits?
> The only users I could see were the block jobs. Some of them should
> indeed be converted to BB; but none of them takes a user-supplied offset
> or size, all work on the full BDS (or only on parts which have been
> modified, etc.). Therefore, it is by design impossible for them to
> exceed the BDS's limits, which makes making all BDS's growable safe.
> 
> 
> v3:
> - Rebased (onto Stefan's branch rebased onto master)
> - Fixed patch 4 [Stefano]

Thanks! It builds correctly and seems to work OK.


> v2:
> - Rebased [Kevin]
> - Patch 2: Added a TODO comment about removing @filename and @flags from
>   blk_new_open() when possible [Kevin]
> 
> 
> git-backport-diff against v2:
> 
> 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:[----] [--] 'block: Lift some BDS functions to the BlockBackend'
> 002/14:[----] [--] 'block: Add blk_new_open()'
> 003/14:[----] [--] 'blockdev: Use blk_new_open() in blockdev_init()'
> 004/14:[0004] [FC] 'block/xen: Use blk_new_open() in blk_connect()'
> 005/14:[----] [--] 'qemu-img: Use blk_new_open() in img_open()'
> 006/14:[----] [-C] 'qemu-img: Use blk_new_open() in img_rebase()'
> 007/14:[----] [-C] 'qemu-img: Use BlockBackend as far as possible'
> 008/14:[----] [--] 'qemu-nbd: Use blk_new_open() in main()'
> 009/14:[----] [--] 'qemu-io: Use blk_new_open() in openfile()'
> 010/14:[----] [--] 'qemu-io: Remove "growable" option'
> 011/14:[----] [--] 'qemu-io: Use BlockBackend'
> 012/14:[----] [--] 'block: Clamp BlockBackend requests'
> 013/14:[----] [--] 'block: Remove "growable" from BDS'
> 014/14:[----] [--] 'block: Keep bdrv_check*_request()'s return value'
> 
> 
> git-backport-diff against v1:
> 
> 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:[----] [--] 'block: Lift some BDS functions to the BlockBackend'
> 002/14:[0006] [FC] 'block: Add blk_new_open()'
> 003/14:[----] [-C] 'blockdev: Use blk_new_open() in blockdev_init()'
> 004/14:[0004] [FC] 'block/xen: Use blk_new_open() in blk_connect()'
> 005/14:[----] [--] 'qemu-img: Use blk_new_open() in img_open()'
> 006/14:[----] [-C] 'qemu-img: Use blk_new_open() in img_rebase()'
> 007/14:[----] [-C] 'qemu-img: Use BlockBackend as far as possible'
> 008/14:[----] [--] 'qemu-nbd: Use blk_new_open() in main()'
> 009/14:[----] [--] 'qemu-io: Use blk_new_open() in openfile()'
> 010/14:[0002] [FC] 'qemu-io: Remove "growable" option'
> 011/14:[----] [--] 'qemu-io: Use BlockBackend'
> 012/14:[----] [--] 'block: Clamp BlockBackend requests'
> 013/14:[----] [--] 'block: Remove "growable" from BDS'
> 014/14:[----] [--] 'block: Keep bdrv_check*_request()'s return value'
> 
> 
> Max Reitz (14):
>   block: Lift some BDS functions to the BlockBackend
>   block: Add blk_new_open()
>   blockdev: Use blk_new_open() in blockdev_init()
>   block/xen: Use blk_new_open() in blk_connect()
>   qemu-img: Use blk_new_open() in img_open()
>   qemu-img: Use blk_new_open() in img_rebase()
>   qemu-img: Use BlockBackend as far as possible
>   qemu-nbd: Use blk_new_open() in main()
>   qemu-io: Use blk_new_open() in openfile()
>   qemu-io: Remove "growable" option
>   qemu-io: Use BlockBackend
>   block: Clamp BlockBackend requests
>   block: Remove "growable" from BDS
>   block: Keep bdrv_check*_request()'s return value
> 
>  block.c                        |  59 +++++-----
>  block/block-backend.c          | 219 +++++++++++++++++++++++++++++++++++++
>  block/qcow2.c                  |   6 --
>  block/raw-posix.c              |   2 +-
>  block/raw-win32.c              |   2 +-
>  block/sheepdog.c               |   2 +-
>  blockdev.c                     |  92 ++++++++--------
>  hmp.c                          |   9 +-
>  hw/block/xen_disk.c            |  28 +++--
>  include/block/block_int.h      |   3 -
>  include/qemu-io.h              |   4 +-
>  include/sysemu/block-backend.h |  12 +++
>  qemu-img.c                     | 171 ++++++++++++++---------------
>  qemu-io-cmds.c                 | 238 
> ++++++++++++++++++++++-------------------
>  qemu-io.c                      |  58 ++++------
>  qemu-nbd.c                     |  25 ++---
>  tests/qemu-iotests/016         |  73 -------------
>  tests/qemu-iotests/016.out     |  23 ----
>  tests/qemu-iotests/051.out     |  60 +++++------
>  tests/qemu-iotests/087.out     |   8 +-
>  tests/qemu-iotests/group       |   1 -
>  21 files changed, 592 insertions(+), 503 deletions(-)
>  delete mode 100755 tests/qemu-iotests/016
>  delete mode 100644 tests/qemu-iotests/016.out
> 
> -- 
> 2.1.0
>