Re: [Xen-devel] [PATCH v4 1/5] remus: don't call stream_continue() when doing failover

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

On Mon, 2016-01-18 at 13:40 +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@xxxxxxxxxxxxxx>
> Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> Âtools/libxl/libxl_stream_read.c | 35 ++++++++++++++++++++++++++++++-----
> Â1 file changed, 30 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/libxl/libxl_stream_read.c
> b/tools/libxl/libxl_stream_read.c
> index 258dec4..24305f4 100644
> --- a/tools/libxl/libxl_stream_read.c
> +++ b/tools/libxl/libxl_stream_read.c
> @@ -101,6 +101,19 @@
> Â *ÂÂÂÂ- stream_write_emulator_done()
> Â *ÂÂÂÂ- stream_continue()
> Â *
> + * 4) Failover for remus

I don't think this is really #4 in the list which precedes it. I think a
section "Failover for remus" would be absolutely fine right that the end of
this comment block though, i.e. right after the "Depending on the
contents..." paragraph.

Andy?

> + *ÂÂÂÂ- we buffer all records until a CHECKPOINT_END record is received
> + *ÂÂÂÂ- we will use the records when a CHECKPOINT_END record is received

"we will consume the buffered records..."


> + *ÂÂÂÂ- if we find some internal error, the rc or retval is not 0 in

s/the/then/

> + *ÂÂÂÂÂÂlibxl__xc_domain_restore_done(). In this case, we don't resume the
> + *ÂÂÂÂÂÂguest
> + *ÂÂÂÂ- if we need to do failover from primary, the rc and retval are 0

s/the/then/ again and I would say "are both 0" for clarity (assuming that
is indeed the requirement).

> + *ÂÂÂÂÂÂin libxl__xc_domain_restore_done(). In this case, the buffered state
> + *ÂÂÂÂÂÂwill be dropped, because we don't receive a CHECKPOINT_END record,

                   Âhaven't received

> + *ÂÂÂÂÂÂand it is a inconsistent state. In libxl__xc_domain_restore_done(),

"an inconsistent".

I think I would say "... and therefore the buffered state is inconsistent".

-ÂÂÂÂÂÂÂÂstream_continue(egc, stream);
> +ÂÂÂÂÂÂÂÂif (checkpointed_stream) {
> +ÂÂÂÂÂÂÂÂÂÂÂÂ/*
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂ* Failover from primary. Domain state is currently at a
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂ* consistent checkpoint, complete the stream, and call
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂ* stream->completion_callback() to resume the guest.
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂ*/
> +ÂÂÂÂÂÂÂÂÂÂÂÂstream_complete(egc, stream, 0);

Is it possible to get here having never received a single CHECKPOINT_END?

Ian.

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