Re: [PATCH v3 1/5] remus: don't call stream_continue() when doing failover

[Ian Campbell <ian.campbell@citrix.com>]

On Fri, 2016-01-08 at 14:38 +0800, Wen Congyang wrote:
> stream_continue() is used for migration to read emulator
> xenstore data and emulator context. For remus, if we do
> failover, we have read it in the checkpoint cycle, and
> we only need to complete the stream.
> 
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
>  tools/libxl/libxl_stream_read.c | 21 ++++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/libxl/libxl_stream_read.c
> b/tools/libxl/libxl_stream_read.c
> index 258dec4..65219d5 100644
> --- a/tools/libxl/libxl_stream_read.c
> +++ b/tools/libxl/libxl_stream_read.c
> @@ -758,6 +758,9 @@ void libxl__xc_domain_restore_done(libxl__egc *egc,
> void *dcs_void,
>      libxl__stream_read_state *stream = &dcs->srs;
>      STATE_AO_GC(dcs->ao);
>  
> +    /* convenience aliases */
> +    const int checkpointed_stream = dcs-
> >restore_params.checkpointed_stream;
> +
>      if (rc)
>          goto err;
>  
> @@ -777,11 +780,19 @@ void libxl__xc_domain_restore_done(libxl__egc *egc,
> void *dcs_void,
>       * If the stream is not still alive, we must not continue any work.
>       */
>      if (libxl__stream_read_inuse(stream)) {
> -        /*
> -         * Libxc has indicated that it is done with the stream.  Resume
> reading
> -         * libxl records from it.
> -         */
> -        stream_continue(egc, stream);
> +        if (checkpointed_stream) {
> +            /*
> +             * Failover from primary. Domain state is currently at a
> +             * consistent checkpoint, ready to go.

This implies that the stream is currently at a consistent point. Whereas
what I think is meant is that things have failed (perhaps halfway through a
checkpoint, i.e. not at a consistent state), therefore we stop and continue
with the previous fully consistent checkpoint (which may have been earlier
in the stream, not at the current point). Is that right?

And what does "ready to go" mean? Does it mean that we will return back to
the next higher level or that we go somewhere else first?

The big comment about flow control at the top of this file doesn't seem to
cover the checkpoint case, if it did I suspect I would have found the
answers there.

> +             */
> +            stream_complete(egc, stream, 0);
> +        } else {
> +            /*
> +             * Libxc has indicated that it is done with the stream.
> +             * Resume reading libxl records from it.
> +             */
> +            stream_continue(egc, stream);
> +        }
>      }
>  }
>  
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel