Re: [Qemu-devel] [PATCH COLO-Frame v8 16/34] COLO failover: Introduce a new command to trigger a failover

[Eric Blake]

On 07/29/2015 02:45 AM, zhanghailiang wrote:
> We leave users to use whatever heartbeat solution they want, if the heartbeat
> is lost, or other errors they detect, they can use command
> 'colo_lost_heartbeat' to tell COLO to do failover, COLO will do operations
> accordingly.
> 
> For example,
> If send the command to PVM, Primary will exit COLO mode, and takeover,
> if to Secondary, Secondary will do failover work and at last takeover server.

Grammar suggestion:

For example, if the command is sent to the PVM, the Primary side will
exit COLO mode and take over operation. If sent to the Secondary, the
secondary will run failover work, then take over server operation to
become the new Primary.

> 
> Cc: Luiz Capitulino <address@hidden>
> Cc: Eric Blake <address@hidden>
> Cc: Markus Armbruster <address@hidden>
> Signed-off-by: zhanghailiang <address@hidden>
> Signed-off-by: Li Zhijian <address@hidden>
> Signed-off-by: Yang Hongyang <address@hidden>
> ---
>  hmp-commands.hx              | 15 +++++++++++++++
>  hmp.c                        |  8 ++++++++
>  hmp.h                        |  1 +
>  include/migration/colo.h     |  4 ++++
>  include/migration/failover.h | 20 ++++++++++++++++++++
>  migration/Makefile.objs      |  2 +-
>  migration/colo-comm.c        | 11 +++++++++++
>  migration/colo-failover.c    | 41 +++++++++++++++++++++++++++++++++++++++++
>  migration/colo.c             |  1 +
>  qapi-schema.json             | 25 +++++++++++++++++++++++++
>  qmp-commands.hx              | 19 +++++++++++++++++++
>  stubs/migration-colo.c       |  8 ++++++++
>  12 files changed, 154 insertions(+), 1 deletion(-)
>  create mode 100644 include/migration/failover.h
>  create mode 100644 migration/colo-failover.c

Just an interface review.

> +++ b/migration/colo.c
> @@ -15,6 +15,7 @@
>  #include "trace.h"
>  #include "qemu/error-report.h"
>  #include "qemu/sockets.h"
> +#include "migration/failover.h"
>  
>  /* Fix me: Convert to use QAPI */

Just noticing this comment; I'm assuming v9 will do more with qapi?

> +++ b/qapi-schema.json
> @@ -666,6 +666,31 @@
>              '*tls-port': 'int', '*cert-subject': 'str' } }
>  
>  ##
> +# @COLOMode
> +#
> +# The colo mode
> +#
> +# @unknown: unknown mode
> +#
> +# @primary: master side
> +#
> +# @secondary: slave side
> +#
> +# Since: 2.4

2.5

> +##
> +{ 'enum': 'COLOMode',
> +  'data': [ 'unknown', 'primary', 'secondary'] }

Where is this enum exposed to the user? Might be worth mentioning in the
commit message if a later patch will use it, and/or reshuffle the series
to have the enum introduced and exposed in one patch (perhaps always
with 'unknown' value), then actually manipulated in later patches.

> +
> +##
> +# @colo-lost-heartbeat
> +#
> +# Tell qemu that heartbeat is lost, request it to do takeover working.

s/working/procedures/

> +#
> +# Since: 2.4

2.5

> +##
> +{ 'command': 'colo-lost-heartbeat' }
> +
> +##
>  # @MouseInfo:
>  #
>  # Information about a mouse device.
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index bb49a1a..28a7962 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -781,6 +781,25 @@ Example:
>  EQMP
>  
>      {
> +        .name       = "colo-lost-heartbeat",
> +        .args_type  = "",
> +        .mhandler.cmd_new = qmp_marshal_input_colo_lost_heartbeat,

Markus' work on introspection will s/_input//; depending on what patches
go in first, there will be an obvious merge needed.

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

signature.asc
Description: OpenPGP digital signature