Re: [Xen-devel] [PATCH v6 11/18] tools/libxl: Add back channel to allow migration target send data back

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

On Wed, Dec 30, 2015 at 10:29:01AM +0800, Wen Congyang wrote:
> In colo mode, slave needs to send data to master, but the io_fd



In previous patches you used COLO in all caps, can that be uniform
across the patches?

Also, slave == secondary and master == primary? Perhaps you
could s/slave/secondary/ s/master/primary/ to sync up with
the other patches?

Thank you!
> only can be written in master, and only can be read in slave.



Could you mention what kind of data the secondary has to send
to the primary? In the previous patch (] tools/libxl:
introduce libxl__domain_common_switch_qemu_logdirty()) it mentioned
dirty page. Is that the case here? If so can you mention
that as well here?


> Save recv_fd in domain_suspend_state, and send_fd in
> domain_create_state.
> Extend libxl_domain_create_restore API, add a send_fd param to
> it.
> Add LIBXL_HAVE_CREATE_RESTORE_SEND_FD to indicate the API change.
> 
> Signed-off-by: Wen Congyang <wency@xxxxxxxxxxxxxx>
> Signed-off-by: Yang Hongyang <hongyang.yang@xxxxxxxxxxxx>
> ---
>  tools/libxl/libxl.c                  |  2 +-
>  tools/libxl/libxl.h                  | 30 ++++++++++++++++++++++++++++--
>  tools/libxl/libxl_create.c           |  9 +++++----
>  tools/libxl/libxl_internal.h         |  2 ++
>  tools/libxl/libxl_types.idl          |  1 +
>  tools/libxl/xl_cmdimpl.c             |  8 +++++++-
>  tools/ocaml/libs/xl/xenlight_stubs.c |  2 +-
>  7 files changed, 45 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 2faea4d..69c8047 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -872,7 +872,7 @@ int libxl_domain_remus_start(libxl_ctx *ctx, 
> libxl_domain_remus_info *info,
>      dss->callback = remus_failover_cb;
>      dss->domid = domid;
>      dss->fd = send_fd;
> -    /* TODO do something with recv_fd */
> +    dss->recv_fd = recv_fd;
>      dss->type = type;
>      dss->live = 1;
>      dss->debug = 0;
> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
> index a01e448..67a4ad7 100644
> --- a/tools/libxl/libxl.h
> +++ b/tools/libxl/libxl.h
> @@ -630,6 +630,15 @@ typedef struct libxl__ctx libxl_ctx;
>  #define LIBXL_HAVE_DOMAIN_CREATE_RESTORE_PARAMS 1
>  
>  /*
> + * LIBXL_HAVE_DOMAIN_CREATE_RESTORE_SEND_FD 1
> + *
> + * If this is defined, libxl_domain_create_restore()'s API has changed to
> + * include a send_fd param which used for libxl migration back channel
> + * during COLO FT.

FT? Could you explain that acronym please?
> + */
> +#define LIBXL_HAVE_DOMAIN_CREATE_RESTORE_SEND_FD 1
> +
> +/*
>   * LIBXL_HAVE_CREATEINFO_PVH
>   * If this is defined, then libxl supports creation of a PVH guest.
>   */
> @@ -1143,7 +1152,7 @@ int libxl_domain_create_new(libxl_ctx *ctx, 
> libxl_domain_config *d_config,
>                              const libxl_asyncprogress_how *aop_console_how)
>                              LIBXL_EXTERNAL_CALLERS_ONLY;
>  int libxl_domain_create_restore(libxl_ctx *ctx, libxl_domain_config 
> *d_config,
> -                                uint32_t *domid, int restore_fd,
> +                                uint32_t *domid, int restore_fd, int send_fd,
>                                  const libxl_domain_restore_params *params,
>                                  const libxl_asyncop_how *ao_how,
>                                  const libxl_asyncprogress_how 
> *aop_console_how)
> @@ -1164,7 +1173,7 @@ int static inline libxl_domain_create_restore_0x040200(
>      libxl_domain_restore_params_init(&params);
>  
>      ret = libxl_domain_create_restore(
> -        ctx, d_config, domid, restore_fd, &params, ao_how, aop_console_how);
> +        ctx, d_config, domid, restore_fd, -1, &params, ao_how, 
> aop_console_how);
>  
>      libxl_domain_restore_params_dispose(&params);
>      return ret;
> @@ -1172,6 +1181,23 @@ int static inline libxl_domain_create_restore_0x040200(
>  
>  #define libxl_domain_create_restore libxl_domain_create_restore_0x040200
>  
> +#elif defined(LIBXL_API_VERSION) && LIBXL_API_VERSION >= 0x040400 \
> +                                 && LIBXL_API_VERSION < 0x040600

s/4060/4070? Or is that suppose to be <= 040600 ?
> +
.. snip..
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index 9aa94be..c5d5d40 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -232,6 +232,7 @@ libxl_hdtype = Enumeration("hdtype", [
>  libxl_checkpointed_stream = Enumeration("checkpointed_stream", [
>      (0, "NONE"),
>      (1, "REMUS"),
> +    (2, "COLO"),

You should also update the migration_stream enum with the extra enum.

And if you follow my idea of adding an assertion in xc_domain_save
for the different checkpointed_stream types then that would need to
be expanded to include support for COLO type as well.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel