Re: [Xen-devel] [PATCH v4 --for 4.6 COLOPre 12/25] tools/libxl: introduce enum type libxl_checkpointed_stream

[Yang Hongyang <yanghy@xxxxxxxxxxxxxx>]

On 07/15/2015 08:34 PM, Ian Campbell wrote:
> On Wed, 2015-07-15 at 15:45 +0800, Yang Hongyang wrote:
> > introduce enum type libxl_checkpointed_stream in IDL.
> > rename the last argument of migrate_receive from "remus" to
> > "checkpointed" since the semantics of this parameter has
> > changed.
> >
> > NOTE:
> >   libxl_domain_restore_params isn't changed here,
> >   checkpointed_stream is still an int.
> >   It has to change eventually and other callers will have to be
> >   updated to cope (and there should be LIBXL_HAVE_...).
> Will this be fixed up later in this series? If so please say so.
It's not fixed in this series, I plan to fix this later, but seems there
will be another round for this series, I can fix this in the next version.
My main concern is that this change is an api change, it will affect the
existing callers.

> > @@ -4282,7 +4282,7 @@ static void migrate_domain(uint32_t domid, const char 
> > *rune, int debug,
> >   }
> >
> >   static void migrate_receive(int debug, int daemonize, int monitor,
> > -                            int send_fd, int recv_fd, int remus)
> > +                            int send_fd, int recv_fd, int checkpointed)
> I think you can start using the new enum type in xl straight away even
> if dom_info.checkpointed_stream remains an int. So that means here.
>
> > @@ -4489,7 +4489,8 @@ int main_restore(int argc, char **argv)
> >
> >   int main_migrate_receive(int argc, char **argv)
> >   {
> > -    int debug = 0, daemonize = 1, monitor = 1, remus = 0;
> > +    int debug = 0, daemonize = 1, monitor = 1;
> > +    int checkpointed = LIBXL_CHECKPOINTED_STREAM_NONE;
> and here.
>
> > @@ -4318,7 +4318,7 @@ static void migrate_receive(int debug, int daemonize, int 
> > monitor,
> >
> >       domid = rc;
> >
> > -    if (remus) {
> > +    if (checkpointed) {
> >           /* If we are here, it means that the sender (primary) has crashed.
> >            * TODO: Split-Brain Check.
> >            */
> Is it the case that we expect all check pointing solutions will use the
> same failover code here? If yes then this should be "if (checkpointed !
> = ...NONE)".
>
> If we think they might differ (even if remus and colo happen to be the
> same) then I think a switch where the NONE case does nothing would be
> more structurally appropriate.
>
> Ian.
>
> .
--
Thanks,
Yang.

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