On 10/09/2018 04:21 PM, Wei Liu wrote:
> 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.dun...@citrix.com>
>>>> ---
>>>> CC: Ian Jackson <ian.jack...@citrix.com>
>>>> CC: Wei Liu <wei.l...@citrix.com>
>>>> CC: Andrew Cooper <andrew.coop...@citrix.com>
>>>> CC: Jan Beulich <jbeul...@suse.com>
>>>> CC: Dario Faggioli <dfaggi...@suse.com>
>>>> CC: Juergen Gross <jgr...@suse.com>
>>>> ---
>>>> 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.
Great, thanks. I'll re-push the credit2 default patch after this gets
pushed to master.
-George
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
