Re: [Xen-devel] [PATCH v3 COLOPre 04/26] tools/libxl: move remus code into libxl_remus.c

[Yang Hongyang <yanghy@xxxxxxxxxxxxxx>]

On 06/29/2015 11:48 PM, Ian Campbell wrote:
> On Thu, 2015-06-25 at 14:25 +0800, Yang Hongyang wrote:
> > Do the following things:
> > - There are 2 checkpoint callbacks, rename them to:
> >    libxl__remus_domain_{save/restore}_checkpoint_callback
> > - Moving remus callbacks into libxl_remus.c, export following
> >    callbacks for internal use:
> >    * libxl__remus_domain_suspend_callback
> >    * libxl__remus_domain_resume_callback
> >    * libxl__remus_domain_save_checkpoint_callback
> >    * libxl__remus_domain_restore_checkpoint_callback
> > - Refactoring Remus setup/teardown, introduce 2 APIs to
> >    setup/teardown Remus:
> >    * libxl__remus_setup
> >    * libxl__remus_teardown
> >
> > Signed-off-by: Yang Hongyang <yanghy@xxxxxxxxxxxxxx>
> > [...]
> > +/* Remus callbacks for save */
> > +_hidden void libxl__remus_domain_suspend_callback(void *data);
> > +_hidden void libxl__remus_domain_resume_callback(void *data);
> At first glance having resume under save rather than under restore
> seemed odd, but I suppose it is just that you resume after a failed
> save, but on restore you just start the VM, is that the right logic?
With Remus, save side should continue to work after migration and
after every checkpoint, while we need to suspend guest to transfer dirty
pages, we must resume it after.

> > +_hidden void libxl__remus_domain_save_checkpoint_callback(void *data);
> > +/* Remus callbacks for restore */
> > +_hidden void libxl__remus_domain_restore_checkpoint_callback(void *data);
> > +/* Remus setup and teardown*/
> > +_hidden void libxl__remus_setup(libxl__egc *egc,
> > +                                libxl__domain_suspend_state *dss);
> > +_hidden void libxl__remus_teardown(libxl__egc *egc,
> > +                                   libxl__domain_suspend_state *dss,
> > +                                   int rc);
> >
> >   /*
> >    * Convenience macros.
> > diff --git a/tools/libxl/libxl_remus.c b/tools/libxl/libxl_remus.c
> > new file mode 100644
> > index 0000000..504cd44
> > --- /dev/null
> > +++ b/tools/libxl/libxl_remus.c
> > @@ -0,0 +1,325 @@
> > +/*
> > + * Copyright (c) 2015 FUJITSU LIMITED
> > + * Author Yang Hongyang <yanghy@xxxxxxxxxxxxxx>
> Given the amount of this which is code motion I think you should also be
> copying the existing copyright and licenses from the files where the
> code originated.
Sure, will add it in the next version.

> Ian.
>
> .
--
Thanks,
Yang.

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