From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56148) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aGCHG-0002pe-Vp for qemu-devel@nongnu.org; Mon, 04 Jan 2016 16:02:56 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aGCHD-0005ui-PT for qemu-devel@nongnu.org; Mon, 04 Jan 2016 16:02:54 -0500 References: <1450936225-4249-1-git-send-email-famz@redhat.com> From: Max Reitz Message-ID: <568ADDEC.2030501@redhat.com> Date: Mon, 4 Jan 2016 22:02:36 +0100 MIME-Version: 1.0 In-Reply-To: <1450936225-4249-1-git-send-email-famz@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="9iQAuJSAdgwJ6UDGkhofPvVWjJbbgOb3t" Subject: Re: [Qemu-devel] [PATCH v4 00/14] qemu-img map: Allow driver to return file of the allocated block List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng , qemu-devel@nongnu.org Cc: Kevin Wolf , pbonzini@redhat.com, Stefan Hajnoczi , qemu-block@nongnu.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --9iQAuJSAdgwJ6UDGkhofPvVWjJbbgOb3t Content-Type: text/plain; charset=iso-8859-15 Content-Transfer-Encoding: quoted-printable On 24.12.2015 06:50, Fam Zheng wrote: > v4: Rebase and resend, adding Eric's and Stefan's reviewed-by. >=20 > Fix one typo in patch 13. >=20 > Drop previous patch 14 for a later rework because it is not a hard > requirement, but it is pending on Eric's QAPI-to-JSON visitor serie= s: >=20 > https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg03929.html= >=20 > v3: Add Eric's rev-by in patches 6, 7, 13, 14. > 12: New, split out from the previous 13. > 12->13: Refactor "entry_mergable" from imp_map(). > Don't mess the merge conditions. [Paolo] > Address Eric's comments: > - Check has_foo before using foo. > - Remove blank line between comments and definition in schema. > - Use PRId64 instead of %ld. > - Merge short lines. >=20 > v2: Add Eric's rev-by in patches 2, 4, 5. > 01: Refering -> referring in commit message. [Eric] > Recurse to "file" for sensible "zero" flag. [Paolo] > 12: New. Make MapEntry a QAPI struct. [Paolo, Markus] >=20 > Original cover letter > --------------------- >=20 > I stumbled upon this when looking at external bitmap formats. >=20 > Current "qemu-img map" command only displays filename if the data is al= located > in bs (bs->file) itself, or in the backing chain. Otherwise, it display= s an > unfriendly error message: >=20 > $ qemu-img create -f vmdk -o subformat=3DmonolithicFlat /tmp/test.v= mdk 1G >=20 > $ qemu-img map /tmp/test.vmdk > Offset Length Mapped to File > qemu-img: File contains external, encrypted or compressed clusters.= >=20 > This can be improved. This series extends the .bdrv_co_get_block_status= > callback, to let block driver return the BDS of file; then updates all = driver > to implement it; and lastly, it changes qemu-img to use this informatio= n in > "map" command: >=20 >=20 > $ qemu-img map /tmp/test.vmdk > Offset Length Mapped to File > 0 0x40000000 0 /tmp/test-flat.vmdk= >=20 > $ qemu-img map --output json /tmp/test.vmdk > [{"length": 1073741824, "start": 0, "zero": false, "offset": 0, "de= pth": 0, > "file": "/tmp/test-flat.vmdk", "data": true} > ] >=20 > Fam Zheng (14): > block: Add "file" output parameter to block status query functions > qcow: Assign bs->file->bs to file in qcow_co_get_block_status > qcow2: Assign bs->file->bs to file in qcow2_co_get_block_status Minor comment: I'd swap these two patches (2 and 3). Patch 1 breaks test 102, patch 3 fixes it again. It would be better to break it for as short a time as possible. Alternatively, in order not to break 102 at all, patch 1 would need to leave the "if (bs->file &&" part of bdrv_co_get_block_status() (@@ -1544,13 +1550,14 @@) as-is and change it only after the format block drivers set *file. Max > raw: Assign bs to file in raw_co_get_block_status > iscsi: Assign bs to file in iscsi_co_get_block_status > parallels: Assign bs->file->bs to file in > parallels_co_get_block_status > qed: Assign bs->file->bs to file in bdrv_qed_co_get_block_status > sheepdog: Assign bs to file in sd_co_get_block_status > vdi: Assign bs->file->bs to file in vdi_co_get_block_status > vpc: Assign bs->file->bs to file in vpc_co_get_block_status > vmdk: Return extent's file in bdrv_get_block_status > qemu-img: In "map", use the returned "file" from bdrv_get_block_statu= s > qemu-img: Make MapEntry a QAPI struct > iotests: Add "qemu-img map" test for VMDK extents >=20 > block/io.c | 42 ++++++++++++++++--------- > block/iscsi.c | 9 ++++-- > block/mirror.c | 3 +- > block/parallels.c | 3 +- > block/qcow.c | 3 +- > block/qcow2.c | 3 +- > block/qed.c | 6 +++- > block/raw-posix.c | 4 ++- > block/raw_bsd.c | 4 ++- > block/sheepdog.c | 5 ++- > block/vdi.c | 3 +- > block/vmdk.c | 13 ++++---- > block/vpc.c | 4 ++- > block/vvfat.c | 2 +- > include/block/block.h | 6 ++-- > include/block/block_int.h | 3 +- > qapi/block-core.json | 27 ++++++++++++++++ > qemu-img.c | 78 ++++++++++++++++++++++++++++----------= -------- > tests/qemu-iotests/059 | 10 ++++++ > tests/qemu-iotests/059.out | 38 ++++++++++++++++++++++ > 20 files changed, 198 insertions(+), 68 deletions(-) >=20 --9iQAuJSAdgwJ6UDGkhofPvVWjJbbgOb3t Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEcBAEBCAAGBQJWit3tAAoJEDuxQgLoOKytMPoH/32WPUddK6JxbE8u4z366/GR efm9IGxBYFkzY0j1X0uB1TS48X5mCmZEslbQz0BVGO0k54ROfMB2YadRW6aGiUCP /AfaV1flsYQ+757rESsrW6ottsHBdkQwranX9EKsZXnpdnZoh3eqUwepKH+qa9c1 GRYHj/NEhcUGgxYuGsMSSzDQ7sov3AlF3p4TBJnKs5y8DTv9t4b89TurAc8F8SAc ZL4naOVjB2Kv5RxSQ4WIz7ondTDcY338Y81/rYvui+v26aPlVuQC2VkELhUk9gRt elDVW0ywSdo8vXz9odUZAnnjUhoQqIzOBZgIp6jUGJeapki2Qxsg5VYqg6aY1so= =e70o -----END PGP SIGNATURE----- --9iQAuJSAdgwJ6UDGkhofPvVWjJbbgOb3t--