Re: [Qemu-devel] [PATCHv2 18/20] qemu-img: add support for fully allocated images

[Peter Lieven]

Am 19.09.2013 um 22:36 schrieb Eric Blake <address@hidden>:

> On 09/17/2013 07:48 AM, Peter Lieven wrote:
>> Signed-off-by: Peter Lieven <address@hidden>
>> ---
>> qemu-img.c |    8 +++++---
>> 1 file changed, 5 insertions(+), 3 deletions(-)
>> 
>> diff --git a/qemu-img.c b/qemu-img.c
>> index 3e5e388..7600b58 100644
>> --- a/qemu-img.c
>> +++ b/qemu-img.c
>> @@ -100,8 +100,10 @@ static void help(void)
>>            "  '-h' with or without a command shows this help and lists the 
>> supported formats\n"
>>            "  '-p' show progress of command (only certain commands)\n"
>>            "  '-q' use Quiet mode - do not print any output (except 
>> errors)\n"
>> -           "  '-S' indicates the consecutive number of bytes that must 
>> contain only zeros\n"
>> -           "       for qemu-img to create a sparse image during 
>> conversion\n"
>> +           "  '-S' indicates the consecutive number of bytes (defaults to 
>> 4096) that must\n"
>> +           "       contain only zeros for qemu-img to create a sparse image 
>> during\n"
>> +           "       conversion. if the number of bytes is 0 sparse files are 
>> disabled and\n"
> 
> s/if/If/
> s/0/0,/
> 
>> +           "       images will always be fully allocated\n"
> 
> IIRC correctly, on NTFS file system, the minimum hole size for a sparse
> file is 64k.  Can you please make this option accept a suffixed number
> (-S 64k) instead of forcing me to spell it out (-S 65536)?

I can do that, of course. But the option is old it was not introduced by me ;-)
I only added the handling for -S 0 ;-)



> 
>>            "  '--output' takes the format in which the output must be done 
>> (human or json)\n"
>>            "  '-n' skips the target volume creation (useful if the volume is 
>> created\n"
>>            "       prior to running qemu-img)\n"
>> @@ -1468,7 +1470,7 @@ static int img_convert(int argc, char **argv)
>>         /* signal EOF to align */
>>         bdrv_write_compressed(out_bs, 0, NULL, 0);
>>     } else {
>> -        int has_zero_init = bdrv_has_zero_init(out_bs);
>> +        int has_zero_init = min_sparse ? bdrv_has_zero_init(out_bs) : 0;
> 
> Is this another function that should be returning bool instead of int?
> But that's a question for a separate patch.

If the answer is yes, bdrv_has_discard_zeroes and bdrv_has_discard_write_zeroes
should return bool as well.

Peter