Re: [Xen-devel] [PATCH] libxl: Restore scheduling parameters after migrate in best-effort fashion

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Tue, Oct 09, 2018 at 04:16:48PM +0100, George Dunlap wrote:
> On 10/09/2018 04:12 PM, Wei Liu wrote:
> > On Tue, Oct 02, 2018 at 04:49:26PM +0100, George Dunlap wrote:
> >> Commit 3b4adba ("tools/libxl: include scheduler parameters in the
> >> output of xl list -l") added scheduling parameters to the set of
> >> information collected by libxl_retrieve_domain_configuration(), in
> >> order to report that information in `xl list -l`.
> >>
> >> Unfortunately, libxl_retrieve_domain_configuration() is also called by
> >> the migration / save code, and the results passed to the restore /
> >> receive code.  This meant scheduler parameters were inadvertently
> >> added to the migration stream, without proper consideration for how to
> >> handle corner cases.  The result was that if migrating from a host
> >> running one scheduler to a host running a different scheduler, the
> >> migration would fail with an error like the following:
> >>
> >> libxl: error: libxl_sched.c:232:sched_credit_domain_set: Domain 1:Getting 
> >> domain sched credit: Invalid argument
> >> libxl: error: libxl_create.c:1275:domcreate_rebuild_done: Domain 1:cannot 
> >> (re-)build domain: -3
> >>
> >> Luckily there's a fairly straightforward way to set parameters in a
> >> "best-effort" fashion.  libxl provides a single struct containing the
> >> parameters of all schedulers, as well as a parameter specifying which
> >> scheduler.  Parameters not used by a given scheduler are ignored.
> >> Additionally, the struct contains a parameter to specify the
> >> scheduler.  If you specify a specific scheduler,
> >> libxl_domain_sched_params_set() will fail if there's a different
> >> scheduler.  However, if you pass LIBXL_SCHEDULER_UNKNOWN, it will use
> >> the value of the current scheduler for that domain.
> >>
> >> In domcreate_stream_done(), before calling libxl__build_post(), set
> >> the scheduler to LIBXL_SCHEDULER_UNKNOWN.  This will propagate
> >> scheduler parameters from the previous instantiation on a best-effort
> >> basis.
> >>
> >> Signed-off-by: George Dunlap <george.dunlap@xxxxxxxxxx>
> >> ---
> >> CC: Ian Jackson <ian.jackson@xxxxxxxxxx>
> >> CC: Wei Liu <wei.liu2@xxxxxxxxxx>
> >> CC: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> >> CC: Jan Beulich <jbeulich@xxxxxxxx>
> >> CC: Dario Faggioli <dfaggioli@xxxxxxxx>
> >> CC: Juergen Gross <jgross@xxxxxxxx>
> >> ---
> >>  tools/libxl/libxl_create.c | 9 +++++++++
> >>  1 file changed, 9 insertions(+)
> >>
> >> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> >> index dcfde7787e..caf79d4620 100644
> >> --- a/tools/libxl/libxl_create.c
> >> +++ b/tools/libxl/libxl_create.c
> >> @@ -1237,6 +1237,15 @@ static void domcreate_stream_done(libxl__egc *egc,
> >>          ret = ERROR_INVAL;
> >>          goto out;
> >>      }
> >> +
> >> +    /* 
> >> +     * The scheduler on the sending domain may be different than the
> >> +     * scheduler running here.  Setting the scheduler to UNKNOWN will
> >> +     * cause the code to take to take whatever parameters are
> >> +     * available in that scheduler, while discarding the rest.
> >> +     */
> >> +    info->sched_params.sched = LIBXL_SCHEDULER_UNKNOWN;
> > 
> > But this function is called by normal domain creation as well. What if
> > libxl user does set the parameter?
> 
> Is it?  I thought the `stream` meant it was only called when doing
> migrate / resume.

Yes, you're right. Feel free to add my ack.

Wei.

> 
>  -George

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel