Re: [Xen-devel] [PATCH for-4.7] tools/libxl: Fix legacy migration following COLO backchannel breakage

[Wen Congyang <wency@xxxxxxxxxxxxxx>]

On 04/15/2016 03:54 AM, Andrew Cooper wrote:
> c/s f5d947bf1b "tools/libxl: add back channel support to read stream"
> made a bogus adjustment to libxl__stream_read_start(), including
> removing the comment hinting at what was going on, which breaks
> conversion of a legacy migration stream.
> 
> Symptoms look like:
> 
>   root@anonymi:~ # xl migrate domU host
>   migration target: Ready to receive domain.
>   Saving to migration stream new xl format (info 0x1/0x0/2677)
>   xc: error: error polling suspend notification channel: -1: Internal error
>   Loading new save file <incoming migration stream> (new xl fmt info 
> 0x1/0x0/2677)
>    Savefile contains xl domain config in JSON format
>   Parsing config from <saved>
>   libxl: error: libxl_stream_read.c:327:stream_header_done: Invalid ident: 
> expected 0x4c6962786c466d74, got 0x01f00f0000000000
>   libxl: error: libxl_utils.c:430:libxl_read_exactly: file/stream truncated 
> reading ipc msg header from domain 1 save/restore helper stdout pipe
> 
> The adjustment is not required for backchannel support (as there is no
> interaction between back channels and legacy conversion), and caused
> stream->fd to be latched in the datacopier before legacy conversion
> substitutes it for the fd which is the output of the conversion script.
> 
> This causes libxl to consume data from the legacy stream rather than the
> v2 stream, and for the conversion script to encounter an error as the
> legacy stream appears to skip ahead.
> 
> Undo the adjustments to libxl__stream_read_start(), and introduce a
> better description of what is going on.  Introduce some extra assertions
> to try and catch similar breakage in the future.
> 
> Reported-by: Olaf Hering <olaf@xxxxxxxxx>
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Reviewed-by: Wen Congyang <wency@xxxxxxxxxxxxxx>

> ---
> CC: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
> CC: Wei Liu <wei.liu2@xxxxxxxxxx>
> CC: Olaf Hering <olaf@xxxxxxxxx>
> CC: Yang Hongyang <hongyang.yang@xxxxxxxxxxxx>
> CC: Wen Congyang <wency@xxxxxxxxxxxxxx>
> CC: Changlong Xie <xiecl.fnst@xxxxxxxxxxxxxx>
> ---
>  tools/libxl/libxl_stream_read.c | 33 ++++++++++++++++++++++++---------
>  1 file changed, 24 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/libxl/libxl_stream_read.c b/tools/libxl/libxl_stream_read.c
> index 9659051..89c2f21 100644
> --- a/tools/libxl/libxl_stream_read.c
> +++ b/tools/libxl/libxl_stream_read.c
> @@ -234,16 +234,16 @@ void libxl__stream_read_start(libxl__egc *egc,
>      stream->running = true;
>      stream->phase   = SRS_PHASE_NORMAL;
>  
> -    dc->ao       = stream->ao;
> -    dc->copywhat = "restore v2 stream";
> -    dc->readfd = stream->fd;
> -    dc->writefd  = -1;
> -
> -    if (stream->back_channel)
> -        return;
> -
>      if (stream->legacy) {
> -        /* Convert the legacy stream. */
> +        /*
> +         * Convert the legacy stream.
> +         *
> +         * This results in a fork()/exec() of conversion helper script.  It 
> is
> +         * passed the exiting stream->fd as an input, and returns the
> +         * transformed stream via a new pipe.  The fd of this new pipe then
> +         * replaces stream->fd, to make the rest of the stream read code
> +         * agnostic to whether legacy conversion is happening or not.
> +         */
>          libxl__conversion_helper_state *chs = &stream->chs;
>  
>          chs->legacy_fd = stream->fd;
> @@ -258,10 +258,25 @@ void libxl__stream_read_start(libxl__egc *egc,
>              goto err;
>          }
>  
> +        /* There should be no interaction of COLO backchannels and legacy
> +         * stream conversion. */
> +        assert(!stream->back_channel);
> +
> +        /* Confirm *dc is still zeroed out, while we shuffle stream->fd. */
> +        assert(dc->ao == NULL);
>          assert(stream->chs.v2_carefd);
>          stream->fd = libxl__carefd_fd(stream->chs.v2_carefd);
>          stream->dcs->libxc_fd = stream->fd;
>      }
> +    /* stream->fd is now a v2 stream. */
> +
> +    dc->ao       = stream->ao;
> +    dc->copywhat = "restore v2 stream";
> +    dc->readfd   = stream->fd;
> +    dc->writefd  = -1;
> +
> +    if (stream->back_channel)
> +        return;
>  
>      /* Start reading the stream header. */
>      rc = setup_read(stream, "stream header",
> 




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