From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47312) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YFkE6-0005Fj-0e for qemu-devel@nongnu.org; Mon, 26 Jan 2015 09:01:15 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YFkE2-0004zx-QF for qemu-devel@nongnu.org; Mon, 26 Jan 2015 09:01:13 -0500 Received: from mx1.redhat.com ([209.132.183.28]:50911) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YFkE2-0004zc-Iv for qemu-devel@nongnu.org; Mon, 26 Jan 2015 09:01:10 -0500 Message-ID: <54C6489E.5050709@redhat.com> Date: Mon, 26 Jan 2015 09:01:02 -0500 From: Max Reitz MIME-Version: 1.0 References: <1421957411-31759-1-git-send-email-mreitz@redhat.com> In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v2 00/14] block: Remove "growable", add blk_new_open() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefano Stabellini Cc: Kevin Wolf , qemu-devel@nongnu.org, Stefan Hajnoczi , Markus Armbruster On 2015-01-26 at 07:18, Stefano Stabellini wrote: > On Thu, 22 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 (i= n >> 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 offs= et >> 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. > Hello Max, > I applied the first four patches in this series, and this is what I get= : > > hw/block/xen_disk.c: In function =E2=80=98blk_connect=E2=80=99: > hw/block/xen_disk.c:907:27: error: implicit declaration of function =E2= =80=98qstring_from_str=E2=80=99 [-Werror=3Dimplicit-function-declaration] > hw/block/xen_disk.c:907:27: error: nested extern declaration of =E2=80=98= qstring_from_str=E2=80=99 [-Werror=3Dnested-externs] > hw/block/xen_disk.c:907:27: error: invalid type argument of =E2=80=98->= =E2=80=99 (have =E2=80=98int=E2=80=99) > hw/block/xen_disk.c:901:22: error: unused variable =E2=80=98drv=E2=80=99= [-Werror=3Dunused-variable] > hw/block/xen_disk.c:900:23: error: unused variable =E2=80=98blk=E2=80=99= [-Werror=3Dunused-variable] > > it would be great if you could build test it. Sorry, of course I build test my patches, but I just don't have some=20 subsystems enabled (there was no real reason not to have the Xen=20 backends enabled, but that's just how it was; another example are=20 Windows-specific areas), so I have to work blind on them. In this case I=20 forgot to include the QMP type headers and to remove the now unused=20 variables. Thank you for looking at my patches and finding this issue! I'll send a=20 fixed v3. Max >> v2: >> - Rebased [Kevin] >> - Patch 2: Added a TODO comment about removing @filename and @flags fr= om >> blk_new_open() when possible [Kevin] >> >> >> 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, res= pectively >> >> 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:[----] [--] 'block/xen: Use blk_new_open() in blk_connect()' >> 005/14:[----] [--] 'qemu-img: Use blk_new_open() in img_open()' >> 006/14:[----] [--] 'qemu-img: Use blk_new_open() in img_rebase()' >> 007/14:[----] [--] '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 | 24 ++--- >> 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, 590 insertions(+), 501 deletions(-) >> delete mode 100755 tests/qemu-iotests/016 >> delete mode 100644 tests/qemu-iotests/016.out >> >> --=20 >> 2.1.0 >>