Re: [PATCH v2 for Xen 4.6 3/4] libxl: enabling XL to set per-VCPU parameters of a domain for RTDS scheduler

From: Chong Li <lichong659@gmail.com>
To: Dario Faggioli <dario.faggioli@citrix.com>
Cc: Chong Li <chong.li@wustl.edu>, Wei Liu <wei.liu2@citrix.com>,
	Ian Campbell <ian.campbell@citrix.com>,
	Sisu Xi <xisisu@gmail.com>,
	George Dunlap <george.dunlap@eu.citrix.com>,
	Ian Jackson <ian.jackson@eu.citrix.com>,
	xen-devel <xen-devel@lists.xen.org>,
	Meng Xu <mengxu@cis.upenn.edu>,
	Dagaen Golomb <dgolomb@seas.upenn.edu>
Subject: Re: [PATCH v2 for Xen 4.6 3/4] libxl: enabling XL to set per-VCPU parameters of a domain for RTDS scheduler
Date: Mon, 8 Jun 2015 15:55:55 -0500	[thread overview]
Message-ID: <CAGHO-ipLRv9FCu234oO3jVeyj0PCchvfmeGTgqS-71m_DNut2w@mail.gmail.com> (raw)
In-Reply-To: <1433778984.2403.27.camel@citrix.com>

On Mon, Jun 8, 2015 at 10:56 AM, Dario Faggioli
<dario.faggioli@citrix.com> wrote:
> On Fri, 2015-06-05 at 12:37 +0100, Ian Campbell wrote:
>> On Mon, 2015-05-25 at 19:09 -0500, Chong Li wrote:
>
>> > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
>> > index 117b61d..d28d274 100644
>> > --- a/tools/libxl/libxl_types.idl
>> > +++ b/tools/libxl/libxl_types.idl
>> > @@ -347,6 +347,17 @@ libxl_domain_restore_params = Struct("domain_restore_params", [
>> >      ("checkpointed_stream", integer),
>> >      ])
>> >

>
> Alternatevily we can just add the per-vcpu stuff (as in 'option 0'), for
> all schedulers, and really leave libxl_domain_sched_params alone (let's
> call this 'option 2'):
>
> libxl_sched_params = Struct("sched_params",[
>     ("weight",       integer, {'init_val': 'LIBXL_PARAM_WEIGHT_DEFAULT'}),
>     ("cap",          integer, {'init_val': 'LIBXL_PARAM_CAP_DEFAULT'}),
>     ("period",       integer, {'init_val': 'LIBXL_PARAM_PERIOD_DEFAULT'}),
>     ("slice",        integer, {'init_val': 'LIBXL_PARAM_SLICE_DEFAULT'}),
>     ("latency",      integer, {'init_val': 'LIBXL_PARAM_LATENCY_DEFAULT'}),
>     ("extratime",    integer, {'init_val': 'LIBXL_PARAM_EXTRATIME_DEFAULT'}),
>     ("budget",       integer, {'init_val': 'LIBXL_PARAM_BUDGET_DEFAULT'}),
>     ])
>
> libxl_vcpu_sched_params = Struct("vcpu_sched_params",[
>     ("sched",        libxl_scheduler),
>     ("vcpus",        Array(libxl_sched_params, "num_vcpus")),
>     ])
>
> In this case the redundancy comes from having libxl_domain_sched_params
> and libxl_sched_params looking a lot similar, but not shared code in
> declaring them.
>
> Maybe we can also consider putting an union inside
> libl_domain_sched_params... but again, quite a severe breakage, and I
> wouldn't be sure about how to 'key it'...
>
> So, Thoughts? What do you think the best way forward could be?

I like option 2 more. But I think we may also need a 'vcpuid' field in
libxl_sched_params.

Chong

>
> Regards,
> Dario
>
> --
> <<This happens because I choose it to happen!>> (Raistlin Majere)
> -----------------------------------------------------------------
> Dario Faggioli, Ph.D, http://about.me/dario.faggioli
> Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

-- 
Chong Li
Department of Computer Science and Engineering
Washington University in St.louis