Re: [Xen-devel] [PATCH 2/2] Introduce migration precopy policy

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Mon, Sep 18, 2017 at 10:05:07AM +0100, Paul Durrant wrote:
> > -----Original Message-----
> > From: Xen-devel [mailto:xen-devel-bounces@xxxxxxxxxxxxx] On Behalf Of
> > Jennifer Herbert
> > Sent: 14 September 2017 16:34
> > To: Ian Jackson <Ian.Jackson@xxxxxxxxxx>; Wei Liu <wei.liu2@xxxxxxxxxx>;
> > xen-devel@xxxxxxxxxxxxxxxxxxxx; jtotto@xxxxxxxxxxxx
> > Cc: Jennifer Herbert <jennifer.herbert@xxxxxxxxxx>
> > Subject: [Xen-devel] [PATCH 2/2] Introduce migration precopy policy
> > 
> > This Patch allows a migration precopy policy to be specified.
> > 
> > The precopy phase of the xc_domain_save() live migration algorithm has
> > historically been implemented to run until either a) (almost) no pages
> > are dirty or b) some fixed, hard-coded maximum number of precopy
> > iterations has been exceeded.  This policy and its implementation are
> > less than ideal for a few reasons:
> > - the logic of the policy is intertwined with the control flow of the
> >   mechanism of the precopy stage
> > - it can't take into account facts external to the immediate
> >   migration context, such external state transfer state, interactive
> >   user input, or the passage of wall-clock time.
> > - it does not permit the user to change their mind, over time, about
> >   what to do at the end of the precopy (they get an unconditional
> >   transition into the stop-and-copy phase of the migration)
> > 
> > To permit callers to implement arbitrary higher-level policies governing
> > when the live migration precopy phase should end, and what should be
> > done next:
> > - add a precopy_policy() callback to the xc_domain_save() user-supplied
> >   callbacks
> > - during the precopy phase of live migrations, consult this policy after
> >   each batch of pages transmitted and take the dictated action, which
> >   may be to a) abort the migration entirely, b) continue with the
> >   precopy, or c) proceed to the stop-and-copy phase.
> > - provide an implementation of the old policy, used when
> >   precopy_policy callback  is not provided.
> > 
> > Signed-off-by: Jennifer Herbert <Jennifer.Herbert@xxxxxxxxxx>
> > 
> > ---
> > 
> > This is updated/modified subset of patch 7/20, part of
> > Joshua Otto's "Add postcopy live migration support." patch,
> > dated 27th March 2017.  As indicated on the original thread,
> > I wish to make use of this this within the XenServer product.
> > I hope this will aid Josh in pushing the remainder of his series.
> 
> Does this patch need to carry Joshua's s-o-b, or at least 'suggested-by'?

I agree. We need to retain Joshua's s-o-b because this patch is based on
his.

> 
> > ---
> >  tools/libxc/include/xenguest.h |  19 ++++++++
> >  tools/libxc/xc_sr_common.h     |   7 ++-
> >  tools/libxc/xc_sr_save.c       | 102 +++++++++++++++++++++++++++++------
> > ------
> >  3 files changed, 94 insertions(+), 34 deletions(-)
> > 
> > diff --git a/tools/libxc/include/xenguest.h b/tools/libxc/include/xenguest.h
> > index 6626f0c..d5908dc 100644
> > --- a/tools/libxc/include/xenguest.h
> > +++ b/tools/libxc/include/xenguest.h
> > @@ -39,6 +39,14 @@
> >   */
> >  struct xenevtchn_handle;
> > 
> > +/* For save's precopy_policy(). */
> > +struct precopy_stats
> > +{
> > +    unsigned iteration;
> > +    unsigned total_written;
> > +    long dirty_count; /* -1 if unknown */
> > +};
> > +
> >  /* callbacks provided by xc_domain_save */
> >  struct save_callbacks {
> >      /* Called after expiration of checkpoint interval,
> > @@ -46,6 +54,17 @@ struct save_callbacks {
> >       */
> >      int (*suspend)(void* data);
> > 
> > +    /* Called after every batch of page data sent during the precopy phase 
> > of
> > a
> > +     * live migration to ask the caller what to do next based on the 
> > current
> > +     * state of the precopy migration.
> > +     */
> > +#define XGS_POLICY_ABORT          (-1) /* Abandon the migration entirely
> > and
> > +                                        * tidy up. */
> > +#define XGS_POLICY_CONTINUE_PRECOPY 0  /* Remain in the precopy
> > phase. */
> > +#define XGS_POLICY_STOP_AND_COPY    1  /* Immediately suspend and
> > transmit the
> > +                                        * remaining dirty pages. */
> > +    int (*precopy_policy)(struct precopy_stats stats, void *data);
> 
> Do we really want to be passing the struct, rather than a pointer to it?
> 

IIRC that was discussed in the past. We passed the struct because we
couldn't pass pointers across process boundary.

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