At 2013-10-01 06:16:34,"Eric Blake" <ebl...@redhat.com> wrote: >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.w...@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.
Agree.
>> -@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?
AFAK, The migration is always detached(In the code, the -d option is always
false), -d and -f can be used at the same time with no doubt.
qmp_migrate(uri, !!blk, blk, !!inc, inc, false, false, !!ft, ft, &err);
By the way, neither -b nor -i could be used at the same time with -f, fault
tolerant needs shared storage.
>> +++ 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?
To align the '=', I will remove them if you like.
>
>> +++ 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?
I'm not clear about how to do that, could you pls give me some hints, where to
add code and documentation.
>Furthermore, 'ft' is an awfully short name;
>for QMP, we prefer to use full words where possible, such as
>'fault-tolerant'.
Agree.
>--
>Eric Blake eblake redhat com +1-919-301-3266
>Libvirt virtualization library http://libvirt.org
>
