Re: [Qemu-devel] [PATCH V7 4/6] qemu-img: add -l for snapshot in convert

[Wenchao Xia <xiawenc@linux.vnet.ibm.com>]

于 2013/12/5 4:42, Eric Blake 写道:
> On 12/04/2013 02:10 AM, Wenchao Xia wrote:
>> Now qemu-img convert have similar options as qemu-nbd for internal
>> snapshot.
>>
>> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>> ---
>
>> @@ -1183,6 +1189,18 @@ static int img_convert(int argc, char **argv)
>>           case 's':
>>               snapshot_name = optarg;
>>               break;
>> +        case 'l':
>> +            if (strstart(optarg, SNAPSHOT_OPT_BASE, NULL)) {
>> +                sn_opts = qemu_opts_parse(&internal_snapshot_opts, optarg, 0);
>> +                if (!sn_opts) {
>> +                    error_report("Failed in parsing snapshot param '%s'",
>> +                                 optarg);
>> +                    return 1;
>> +                }
>> +            } else {
>> +                snapshot_name = optarg;
>> +            }
>> +            break;
>
> Do we want a followup patch that makes it an error to use -l and -s
> together?  Without such a patch, we have the odd behavior that:
>
> convert -l name1 -s name2
>
> loads name2, but:
>
> convert -l snapshot.name=name1 -s name2
>
> loads name1.  Confusing that the choice of HOW the argument to -l is
> specified determines whether the -s has any impact.
>
> For that matter, why can't '-s' and '-l' be made synonyms of each other?
>   In other words, why not support:
>
> convert -s snapshot.name=name1
>
   Previous I planned to use -l for internal snapshot in all possible
program, since -s is taken as external snapshot in qemu, qemu-nbd.
let -s stands for internal in qemu-img convert only, may bring
confuse to user, so I deprecated it instead of enhance it(I want
to remove it but may bring compatiablity issue).
   Yes, it should report error when both specified, will send a patch
if you agree '-l' should still be used.