diff mbox series

[v6,09/18] libxl: add save/restore support for qemu-xen in stubdomain

Message ID 20200518011353.326287-10-jandryuk@gmail.com (mailing list archive)
State Superseded
Headers show
Series Add support for qemu-xen runnning in a Linux-based stubdomain | expand

Commit Message

Jason Andryuk May 18, 2020, 1:13 a.m. UTC 
From: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

Rely on a wrapper script in stubdomain to attach relevant consoles to
qemu.  The save console (1) must be attached to fdset/1.  When
performing a restore, $STUBDOM_RESTORE_INCOMING_ARG must be replaced on
the qemu command line by "fd:$FD", where $FD is an open file descriptor
number to the restore console (2).

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

Address TODO in dm_state_save_to_fdset: Only remove savefile for
non-stubdom.
Use $STUBDOM_RESTORE_INCOMING_ARG instead of fd:3 and update commit
message.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
Changes in v3:
 - adjust for qmp_ev*
 - assume specific fdset id in qemu set in stubdomain
Changes in v5:
 - Only remove savefile for non-stubdom
Chanres in v6:
 - Replace hardcoded fd:3 with placeholder $STUBDOM_RESTORE_INCOMING_ARG
---
 tools/libxl/libxl_dm.c  | 25 +++++++++++++------------
 tools/libxl/libxl_qmp.c | 27 +++++++++++++++++++++++++--
 2 files changed, 38 insertions(+), 14 deletions(-)

Comments

