[PATCH v4 21/29] tools/libxc+libxl+xl: Restore v2 streams

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

This is a complicated set of changes which must be done together for
bisectability.

 * libxl-save-helper is updated to unconditionally use libxc migration
   v2.
 * libxl compatibility workarounds in libxc are disabled for restore
   operations.
 * libxl__stream_read_start() is logically spliced into the event
   location where libxl__xc_domain_restore() used to reside.
 * Ownership of the save_helper_state moves to stream_read_state.

The parameters 'hvm', 'pae', and 'superpages' were previously
superfluous, and are completely unused in migration
v2. callbacks->toolstack_restore is handled via a migration v2 record
now, rather than via a callback from libxc.

NB: this change breaks Remus.  Further untangling needs to happen
before Remus will function.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>

---
v3:
 * Simplify from v2.
 * Alter the ownership of save_helper_state

v2:
 * Drop "legacy_width" from the IDL
 * Gain a LIBXL_HAVE_ to signify support of migration v2 streams
---
 tools/libxc/Makefile            |    4 ++--
 tools/libxl/libxl.h             |   17 ++++++++++++++
 tools/libxl/libxl_create.c      |   49 +++++++++++++--------------------------
 tools/libxl/libxl_internal.h    |    3 ++-
 tools/libxl/libxl_save_helper.c |    2 +-
 tools/libxl/libxl_stream_read.c |   35 +++++++++++++++++++++++++++-
 tools/libxl/libxl_types.idl     |    1 +
 tools/libxl/xl_cmdimpl.c        |    7 +++++-
 8 files changed, 79 insertions(+), 39 deletions(-)

diff --git a/tools/libxc/Makefile b/tools/libxc/Makefile
index b659df4..2cd0b1a 100644
--- a/tools/libxc/Makefile
+++ b/tools/libxc/Makefile
@@ -64,8 +64,8 @@ GUEST_SRCS-$(CONFIG_X86) += xc_sr_save_x86_hvm.c
 GUEST_SRCS-y += xc_sr_restore.c
 GUEST_SRCS-y += xc_sr_save.c
 GUEST_SRCS-y += xc_offline_page.c xc_compression.c
-$(patsubst %.c,%.o,$(GUEST_SRCS-y)): CFLAGS += -DXG_LIBXL_HVM_COMPAT
-$(patsubst %.c,%.opic,$(GUEST_SRCS-y)): CFLAGS += -DXG_LIBXL_HVM_COMPAT
+xc_sr_save_x86_hvm.o: CFLAGS += -DXG_LIBXL_HVM_COMPAT
+xc_sr_save_x86_hvm.opic: CFLAGS += -DXG_LIBXL_HVM_COMPAT
 else
 GUEST_SRCS-y += xc_nomigrate.c
 endif
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index e9d63c9..74b0829 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -807,6 +807,23 @@
  */
 #define LIBXL_HAVE_SOCKET_BITMAP_ALLOC 1
 
