From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49523) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YFlv5-0007Bx-GV for qemu-devel@nongnu.org; Mon, 26 Jan 2015 10:49:44 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YFlv2-0000Sq-A8 for qemu-devel@nongnu.org; Mon, 26 Jan 2015 10:49:43 -0500 Received: from smtp02.citrix.com ([66.165.176.63]:57108) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YFlv2-0000Si-2M for qemu-devel@nongnu.org; Mon, 26 Jan 2015 10:49:40 -0500 Date: Mon, 26 Jan 2015 15:49:15 +0000 From: Stefano Stabellini In-Reply-To: <1422284444-12529-1-git-send-email-mreitz@redhat.com> Message-ID: References: <1422284444-12529-1-git-send-email-mreitz@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Subject: Re: [Qemu-devel] [PATCH v3 00/14] block: Remove "growable", add blk_new_open() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz Cc: Kevin Wolf , Stefano Stabellini , qemu-devel@nongnu.org, Stefan Hajnoczi , Markus Armbruster 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 >