qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2 2/4] Curling: cmdline interface.


From: Eric Blake
Subject: Re: [Qemu-devel] [PATCH v2 2/4] Curling: cmdline interface.
Date: Mon, 30 Sep 2013 16:16:34 -0600
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130923 Thunderbird/17.0.9

On 09/29/2013 02:14 PM, Jules Wang wrote:
> Add an option '-f' to migration cmdline.
> Indicating whether to enable fault tolerant or not.
> 
> Signed-off-by: Jules Wang <address@hidden>
> ---
>          .help       = "migrate to URI (using -d to not wait for completion)"
>                     "\n\t\t\t -b for migration without shared storage with"
>                     " full copy of disk\n\t\t\t -i for migration without "
>                     "shared storage with incremental copy of disk "
> -                   "(base image shared between src and destination)",
> +                   "(base image shared between src and destination)"
> +                   "\n\t\t\t -f for fault tolerant, this is another "
> +                   "feature rather than migrate",

That sounds awkward, and overly long.  Maybe go with just:

-f for fault tolerance mode

and let the user then read the full documentation for what it entails.

> address@hidden migrate [-d] [-b] [-i] @var{uri}
> address@hidden migrate [-d] [-b] [-i] [-f] @var{uri}
>  @findex migrate
>  Migrate to @var{uri} (using -d to not wait for completion).
>       -b for migration with full copy of disk
>       -i for migration with incremental copy of disk (base image is shared)
> +     -f for fault tolerant

Can -d and -f be used at the same time, or are they exclusive?

> +++ b/hmp.c
> @@ -1213,10 +1213,11 @@ void hmp_migrate(Monitor *mon, const QDict *qdict)
>      int detach = qdict_get_try_bool(qdict, "detach", 0);
>      int blk = qdict_get_try_bool(qdict, "blk", 0);
>      int inc = qdict_get_try_bool(qdict, "inc", 0);
> +    int ft  = qdict_get_try_bool(qdict, "ft", 0);

Why two spaces?

> +++ b/qapi-schema.json
> @@ -2420,7 +2420,8 @@
>  # Since: 0.14.0
>  ##
>  { 'command': 'migrate',
> -  'data': {'uri': 'str', '*blk': 'bool', '*inc': 'bool', '*detach': 'bool' } 
> }
> +  'data': {'uri': 'str', '*blk': 'bool', '*inc': 'bool', '*detach': 'bool',
> +           '*ft': 'bool' } }

Missing documentation, including mention that the new option was only
made available in 1.7.  We still don't have introspection; is there some
other means by which libvirt and other management apps can tell whether
this feature is available?  Furthermore, 'ft' is an awfully short name;
for QMP, we prefer to use full words where possible, such as
'fault-tolerant'.

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

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]