[prev in list] [next in list] [prev in thread] [next in thread] 

List:       qemu-devel
Subject:    Re: [Qemu-devel] [PATCH v2 2/4] Curling: cmdline interface.
From:       Eric Blake <eblake () redhat ! com>
Date:       2013-09-30 22:16:34
Message-ID: 5249F842.8050405 () redhat ! com
[Download RAW message or body]


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 <junqing.wang@cs2c.com.cn>
> ---
>          .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.

> -@item migrate [-d] [-b] [-i] @var{uri}
> +@item 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


["signature.asc" (application/pgp-signature)]

[prev in list] [next in list] [prev in thread] [next in thread]