Re: [PATCH v8 --for 4.6 COLO 08/25] tools/libxl: handle colo_context records in a libxl migration v2 read stream

[Yang Hongyang <yanghy@cn.fujitsu.com>]

On 07/16/2015 01:44 AM, Andrew Cooper wrote:
> On 15/07/15 10:18, Yang Hongyang wrote:
[...]
>> +
>>       if (!stream->rc)
>>           stream->rc = rc;
>>       stream_done(egc, stream);
>> @@ -794,6 +815,36 @@ static void check_all_finished(libxl__egc *egc,
>>       stream->completion_callback(egc, stream, stream->rc);
>>   }
>>
>> +/*----- COLO context handlers -----*/
>> +
>> +void libxl__stream_read_colo_context(libxl__egc *egc,
>> +                                     libxl__stream_read_state *stream)
>
> A name like this makes the erroneous assumption that a COLO\_CONTEXT
> record is what is going to be found next in the stream.
>
> Where and when is a COLO\_CONTEXT record expected, and is it only in the
> backchannel?

A COLO CONTEXT is actually a set of control command (I'm not sure command
here is a suitable description) which used to sync the checkpoint steps between
primary and secondary. So it is not only in the back channel.

control_id       0x00000000: Secondary VM is out of sync, start a new checkpoint
                  0x00000001: Secondary VM is suspended
                  0x00000002: Secondary VM is ready
                  0x00000003: Secondary VM is resumed

First boot:
When doing COLO, primary must be start with -p, then start COLO, the
first step is live migration, after migration, when secondary is ready,
we will resume both side.


   control_id      Primary                 Secondary
                   start with -p
                   live migrate
                                           Receive&load state
   0x00000002                              @
                   Resume                  Resume
   0x00000003                              @
                   Start Comparing Packets


At checkpoint:

   control_id      Primary                 Secondary
   0x00000000      @
                                           Suspend
   0x00000001                              @
                   Suspend
                   Send state              Receive state
                   Flush Network           Load state
                   Resume                  Resume
   0x00000003                              @
                   Start Comparing Packets

NOTE:
  1) '@' who sends the message
  2) Every sync-point is synchronized by two sides with only
     one handshake(single direction) for low-latency.
     If more strict synchronization is required, a opposite direction
     sync-point should be added.
  3) Since sync-points are single direction, the remote side may
     go forward a lot when this side just receives the sync-point.

>
>> +{
>> +    assert(stream->running);
>> +    assert(!stream->in_checkpoint);
>> +    assert(!stream->in_colo_context);
>> +    stream->in_colo_context = true;
>> +
>> +    setup_read_record(egc, stream);
>> +}
>> +
>> +static void handle_colo_context(libxl__egc *egc,
>> +                                libxl__stream_read_state *stream,
>> +                                libxl__sr_record_buf *rec)
>> +{
>> +    libxl_sr_colo_context *colo_context = rec->body;
>> +
>> +    colo_context_done(egc, stream, colo_context->id);
>
> A handler this trivial should just be done in the switch statement in
> process_record().  No need for its own function for a single forward call.

Ok

>
> ~Andrew
> .
>

-- 
Thanks,
Yang.