Re: [PATCH] libxl: Reset toolstack_save file position in libxl

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

On 17/05/14 16:35, Jason Andryuk wrote:
> toolstack_save data is written to a temporary file in libxl and read
> back in libxl-save-helper.  The file position must be reset prior to
> reading the file, which is done in libxl-save-helper with lseek.
>
> lseek is unsupported for pipes and sockets, so a wrapper passing such an
> fd to libxl-save-helper fails the lseek.  Moving the lseek to libxl
> avoids the error, allowing the save to continue.
>
> Signed-off-by: Jason Andryuk <andryuk@aero.org>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> with one minor nit.

FWIW, "toolstack data" will be abolished in its entirety as part of the
new migration work, but this fix could do with backporting.

~Andrew

> ---
>  tools/libxl/libxl_save_callout.c | 6 ++++++
>  tools/libxl/libxl_save_helper.c  | 5 ++---
>  2 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/tools/libxl/libxl_save_callout.c b/tools/libxl/libxl_save_callout.c
> index e3bda8f..d907e2d 100644
> --- a/tools/libxl/libxl_save_callout.c
> +++ b/tools/libxl/libxl_save_callout.c
> @@ -105,6 +105,12 @@ 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; }
> +
> +        /* 
> +         * file position must be reset before passing to libxl-save-helper.
> +         */

This could easily be a single line comment.

> +        r = lseek(toolstack_data_fd, 0, SEEK_SET);
> +        if (r) { 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 c36314c..b259bd0 100644
> --- a/tools/libxl/libxl_save_helper.c
> +++ b/tools/libxl/libxl_save_helper.c
> @@ -163,10 +163,9 @@ static uint32_t toolstack_save_len;
>  static int toolstack_save_cb(uint32_t domid, uint8_t **buf,
>                               uint32_t *len, void *data)
>  {
> -    assert(toolstack_save_fd > 0);
> +    int r;
>  
> -    int r = lseek(toolstack_save_fd, 0, SEEK_SET);
> -    if (r) fail(errno,"rewind toolstack data tmpfile");
> +    assert(toolstack_save_fd > 0);
>  
>      *buf = xmalloc(toolstack_save_len);
>      r = read_exactly(toolstack_save_fd, *buf, toolstack_save_len);