Re: [PATCH 17/27] tools/libxl: Support converting a legacy stream to a v2 stream

[Andrew Cooper <andrew.cooper3@citrix.com>]

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 <andrew.cooper3@citrix.com>
>> CC: Ian Campbell <Ian.Campbell@citrix.com>
>> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
>> CC: Wei Liu <wei.liu2@citrix.com>
>> ---
>>  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