At 2013-10-01 06:16:34,"Eric Blake" <address@hidden> 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 <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.
Agree.
>> 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?
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
>