[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 00/16] block: Remove "growable", add blk_new_
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH v4 00/16] block: Remove "growable", add blk_new_open() |
Date: |
Fri, 13 Feb 2015 16:47:10 +0000 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
On Thu, Feb 05, 2015 at 01:58:09PM -0500, 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.
>
>
> v4:
> - Patch 1: Added blk_nb_sectors() [Eric]
> - Patch 3: Added in order to get a more useful error message than just
> "Unknown protocol" [Eric]
> - Patch 4: Added [Eric]
> - Patch 5:
> - Use qdict_put() instead of qdict_put_obj() [Kevin]
> - Rebased onto the new patches 3 and 4
> - Patch 6:
> - Note the fix for invalid drivers in the commit message; and note
> that the fix is not complete [Kevin]
> - Use qdict_put() instead of qdict_put_obj() [Kevin]
> - Patch 7: Use qdict_put() instead of qdict_put_obj() [Kevin]
> - Patch 8:
> - Do not skip output format validity test on unsafe [Kevin]
> - Dropped superfluous !unsafe in condition [Eric]
> - Use qdict_put() instead of qdict_put_obj() [Kevin]
> - Patch 9: Use blk_nb_sectors() to replace bdrv_nb_sectors() (instead of
> blk_getlength() with a later division by BDRV_SECTOR_SIZE) [Eric]
> - Patch 10: Use qdict_put() instead of qdict_put_obj() [Kevin]
> - Patch 11: Dropped the hunk which removed the reset of qemuio_bs to
> NULL in close_f() [Kevin]
> - Patch 12: Added a comment to the group file stating that 016 should
> not be reused [Eric/Kevin]
> - Patch 13:
> - Use blk_nb_sectors() to replace bdrv_nb-sectors() [Eric]
> - Drop qemuio_bs [Kevin]
> - Patch 15: Kept the "if (offset < 0)" check in
> bdrv_check_byte_request() [Kevin]
>
> v3:
> - Rebased (onto Stefan's branch rebased onto master)
> - Fixed patch 4 [Stefano]
>
> 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 v3:
>
> 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/16:[0006] [FC] 'block: Lift some BDS functions to the BlockBackend'
> 002/16:[----] [--] 'block: Add blk_new_open()'
> 003/16:[down] 'block: Add Error parameter to bdrv_find_protocol()'
> 004/16:[down] 'iotests: Add test for driver=qcow2,format=qcow2'
> 005/16:[0012] [FC] 'blockdev: Use blk_new_open() in blockdev_init()'
> 006/16:[0003] [FC] 'block/xen: Use blk_new_open() in blk_connect()'
> 007/16:[0002] [FC] 'qemu-img: Use blk_new_open() in img_open()'
> 008/16:[0016] [FC] 'qemu-img: Use blk_new_open() in img_rebase()'
> 009/16:[0030] [FC] 'qemu-img: Use BlockBackend as far as possible'
> 010/16:[0002] [FC] 'qemu-nbd: Use blk_new_open() in main()'
> 011/16:[0003] [FC] 'qemu-io: Use blk_new_open() in openfile()'
> 012/16:[0001] [FC] 'qemu-io: Remove "growable" option'
> 013/16:[0006] [FC] 'qemu-io: Use BlockBackend'
> 014/16:[----] [--] 'block: Clamp BlockBackend requests'
> 015/16:[0004] [FC] 'block: Remove "growable" from BDS'
> 016/16:[----] [--] 'block: Keep bdrv_check*_request()'s return value'
>
>
>
> Max Reitz (16):
> block: Lift some BDS functions to the BlockBackend
> block: Add blk_new_open()
> block: Add Error parameter to bdrv_find_protocol()
> iotests: Add test for driver=qcow2,format=qcow2
> 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 | 73 +++++++------
> block/block-backend.c | 224 ++++++++++++++++++++++++++++++++++++++
> block/qcow2.c | 6 --
> block/raw-posix.c | 2 +-
> block/raw-win32.c | 2 +-
> block/sheepdog.c | 4 +-
> blockdev.c | 92 ++++++++--------
> hmp.c | 9 +-
> hw/block/xen_disk.c | 27 ++---
> include/block/block.h | 3 +-
> include/block/block_int.h | 3 -
> include/qemu-io.h | 4 +-
> include/sysemu/block-backend.h | 13 +++
> qemu-img.c | 158 +++++++++++++--------------
> qemu-io-cmds.c | 237
> +++++++++++++++++++++--------------------
> qemu-io.c | 58 +++-------
> qemu-nbd.c | 25 ++---
> tests/qemu-iotests/016 | 73 -------------
> tests/qemu-iotests/016.out | 23 ----
> tests/qemu-iotests/051 | 1 +
> tests/qemu-iotests/051.out | 63 +++++------
> tests/qemu-iotests/087.out | 8 +-
> tests/qemu-iotests/group | 2 +-
> 23 files changed, 603 insertions(+), 507 deletions(-)
> delete mode 100755 tests/qemu-iotests/016
> delete mode 100644 tests/qemu-iotests/016.out
>
> --
> 2.1.0
>
>
Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block
Stefan
pgpGGFtCJ3cpv.pgp
Description: PGP signature
- [Qemu-devel] [PATCH v4 11/16] qemu-io: Use blk_new_open() in openfile(), (continued)
- [Qemu-devel] [PATCH v4 11/16] qemu-io: Use blk_new_open() in openfile(), Max Reitz, 2015/02/05
- [Qemu-devel] [PATCH v4 13/16] qemu-io: Use BlockBackend, Max Reitz, 2015/02/05
- [Qemu-devel] [PATCH v4 16/16] block: Keep bdrv_check*_request()'s return value, Max Reitz, 2015/02/05
- [Qemu-devel] [PATCH v4 15/16] block: Remove "growable" from BDS, Max Reitz, 2015/02/05
- Re: [Qemu-devel] [PATCH v4 00/16] block: Remove "growable", add blk_new_open(),
Stefan Hajnoczi <=