Re: [PATCH 20/27] tools/libxl: Infrastructure for writing a v2 stream

[Ian Campbell <ian.campbell@citrix.com>]

On Mon, 2015-06-15 at 14:44 +0100, Andrew Cooper wrote:
> From: Ross Lagerwall <ross.lagerwall@citrix.com>
> 
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
> 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             |    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?

> +
> +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()


> +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?