Ian Jackson May 18, 2020, 2:24 p.m. UTC | #1 
Jason Andryuk writes ("[PATCH v6 09/18] libxl: add save/restore support for qemu-xen in stubdomain"):
> From: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
...
>      if (state->saved_state) {
> -        /* This file descriptor is meant to be used by QEMU */
> -        *dm_state_fd = open(state->saved_state, O_RDONLY);
> -        flexarray_append(dm_args, "-incoming");
> -        flexarray_append(dm_args, GCSPRINTF("fd:%d",*dm_state_fd));
> +        if (is_stubdom) {
> +            /* Linux stubdomain must replace $STUBDOM_RESTORE_INCOMING_ARG
> +             * with the approriate fd:$num argument for the
> +             * STUBDOM_CONSOLE_RESTORE console 2.
> +             */
> +            flexarray_append(dm_args, "-incoming");
> +            flexarray_append(dm_args, "$STUBDOM_RESTORE_INCOMING_ARG");
> +        } else {
> +            /* This file descriptor is meant to be used by QEMU */
> +            *dm_state_fd = open(state->saved_state, O_RDONLY);
> +            flexarray_append(dm_args, "-incoming");
> +            flexarray_append(dm_args, GCSPRINTF("fd:%d",*dm_state_fd));

Hrk.  The stubdom script is expected to spot this particular value in
the dm_args array and seddery it.  OK.  This is, at leasst, sound.
I'm happy with the code and the protocol.

I think this needs a change to this doc:

  Subject: [PATCH v6 01/18] Document ioemu MiniOS stubdomain protocol

  +Toolstack to MiniOS ioemu stubdomain protocol
  +---------------------------------------------

Provided that you update the docs commit and take my ack off that,
please add my ack to this code :-).

Or you can fold the docs change into this commit, if you prefer, in
which case I'll review this patch again.

Thanks,
Ian.
Jason Andryuk May 18, 2020, 3:30 p.m. UTC | #2 
On Mon, May 18, 2020 at 10:24 AM Ian Jackson <ian.jackson@citrix.com> wrote:
>
> Jason Andryuk writes ("[PATCH v6 09/18] libxl: add save/restore support for qemu-xen in stubdomain"):
> > From: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> ...
> >      if (state->saved_state) {
> > -        /* This file descriptor is meant to be used by QEMU */
> > -        *dm_state_fd = open(state->saved_state, O_RDONLY);
> > -        flexarray_append(dm_args, "-incoming");
> > -        flexarray_append(dm_args, GCSPRINTF("fd:%d",*dm_state_fd));
> > +        if (is_stubdom) {
> > +            /* Linux stubdomain must replace $STUBDOM_RESTORE_INCOMING_ARG
> > +             * with the approriate fd:$num argument for the
> > +             * STUBDOM_CONSOLE_RESTORE console 2.
> > +             */
> > +            flexarray_append(dm_args, "-incoming");
> > +            flexarray_append(dm_args, "$STUBDOM_RESTORE_INCOMING_ARG");
> > +        } else {
> > +            /* This file descriptor is meant to be used by QEMU */
> > +            *dm_state_fd = open(state->saved_state, O_RDONLY);
> > +            flexarray_append(dm_args, "-incoming");
> > +            flexarray_append(dm_args, GCSPRINTF("fd:%d",*dm_state_fd));
>
> Hrk.  The stubdom script is expected to spot this particular value in
> the dm_args array and seddery it.  OK.  This is, at leasst, sound.
> I'm happy with the code and the protocol.
>
> I think this needs a change to this doc:
>
>   Subject: [PATCH v6 01/18] Document ioemu MiniOS stubdomain protocol
>
>   +Toolstack to MiniOS ioemu stubdomain protocol
>   +---------------------------------------------
>
> Provided that you update the docs commit and take my ack off that,
> please add my ack to this code :-).

I updated "[PATCH v6 02/18] Document ioemu Linux stubdomain protocol"
to mention $STUBDOM_RESTORE_INCOMING_ARG as well as the xenstore
directory change to "dm-argv" in this v6, but I left your Ack on it.
Sorry about that.  I'll remove your Ack from 02/18 when I post v7,
but I'll add the Ack to this 09/18.

-Jason
Ian Jackson May 18, 2020, 4:29 p.m. UTC | #3 
Jason Andryuk writes ("Re: [PATCH v6 09/18] libxl: add save/restore support for qemu-xen in stubdomain"):
> On Mon, May 18, 2020 at 10:24 AM Ian Jackson <ian.jackson@citrix.com> wrote:
> > Provided that you update the docs commit and take my ack off that,
> > please add my ack to this code :-).
> 
> I updated "[PATCH v6 02/18] Document ioemu Linux stubdomain protocol"
> to mention $STUBDOM_RESTORE_INCOMING_ARG as well as the xenstore
> directory change to "dm-argv" in this v6, but I left your Ack on it.
> Sorry about that.  I'll remove your Ack from 02/18 when I post v7,
> but I'll add the Ack to this 09/18.

Oh, that's why I didn't see that docs change.  I went to look what you
wrote in v6 02/18.  It LGTM.  Please put my ack back :-).

Thanks,
Ian.
diff mbox series

Patch

diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index eaed6e8ee7..a4f8866d33 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1745,10 +1745,19 @@  static int libxl__build_device_model_args_new(libxl__gc *gc,
     }
 
     if (state->saved_state) {
-        /* This file descriptor is meant to be used by QEMU */
-        *dm_state_fd = open(state->saved_state, O_RDONLY);
-        flexarray_append(dm_args, "-incoming");
-        flexarray_append(dm_args, GCSPRINTF("fd:%d",*dm_state_fd));
+        if (is_stubdom) {
+            /* Linux stubdomain must replace $STUBDOM_RESTORE_INCOMING_ARG
+             * with the approriate fd:$num argument for the
+             * STUBDOM_CONSOLE_RESTORE console 2.
+             */
+            flexarray_append(dm_args, "-incoming");
+            flexarray_append(dm_args, "$STUBDOM_RESTORE_INCOMING_ARG");
+        } else {
+            /* This file descriptor is meant to be used by QEMU */
+            *dm_state_fd = open(state->saved_state, O_RDONLY);
+            flexarray_append(dm_args, "-incoming");
+            flexarray_append(dm_args, GCSPRINTF("fd:%d",*dm_state_fd));
+        }
     }
     for (i = 0; b_info->extra && b_info->extra[i] != NULL; i++)
         flexarray_append(dm_args, b_info->extra[i]);
@@ -2236,14 +2245,6 @@  void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss)
 
     assert(libxl_defbool_val(guest_config->b_info.device_model_stubdomain));
 
-    if (libxl__stubdomain_is_linux(&guest_config->b_info)) {
-        if (d_state->saved_state) {
-            LOG(ERROR, "Save/Restore not supported yet with Linux Stubdom.");
-            ret = -1;
-            goto out;
-        }
-    }
-
     sdss->pvqemu.guest_domid = INVALID_DOMID;
 
     libxl_domain_create_info_init(&dm_config->c_info);
diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index efaba91086..c394000ea9 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -962,6 +962,7 @@  static void dm_stopped(libxl__egc *egc, libxl__ev_qmp *ev,
                        const libxl__json_object *response, int rc);
 static void dm_state_fd_ready(libxl__egc *egc, libxl__ev_qmp *ev,
                               const libxl__json_object *response, int rc);
+static void dm_state_save_to_fdset(libxl__egc *egc, libxl__ev_qmp *ev, int fdset);
 static void dm_state_saved(libxl__egc *egc, libxl__ev_qmp *ev,
                            const libxl__json_object *response, int rc);
 
@@ -994,10 +995,17 @@  static void dm_stopped(libxl__egc *egc, libxl__ev_qmp *ev,
     EGC_GC;
     libxl__domain_suspend_state *dsps = CONTAINER_OF(ev, *dsps, qmp);
     const char *const filename = dsps->dm_savefile;
+    uint32_t dm_domid = libxl_get_stubdom_id(CTX, dsps->domid);
 
     if (rc)
         goto error;
 
+    if (dm_domid) {
+        /* see Linux stubdom interface in docs/stubdom.txt */
+        dm_state_save_to_fdset(egc, ev, 1);
+        return;
+    }
+
     ev->payload_fd = open(filename, O_WRONLY | O_CREAT, 0600);
     if (ev->payload_fd < 0) {
         LOGED(ERROR, ev->domid,
@@ -1028,7 +1036,6 @@  static void dm_state_fd_ready(libxl__egc *egc, libxl__ev_qmp *ev,
     EGC_GC;
     int fdset;
     const libxl__json_object *o;
-    libxl__json_object *args = NULL;
     libxl__domain_suspend_state *dsps = CONTAINER_OF(ev, *dsps, qmp);
 
     close(ev->payload_fd);
@@ -1043,6 +1050,21 @@  static void dm_state_fd_ready(libxl__egc *egc, libxl__ev_qmp *ev,
         goto error;
     }
     fdset = libxl__json_object_get_integer(o);
+    dm_state_save_to_fdset(egc, ev, fdset);
+    return;
+
+error:
+    assert(rc);
+    libxl__remove_file(gc, dsps->dm_savefile);
+    dsps->callback_device_model_done(egc, dsps, rc);
+}
+
+static void dm_state_save_to_fdset(libxl__egc *egc, libxl__ev_qmp *ev, int fdset)
+{
+    EGC_GC;
+    int rc;
+    libxl__json_object *args = NULL;
+    libxl__domain_suspend_state *dsps = CONTAINER_OF(ev, *dsps, qmp);
 
     ev->callback = dm_state_saved;
 
@@ -1060,7 +1082,8 @@  static void dm_state_fd_ready(libxl__egc *egc, libxl__ev_qmp *ev,
 
 error:
     assert(rc);
-    libxl__remove_file(gc, dsps->dm_savefile);
+    if (!libxl_get_stubdom_id(CTX, dsps->domid))
+        libxl__remove_file(gc, dsps->dm_savefile);
     dsps->callback_device_model_done(egc, dsps, rc);
 }