From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konrad Rzeszutek Wilk Subject: Re: [PATCH v6 11/18] tools/libxl: Add back channel to allow migration target send data back Date: Mon, 25 Jan 2016 14:17:43 -0500 Message-ID: <20160125191743.GQ14977@char.us.oracle.com> References: <1451442548-26974-1-git-send-email-wency@cn.fujitsu.com> <1451442548-26974-12-git-send-email-wency@cn.fujitsu.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <1451442548-26974-12-git-send-email-wency@cn.fujitsu.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Wen Congyang Cc: Lars Kurth , Changlong Xie , Wei Liu , Ian Campbell , Andrew Cooper , Jiang Yunhong , Ian Jackson , xen devel , Dong Eddie , Gui Jianfeng , Shriram Rajagopalan , Yang Hongyang List-Id: xen-devel@lists.xenproject.org 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 > Signed-off-by: Yang Hongyang > --- > 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(¶ms); > > ret = libxl_domain_create_restore( > - ctx, d_config, domid, restore_fd, ¶ms, ao_how, aop_console_how); > + ctx, d_config, domid, restore_fd, -1, ¶ms, ao_how, aop_console_how); > > libxl_domain_restore_params_dispose(¶ms); > 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.