Re: [PATCH v4 --for 4.6 COLOPre 24/25] tools/libxl: move remus state into a seperate structure

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

On Wed, 2015-07-15 at 21:50 +0800, Yang Hongyang wrote:
> 
> On 07/15/2015 09:28 PM, Ian Campbell wrote:
> > On Wed, 2015-07-15 at 15:45 +0800, Yang Hongyang wrote:
> >> @@ -2921,6 +2911,26 @@ _hidden void libxl__checkpoint_devices_preresume(libxl__egc *egc,
> >>                                           libxl__checkpoint_devices_state *cds);
> >>   _hidden void libxl__checkpoint_devices_commit(libxl__egc *egc,
> >>                                           libxl__checkpoint_devices_state *cds);
> >> +
> >> +/*----- Remus related state structure -----*/
> >> +typedef struct libxl__remus_state libxl__remus_state;
> >> +struct libxl__remus_state {
> >> +    /* private */
> >> +    libxl__ev_time checkpoint_timeout; /* used for Remus checkpoint */
> >> +    int interval; /* checkpoint interval */
> >> +
> >> +    /* abstract layer */
> >> +    libxl__checkpoint_devices_state cds;
> >
> > This mostly makes sense, I think, but this one field feels like it will
> > be wanted by colo too. Does that mean we will end up with dss->rs.cds
> > and dss->colo.cds doing effectively the same thing?
> 
> Yes, checkpoint device is an abstract layer, used by both Remus & colo,
> in the abstract layer, we do not aware of remus or colo, in Remus or colo,
> we can use container of cds to retrive Remus/colo state.

This is because the cds callbacks receive a
libxl__checkpoint_devices_state * but are specific to either Remus of
Colo?

I think the usual way to solve that would be for the callback to take a
void *data "closure" field, which is registered along with the callbacks
and passed to all callbacks, or in this case perhaps you can get away
with just including it in the cds itself.

Ian, what do you think?

Ian.