From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH 17/27] tools/libxl: Support converting a legacy stream to a v2 stream Date: Tue, 16 Jun 2015 16:13:01 +0100 Message-ID: <55803CFD.6060202@citrix.com> References: <1434375880-30914-1-git-send-email-andrew.cooper3@citrix.com> <1434375880-30914-18-git-send-email-andrew.cooper3@citrix.com> <1434465530.13744.199.camel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1434465530.13744.199.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: Wei Liu , Yang Hongyang , Ian Jackson , Xen-devel List-Id: xen-devel@lists.xenproject.org On 16/06/15 15:38, Ian Campbell wrote: > On Mon, 2015-06-15 at 14:44 +0100, Andrew Cooper wrote: >> When a legacy stream is found, it needs to be converted to a v2 stream for the >> reading logic. This is done by exec()ing the python conversion utility. >> >> One complication is that the caller of this interface needs to assume >> ownership of the output fd, to prevent it being closed while still in use in a >> datacopier. >> >> Signed-off-by: Andrew Cooper >> CC: Ian Campbell >> CC: Ian Jackson >> CC: Wei Liu >> --- >> tools/libxl/Makefile | 1 + >> tools/libxl/libxl_convert_callout.c | 146 +++++++++++++++++++++++++++++++++++ >> tools/libxl/libxl_internal.h | 32 ++++++++ >> 3 files changed, 179 insertions(+) >> create mode 100644 tools/libxl/libxl_convert_callout.c >> >> diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile >> index c71c5fe..ca0ae3e 100644 >> --- a/tools/libxl/Makefile >> +++ b/tools/libxl/Makefile >> @@ -96,6 +96,7 @@ LIBXL_OBJS = flexarray.o libxl.o libxl_create.o libxl_dm.o libxl_pci.o \ >> libxl_json.o libxl_aoutils.o libxl_numa.o libxl_vnuma.o \ >> libxl_stream_read.o \ >> libxl_save_callout.o _libxl_save_msgs_callout.o \ >> + libxl_convert_callout.o \ > Could we arrange for this to be x86 only, please (both here while > compiling and at runtime) Yes > >> libxl_qmp.o libxl_event.o libxl_fork.o $(LIBXL_OBJS-y) >> LIBXL_OBJS += libxl_genid.o >> LIBXL_OBJS += _libxl_types.o libxl_flask.o _libxl_types_internal.o >> diff --git a/tools/libxl/libxl_convert_callout.c b/tools/libxl/libxl_convert_callout.c >> new file mode 100644 >> index 0000000..9050bb9 >> --- /dev/null >> +++ b/tools/libxl/libxl_convert_callout.c >> + >> +static void helper_failed(libxl__egc *egc, >> + libxl__conversion_helper_state *chs, int rc); >> +static void helper_exited(libxl__egc *egc, libxl__ev_child *ch, >> + pid_t pid, int status); >> +static void helper_done(libxl__egc *egc, >> + libxl__conversion_helper_state *chs); > A lot of this stuff looks a lot like the contents of > libxl_save_callout.c, is there no scope for sharing any of it? I don't believe so. About the only thing they actually have in common is that they are looking after a child process. Starting, error and termination conditions are all different, as well as ownership of various bits of state. > > Since we only support N->N+1 we could perhaps tolerate the duplication > if we agreed upon a reasonable schedule to remove all this compat stuff, > e.g. in 4.7 or 4.8. N->N+1 is another item on the "needs to be resolved before Xapi can use libxl" list. In XenServer, we still have support for importing VMs from XenServer 4.0, which is certainly older than Xen 3.2 It might be reasonable to make a compile time option to omit the legacy conversion, but conversion should absolutely be on by default for the first release after this series being accepted. ~Andrew