From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49389) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vgla4-0005Hq-AH for qemu-devel@nongnu.org; Wed, 13 Nov 2013 20:18:54 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VglZy-0002Dv-BL for qemu-devel@nongnu.org; Wed, 13 Nov 2013 20:18:48 -0500 Received: from mx1.redhat.com ([209.132.183.28]:5470) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VglZy-0002Df-3J for qemu-devel@nongnu.org; Wed, 13 Nov 2013 20:18:42 -0500 Message-ID: <528424EB.90809@redhat.com> Date: Thu, 14 Nov 2013 09:18:35 +0800 From: Fam Zheng MIME-Version: 1.0 References: <1384303994-26796-1-git-send-email-famz@redhat.com> <1384303994-26796-2-git-send-email-famz@redhat.com> <20131113152314.GA22807@localhost.localdomain> In-Reply-To: <20131113152314.GA22807@localhost.localdomain> Content-Type: text/plain; charset=UTF-8; format=flowed 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: Jeff Cody Cc: kwolf@redhat.com, gentoo.integer@gmail.com, qemu-devel@nongnu.org, stefanha@redhat.com 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)' > Moving it up forces bdrv_get_info to succeed for both compress case and=20 non compress case, which is too strict and fails when target driver=20 doesn't support it. I see it's just odd to assign ret outside if block and check it in only=20 one branch, but I don't have a better way to do this here. Fam