[PATCH 27/29] [VERY RFC] tools/libxl: Support restoring legacy streams

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

This WorksForMe in the success case, but the error handling is certainly lacking.

Specifically, the conversion scripts output fd can't be closed until the v2
read loop has exited (cleanly or otherwise), without risking a close()/open()
race silently replacing the fd behind the loops back.

However, it can't be closed when the read loop exits, as the conversion script
child might still be alive, and would prefer terminating cleaning than failing
with a bad FD.

Obviously, having one error handler block for the success/failure of the other
side is a no-go, and would still involve a preselecting which was expected to
exit first.

Does anyone have any clever ideas of how to asynchronously collect the events
"the conversion script has exited", "the save helper has exited" and "the v2
read loop has finished" given the available infrastructure, to kick of a
combined cleanup of all 3?

(I also need to fix the conversion script info/error logging, but that is a
distinctly more minor problem.)
---
 tools/libxl/Makefile                |    1 +
 tools/libxl/libxl_convert_callout.c |  127 +++++++++++++++++++++++++++++++++++
 tools/libxl/libxl_create.c          |   43 +++++++++---
 tools/libxl/libxl_internal.h        |   16 +++++
 tools/libxl/libxl_types.idl         |    1 +
 5 files changed, 178 insertions(+), 10 deletions(-)
 create mode 100644 tools/libxl/libxl_convert_callout.c

diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
index 4bee4af..d7b5036 100644
--- a/tools/libxl/Makefile
+++ b/tools/libxl/Makefile
@@ -80,6 +80,7 @@ LIBXL_OBJS = flexarray.o libxl.o libxl_create.o libxl_dm.o libxl_pci.o \
 			libxl_internal.o libxl_utils.o libxl_uuid.o \
 			libxl_json.o libxl_aoutils.o libxl_numa.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)
 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..9d31b91
--- /dev/null
+++ b/tools/libxl/libxl_convert_callout.c
@@ -0,0 +1,127 @@
+/*
+ * Copyright (C) 2014      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"
+
+#include "libxl_internal.h"
+
+static void stream_convert_done(libxl__egc *egc, libxl__conversion_helper_state *chs)
+{
+    STATE_AO_GC(chs->ao);
+
+    if (chs->rc)
+        libxl__ao_complete(egc, ao, chs->rc);
+}
+
+static void helper_done(libxl__egc *egc, libxl__conversion_helper_state *chs)
+{
+    STATE_AO_GC(chs->ao);
+
+    if (chs->rc)
+        LOG(ERROR, "conversion script failed");
+    else
+        LOG(INFO, "conversion script succeeded");
+
+    chs->completion_callback(egc, chs);
+}
+
+static void helper_exited(libxl__egc *egc, libxl__ev_child *ch,
+                          pid_t pid, int status)
+{
+    libxl__conversion_helper_state *chs = CONTAINER_OF(ch, *chs, child);
+    STATE_AO_GC(chs->ao);
+
+    if (status) {
+        libxl_report_child_exitstatus(CTX, XTL_INFO, "conversion", pid, status);
+        chs->rc = ERROR_FAIL;
+    }
+
+    helper_done(egc, chs);
+}
+
+/*----- entrypoints -----*/
+int libxl__convert_legacy_stream(libxl__egc *egc,
+                                 libxl__domain_create_state *dcs)
+{
+    STATE_AO_GC(dcs->ao);
+    libxl__conversion_helper_state *chs = &dcs->chs;
+    int rc = 0;
+
+    chs->ao = ao;
+    chs->completion_callback = stream_convert_done;
+
+    chs->rc = 0;
+    libxl__ev_child_init(&chs->child);
+
+    int legacy_fd = dcs->restore_fd;
+    libxl__carefd *child_in = NULL;
+
+    libxl__carefd_begin();
+    int fds[2];
+    if (libxl_pipe(CTX, fds)) {
+        rc = ERROR_FAIL;
+        libxl__carefd_unlock();
+        goto err;
+
+    }
+    chs->child_out = libxl__carefd_record(CTX, fds[0]);
+    child_in       = libxl__carefd_record(CTX, fds[1]);
+    libxl__carefd_unlock();
+
+    pid_t pid = libxl__ev_child_fork(gc, &chs->child, helper_exited);
+    if (!pid) {
+        LOG(INFO, "In child!");
+
+        char * const args[] =
+        {
+            getenv("LIBXL_CONVERT_HELPER") ?: PRIVATE_BINDIR "/convert-legacy-stream",
+            "--in", GCSPRINTF("%d", legacy_fd),
+            "--out", GCSPRINTF("%d", fds[1]),
+            "--width",
+#ifdef __i386__
+            "32",
+#else
+            "64",
+#endif
+            "--guest", "pv",
+            "--format", "libxl",
+            /* "--verbose", */
+            NULL,
+        };
+
+        libxl_fd_set_cloexec(CTX, legacy_fd, 0);
+        libxl_fd_set_cloexec(CTX, libxl__carefd_fd(child_in), 0);
+
+        libxl__exec(gc,
+                    -1, -1, -1,
+                    args[0], args, NULL);
+    }
+
+    LOG(INFO, "In parent!");
+
+    libxl__carefd_close(child_in);
+
+    LOG(INFO, "Updating restore_fd %d -> %d",
+        dcs->restore_fd, libxl__carefd_fd(chs->child_out));
+    dcs->restore_fd = libxl__carefd_fd(chs->child_out);
+
+    /* TODO - how to libxl__carefd_close(chs->child_out) ? */
+
+    assert(!rc);
+    return rc;
+
+ err:
+    assert(rc);
+    return rc;
+}
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 9661f78..a1935ec 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -1514,16 +1514,6 @@ int libxl_domain_create_new(libxl_ctx *ctx, libxl_domain_config *d_config,
                             ao_how, aop_console_how);
 }
 
