From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH 20/27] tools/libxl: Infrastructure for writing a v2 stream Date: Tue, 16 Jun 2015 16:28:11 +0100 Message-ID: <5580408B.9000408@citrix.com> References: <1434375880-30914-1-git-send-email-andrew.cooper3@citrix.com> <1434375880-30914-21-git-send-email-andrew.cooper3@citrix.com> <1434466626.13744.208.camel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1434466626.13744.208.camel@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: Ian Campbell Cc: Ross Lagerwall , Wei Liu , Yang Hongyang , Ian Jackson , Xen-devel List-Id: xen-devel@lists.xenproject.org On 16/06/15 15:57, Ian Campbell wrote: > On Mon, 2015-06-15 at 14:44 +0100, Andrew Cooper wrote: >> From: Ross Lagerwall >> >> Signed-off-by: Ross Lagerwall >> Signed-off-by: Andrew Cooper >> CC: Ian Campbell >> CC: Ian Jackson >> CC: Wei Liu >> --- >> tools/libxl/Makefile | 2 +- >> tools/libxl/libxl_internal.h | 33 +++ >> tools/libxl/libxl_stream_write.c | 536 ++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 570 insertions(+), 1 deletion(-) >> create mode 100644 tools/libxl/libxl_stream_write.c >> >> diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile >> index ca0ae3e..63e32f7 100644 >> --- a/tools/libxl/Makefile >> +++ b/tools/libxl/Makefile >> @@ -94,7 +94,7 @@ LIBXL_OBJS = flexarray.o libxl.o libxl_create.o libxl_dm.o libxl_pci.o \ >> libxl_dom.o libxl_exec.o libxl_xshelp.o libxl_device.o \ >> libxl_internal.o libxl_utils.o libxl_uuid.o \ >> libxl_json.o libxl_aoutils.o libxl_numa.o libxl_vnuma.o \ >> - libxl_stream_read.o \ >> + libxl_stream_read.o libxl_stream_write.o \ >> libxl_save_callout.o _libxl_save_msgs_callout.o \ >> libxl_convert_callout.o \ >> libxl_qmp.o libxl_event.o libxl_fork.o $(LIBXL_OBJS-y) >> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h >> index 5482950..82cd792 100644 >> --- a/tools/libxl/libxl_internal.h >> +++ b/tools/libxl/libxl_internal.h >> @@ -2868,6 +2868,38 @@ typedef void libxl__domain_suspend_cb(libxl__egc*, >> typedef void libxl__save_device_model_cb(libxl__egc*, >> libxl__domain_suspend_state*, int rc); >> >> +/* State for writing a libxl migration v2 stream */ >> +typedef struct libxl__stream_write_state libxl__stream_write_state; >> + >> +struct libxl__stream_write_state { >> + /* filled by the user */ >> + libxl__ao *ao; >> + int fd; >> + uint32_t domid; >> + void (*completion_callback)(libxl__egc *egc, >> + libxl__domain_suspend_state *dss, >> + int rc); >> + /* Private */ >> + int rc; >> + int joined_rc; >> + size_t padding; >> + bool running; >> + libxl__datacopier_state dc; >> +}; >> + >> +_hidden void libxl__stream_write_start(libxl__egc *egc, >> + libxl__stream_write_state *stream); >> + >> +_hidden void libxl__stream_write_abort(libxl__egc *egc, >> + libxl__stream_write_state *stream, >> + int rc); >> + >> +static inline bool libxl__stream_write_inuse( >> + const libxl__stream_write_state *stream) >> +{ >> + return stream->running; >> +} >> + >> typedef struct libxl__logdirty_switch { >> const char *cmd; >> const char *cmd_path; >> @@ -2907,6 +2939,7 @@ struct libxl__domain_suspend_state { >> /* private for libxl__domain_save_device_model */ >> libxl__save_device_model_cb *save_dm_callback; >> libxl__datacopier_state save_dm_datacopier; >> + libxl__stream_write_state sws; >> }; >> >> >> diff --git a/tools/libxl/libxl_stream_write.c b/tools/libxl/libxl_stream_write.c >> new file mode 100644 >> index 0000000..856d72e >> --- /dev/null >> +++ b/tools/libxl/libxl_stream_write.c >> @@ -0,0 +1,536 @@ >> +/* >> + * Copyright (C) 2015 Citrix Ltd. >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU Lesser General Public License as published >> + * by the Free Software Foundation; version 2.1 only. with the special >> + * exception on linking described in file LICENSE. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU Lesser General Public License for more details. >> + */ >> + >> +#include "libxl_osdeps.h" /* must come before any other headers */ >> + >> +#include "libxl_internal.h" >> + >> +/* >> + * Infrastructure for writing a domain to a libxl migration v2 stream. >> + * >> + * Entry points from outside: >> + * - libxl__stream_write_start() >> + * - Start writing a stream from the start. >> + * >> + * In normal operation, there are two tasks running at once; this stream >> + * processing, and the the libxl-save-helper. check_stream_finished() is used > "the the". > >> + * to join all the tasks in both success and error cases. >> + * >> + * Nomenclature for event callbacks: >> + * - $FOO_done(): Completion callback for $FOO >> + * - write_$FOO(): Set up writing a $FOO > Set up or actually write? Set up the dc to write $FOO The write doesn't actually happen until we get the dc callback. > >> + >> +void libxl__stream_write_start(libxl__egc *egc, >> + libxl__stream_write_state *stream) >> +{ >> + libxl__datacopier_state *dc = &stream->dc; >> + STATE_AO_GC(stream->ao); >> + struct libxl_sr_hdr hdr = { 0 }; >> + int ret = 0; >> + >> + assert(!stream->running); >> + stream->running = true; >> + >> + memset(dc, 0, sizeof(*dc)); > Please use the _init() _init() is already called for us. > > >> +static void check_stream_finished(libxl__egc *egc, >> + libxl__domain_suspend_state *dss, >> + int rc, const char *what) >> +{ >> + libxl__stream_write_state *stream = &dss->sws; >> + STATE_AO_GC(dss->ao); >> + >> + LOG(INFO, "Task '%s' joining (rc %d)", what, rc); >> + >> + if (rc && !stream->joined_rc) { >> + bool skip = false; >> + /* First reported failure from joining tasks. Tear everything down */ >> + stream->joined_rc = rc; > > This (not just this, but a bunch of the preceeding helpers) all looks > rather familiar, can it be shared to some extent? The types are all different, including the signature of the eventual callback to use. As COLO gets built on top, I expect these to diverge further. ~Andrew