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

[Eric Blake]

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 <address@hidden>
> Signed-off-by: Wen Congyang <address@hidden>
> ---
>  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?).


> +++ 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.

> +
> +##
> +# @xen-do-checkpoint
> +#
> +# Do checkpoint.
> +#
> +# Returns: nothing.
> +#
> +# Since: 2.9
> +##
> +{ 'command': 'xen-do-checkpoint' }

This one is probably okay.


-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature