[PATCH] libxl: Rewind toolstack_save_fd in libxl_save_helper when using remus

[PATCH] libxl: Rewind toolstack_save_fd in libxl_save_helper when using remus

[Yang Hongyang]

commit(b327a3f421bb57d262b7d1fb3c43b710852b103b) move the rewinding
of toolstack_save_fd to libxl, this breaks remus, because in remus
mode, toolstack_save_cb will be called in every checkpoint, if
we don't rewind it in libxl_save_helper, it will surely encounted
error.

I know this fix is just a hack, in fact the whole toolstack save
thing should be done in libxl. But this fix should solve Jason's
case and remus thing. Until migration v2 is upstreamed, this fix
should be fine.

Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
CC: Jason Andryuk <andryuk@aero.org>
CC: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Ian Jackson <ian.jackson@eu.citrix.com>
---
 tools/libxl/libxl_save_helper.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/tools/libxl/libxl_save_helper.c b/tools/libxl/libxl_save_helper.c
index b259bd0..4619fa3 100644
--- a/tools/libxl/libxl_save_helper.c
+++ b/tools/libxl/libxl_save_helper.c
@@ -159,6 +159,7 @@ int helper_getreply(void *user)
 
 static int toolstack_save_fd;
 static uint32_t toolstack_save_len;
+static struct save_callbacks helper_save_callbacks;
 
 static int toolstack_save_cb(uint32_t domid, uint8_t **buf,
                              uint32_t *len, void *data)
@@ -167,6 +168,12 @@ static int toolstack_save_cb(uint32_t domid, uint8_t **buf,
 
     assert(toolstack_save_fd > 0);
 
+    /* This is a hack for remus */
+    if (helper_save_callbacks.checkpoint) {
+        r = lseek(toolstack_save_fd, 0, SEEK_SET);
+        if (r) fail(errno,"rewind toolstack data tmpfile");
+    }
+
     *buf = xmalloc(toolstack_save_len);
     r = read_exactly(toolstack_save_fd, *buf, toolstack_save_len);
     if (r<0) fail(errno,"read toolstack data");
@@ -191,7 +198,6 @@ static void complete(int retval) {
     exit(0);
 }
 
-static struct save_callbacks helper_save_callbacks;
 static struct restore_callbacks helper_restore_callbacks;
 
 int main(int argc, char **argv)
-- 
1.7.1

Re: [PATCH] libxl: Rewind toolstack_save_fd in libxl_save_helper when using remus

[Andrew Cooper]

On 20/06/14 07:59, Yang Hongyang wrote:
> commit(b327a3f421bb57d262b7d1fb3c43b710852b103b) move the rewinding
> of toolstack_save_fd to libxl, this breaks remus, because in remus
> mode, toolstack_save_cb will be called in every checkpoint, if
> we don't rewind it in libxl_save_helper, it will surely encounted
> error.
>
> I know this fix is just a hack, in fact the whole toolstack save
> thing should be done in libxl. But this fix should solve Jason's
> case and remus thing. Until migration v2 is upstreamed, this fix
> should be fine.
>
> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
> CC: Jason Andryuk <andryuk@aero.org>
> CC: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Ian Jackson <ian.jackson@eu.citrix.com>

Hmm - this is very hacky, but I don't see an alternative.

As b327a3f421 did regress remus, this fix should be taken.  It will only
be temporary until the libxl layering violation fixes from migration v2
appear.

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>


> ---
>  tools/libxl/libxl_save_helper.c |    8 +++++++-
>  1 files changed, 7 insertions(+), 1 deletions(-)
>
> diff --git a/tools/libxl/libxl_save_helper.c b/tools/libxl/libxl_save_helper.c
> index b259bd0..4619fa3 100644
> --- a/tools/libxl/libxl_save_helper.c
> +++ b/tools/libxl/libxl_save_helper.c
> @@ -159,6 +159,7 @@ int helper_getreply(void *user)
>  
>  static int toolstack_save_fd;
>  static uint32_t toolstack_save_len;
> +static struct save_callbacks helper_save_callbacks;
>  
>  static int toolstack_save_cb(uint32_t domid, uint8_t **buf,
>                               uint32_t *len, void *data)
> @@ -167,6 +168,12 @@ static int toolstack_save_cb(uint32_t domid, uint8_t **buf,
>  
>      assert(toolstack_save_fd > 0);
>  
> +    /* This is a hack for remus */
> +    if (helper_save_callbacks.checkpoint) {
> +        r = lseek(toolstack_save_fd, 0, SEEK_SET);
> +        if (r) fail(errno,"rewind toolstack data tmpfile");
> +    }
> +
>      *buf = xmalloc(toolstack_save_len);
>      r = read_exactly(toolstack_save_fd, *buf, toolstack_save_len);
>      if (r<0) fail(errno,"read toolstack data");
> @@ -191,7 +198,6 @@ static void complete(int retval) {
>      exit(0);
>  }
>  
> -static struct save_callbacks helper_save_callbacks;
>  static struct restore_callbacks helper_restore_callbacks;
>  
>  int main(int argc, char **argv)

Re: [PATCH] libxl: Rewind toolstack_save_fd in libxl_save_helper when using remus

[Jason Andryuk]

On 6/20/2014 2:59 AM, Yang Hongyang wrote:
> commit(b327a3f421bb57d262b7d1fb3c43b710852b103b) move the rewinding
> of toolstack_save_fd to libxl, this breaks remus, because in remus
> mode, toolstack_save_cb will be called in every checkpoint, if
> we don't rewind it in libxl_save_helper, it will surely encounted
> error.
> 
> I know this fix is just a hack, in fact the whole toolstack save
> thing should be done in libxl. But this fix should solve Jason's
> case and remus thing. Until migration v2 is upstreamed, this fix
> should be fine.
> 
> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
> CC: Jason Andryuk <andryuk@aero.org>
> CC: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Ian Jackson <ian.jackson@eu.citrix.com>

Whoops, I didn't realize I broke Remus.  

A quick test has migration still working for me.

-Jason

> ---
>   tools/libxl/libxl_save_helper.c |    8 +++++++-
>   1 files changed, 7 insertions(+), 1 deletions(-)
> 
> diff --git a/tools/libxl/libxl_save_helper.c b/tools/libxl/libxl_save_helper.c
> index b259bd0..4619fa3 100644
> --- a/tools/libxl/libxl_save_helper.c
> +++ b/tools/libxl/libxl_save_helper.c
> @@ -159,6 +159,7 @@ int helper_getreply(void *user)
>   
>   static int toolstack_save_fd;
>   static uint32_t toolstack_save_len;
> +static struct save_callbacks helper_save_callbacks;
>   
>   static int toolstack_save_cb(uint32_t domid, uint8_t **buf,
>                                uint32_t *len, void *data)
> @@ -167,6 +168,12 @@ static int toolstack_save_cb(uint32_t domid, uint8_t **buf,
>   
>       assert(toolstack_save_fd > 0);
>   
> +    /* This is a hack for remus */
> +    if (helper_save_callbacks.checkpoint) {
> +        r = lseek(toolstack_save_fd, 0, SEEK_SET);
> +        if (r) fail(errno,"rewind toolstack data tmpfile");
> +    }
> +
>       *buf = xmalloc(toolstack_save_len);
>       r = read_exactly(toolstack_save_fd, *buf, toolstack_save_len);
>       if (r<0) fail(errno,"read toolstack data");
> @@ -191,7 +198,6 @@ static void complete(int retval) {
>       exit(0);
>   }
>   
> -static struct save_callbacks helper_save_callbacks;
>   static struct restore_callbacks helper_restore_callbacks;
>   
>   int main(int argc, char **argv)
> 

Re: [PATCH] libxl: Rewind toolstack_save_fd in libxl_save_helper when using remus

[Ian Jackson]

Jason Andryuk writes ("Re: [PATCH] libxl: Rewind toolstack_save_fd in libxl_save_helper when using remus"):
> On 6/20/2014 2:59 AM, Yang Hongyang wrote:
> > commit(b327a3f421bb57d262b7d1fb3c43b710852b103b) move the rewinding
> > of toolstack_save_fd to libxl, this breaks remus, because in remus
> > mode, toolstack_save_cb will be called in every checkpoint, if
> > we don't rewind it in libxl_save_helper, it will surely encounted
> > error.
> > 
> > I know this fix is just a hack, in fact the whole toolstack save
> > thing should be done in libxl. But this fix should solve Jason's
> > case and remus thing. Until migration v2 is upstreamed, this fix
> > should be fine.
> > 
> > Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
> > CC: Jason Andryuk <andryuk@aero.org>
> > CC: Andrew Cooper <andrew.cooper3@citrix.com>
> > CC: Ian Jackson <ian.jackson@eu.citrix.com>
> 
> Whoops, I didn't realize I broke Remus.  
> 
> A quick test has migration still working for me.

Thanks to Yang Hongyang for the patch, to Andrew for the review, and
Jason for the test.  I have applied it.  I reworded the commit
message.

Regards,
Ian.