From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH v8 --for 4.6 COLO 08/25] tools/libxl: handle colo_context records in a libxl migration v2 read stream Date: Wed, 15 Jul 2015 18:44:19 +0100 Message-ID: <55A69BF3.3030100@citrix.com> References: <1436951936-3447-1-git-send-email-yanghy@cn.fujitsu.com> <1436951936-3447-9-git-send-email-yanghy@cn.fujitsu.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1436951936-3447-9-git-send-email-yanghy@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: Yang Hongyang , xen-devel@lists.xen.org Cc: wei.liu2@citrix.com, ian.campbell@citrix.com, wency@cn.fujitsu.com, guijianfeng@cn.fujitsu.com, yunhong.jiang@intel.com, eddie.dong@intel.com, rshriram@cs.ubc.ca, ian.jackson@eu.citrix.com List-Id: xen-devel@lists.xenproject.org On 15/07/15 10:18, Yang Hongyang wrote: > Read a colo_context and call stream->checkpoint_callback to handle it. > > Signed-off-by: Yang Hongyang > Signed-off-by: Wen Congyang > --- > tools/libxl/libxl_internal.h | 3 +++ > tools/libxl/libxl_stream_read.c | 51 +++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 54 insertions(+) > > diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h > index 05cee04..1be2a4a 100644 > --- a/tools/libxl/libxl_internal.h > +++ b/tools/libxl/libxl_internal.h > @@ -3369,6 +3369,7 @@ struct libxl__stream_read_state { > int rc; > bool running; > bool in_checkpoint; > + bool in_colo_context; > libxl__save_helper_state shs; > libxl__conversion_helper_state chs; > > @@ -3396,6 +3397,8 @@ _hidden void libxl__stream_read_start(libxl__egc *egc, > libxl__stream_read_state *stream); > _hidden void libxl__stream_read_start_checkpoint(libxl__egc *egc, > libxl__stream_read_state *stream); > +_hidden void libxl__stream_read_colo_context(libxl__egc *egc, > + libxl__stream_read_state *stream); > _hidden void libxl__stream_read_abort(libxl__egc *egc, > libxl__stream_read_state *stream, int rc); > static inline bool > diff --git a/tools/libxl/libxl_stream_read.c b/tools/libxl/libxl_stream_read.c > index b924f05..ab47251 100644 > --- a/tools/libxl/libxl_stream_read.c > +++ b/tools/libxl/libxl_stream_read.c > @@ -152,6 +152,13 @@ static void write_emulator_done(libxl__egc *egc, > libxl__datacopier_state *dc, > int rc, int onwrite, int errnoval); > > +/* Handlers for colo context mini-loop */ > +static void handle_colo_context(libxl__egc *egc, > + libxl__stream_read_state *stream, > + libxl__sr_record_buf *rec); > +static void colo_context_done(libxl__egc *egc, > + libxl__stream_read_state *stream, int rc); > + > /*----- Helpers -----*/ > > /* Helper to set up reading some data from the stream. */ > @@ -569,6 +576,15 @@ static bool process_record(libxl__egc *egc, > checkpoint_done(egc, stream, 0); > break; > > + case REC_TYPE_COLO_CONTEXT: > + if (!stream->in_colo_context) { > + LOG(ERROR, "Unexpected COLO_CONTEXT record in stream"); > + rc = ERROR_FAIL; > + goto err; > + } > + handle_colo_context(egc, stream, rec); > + break; > + > default: > LOG(ERROR, "Unrecognised record 0x%08x", rec->hdr.type); > rc = ERROR_FAIL; > @@ -678,6 +694,11 @@ static void stream_complete(libxl__egc *egc, > return; > } > > + if (stream->in_colo_context) { > + colo_context_done(egc, stream, rc); > + return; > + } > + > if (!stream->rc) > stream->rc = rc; > stream_done(egc, stream); > @@ -794,6 +815,36 @@ static void check_all_finished(libxl__egc *egc, > stream->completion_callback(egc, stream, stream->rc); > } > > +/*----- COLO context handlers -----*/ > + > +void libxl__stream_read_colo_context(libxl__egc *egc, > + libxl__stream_read_state *stream) A name like this makes the erroneous assumption that a COLO\_CONTEXT record is what is going to be found next in the stream. Where and when is a COLO\_CONTEXT record expected, and is it only in the backchannel? > +{ > + assert(stream->running); > + assert(!stream->in_checkpoint); > + assert(!stream->in_colo_context); > + stream->in_colo_context = true; > + > + setup_read_record(egc, stream); > +} > + > +static void handle_colo_context(libxl__egc *egc, > + libxl__stream_read_state *stream, > + libxl__sr_record_buf *rec) > +{ > + libxl_sr_colo_context *colo_context = rec->body; > + > + colo_context_done(egc, stream, colo_context->id); A handler this trivial should just be done in the switch statement in process_record(). No need for its own function for a single forward call. ~Andrew