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

[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 <address@hidden>
> ---

> @@ -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

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature