Re: [Xen-devel] [PATCH v5 00/21] libxl: domain save/restore: run in a separate process

[Shriram Rajagopalan <rshriram@xxxxxxxxx>]

On Wed, Jun 27, 2012 at 11:59 AM, Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx> wrote:

Ian Jackson writes ("Re: [PATCH v5 00/21] libxl: domain save/restore: run in a separate process"):

> However, when I apply my series, I can indeed produce an assertion
> failure:

...

> So I have indeed made matters worse.

I found two bugs:

1. The void* passed to the callback was being treated as a
libxl__domain_suspend_state* by the remus callbacks; this is a
holdover from a much earlier version of the series.  It should be
converted to a libxl__save_helper_state and then the dss extracted
with CONTAINER_OF.

2. The way remus works means that the toolstack save callback is
invoked more than once, which the helper's implementation was not
prepared to deal with.  Fix this by moving the rewind of the fd into
the helper.

Fixes for these are below.  With this, on top of my series, seem to I
get the same behaviour as with the baseline.  Would you like to try it ?

Sure, I ll give it a shot.

Btw, my earlier mail was in response to remus not

working on the baseline setup on your dev environment.

 

Thanks,
Ian.

diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index abc5932..069aca1 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -984,7 +984,8 @@ static int libxl__remus_domain_suspend_callback(void *data)

 static int libxl__remus_domain_resume_callback(void *data)

 {
-    libxl__domain_suspend_state *dss = data;

+    libxl__save_helper_state *shs = data;

+    libxl__domain_suspend_state *dss = CONTAINER_OF(shs, *dss, shs);
    STATE_AO_GC(dss->ao);

    /* Resumes the domain and the device model */
@@ -1002,7 +1003,8 @@ static void remus_checkpoint_dm_saved(libxl__egc *egc,

 static void libxl__remus_domain_checkpoint_callback(void *data)

 {
-    libxl__domain_suspend_state *dss = data;

+    libxl__save_helper_state *shs = data;
+    libxl__domain_suspend_state *dss = CONTAINER_OF(shs, *dss, shs);
    libxl__egc *egc = dss->shs.egc;
    STATE_AO_GC(dss->ao);

diff --git a/tools/libxl/libxl_save_callout.c b/tools/libxl/libxl_save_callout.c
index 6332beb..078b7ee 100644
--- a/tools/libxl/libxl_save_callout.c
+++ b/tools/libxl/libxl_save_callout.c
@@ -105,13 +105,6 @@ void libxl__xc_domain_save(libxl__egc *egc, libxl__domain_suspend_state *dss,
                                toolstack_data_buf, toolstack_data_len,

                                "toolstack data tmpfile", 0);

        if (r) { rc = ERROR_FAIL; goto out; }

-
-        r = lseek(toolstack_data_fd, 0, SEEK_SET);
-        if (r) {
-            LOGE(ERROR, "rewind toolstack data tmpfile");

-            rc = ERROR_FAIL;
-            goto out;

-        }
    }

    const unsigned long argnums[] = {

diff --git a/tools/libxl/libxl_save_helper.c b/tools/libxl/libxl_save_helper.c
index 3bdfa28..772251a 100644
--- a/tools/libxl/libxl_save_helper.c
+++ b/tools/libxl/libxl_save_helper.c
@@ -171,12 +171,14 @@ static int toolstack_save_cb(uint32_t domid, uint8_t **buf,
 {
    assert(toolstack_save_fd > 0);

+    int r = lseek(toolstack_save_fd, 0, SEEK_SET);
+    if (r) fail(errno,"rewind toolstack data tmpfile");
+

    *buf = xmalloc(toolstack_save_len);
-    int r = read_exactly(toolstack_save_fd, *buf, toolstack_save_len);
+    r = read_exactly(toolstack_save_fd, *buf, toolstack_save_len);
    if (r<0) fail(errno,"read toolstack data");

    if (r==0) fail(0,"read toolstack data eof");

-    toolstack_save_fd = -1;
    *len = toolstack_save_len;
    return 0;
 }

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel