Re: [PATCH] qemu-img: Add -F shorthand to convert

[Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>]

17.09.2021 23:13, Eric Blake wrote:
> On Mon, Sep 13, 2021 at 08:17:35AM -0500, Eric Blake wrote:
>> Although we have long supported 'qemu-img convert -o
>> backing_file=foo,backing_fmt=bar', the fact that we have a shortcut -B
>> for backing_file but none for backing_fmt has made it more likely that
>> users accidentally run into:
>>
>> qemu-img: warning: Deprecated use of backing file without explicit backing format
>>
>> when using -B instead of -o.  For similarity with other qemu-img
>> commands, such as create and compare, add '-F $fmt' as the shorthand
>> for '-o backing_fmt=$fmt'.  Update iotest 122 for coverage of both
>> spellings.
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>> ---
>>
> 
> Self-review (and late, too), but...
> 
>> +++ b/docs/tools/qemu-img.rst
>> @@ -414,7 +414,7 @@ Command description:
>>     4
>>       Error on reading data
>>
>> -.. option:: convert [--object OBJECTDEF] [--image-opts] [--target-image-opts] [--target-is-zero] [--bitmaps [--skip-broken-bitmaps]] [-U] [-C] [-c] [-p] [-q] [-n] [-f FMT] [-t CACHE] [-T SRC_CACHE] [-O OUTPUT_FMT] [-B BACKING_FILE] [-o OPTIONS] [-l SNAPSHOT_PARAM] [-S SPARSE_SIZE] [-r RATE_LIMIT] [-m NUM_COROUTINES] [-W] FILENAME [FILENAME2 [...]] OUTPUT_FILENAME
>> +.. option:: convert [--object OBJECTDEF] [--image-opts] [--target-image-opts] [--target-is-zero] [--bitmaps [--skip-broken-bitmaps]] [-U] [-C] [-c] [-p] [-q] [-n] [-f FMT] [-t CACHE] [-T SRC_CACHE] [-O OUTPUT_FMT] [-B BACKING_FILE [-F backing_fmt]] [-o OPTIONS] [-l SNAPSHOT_PARAM] [-S SPARSE_SIZE] [-r RATE_LIMIT] [-m NUM_COROUTINES] [-W] FILENAME [FILENAME2 [...]] OUTPUT_FILENAME
> 
> s/backing_fmt/BACKING_FMT/ would be more consistent here
> 
>> +++ b/qemu-img-cmds.hx
>> @@ -46,7 +46,7 @@ SRST
>>   ERST
>>
>>   DEF("convert", img_convert,
>> -    "convert [--object objectdef] [--image-opts] [--target-image-opts] [--target-is-zero] [--bitmaps] [-U] [-C] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-B backing_file] [-o options] [-l snapshot_param] [-S sparse_size] [-r rate_limit] [-m num_coroutines] [-W] [--salvage] filename [filename2 [...]] output_filename")
>> +    "convert [--object objectdef] [--image-opts] [--target-image-opts] [--target-is-zero] [--bitmaps] [-U] [-C] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-B backing_file [-F backing_fmt]] [-o options] [-l snapshot_param] [-S sparse_size] [-r rate_limit] [-m num_coroutines] [-W] [--salvage] filename [filename2 [...]] output_filename")
>>   SRST
>>   .. option:: convert [--object OBJECTDEF] [--image-opts] [--target-image-opts] [--target-is-zero] [--bitmaps] [-U] [-C] [-c] [-p] [-q] [-n] [-f FMT] [-t CACHE] [-T SRC_CACHE] [-O OUTPUT_FMT] [-B BACKING_FILE] [-o OPTIONS] [-l SNAPSHOT_PARAM] [-S SPARSE_SIZE] [-r RATE_LIMIT] [-m NUM_COROUTINES] [-W] [--salvage] FILENAME [FILENAME2 [...]] OUTPUT_FILENAME
> 
> and I missed this line here. We have too much not-quite-identical
> duplication :( Followup patch coming.
> 

That's for sure accidental complexity :)

-- 
Best regards,
Vladimir