From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58288) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vglp9-0003xX-JJ for qemu-devel@nongnu.org; Wed, 13 Nov 2013 20:34:29 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Vglp3-0000XH-FM for qemu-devel@nongnu.org; Wed, 13 Nov 2013 20:34:23 -0500 Received: from mx1.redhat.com ([209.132.183.28]:18021) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vglp3-0000W5-5S for qemu-devel@nongnu.org; Wed, 13 Nov 2013 20:34:17 -0500 Date: Wed, 13 Nov 2013 20:34:13 -0500 From: Jeff Cody Message-ID: <20131114013413.GA4089@localhost.localdomain> References: <1384303994-26796-1-git-send-email-famz@redhat.com> <1384303994-26796-2-git-send-email-famz@redhat.com> <20131113152314.GA22807@localhost.localdomain> <528424EB.90809@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <528424EB.90809@redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 1/5] qemu-img: Convert by cluster size if target is compressed List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng Cc: kwolf@redhat.com, gentoo.integer@gmail.com, qemu-devel@nongnu.org, stefanha@redhat.com On Thu, Nov 14, 2013 at 09:18:35AM +0800, Fam Zheng wrote: > On 2013=E5=B9=B411=E6=9C=8813=E6=97=A5 23:23, Jeff Cody wrote: > >On Wed, Nov 13, 2013 at 08:53:10AM +0800, Fam Zheng wrote: > >>If target block driver forces compression, qemu-img convert needs to > >>write by cluster size as well as "-c" option. > >> > >>Particularly, this applies for converting to VMDK streamOptimized > >>format. > >> > >>Signed-off-by: Fam Zheng > >>--- > >> include/block/block.h | 1 + > >> qemu-img.c | 5 ++++- > >> 2 files changed, 5 insertions(+), 1 deletion(-) > >> > >>diff --git a/include/block/block.h b/include/block/block.h > >>index 3560deb..169c092 100644 > >>--- a/include/block/block.h > >>+++ b/include/block/block.h > >>@@ -18,6 +18,7 @@ typedef struct BlockDriverInfo { > >> /* offset at which the VM state can be saved (0 if not possible= ) */ > >> int64_t vm_state_offset; > >> bool is_dirty; > >>+ bool is_compressed; > >> } BlockDriverInfo; > >> > >> typedef struct BlockFragInfo { > >>diff --git a/qemu-img.c b/qemu-img.c > >>index bf3fb4f..09ed9b2 100644 > >>--- a/qemu-img.c > >>+++ b/qemu-img.c > >>@@ -1383,8 +1383,11 @@ static int img_convert(int argc, char **argv) > >> } > >> } > >> > >>+ ret =3D bdrv_get_info(out_bs, &bdi); > >>+ if (ret =3D=3D 0) { > >>+ compress =3D compress || bdi.is_compressed; > >>+ } > >> if (compress) { > >>- ret =3D bdrv_get_info(out_bs, &bdi); > >> if (ret < 0) { > >> error_report("could not get block driver info"); > >> goto out; > > > >You need to move the error checking up as well, above the > >'if (compress)' > > >=20 > Moving it up forces bdrv_get_info to succeed for both compress case > and non compress case, which is too strict and fails when target > driver doesn't support it. bdrv_get_info() will return -ENOTSUP if it is not supported. If we do call down to the driver layer bdrv_get_info() and get an error, I think we should pass it up. So just ignore -ENOTSUP if we don't care about the case when bdrv_get_info() is not supported by a target. >=20 > I see it's just odd to assign ret outside if block and check it in > only one branch, but I don't have a better way to do this here. >=20 > Fam >=20