From mboxrd@z Thu Jan 1 00:00:00 1970 From: Yang Hongyang Subject: Re: [PATCH v8 --for 4.6 COLO 05/25] tools/libxl: add back channel support to write stream Date: Thu, 16 Jul 2015 15:21:22 +0800 Message-ID: <55A75B72.7030106@cn.fujitsu.com> References: <1436951936-3447-1-git-send-email-yanghy@cn.fujitsu.com> <1436951936-3447-6-git-send-email-yanghy@cn.fujitsu.com> <55A69771.9030103@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <55A69771.9030103@citrix.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: Andrew Cooper , 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 07/16/2015 01:25 AM, Andrew Cooper wrote: > On 15/07/15 10:18, Yang Hongyang wrote: >> Add back channel support to write stream. If the write stream is >> a back channel stream, this means the write stream is used by >> Secondary to send some records back. >> >> Signed-off-by: Yang Hongyang >> --- >> tools/libxl/libxl_dom_save.c | 1 + >> tools/libxl/libxl_internal.h | 1 + >> tools/libxl/libxl_stream_write.c | 16 ++++++++++++++++ >> 3 files changed, 18 insertions(+) >> >> diff --git a/tools/libxl/libxl_dom_save.c b/tools/libxl/libxl_dom_save.c >> index 9b7159f..25813ce 100644 >> --- a/tools/libxl/libxl_dom_save.c >> +++ b/tools/libxl/libxl_dom_save.c >> @@ -445,6 +445,7 @@ void libxl__domain_save(libxl__egc *egc, libxl__domain_save_state *dss) >> dss->sws.ao = dss->ao; >> dss->sws.dss = dss; >> dss->sws.fd = dss->fd; >> + dss->sws.back_channel = false; >> dss->sws.completion_callback = stream_done; >> >> libxl__stream_write_start(egc, &dss->sws); >> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h >> index 9c81d8d..a83d6a5 100644 >> --- a/tools/libxl/libxl_internal.h >> +++ b/tools/libxl/libxl_internal.h >> @@ -2989,6 +2989,7 @@ struct libxl__stream_write_state { >> libxl__ao *ao; >> libxl__domain_save_state *dss; >> int fd; >> + bool back_channel; >> void (*completion_callback)(libxl__egc *egc, >> libxl__stream_write_state *sws, >> int rc); >> diff --git a/tools/libxl/libxl_stream_write.c b/tools/libxl/libxl_stream_write.c >> index 16f667a..df55277 100644 >> --- a/tools/libxl/libxl_stream_write.c >> +++ b/tools/libxl/libxl_stream_write.c >> @@ -47,6 +47,13 @@ >> * - Toolstack record >> * - if (hvm), Qemu record >> * - Checkpoint end record >> + * >> + * For back channel stream: >> + * - libxl__stream_write_start() >> + * - Set up the stream to running state >> + * >> + * - Add a new API to write the record. When the record is written >> + * out, call stream->checkpoint_callback() to return. >> */ >> >> /* Success/error/cleanup handling. */ >> @@ -178,6 +185,9 @@ void libxl__stream_write_start(libxl__egc *egc, >> >> stream->running = true; >> >> + if (stream->back_channel) >> + return; >> + > > Some of the setup below should not be skipped. > > While it makes the diff bigger, the end result would be more logical as > > dc->ao = ao; > dc->readfd = -1; > dc->writefd = stream->fd; > dc->maxsz = -1; > > if (!stream->back_channel) { > /* Write the stream header. */ > dc->writewhat = "save/migration stream"; > dc->callback = stream_header_done; > > rc = ibxl__datacopier_start(dc); > .... > } > > To split the object setup from the action of sending the stream header, > which are currently mixed. > > What happened to the backchannel negotiation header? There's no negotiation header currently...If it is a broken back channel, COLO will fail. > > ~Andrew > >> dc->ao = ao; >> dc->readfd = -1; >> dc->writewhat = "save/migration stream"; >> @@ -207,6 +217,7 @@ void libxl__stream_write_start_checkpoint(libxl__egc *egc, >> { >> assert(stream->running); >> assert(!stream->in_checkpoint); >> + assert(!stream->back_channel); >> stream->in_checkpoint = true; >> >> write_toolstack_record(egc, stream); >> @@ -500,6 +511,11 @@ static void stream_done(libxl__egc *egc, >> assert(stream->running); >> stream->running = false; >> >> + if (stream->back_channel) { >> + stream->completion_callback(egc, stream, stream->rc); >> + return; >> + } >> + >> if (stream->emu_carefd) >> libxl__carefd_close(stream->emu_carefd); >> free(stream->emu_body); > > . > -- Thanks, Yang.