-int libxl_domain_create_restore(libxl_ctx *ctx, libxl_domain_config *d_config,
-                                uint32_t *domid, int restore_fd,
-                                const libxl_domain_restore_params *params,
-                                const libxl_asyncop_how *ao_how,
-                                const libxl_asyncprogress_how *aop_console_how)
-{
-    return do_domain_create(ctx, d_config, domid, restore_fd,
-                            params->checkpointed_stream, ao_how, aop_console_how);
-}
-
 static void read_restore_rec_hdr(libxl__app_domain_create_state *cdcs);
 
 static void read_padding_cb(libxl__egc *egc, libxl__datacopier_state *dc,
@@ -1810,6 +1800,39 @@ static void libxl__domain_restore(libxl__egc *egc,
     libxl__datacopier_start(dc);
 }
 
+int libxl_domain_create_restore(libxl_ctx *ctx, libxl_domain_config *d_config,
+                                uint32_t *domid, int restore_fd,
+                                const libxl_domain_restore_params *params,
+                                const libxl_asyncop_how *ao_how,
+                                const libxl_asyncprogress_how *aop_console_how)
+{
+    AO_CREATE(ctx, 0, ao_how);
+    libxl__app_domain_create_state *cdcs;
+    int ret = 0;
+
+    GCNEW(cdcs);
+    cdcs->dcs.ao = ao;
+    cdcs->dcs.guest_config = d_config;
+    cdcs->dcs.restore_fd = restore_fd;
+    cdcs->dcs.callback = domain_create_cb;
+    cdcs->dcs.checkpointed_stream = 0;
+    cdcs->dcs.rebuild_callback = restore_rebuild_complete;
+    libxl__ao_progress_gethow(&cdcs->dcs.aop_console_how, aop_console_how);
+    cdcs->domid_out = domid;
+
+    if (params->stream_version == 1) {
+        LOG(DEBUG, "Converting legacy stream");
+        ret = libxl__convert_legacy_stream(egc, &cdcs->dcs);
+    }
+
+    if (!ret)
+        libxl__domain_restore(egc, cdcs);
+
+    if (ret)
+        return AO_ABORT(ret);
+    else
+        return AO_INPROGRESS;
+}
 
 /*
  * Local variables:
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 3964009..c56a167 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2497,6 +2497,19 @@ typedef struct libxl__save_helper_state {
                       * marshalling and xc callback functions */
 } libxl__save_helper_state;
 
+/*----- Legacy conversion helper -----*/
+typedef struct libxl__conversion_helper_state libxl__conversion_helper_state;
+
+struct libxl__conversion_helper_state {
+    /* public */
+    libxl__ao *ao;
+    void (*completion_callback)(libxl__egc *egc,
+                                libxl__conversion_helper_state *chs);
+    /* private */
+    int rc;
+    libxl__ev_child child;
+    libxl__carefd *child_out;
+};
 
 /*----- Domain suspend (save) state structure -----*/
 
@@ -2806,6 +2819,7 @@ struct libxl__domain_create_state {
         /* If we're not doing stubdom, we use only dmss.dm,
          * for the non-stubdom device model. */
     libxl__save_helper_state shs;
+    libxl__conversion_helper_state chs;
     /* necessary if the domain creation failed and we have to destroy it */
     libxl__domain_destroy_state dds;
     libxl__multidev multidev;
@@ -3283,6 +3297,8 @@ static inline void libxl__update_config_vtpm(libxl__gc *gc,
     libxl_uuid_copy(CTX, &dst->uuid, &src->uuid);
 }
 
+int libxl__convert_legacy_stream(libxl__egc *egc,
+                                 libxl__domain_create_state *dcs);
 #endif
 
 /*
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index f1fcbc3..fdd3781 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -308,6 +308,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",[
-- 
1.7.10.4