From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60841) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vgm4L-0007ZH-8U for qemu-devel@nongnu.org; Wed, 13 Nov 2013 20:50:11 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Vgm4F-0004nW-8Y for qemu-devel@nongnu.org; Wed, 13 Nov 2013 20:50:05 -0500 Received: from mx1.redhat.com ([209.132.183.28]:1064) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vgm4F-0004nQ-07 for qemu-devel@nongnu.org; Wed, 13 Nov 2013 20:49:59 -0500 Message-ID: <52842C40.7000106@redhat.com> Date: Thu, 14 Nov 2013 09:49:52 +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> <528424EB.90809@redhat.com> <20131114013413.GA4089@localhost.localdomain> In-Reply-To: <20131114013413.GA4089@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=8814=E6=97=A5 09:34, Jeff Cody wrote: > 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 possibl= e) */ >>>> 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 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. > We care for -ENOTSUP as well as other err code in the compressed case,=20 and don't care the call to bdrv_get_info in the other case. I get your=20 point, I'll add "else if (ret !=3D -ENOTSUP) {...}" below the call, and=20 keep the check of ret in "if (compressed)" unchanged. Fam