On 02/21/2017 08:07 AM, Markus Armbruster wrote: > Zhang Chen <[email protected]> writes: > >> On 02/21/2017 07:15 PM, Markus Armbruster wrote: >>> Zhang Chen <[email protected]> writes: >>> >>>> We can call this qmp command to do checkpoint outside of qemu. >>>> Xen colo will need this function. >>> I know nothing about "Xen colo", but I'll try anyway. >>> >>> I *guess* "Xen colo" is a long-running activity, and the new commands >>> interact with it. Correct? >> >> Yes, you are right. > > We need to build a generic framework for interacting with long-running > activities. But asking you to wait for it wouldn't be fair. > >> By the way this patch has been reviewed by Eric black. >> >> https://lists.nongnu.org/archive/html/qemu-devel/2017-02/msg03435.html
Blake, but you're not the first to mis-type it.
>>>> +{
>>>> + Error *err = NULL;
>>>> + ReplicationResult *result = g_new0(ReplicationResult, 1);
>>>> + replication_get_error_all(&err);
>>>> + result->status = err ?
>>>> + REPLICATION_STATUS_ERROR :
>>>> + REPLICATION_STATUS_NORMAL;
>>> Reports only that there is an error, throws away the actual error
>>> message. Naive question: could the error message be good to know for
>>> the QMP user?
>>
>> Yes, Xen colo will handle it.
>
> Is that "yes, the QMP user could use the error message, but we're not
> giving it to him regardless", or "no, the QMP user does not need to
> know, and that's because we don't give it to him"?
>
> Even if we want QMP clients to treat all errors the same, transmitting
> the error message can still be useful for troubleshooting.
Ah, as in:
if (err) {
result->status = REPLICATION_STATUS_ERROR;
result->has_desc = true;
result->desc = ...extract string from err
} else {
result->status = REPLICATION_STATUS_NORMAL;
}
by modifying the JSON [1]
>>>> +##
>>>> +{ 'enum': 'ReplicationStatus',
>>>> + 'data': [ 'error', 'normal' ] }
>>> Do you expect more status values in the future?
>>>
>>> If yes, what should clients do when they see a status value they don't
>>> know?
>>
>> We will add other status for it, now, that's enough.
>
> What should a QMP client do when it sees a ReplicationStatus value other
> than 'error' and 'normal'?
>
> You need to provide some guidance, or else you won't be able to add
> status values without breaking clients!
>
>>> If no, why not simply use bool?
Off-hand, what other states do you envision adding?
>>>
>>>> +
>>>> +##
>>>> +# @ReplicationResult:
>>>> +#
>>>> +# The result format for 'query-xen-replication-status'.
>>>> +#
>>>> +# @status: enum of @ReplicationStatus, which shows current
>>>> +# replication error status
>>>> +#
>>>> +# Since: 2.9
>>>> +##
>>>> +{ 'struct': 'ReplicationResult',
>>>> + 'data': { 'status': 'ReplicationStatus'} }
[1] the modification would be:
'data': { 'status': 'ReplicationStatus', '*desc': 'str' }
with documentation that @desc is #optional, is only for human
consumption, and is only present when @status indicates an error.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
