Re: [PATCH v3 COLOPre 19/26] libxc/migration: Specification update for DIRTY_BITMAP records

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

On Wed, 2015-07-01 at 11:16 +0100, Andrew Cooper wrote:
> On 01/07/15 04:07, Yang Hongyang wrote:
> >
> >
> > On 06/30/2015 06:24 PM, Ian Campbell wrote:
> >> On Thu, 2015-06-25 at 14:25 +0800, Yang Hongyang wrote:
> >>> Used by secondary to send it's dirty bitmap to primary under COLO.
> >>
> >> This is the backchannel, right?
> >
> > Right.
> >
> >>
> >> It seems to me that this ought to be described more clearly as a
> >> separate stream in the opposite direction, rather than looking like just
> >> another record in the forward channel.
> >
> > Agreed, I'm not sure if having this back channel record is eligible,
> > Andy, thoughts?
> >
> >>
> >> Does the back channel not also need some sort of negotiation phase where
> >> we check both ends are compatible (i.e. like the forward channel's
> >> header). This might be easier than with the forward channel since you
> >> might assert that the versions must match exactly for COLO to be
> >> possible, that might not be true of some potential future user of the
> >> backchannel though.
> >
> > The negotiation record for COLO is introduced in the following patch
> > on libxl side. But that might be diffrent form what you said here, we
> > don't have a version check currently, if the 2 side doesn't match, for
> > example one has colo feature enabled and the other end do not, the
> > migration will simply fail.
> 
> I do think that each backchannel level needs some kind of initial
> negotiation to confirm everything is set up and working, but I think the
> backchannel should also match the spec for its level, and all contained
> in the single spec document.

In the same spec, sure. It's the presenting it as just another record
mixed in with all the others which I think is a problem.

At the very least every record should be tagged as either forward,
backward or bidirectional to indicate who can produce and who should
consume it.

Even better would br if we can convince ourselves there should be no
bidirectional fields, in which case I'd be further inclined to say that
the record space should be explicitly separate. i.e. the backchannel
should be a separate chapter in the doc and the records.

> So for both the libxc and libxl backchannels, I would have thought
> something along these lines to be sensible:
> 
> Forwards sends a LIBX{C,L}_BACKCHANNEL_INIT, and waits to find a
> LIBX{C,L}_BACKCHANNEL_REPLY on the backchannel.  After that, processing
> continues as normal, with records arriving on the backchannel when
> appropriate.

Yes, for init time something like this sounds sensible.

Ian.