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

[Shriram Rajagopalan <rshriram@cs.ubc.ca>]

[-- Attachment #1.1: Type: text/plain, Size: 3673 bytes --]

On Wed, Jun 27, 2012 at 11:59 AM, Ian Jackson <Ian.Jackson@eu.citrix.com>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;
>  }
>
>

[-- Attachment #1.2: Type: text/html, Size: 4715 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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