Re: [Qemu-devel] [PATCH for-2.9 V4 2/2] Add a new qmp command to do checkpoint, get replication error

[Zhang Chen <zhangchen.fnst@cn.fujitsu.com>]

On 12/20/2016 10:42 PM, Eric Blake wrote:
> On 12/15/2016 08:46 PM, Zhang Chen wrote:
>> We can call this qmp command to do checkpoint outside of qemu.
>> Like Xen colo need this function.
>>
>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>> Signed-off-by: Wen Congyang <wencongyang@gmail.com>
>> ---
>>   docs/qmp-commands.txt | 24 ++++++++++++++++++++++++
>>   migration/colo.c      | 10 ++++++++++
>>   qapi-schema.json      | 22 ++++++++++++++++++++++
>>   3 files changed, 56 insertions(+)
>>
>>   
>> +void qmp_xen_get_replication_error(Error **errp)
>> +    {
>> +        replication_get_error_all(errp);
>> +    }
> Is this trying to cause a replication error, or is it trying to collect
> status on whether an error has occurred? The name 'get' usually implies
> a query that does not change state, but a query usually needs some way
> to return what was queried back to the user, so a void function with no
> parameters other than error is odd (either the command succeeds because
> there was no error, or the command fails with an error message because
> the query had something to get?).

Make sense, this command trying to collect status on whether
an error has occurred, and the "replication_get_error_all(errp)"
is always succeeds. So, Can you suggest to me the right name?


>
>
>> +++ b/qapi-schema.json
>> @@ -4695,6 +4695,28 @@
>>     'data': { 'enable': 'bool', 'primary': 'bool', '*failover' : 'bool' } }
>>   
>>   ##
>> +# @xen-get-replication-error
>> +#
>> +# Get replication error that occurs when the vm is running.
>> +#
>> +# Returns: nothing.
>> +#
>> +# Since: 2.9
>> +##
>> +{ 'command': 'xen-get-replication-error' }
> So I don't think this is the right name for its signature, but I don't
> know what you are trying to accomplish with this command to suggest the
> right name.

This interface is called to check if error happened in replication.

Thanks
Zhang Chen


>> +
>> +##
>> +# @xen-do-checkpoint
>> +#
>> +# Do checkpoint.
>> +#
>> +# Returns: nothing.
>> +#
>> +# Since: 2.9
>> +##
>> +{ 'command': 'xen-do-checkpoint' }
> This one is probably okay.
>
>

-- 
Thanks
Zhang Chen