On 03/03/2017 07:45 AM, Markus Armbruster wrote: > Eric Blake <[email protected]> writes: > >> On 02/23/2017 01:14 AM, Zhang Chen wrote: >>> We can call this qmp command to do checkpoint outside of qemu. >>> Xen colo will need this function. >>> >>> Signed-off-by: Zhang Chen <[email protected]> >>> Signed-off-by: Wen Congyang <[email protected]> >>> Reviewed-by: Eric Blake <[email protected]> >> >> You made a substantial change to this patch since v7 in response to my >> comments; when you do that, it's best to remove the R-b to make sure I >> re-review the changes and am still happy with them. >>
>>> +# @ReplicationStatus:
>>> +#
>>> +# The result format for 'query-xen-replication-status'.
>>> +#
>>> +# @status: true to error, false to normal.
>>
>> This is now a poor name for the parameter. Please rename it; probably
>> to @error (if you want to keep true meaning a problem has been
>> detected), or to @okay (if you want to invert the sense, and @desc is
>> only present when @okay is false).
>
> We could replace both members by '*error': 'str', present exactly when
> status is "bad".
>
>>> +#
>>> +# @desc: #optional the human readable error description string, when
>>> +# @status is 'true'.
>>> +#
>>> +# Since: 2.9
>>> +##
>>> +{ 'struct': 'ReplicationStatus',
>>> + 'data': { 'status': 'bool', '*desc': 'str' } }
Except that ReplicationStatus would then be the empty struct on success.
I guess that's okay, but seems a bit odd.
>>> +# Example:
>>> +#
>>> +# -> { "execute": "query-xen-replication-status" }
>>> +# <- { "return": { "status": "normal" } }
>>
>> This example is now wrong.
>>
>> You'll need a v9.
which has already been submitted and turned into a pull request:
https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg07652.html
It pre-dated your mandate of "all new QMP commands must have a test":
https://lists.gnu.org/archive/html/qemu-devel/2017-03/msg00296.html
but the idea is still relevant, so a followup patch for 2.9 that adds
testsuite coverage of the new commands is appropriate during soft freeze.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
