Re: [Qemu-devel] [PATCH 1/5] qemu-img: Convert by cluster size if target is compressed

[Fam Zheng <famz@redhat.com>]

On 2013年11月13日 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 <famz@redhat.com>
>> ---
>>   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 = bdrv_get_info(out_bs, &bdi);
>> +    if (ret == 0) {
>> +        compress = compress || bdi.is_compressed;
>> +    }
>>       if (compress) {
>> -        ret = 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.

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.

Fam