+/*
+ * LIBXL_HAVE_SRM_V2
+ *
+ * If this is defined, then the libxl_domain_create_restore() interface takes
+ * a "stream_version" parameter and supports a value of 2.
+ */
+#define LIBXL_HAVE_SRM_V2 1
+
+/*
+ * LIBXL_HAVE_SRM_V1
+ *
+ * In the case that LIBXL_HAVE_SRM_V2 is set, LIBXL_HAVE_SRM_V1
+ * indicates that libxl_domain_create_restore() can handle a "stream_version"
+ * parameter of 1, and convert the stream format automatically.
+ */
+#define LIBXL_HAVE_SRM_V1 1
+
 typedef char **libxl_string_list;
 void libxl_string_list_dispose(libxl_string_list *sl);
 int libxl_string_list_length(const libxl_string_list *sl);
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 7f0ffc6..c51d64f 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -704,6 +704,10 @@ static void domcreate_attach_dtdev(libxl__egc *egc,
 static void domcreate_console_available(libxl__egc *egc,
                                         libxl__domain_create_state *dcs);
 
+static void domcreate_stream_done(libxl__egc *egc,
+                                  libxl__stream_read_state *srs,
+                                  int ret);
+
 static void domcreate_rebuild_done(libxl__egc *egc,
                                    libxl__domain_create_state *dcs,
                                    int ret);
@@ -933,11 +937,8 @@ static void domcreate_bootloader_done(libxl__egc *egc,
     /* convenience aliases */
     const uint32_t domid = dcs->guest_domid;
     libxl_domain_config *const d_config = dcs->guest_config;
-    libxl_domain_build_info *const info = &d_config->b_info;
     const int restore_fd = dcs->restore_fd;
     libxl__domain_build_state *const state = &dcs->build_state;
-    libxl__srm_restore_autogen_callbacks *const callbacks =
-        &dcs->shs.callbacks.restore.a;
 
     if (rc) {
         domcreate_rebuild_done(egc, dcs, rc);
@@ -970,30 +971,17 @@ static void domcreate_bootloader_done(libxl__egc *egc,
     if (rc)
         goto out;
 
-    /* read signature */
-    int hvm, pae, superpages;
-    switch (info->type) {
-    case LIBXL_DOMAIN_TYPE_HVM:
-        hvm = 1;
-        superpages = 1;
-        pae = libxl_defbool_val(info->u.hvm.pae);
-        callbacks->toolstack_restore = libxl__toolstack_restore;
-        break;
-    case LIBXL_DOMAIN_TYPE_PV:
-        hvm = 0;
-        superpages = 0;
-        pae = 1;
-        break;
-    default:
-        rc = ERROR_INVAL;
-        goto out;
-    }
-    libxl__xc_domain_restore(egc, dcs, &dcs->shs,
-                             hvm, pae, superpages);
+    dcs->srs.ao = ao;
+    dcs->srs.dcs = dcs;
+    dcs->srs.fd = restore_fd;
+    dcs->srs.legacy = (dcs->restore_params.stream_version == 1);
+    dcs->srs.completion_callback = domcreate_stream_done;
+
+    libxl__stream_read_start(egc, &dcs->srs);
     return;
 
  out:
-    libxl__xc_domain_restore_done(egc, dcs, rc, 0, 0);
+    domcreate_stream_done(egc, &dcs->srs, rc);
 }
 
 void libxl__srm_callout_callback_restore_results(unsigned long store_mfn,
@@ -1009,10 +997,11 @@ void libxl__srm_callout_callback_restore_results(unsigned long store_mfn,
     shs->need_results =           0;
 }
 
-void libxl__xc_domain_restore_done(libxl__egc *egc, void *dcs_void,
-                                   int ret, int retval, int errnoval)
+static void domcreate_stream_done(libxl__egc *egc,
+                                  libxl__stream_read_state *srs,
+                                  int ret)
 {
-    libxl__domain_create_state *dcs = dcs_void;
+    libxl__domain_create_state *dcs = srs->dcs;
     STATE_AO_GC(dcs->ao);
     libxl_ctx *ctx = libxl__gc_owner(gc);
     char **vments = NULL, **localents = NULL;
@@ -1029,12 +1018,6 @@ void libxl__xc_domain_restore_done(libxl__egc *egc, void *dcs_void,
     if (ret)
         goto out;
 
-    if (retval) {
-        LOGEV(ERROR, errnoval, "restoring domain");
-        ret = ERROR_FAIL;
-        goto out;
-    }
-
     gettimeofday(&start_time, NULL);
 
     switch (info->type) {
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index b65019b..0a13c7f 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -3265,6 +3265,7 @@ struct libxl__stream_read_state {
     /* Private */
     int rc;
     bool running;
+    libxl__save_helper_state shs;
     libxl__conversion_helper_state chs;
 
     /* Main stream-reading data. */
@@ -3312,7 +3313,7 @@ struct libxl__domain_create_state {
     libxl__stub_dm_spawn_state dmss;
         /* If we're not doing stubdom, we use only dmss.dm,
          * for the non-stubdom device model. */
-    libxl__save_helper_state shs;
+    libxl__stream_read_state srs;
     /* necessary if the domain creation failed and we have to destroy it */
     libxl__domain_destroy_state dds;
     libxl__multidev multidev;
diff --git a/tools/libxl/libxl_save_helper.c b/tools/libxl/libxl_save_helper.c
index 14675ae..efbe2eb 100644
--- a/tools/libxl/libxl_save_helper.c
+++ b/tools/libxl/libxl_save_helper.c
@@ -313,7 +313,7 @@ int main(int argc, char **argv)
         startup("restore");
         setup_signals(SIG_DFL);
 
-        r = xc_domain_restore(xch, io_fd, dom, store_evtchn, &store_mfn,
+        r = xc_domain_restore2(xch, io_fd, dom, store_evtchn, &store_mfn,
                               store_domid, console_evtchn, &console_mfn,
                               console_domid, hvm, pae, superpages,
                               checkpointed,
diff --git a/tools/libxl/libxl_stream_read.c b/tools/libxl/libxl_stream_read.c
index 4b677f6..3051a2b 100644
--- a/tools/libxl/libxl_stream_read.c
+++ b/tools/libxl/libxl_stream_read.c
@@ -159,6 +159,7 @@ void libxl__stream_read_init(libxl__stream_read_state *stream)
 {
     stream->rc = 0;
     stream->running = false;
+    libxl__save_helper_init(&stream->shs);
     libxl__conversion_helper_init(&stream->chs);
     FILLZERO(stream->dc);
     FILLZERO(stream->hdr);
@@ -445,9 +446,13 @@ static bool process_record(libxl__egc *egc,
         stream_complete(egc, stream, 0);
         break;
 
+    case REC_TYPE_LIBXC_CONTEXT:
+        libxl__xc_domain_restore(egc, dcs, &stream->shs, 0, 0, 0);
+        break;
+
     case REC_TYPE_XENSTORE_DATA:
         rc = libxl__toolstack_restore(dcs->guest_domid, rec->body,
-                                      rec->hdr.length, &dcs->shs);
+                                      rec->hdr.length, &stream->shs);
         if (rc)
             goto err;
 
@@ -592,6 +597,32 @@ static void stream_done(libxl__egc *egc,
     check_all_finished(egc, stream, stream->rc);
 }
 
+void libxl__xc_domain_restore_done(libxl__egc *egc, void *dcs_void,
+                                   int rc, int retval, int errnoval)
+{
+    libxl__domain_create_state *dcs = dcs_void;
+    libxl__stream_read_state *stream = &dcs->srs;
+    STATE_AO_GC(dcs->ao);
+
+    if (rc)
+        goto err;
+
+    if (retval) {
+        LOGEV(ERROR, errnoval, "restoring domain");
+        rc = ERROR_FAIL;
+        goto err;
+    }
+
+    /*
+     * Libxc has indicated that it is done with the stream.  Resume reading
+     * libxl records from it.
+     */
+    stream_continue(egc, stream);
+
+ err:
+    check_all_finished(egc, stream, rc);
+}
+
 static void conversion_done(libxl__egc *egc,
                             libxl__conversion_helper_state *chs, int rc)
 {
@@ -610,11 +641,13 @@ static void check_all_finished(libxl__egc *egc,
         stream->rc = rc;
 
         libxl__stream_read_abort(egc, stream, rc);
+        libxl__save_helper_abort(egc, &stream->shs);
         libxl__conversion_helper_abort(egc, &stream->chs, rc);
     }
 
     /* Don't fire the callback until all our parallel tasks have stopped. */
     if (libxl__stream_read_inuse(stream) ||
+        libxl__save_helper_inuse(&stream->shs) ||
         libxl__conversion_helper_inuse(&stream->chs))
         return;
 
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 8dacf8d..bc0c4ef 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -355,6 +355,7 @@ libxl_domain_create_info = Struct("domain_create_info",[
 
 libxl_domain_restore_params = Struct("domain_restore_params", [
     ("checkpointed_stream", integer),
+    ("stream_version", uint32, {'init_val': '1'}),
     ])
 
 libxl_domain_sched_params = Struct("domain_sched_params",[
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 26b1e7d..71a22b8 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -110,7 +110,9 @@
 
 #define XL_MANDATORY_FLAG_JSON (1U << 0) /* config data is in JSON format */
 #define XL_MANDATORY_FLAG_STREAMv2 (1U << 1) /* stream is v2 */
-#define XL_MANDATORY_FLAG_ALL  (XL_MANDATORY_FLAG_JSON)
+#define XL_MANDATORY_FLAG_ALL  (XL_MANDATORY_FLAG_JSON |        \
+                                XL_MANDATORY_FLAG_STREAMv2)
+
 struct save_file_header {
     char magic[32]; /* savefileheader_magic */
     /* All uint32_ts are in domain's byte order. */
@@ -2757,6 +2759,9 @@ static uint32_t create_domain(struct domain_create *dom_info)
         libxl_domain_restore_params_init(&params);
 
         params.checkpointed_stream = dom_info->checkpointed_stream;
+        params.stream_version =
+            (hdr.mandatory_flags & XL_MANDATORY_FLAG_STREAMv2) ? 2 : 1;
+
         ret = libxl_domain_create_restore(ctx, &d_config,
                                           &domid, restore_fd,
                                           &params,
-- 
1.7.10.4