[PATCH 0 of 3] support of setting scheduler parameters on domain creation

[PATCH 0 of 3] support of setting scheduler parameters on domain creation

[Juergen Gross]

Support setting scheduling parameters on domain creation.

Depending on the scheduler of the cpupool in which the domain is started,
the default parameters are obtained from the hypervisor. Any scheduling
parameters specified during domain creation will modify these defaults.

This patch series consists of 3 patches:

1: support of getting scheduler default parameters in the hypervisor
2: support in libxc
3: support in xl/libxl

14 files changed, 214 insertions(+), 28 deletions(-)
tools/libxc/xc_csched.c     |   21 +++++++++++++
tools/libxc/xc_csched2.c    |   21 +++++++++++++
tools/libxc/xc_sedf.c       |   21 +++++++++++++
tools/libxc/xenctrl.h       |   10 ++++++
tools/libxl/libxl.h         |    2 +
tools/libxl/libxl_dom.c     |   65 +++++++++++++++++++++++++++++++++++++++++--
tools/libxl/libxl_types.idl |    1 
tools/libxl/xl_cmdimpl.c    |   10 ++++--
xen/common/sched_credit.c   |    5 +++
xen/common/sched_credit2.c  |   17 +++++++++++
xen/common/sched_sedf.c     |   20 +++++++++++++
xen/common/schedule.c       |    5 +--
xen/include/public/domctl.h |   38 +++++++++++++------------
xen/include/public/sysctl.h |    6 ++-

[PATCH 1 of 3] Support of getting scheduler defaults

[Juergen Gross]

[-- Attachment #1: Type: text/plain, Size: 654 bytes --]

Support a new sysctl schedop sub-command to get the scheduling defaults of a
specific scheduler.

Additionally correct parameter checking of the sysctl handling in schedule.c
(checked wrong sub-commands: domctl instead of sysctl).

Signed-off-by: Juergen Gross <juergen.gross@ts.fujitsu.com>


6 files changed, 69 insertions(+), 22 deletions(-)
xen/common/sched_credit.c   |    5 +++++
xen/common/sched_credit2.c  |   17 +++++++++++++++++
xen/common/sched_sedf.c     |   20 ++++++++++++++++++++
xen/common/schedule.c       |    5 +++--
xen/include/public/domctl.h |   38 ++++++++++++++++++++------------------
xen/include/public/sysctl.h |    6 ++++--



[-- Attachment #2: xen-staging.hg-3.patch --]
[-- Type: text/x-patch, Size: 6493 bytes --]

# HG changeset patch
# User Juergen Gross <juergen.gross@ts.fujitsu.com>
# Date 1337674472 -7200
# Node ID 56c50b3f6cc3eb1de8b86024d0e41e65345d9a79
# Parent  238900a4ed227d04c164d4cd12dfc66f7a25b946
Support of getting scheduler defaults

Support a new sysctl schedop sub-command to get the scheduling defaults of a
specific scheduler.

Additionally correct parameter checking of the sysctl handling in schedule.c
(checked wrong sub-commands: domctl instead of sysctl).

Signed-off-by: Juergen Gross <juergen.gross@ts.fujitsu.com>

diff -r 238900a4ed22 -r 56c50b3f6cc3 xen/common/sched_credit.c
--- a/xen/common/sched_credit.c	Mon May 21 12:03:32 2012 +0200
+++ b/xen/common/sched_credit.c	Tue May 22 10:14:32 2012 +0200
@@ -858,6 +858,11 @@ csched_sys_cntl(const struct scheduler *
         params->ratelimit_us = prv->ratelimit_us;
         rc = 0;
         break;
+    case XEN_SYSCTL_SCHEDOP_getdefaults:
+        sc->u.defaults.credit.weight = CSCHED_DEFAULT_WEIGHT;
+        sc->u.defaults.credit.cap = 0U;
+        rc = 0;
+        break;
     }
     out:
     return rc;
diff -r 238900a4ed22 -r 56c50b3f6cc3 xen/common/sched_credit2.c
--- a/xen/common/sched_credit2.c	Mon May 21 12:03:32 2012 +0200
+++ b/xen/common/sched_credit2.c	Tue May 22 10:14:32 2012 +0200
@@ -1423,6 +1423,22 @@ csched_dom_cntl(
     return 0;
 }
 
+static int
+csched_sys_cntl(const struct scheduler *ops,
+                        struct xen_sysctl_scheduler_op *sc)
+{
+    int rc = -EINVAL;
+
+    switch ( sc->cmd )
+    {
+    case XEN_SYSCTL_SCHEDOP_getdefaults:
+        sc->u.defaults.credit2.weight = CSCHED_DEFAULT_WEIGHT;
+        rc = 0;
+        break;
+    }
+    return rc;
+}
+
 static void *
 csched_alloc_domdata(const struct scheduler *ops, struct domain *dom)
 {
@@ -2110,6 +2126,7 @@ const struct scheduler sched_credit2_def
     .wake           = csched_vcpu_wake,
 
     .adjust         = csched_dom_cntl,
+    .adjust_global  = csched_sys_cntl,
 
     .pick_cpu       = csched_cpu_pick,
     .migrate        = csched_vcpu_migrate,
diff -r 238900a4ed22 -r 56c50b3f6cc3 xen/common/sched_sedf.c
--- a/xen/common/sched_sedf.c	Mon May 21 12:03:32 2012 +0200
+++ b/xen/common/sched_sedf.c	Tue May 22 10:14:32 2012 +0200
@@ -1502,6 +1502,25 @@ out:
     return rc;
 }
 
+static int sedf_adjust_global(const struct scheduler *ops,
+                              struct xen_sysctl_scheduler_op *sc)
+{
+    int rc = -EINVAL;
+
+    switch ( sc->cmd )
+    {
+    case XEN_SYSCTL_SCHEDOP_getdefaults:
+        sc->u.defaults.sedf.period = WEIGHT_PERIOD;
+        sc->u.defaults.sedf.slice = 0;
+        sc->u.defaults.sedf.latency = 0;
+        sc->u.defaults.sedf.extratime = EXTRA_AWARE;
+        sc->u.defaults.sedf.weight = 0;
+        rc = 0;
+        break;
+    }
+    return rc;
+}
+
 static struct sedf_priv_info _sedf_priv;
 
 const struct scheduler sched_sedf_def = {
@@ -1531,6 +1550,7 @@ const struct scheduler sched_sedf_def = 
     .sleep          = sedf_sleep,
     .wake           = sedf_wake,
     .adjust         = sedf_adjust,
+    .adjust_global  = sedf_adjust_global,
 };
 
 /*
diff -r 238900a4ed22 -r 56c50b3f6cc3 xen/common/schedule.c
--- a/xen/common/schedule.c	Mon May 21 12:03:32 2012 +0200
+++ b/xen/common/schedule.c	Tue May 22 10:14:32 2012 +0200
@@ -1029,8 +1029,9 @@ long sched_adjust_global(struct xen_sysc
     struct cpupool *pool;
     int rc;
 
-    if ( (op->cmd != XEN_DOMCTL_SCHEDOP_putinfo) &&
-         (op->cmd != XEN_DOMCTL_SCHEDOP_getinfo) )
+    if ( (op->cmd != XEN_SYSCTL_SCHEDOP_putinfo) &&
+         (op->cmd != XEN_SYSCTL_SCHEDOP_getinfo) &&
+         (op->cmd != XEN_SYSCTL_SCHEDOP_getdefaults))
         return -EINVAL;
 
     pool = cpupool_get_by_id(op->cpupool_id);
diff -r 238900a4ed22 -r 56c50b3f6cc3 xen/include/public/domctl.h
--- a/xen/include/public/domctl.h	Mon May 21 12:03:32 2012 +0200
+++ b/xen/include/public/domctl.h	Tue May 22 10:14:32 2012 +0200
@@ -303,28 +303,30 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_max_v
 #define XEN_SCHEDULER_CREDIT   5
 #define XEN_SCHEDULER_CREDIT2  6
 #define XEN_SCHEDULER_ARINC653 7
+/* Scheduling parameters (used in sysctl.h, too) */
+union xen_sched_par {
+    struct xen_domctl_sched_sedf {
+        uint64_aligned_t period;
+        uint64_aligned_t slice;
+        uint64_aligned_t latency;
+        uint32_t extratime;
+        uint32_t weight;
+    } sedf;
+    struct xen_domctl_sched_credit {
+        uint16_t weight;
+        uint16_t cap;
+    } credit;
+    struct xen_domctl_sched_credit2 {
+        uint16_t weight;
+    } credit2;
+};
 /* Set or get info? */
-#define XEN_DOMCTL_SCHEDOP_putinfo 0
-#define XEN_DOMCTL_SCHEDOP_getinfo 1
+#define XEN_DOMCTL_SCHEDOP_putinfo  0
+#define XEN_DOMCTL_SCHEDOP_getinfo  1
 struct xen_domctl_scheduler_op {
     uint32_t sched_id;  /* XEN_SCHEDULER_* */
     uint32_t cmd;       /* XEN_DOMCTL_SCHEDOP_* */
-    union {
-        struct xen_domctl_sched_sedf {
-            uint64_aligned_t period;
-            uint64_aligned_t slice;
-            uint64_aligned_t latency;
-            uint32_t extratime;
-            uint32_t weight;
-        } sedf;
-        struct xen_domctl_sched_credit {
-            uint16_t weight;
-            uint16_t cap;
-        } credit;
-        struct xen_domctl_sched_credit2 {
-            uint16_t weight;
-        } credit2;
-    } u;
+    union xen_sched_par u;
 };
 typedef struct xen_domctl_scheduler_op xen_domctl_scheduler_op_t;
 DEFINE_XEN_GUEST_HANDLE(xen_domctl_scheduler_op_t);
diff -r 238900a4ed22 -r 56c50b3f6cc3 xen/include/public/sysctl.h
--- a/xen/include/public/sysctl.h	Mon May 21 12:03:32 2012 +0200
+++ b/xen/include/public/sysctl.h	Tue May 22 10:14:32 2012 +0200
@@ -579,8 +579,9 @@ DEFINE_XEN_GUEST_HANDLE(xen_sysctl_credi
 
 /* XEN_SYSCTL_scheduler_op */
 /* Set or get info? */
-#define XEN_SYSCTL_SCHEDOP_putinfo 0
-#define XEN_SYSCTL_SCHEDOP_getinfo 1
+#define XEN_SYSCTL_SCHEDOP_putinfo     0
+#define XEN_SYSCTL_SCHEDOP_getinfo     1
+#define XEN_SYSCTL_SCHEDOP_getdefaults 2
 struct xen_sysctl_scheduler_op {
     uint32_t cpupool_id; /* Cpupool whose scheduler is to be targetted. */
     uint32_t sched_id;   /* XEN_SCHEDULER_* (domctl.h) */
@@ -590,6 +591,7 @@ struct xen_sysctl_scheduler_op {
             XEN_GUEST_HANDLE_64(xen_sysctl_arinc653_schedule_t) schedule;
         } sched_arinc653;
         struct xen_sysctl_credit_schedule sched_credit;
+        union xen_sched_par defaults;
     } u;
 };
 typedef struct xen_sysctl_scheduler_op xen_sysctl_scheduler_op_t;

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

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

Re: [PATCH 1 of 3] Support of getting scheduler defaults

[Ian Campbell]

On Tue, 2012-05-22 at 10:16 +0100, Juergen Gross wrote:
> Support a new sysctl schedop sub-command to get the scheduling defaults of a
> specific scheduler.

Why is XEN_DOMCTL_SCHEDOP_getinfo not sufficient here? 

Isn't it actually more useful/meaningful to get the current actual
setting rather than a default?

Ian.

> 
> Additionally correct parameter checking of the sysctl handling in schedule.c
> (checked wrong sub-commands: domctl instead of sysctl).
> 
> Signed-off-by: Juergen Gross <juergen.gross@ts.fujitsu.com>
> 
> 
> 6 files changed, 69 insertions(+), 22 deletions(-)
> xen/common/sched_credit.c   |    5 +++++
> xen/common/sched_credit2.c  |   17 +++++++++++++++++
> xen/common/sched_sedf.c     |   20 ++++++++++++++++++++
> xen/common/schedule.c       |    5 +++--
> xen/include/public/domctl.h |   38 ++++++++++++++++++++------------------
> xen/include/public/sysctl.h |    6 ++++--
> 
> 

Re: [PATCH 1 of 3] Support of getting scheduler defaults

[Juergen Gross]

On 05/22/2012 02:22 PM, Ian Campbell wrote:
> On Tue, 2012-05-22 at 10:16 +0100, Juergen Gross wrote:
>> Support a new sysctl schedop sub-command to get the scheduling defaults of a
>> specific scheduler.
> Why is XEN_DOMCTL_SCHEDOP_getinfo not sufficient here?
>
> Isn't it actually more useful/meaningful to get the current actual
> setting rather than a default?

When setting the parameters from the config file we have no domain yet which
we would have to specify for XEN_DOMCTL_SCHEDOP_getinfo. I didn't want to
parse part of the config only after domain creation.

A default should be okay, as this is what we want to modify. :-)
The scheduler should initialize a new domain with this default, of course.


Juergen

-- 
Juergen Gross                 Principal Developer Operating Systems
PDG ES&S SWE OS6                       Telephone: +49 (0) 89 3222 2967
Fujitsu Technology Solutions              e-mail: juergen.gross@ts.fujitsu.com
Domagkstr. 28                           Internet: ts.fujitsu.com
D-80807 Muenchen                 Company details: ts.fujitsu.com/imprint.html

Re: [PATCH 1 of 3] Support of getting scheduler defaults

[Ian Campbell]

On Tue, 2012-05-22 at 13:29 +0100, Juergen Gross wrote:
> On 05/22/2012 02:22 PM, Ian Campbell wrote:
> > On Tue, 2012-05-22 at 10:16 +0100, Juergen Gross wrote:
> >> Support a new sysctl schedop sub-command to get the scheduling defaults of a
> >> specific scheduler.
> > Why is XEN_DOMCTL_SCHEDOP_getinfo not sufficient here?
> >
> > Isn't it actually more useful/meaningful to get the current actual
> > setting rather than a default?
> 
> When setting the parameters from the config file we have no domain yet which
> we would have to specify for XEN_DOMCTL_SCHEDOP_getinfo. I didn't want to
> parse part of the config only after domain creation.

That seems like another problem with doing this up front instead of
doing read-modify-write when we come to set the values for the domain.

> A default should be okay, as this is what we want to modify. :-)
> The scheduler should initialize a new domain with this default, of course.

So I can't use this interface to change the current one of the current
settings for running a domain, while leaving the others at their current
value?

Which interface can I use for that and why isn't it the same as this
one?
Ian.

Re: [PATCH 1 of 3] Support of getting scheduler defaults

[Juergen Gross]

On 05/22/2012 02:32 PM, Ian Campbell wrote:
> On Tue, 2012-05-22 at 13:29 +0100, Juergen Gross wrote:
>> On 05/22/2012 02:22 PM, Ian Campbell wrote:
>>> On Tue, 2012-05-22 at 10:16 +0100, Juergen Gross wrote:
>>>> Support a new sysctl schedop sub-command to get the scheduling defaults of a
>>>> specific scheduler.
>>> Why is XEN_DOMCTL_SCHEDOP_getinfo not sufficient here?
>>>
>>> Isn't it actually more useful/meaningful to get the current actual
>>> setting rather than a default?
>> When setting the parameters from the config file we have no domain yet which
>> we would have to specify for XEN_DOMCTL_SCHEDOP_getinfo. I didn't want to
>> parse part of the config only after domain creation.
> That seems like another problem with doing this up front instead of
> doing read-modify-write when we come to set the values for the domain.

Do you think it would be okay to parse the scheduler config data _after_
domain creation? If yes, the read-modify-write approach is simple and always
correct. If no, you will have to initialize the parameters to something
scheduler specific, and not domain specific.

>> A default should be okay, as this is what we want to modify. :-)
>> The scheduler should initialize a new domain with this default, of course.
> So I can't use this interface to change the current one of the current
> settings for running a domain, while leaving the others at their current
> value?

I don't understand this sentence :-) ...

> Which interface can I use for that and why isn't it the same as this
> one?

... making it hard to answer to this one. :-)
Nevertheless trying: you can change the scheduling parameters of a running
domain with xl sched-credit/sched-sedf/...


Juergen

-- 
Juergen Gross                 Principal Developer Operating Systems
PDG ES&S SWE OS6                       Telephone: +49 (0) 89 3222 2967
Fujitsu Technology Solutions              e-mail: juergen.gross@ts.fujitsu.com
Domagkstr. 28                           Internet: ts.fujitsu.com
D-80807 Muenchen                 Company details: ts.fujitsu.com/imprint.html

Re: [PATCH 1 of 3] Support of getting scheduler defaults

[George Dunlap]

On Tue, May 22, 2012 at 1:58 PM, Juergen Gross
<juergen.gross@ts.fujitsu.com> wrote:
>>> When setting the parameters from the config file we have no domain yet
>>> which
>>> we would have to specify for XEN_DOMCTL_SCHEDOP_getinfo. I didn't want to
>>> parse part of the config only after domain creation.
>>
>> That seems like another problem with doing this up front instead of
>> doing read-modify-write when we come to set the values for the domain.
>
>
> Do you think it would be okay to parse the scheduler config data _after_
> domain creation? If yes, the read-modify-write approach is simple and always
> correct. If no, you will have to initialize the parameters to something
> scheduler specific, and not domain specific.
>
>
>>> A default should be okay, as this is what we want to modify. :-)
>>> The scheduler should initialize a new domain with this default, of
>>> course.
>>
>> So I can't use this interface to change the current one of the current
>> settings for running a domain, while leaving the others at their current
>> value?
>
>
> I don't understand this sentence :-) ...

I think he means this:  Suppose after the domain is created, you want
to set the weight.  With the proposed interface, you have to manually
read the values and change them yourself.  With an interface that
allows -1 to be default, libxl can do that for you (making programming
easier).

>
>
>> Which interface can I use for that and why isn't it the same as this
>> one?
>
>
> ... making it hard to answer to this one. :-)
> Nevertheless trying: you can change the scheduling parameters of a running
> domain with xl sched-credit/sched-sedf/...

I think Ian has convinced me that just using -1 for default values
would be the best option:
* It doesn't require a new special 'what are the defaults" hypercall
* The same interface can be used to set only a subset of the options
(leaving the other ones alone) *after* the domain is created.

Thanks for doing the work to code it up, though Juergen.

 -George

Re: [PATCH 1 of 3] Support of getting scheduler defaults

[Ian Campbell]

On Tue, 2012-05-22 at 14:05 +0100, George Dunlap wrote:
> On Tue, May 22, 2012 at 1:58 PM, Juergen Gross
> <juergen.gross@ts.fujitsu.com> wrote:
> >>> When setting the parameters from the config file we have no domain yet
> >>> which
> >>> we would have to specify for XEN_DOMCTL_SCHEDOP_getinfo. I didn't want to
> >>> parse part of the config only after domain creation.
> >>
> >> That seems like another problem with doing this up front instead of
> >> doing read-modify-write when we come to set the values for the domain.
> >
> >
> > Do you think it would be okay to parse the scheduler config data _after_
> > domain creation?

No, I don't think there's any sensible way to make this work at the
libxl interface, you'd need to call back out to xl half way through
domain build to ask it to parse the sched params for you.

>  If yes, the read-modify-write approach is simple and always
> > correct. If no, you will have to initialize the parameters to something
> > scheduler specific, and not domain specific.
> >
> >
> >>> A default should be okay, as this is what we want to modify. :-)
> >>> The scheduler should initialize a new domain with this default, of
> >>> course.
> >>
> >> So I can't use this interface to change the current one of the current
> >> settings for running a domain, while leaving the others at their current
> >> value?
> >
> >
> > I don't understand this sentence :-) ...
> 
> I think he means this:  Suppose after the domain is created, you want
> to set the weight.  With the proposed interface, you have to manually
> read the values and change them yourself.  With an interface that
> allows -1 to be default, libxl can do that for you (making programming
> easier).

Right. And furthermore with this patch the interface for setting the
initial values is different to the one for updating them, since this
patch gets you the defaults which you change.

> 
> >
> >
> >> Which interface can I use for that and why isn't it the same as this
> >> one?
> >
> >
> > ... making it hard to answer to this one. :-)
> > Nevertheless trying: you can change the scheduling parameters of a running
> > domain with xl sched-credit/sched-sedf/...

Right, my question above was -- why I can't I use the same libxl
interface as these commands at build time (perhaps internally inside
libxl)

> I think Ian has convinced me that just using -1 for default values
> would be the best option:
> * It doesn't require a new special 'what are the defaults" hypercall
> * The same interface can be used to set only a subset of the options
> (leaving the other ones alone) *after* the domain is created.
> 
> Thanks for doing the work to code it up, though Juergen.
> 
>  -George

Re: [PATCH 1 of 3] Support of getting scheduler defaults

[Ian Campbell]

On Tue, 2012-05-22 at 14:05 +0100, George Dunlap wrote:
> 
> I think Ian has convinced me that just using -1 for default values
> would be the best option: 

Perhaps I should code up what I'm talking about for comparison. 

Ian.

Re: [PATCH 1 of 3] Support of getting scheduler defaults

[Ian Campbell]

On Tue, 2012-05-22 at 14:40 +0100, Ian Campbell wrote:
> On Tue, 2012-05-22 at 14:05 +0100, George Dunlap wrote:
> > 
> > I think Ian has convinced me that just using -1 for default values
> > would be the best option: 
> 
> Perhaps I should code up what I'm talking about for comparison. 

Like the below. Lightly tested with the credit scheduler.

I think CAP is the only one for which 0 is a real valid value, but I'm
not sure (especially with the SEDF ones which didn't have existing
limits checks in the set function I could crib from...).

I'm pretty sure that libxl__sched_set_params needs to get the correct
scheduler for the particular domain, but I've no idea how to get that...

8<---------------------------

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1337698727 -3600
# Node ID 355030f95eb313605a0e43aa7328e731b28a28b3
# Parent  426bbf58cea4559464b6e5d3ff0f65324a5f5926
libxl: make it possible to explicitly specify default sched params

To do so we define a descriminating value which is never a valid real value for
each parameter.

While there:

 - remove tslice_ms and ratelimit_us from libxl_sched_params and from the xl
   domain config parser. These are global scheduler properties, not per-domain
   ones (and were never read in any case).
 - removed libxl_sched_*_domain in favour of libxl_sched_params.
 - rename libxl_sched_params to libxl_sched_domain_params for clarity.
 - use this new functionality for the various xl commands which set sched
   parameters, which saves an explicit read-modify-write in xl.
 - removed call of xc_domain_getinfolist from a few functions which weren't
   actually using the result (looks like a cut and paste error)
 - fix xl which was setting period for a variety of different config keys.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

diff -r 426bbf58cea4 -r 355030f95eb3 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Tue May 22 14:19:07 2012 +0100
+++ b/tools/libxl/libxl.c	Tue May 22 15:58:47 2012 +0100
@@ -3168,19 +3168,19 @@ libxl_scheduler libxl_get_scheduler(libx
 }
 
 int libxl_sched_credit_domain_get(libxl_ctx *ctx, uint32_t domid,
-                                  libxl_sched_credit_domain *scinfo)
+                                  libxl_sched_domain_params *scinfo)
 {
     struct xen_domctl_sched_credit sdom;
     int rc;
 
-    libxl_sched_credit_domain_init(scinfo);
-
     rc = xc_sched_credit_domain_get(ctx->xch, domid, &sdom);
     if (rc != 0) {
         LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain sched credit");
         return ERROR_FAIL;
     }
 
+    libxl_sched_domain_params_init(scinfo);
+    scinfo->sched = LIBXL_SCHEDULER_CREDIT;
     scinfo->weight = sdom.weight;
     scinfo->cap = sdom.cap;
 
@@ -3188,7 +3188,7 @@ int libxl_sched_credit_domain_get(libxl_
 }
 
 int libxl_sched_credit_domain_set(libxl_ctx *ctx, uint32_t domid,
-                                  libxl_sched_credit_domain *scinfo)
+                                  libxl_sched_domain_params *scinfo)
 {
     struct xen_domctl_sched_credit sdom;
     xc_domaininfo_t domaininfo;
@@ -3202,22 +3202,33 @@ int libxl_sched_credit_domain_set(libxl_
     if (rc != 1 || domaininfo.domain != domid)
         return ERROR_INVAL;
 
-
-    if (scinfo->weight < 1 || scinfo->weight > 65535) {
-        LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
-            "Cpu weight out of range, valid values are within range from 1 to 65535");
-        return ERROR_INVAL;
+    rc = xc_sched_credit_domain_get(ctx->xch, domid, &sdom);
+    if (rc != 0) {
+        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain sched credit");
+        return ERROR_FAIL;
     }
 
-    if (scinfo->cap < 0 || scinfo->cap > (domaininfo.max_vcpu_id + 1) * 100) {
-        LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
-            "Cpu cap out of range, valid range is from 0 to %d for specified number of vcpus",
-            ((domaininfo.max_vcpu_id + 1) * 100));
-        return ERROR_INVAL;
+    if (scinfo->weight != LIBXL_SCHED_DOMAIN_PARAM_WEIGHT_DEFAULT) {
+        if (scinfo->weight < 1 || scinfo->weight > 65535) {
+            LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                       "Cpu weight out of range, "
+                       "valid values are within range from 1 to 65535");
+            return ERROR_INVAL;
+        }
+        sdom.weight = scinfo->weight;
     }
 
-    sdom.weight = scinfo->weight;
-    sdom.cap = scinfo->cap;
+    if (scinfo->cap != LIBXL_SCHED_DOMAIN_PARAM_CAP_DEFAULT) {
+        if (scinfo->cap < 0
+            || scinfo->cap > (domaininfo.max_vcpu_id + 1) * 100) {
+            LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                "Cpu cap out of range, "
+                "valid range is from 0 to %d for specified number of vcpus",
+                       ((domaininfo.max_vcpu_id + 1) * 100));
+            return ERROR_INVAL;
+        }
+        sdom.cap = scinfo->cap;
+    }
 
     rc = xc_sched_credit_domain_set(ctx->xch, domid, &sdom);
     if ( rc < 0 ) {
@@ -3290,13 +3301,11 @@ int libxl_sched_credit_params_set(libxl_
 }
 
 int libxl_sched_credit2_domain_get(libxl_ctx *ctx, uint32_t domid,
-                                   libxl_sched_credit2_domain *scinfo)
+                                   libxl_sched_domain_params *scinfo)
 {
     struct xen_domctl_sched_credit2 sdom;
     int rc;
 
-    libxl_sched_credit2_domain_init(scinfo);
-
     rc = xc_sched_credit2_domain_get(ctx->xch, domid, &sdom);
     if (rc != 0) {
         LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
@@ -3304,36 +3313,37 @@ int libxl_sched_credit2_domain_get(libxl
         return ERROR_FAIL;
     }
 
+    libxl_sched_domain_params_init(scinfo);
+    scinfo->sched = LIBXL_SCHEDULER_CREDIT2;
     scinfo->weight = sdom.weight;
 
     return 0;
 }
 
 int libxl_sched_credit2_domain_set(libxl_ctx *ctx, uint32_t domid,
-                                   libxl_sched_credit2_domain *scinfo)
+                                   libxl_sched_domain_params *scinfo)
 {
     struct xen_domctl_sched_credit2 sdom;
-    xc_domaininfo_t domaininfo;
     int rc;
 
-    rc = xc_domain_getinfolist(ctx->xch, domid, 1, &domaininfo);
-    if (rc < 0) {
-        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain info list");
+    rc = xc_sched_credit2_domain_get(ctx->xch, domid, &sdom);
+    if (rc != 0) {
+        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
+                         "getting domain sched credit2");
         return ERROR_FAIL;
     }
-    if (rc != 1 || domaininfo.domain != domid)
-        return ERROR_INVAL;
-
-
-    if (scinfo->weight < 1 || scinfo->weight > 65535) {
-        LIBXL__LOG_ERRNOVAL(ctx, LIBXL__LOG_ERROR, rc,
-            "Cpu weight out of range, valid values are within range from "
-            "1 to 65535");
-        return ERROR_INVAL;
+
+    if (scinfo->weight != LIBXL_SCHED_DOMAIN_PARAM_WEIGHT_DEFAULT) {
+        if (scinfo->weight < 1 || scinfo->weight > 65535) {
+            LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                       "Cpu weight out of range, "
+                       "valid values are within range from "
+                       "1 to 65535");
+            return ERROR_INVAL;
+        }
+        sdom.weight = scinfo->weight;
     }
 
-    sdom.weight = scinfo->weight;
-
     rc = xc_sched_credit2_domain_set(ctx->xch, domid, &sdom);
     if ( rc < 0 ) {
         LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
@@ -3345,7 +3355,7 @@ int libxl_sched_credit2_domain_set(libxl
 }
 
 int libxl_sched_sedf_domain_get(libxl_ctx *ctx, uint32_t domid,
-                                libxl_sched_sedf_domain *scinfo)
+                                libxl_sched_domain_params *scinfo)
 {
     uint64_t period;
     uint64_t slice;
@@ -3354,8 +3364,6 @@ int libxl_sched_sedf_domain_get(libxl_ct
     uint16_t weight;
     int rc;
 
-    libxl_sched_sedf_domain_init(scinfo);
-
     rc = xc_sedf_domain_get(ctx->xch, domid, &period, &slice, &latency,
                             &extratime, &weight);
     if (rc != 0) {
@@ -3363,6 +3371,8 @@ int libxl_sched_sedf_domain_get(libxl_ct
         return ERROR_FAIL;
     }
 
+    libxl_sched_domain_params_init(scinfo);
+    scinfo->sched = LIBXL_SCHEDULER_SEDF;
     scinfo->period = period / 1000000;
     scinfo->slice = slice / 1000000;
     scinfo->latency = latency / 1000000;
@@ -3373,24 +3383,37 @@ int libxl_sched_sedf_domain_get(libxl_ct
 }
 
 int libxl_sched_sedf_domain_set(libxl_ctx *ctx, uint32_t domid,
-                                libxl_sched_sedf_domain *scinfo)
+                                libxl_sched_domain_params *scinfo)
 {
-    xc_domaininfo_t domaininfo;
-    int rc;
-
-    rc = xc_domain_getinfolist(ctx->xch, domid, 1, &domaininfo);
-    if (rc < 0) {
-        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain info list");
+    uint64_t period;
+    uint64_t slice;
+    uint64_t latency;
+    uint16_t extratime;
+    uint16_t weight;
+
+    int ret;
+
+    ret = xc_sedf_domain_get(ctx->xch, domid, &period, &slice, &latency,
+                            &extratime, &weight);
+    if (ret != 0) {
+        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain sched sedf");
         return ERROR_FAIL;
     }
-    if (rc != 1 || domaininfo.domain != domid)
-        return ERROR_INVAL;
-
-
-    rc = xc_sedf_domain_set(ctx->xch, domid, scinfo->period * 1000000,
-                            scinfo->slice * 1000000, scinfo->latency * 1000000,
-                            scinfo->extratime, scinfo->weight);
-    if ( rc < 0 ) {
+
+    if (scinfo->period != LIBXL_SCHED_DOMAIN_PARAM_PERIOD_DEFAULT)
+        period = scinfo->period * 1000000;
+    if (scinfo->slice != LIBXL_SCHED_DOMAIN_PARAM_SLICE_DEFAULT)
+        period = scinfo->slice * 1000000;
+    if (scinfo->latency != LIBXL_SCHED_DOMAIN_PARAM_LATENCY_DEFAULT)
+        period = scinfo->latency * 1000000;
+    if (scinfo->extratime != LIBXL_SCHED_DOMAIN_PARAM_EXTRATIME_DEFAULT)
+        period = scinfo->extratime;
+    if (scinfo->weight != LIBXL_SCHED_DOMAIN_PARAM_WEIGHT_DEFAULT)
+        period = scinfo->weight;
+
+    ret = xc_sedf_domain_set(ctx->xch, domid, period, slice, latency,
+                            extratime, weight);
+    if ( ret < 0 ) {
         LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "setting domain sched sedf");
         return ERROR_FAIL;
     }
diff -r 426bbf58cea4 -r 355030f95eb3 tools/libxl/libxl.h
--- a/tools/libxl/libxl.h	Tue May 22 14:19:07 2012 +0100
+++ b/tools/libxl/libxl.h	Tue May 22 15:58:47 2012 +0100
@@ -805,23 +805,33 @@ int libxl_set_vcpuonline(libxl_ctx *ctx,
 
 libxl_scheduler libxl_get_scheduler(libxl_ctx *ctx);
 
-
-int libxl_sched_credit_domain_get(libxl_ctx *ctx, uint32_t domid,
-                                  libxl_sched_credit_domain *scinfo);
-int libxl_sched_credit_domain_set(libxl_ctx *ctx, uint32_t domid,
-                                  libxl_sched_credit_domain *scinfo);
+/* Per-scheduler parameters */
 int libxl_sched_credit_params_get(libxl_ctx *ctx, uint32_t poolid,
                                   libxl_sched_credit_params *scinfo);
 int libxl_sched_credit_params_set(libxl_ctx *ctx, uint32_t poolid,
                                   libxl_sched_credit_params *scinfo);
+
+/* Scheduler Per-domain parameters */
+
+#define LIBXL_SCHED_DOMAIN_PARAM_WEIGHT_DEFAULT    0
+#define LIBXL_SCHED_DOMAIN_PARAM_CAP_DEFAULT       -1
+#define LIBXL_SCHED_DOMAIN_PARAM_PERIOD_DEFAULT    0
+#define LIBXL_SCHED_DOMAIN_PARAM_SLICE_DEFAULT     0
+#define LIBXL_SCHED_DOMAIN_PARAM_LATENCY_DEFAULT   0
+#define LIBXL_SCHED_DOMAIN_PARAM_EXTRATIME_DEFAULT 0
+
+int libxl_sched_credit_domain_get(libxl_ctx *ctx, uint32_t domid,
+                                  libxl_sched_domain_params *scinfo);
+int libxl_sched_credit_domain_set(libxl_ctx *ctx, uint32_t domid,
+                                  libxl_sched_domain_params *scinfo);
 int libxl_sched_credit2_domain_get(libxl_ctx *ctx, uint32_t domid,
-                                   libxl_sched_credit2_domain *scinfo);
+                                   libxl_sched_domain_params *scinfo);
 int libxl_sched_credit2_domain_set(libxl_ctx *ctx, uint32_t domid,
-                                   libxl_sched_credit2_domain *scinfo);
+                                   libxl_sched_domain_params *scinfo);
 int libxl_sched_sedf_domain_get(libxl_ctx *ctx, uint32_t domid,
-                                libxl_sched_sedf_domain *scinfo);
+                                libxl_sched_domain_params *scinfo);
 int libxl_sched_sedf_domain_set(libxl_ctx *ctx, uint32_t domid,
-                                libxl_sched_sedf_domain *scinfo);
+                                libxl_sched_domain_params *scinfo);
 int libxl_send_trigger(libxl_ctx *ctx, uint32_t domid,
                        libxl_trigger trigger, uint32_t vcpuid);
 int libxl_send_sysrq(libxl_ctx *ctx, uint32_t domid, char sysrq);
diff -r 426bbf58cea4 -r 355030f95eb3 tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c	Tue May 22 14:19:07 2012 +0100
+++ b/tools/libxl/libxl_dom.c	Tue May 22 15:58:47 2012 +0100
@@ -42,36 +42,30 @@ libxl_domain_type libxl__domain_type(lib
         return LIBXL_DOMAIN_TYPE_PV;
 }
 
-int libxl__sched_set_params(libxl__gc *gc, uint32_t domid, libxl_sched_params *scparams)
+int libxl__sched_set_params(libxl__gc *gc, uint32_t domid,
+                            libxl_sched_domain_params *scparams)
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
-    libxl_scheduler sched;
-    libxl_sched_sedf_domain sedf_info;
-    libxl_sched_credit_domain credit_info;
-    libxl_sched_credit2_domain credit2_info;
+    libxl_scheduler sched = scparams->sched;
     int ret;
 
-    sched = libxl_get_scheduler (ctx);
+    if (sched == LIBXL_SCHEDULER_UNKNOWN)
+        sched = libxl_get_scheduler(ctx);
+
     switch (sched) {
     case LIBXL_SCHEDULER_SEDF:
-      sedf_info.period = scparams->period;
-      sedf_info.slice = scparams->slice;
-      sedf_info.latency = scparams->latency;
-      sedf_info.extratime = scparams->extratime;
-      sedf_info.weight = scparams->weight;
-      ret=libxl_sched_sedf_domain_set(ctx, domid, &sedf_info);
-      break;
+        ret=libxl_sched_sedf_domain_set(ctx, domid, scparams);
+        break;
     case LIBXL_SCHEDULER_CREDIT:
-      credit_info.weight = scparams->weight;
-      credit_info.cap = scparams->cap;
-      ret=libxl_sched_credit_domain_set(ctx, domid, &credit_info);
-      break;
+        ret=libxl_sched_credit_domain_set(ctx, domid, scparams);
+        break;
     case LIBXL_SCHEDULER_CREDIT2:
-      credit2_info.weight = scparams->weight;
-      ret=libxl_sched_credit2_domain_set(ctx, domid, &credit2_info);
-      break;
+        ret=libxl_sched_credit2_domain_set(ctx, domid, scparams);
+        break;
     default:
-      ret=-1;
+        LOG(ERROR, "Unknown scheduler");
+        ret=ERROR_INVAL;
+        break;
     }
     return ret;
 }
diff -r 426bbf58cea4 -r 355030f95eb3 tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h	Tue May 22 14:19:07 2012 +0100
+++ b/tools/libxl/libxl_internal.h	Tue May 22 15:58:47 2012 +0100
@@ -716,7 +716,7 @@ _hidden int libxl__atfork_init(libxl_ctx
 /* from xl_dom */
 _hidden libxl_domain_type libxl__domain_type(libxl__gc *gc, uint32_t domid);
 _hidden int libxl__domain_shutdown_reason(libxl__gc *gc, uint32_t domid);
-_hidden int libxl__sched_set_params(libxl__gc *gc, uint32_t domid, libxl_sched_params *scparams);
+_hidden int libxl__sched_set_params(libxl__gc *gc, uint32_t domid, libxl_sched_domain_params *scparams);
 #define LIBXL__DOMAIN_IS_TYPE(gc, domid, type) \
     libxl__domain_type((gc), (domid)) == LIBXL_DOMAIN_TYPE_##type
 typedef struct {
diff -r 426bbf58cea4 -r 355030f95eb3 tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl	Tue May 22 14:19:07 2012 +0100
+++ b/tools/libxl/libxl_types.idl	Tue May 22 15:58:47 2012 +0100
@@ -109,7 +109,9 @@ libxl_bios_type = Enumeration("bios_type
     ])
 
 # Consistent with values defined in domctl.h
+# Except unknown which is special.
 libxl_scheduler = Enumeration("scheduler", [
+    (0, "unknown"),
     (4, "sedf"),
     (5, "credit"),
     (6, "credit2"),
@@ -224,15 +226,14 @@ libxl_domain_create_info = Struct("domai
 
 MemKB = UInt(64, init_val = "LIBXL_MEMKB_DEFAULT")
 
-libxl_sched_params = Struct("sched_params",[
-    ("weight",       integer),
-    ("cap",          integer),
-    ("tslice_ms",    integer),
-    ("ratelimit_us", integer),
-    ("period",       integer),
-    ("slice",        integer),
-    ("latency",      integer),
-    ("extratime",    integer),
+libxl_sched_domain_params = Struct("sched_domain_params",[
+    ("sched",        libxl_scheduler),
+    ("weight",       integer, {'init_val': 'LIBXL_SCHED_DOMAIN_PARAM_WEIGHT_DEFAULT'}),
+    ("cap",          integer, {'init_val': 'LIBXL_SCHED_DOMAIN_PARAM_CAP_DEFAULT'}),
+    ("period",       integer, {'init_val': 'LIBXL_SCHED_DOMAIN_PARAM_PERIOD_DEFAULT'}),
+    ("slice",        integer, {'init_val': 'LIBXL_SCHED_DOMAIN_PARAM_SLICE_DEFAULT'}),
+    ("latency",      integer, {'init_val': 'LIBXL_SCHED_DOMAIN_PARAM_LATENCY_DEFAULT'}),
+    ("extratime",    integer, {'init_val': 'LIBXL_SCHED_DOMAIN_PARAM_EXTRATIME_DEFAULT'}),
     ], dir=DIR_IN)
 
 # Instances of libxl_file_reference contained in this struct which
@@ -267,7 +268,7 @@ libxl_domain_build_info = Struct("domain
     # extra parameters pass directly to qemu for HVM guest, NULL terminated
     ("extra_hvm",        libxl_string_list),
     #  parameters for all type of scheduler
-    ("sched_params",     libxl_sched_params),
+    ("sched_params",     libxl_sched_domain_params),
 
     ("u", KeyedUnion(None, libxl_domain_type, "type",
                 [("hvm", Struct(None, [("firmware",         string),
@@ -432,28 +433,11 @@ libxl_cputopology = Struct("cputopology"
     ("node", uint32),
     ], dir=DIR_OUT)
 
-libxl_sched_credit_domain = Struct("sched_credit_domain", [
-    ("weight", integer),
-    ("cap", integer),
-    ])
-
 libxl_sched_credit_params = Struct("sched_credit_params", [
     ("tslice_ms", integer),
     ("ratelimit_us", integer),
     ], dispose_fn=None)
 
-libxl_sched_credit2_domain = Struct("sched_credit2_domain", [
-    ("weight", integer),
-    ])
-
-libxl_sched_sedf_domain = Struct("sched_sedf_domain", [
-    ("period", integer),
-    ("slice", integer),
-    ("latency", integer),
-    ("extratime", integer),
-    ("weight", integer),
-    ])
-
 libxl_domain_remus_info = Struct("domain_remus_info",[
     ("interval",     integer),
     ("blackhole",    bool),
diff -r 426bbf58cea4 -r 355030f95eb3 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Tue May 22 14:19:07 2012 +0100
+++ b/tools/libxl/xl_cmdimpl.c	Tue May 22 15:58:47 2012 +0100
@@ -627,23 +627,20 @@ static void parse_config_data(const char
 
     libxl_domain_build_info_init_type(b_info, c_info->type);
 
-    /* the following is the actual config parsing with overriding values in the structures */
+    /* the following is the actual config parsing with overriding
+     * values in the structures */
     if (!xlu_cfg_get_long (config, "cpu_weight", &l, 0))
         b_info->sched_params.weight = l;
     if (!xlu_cfg_get_long (config, "cap", &l, 0))
         b_info->sched_params.cap = l;
-    if (!xlu_cfg_get_long (config, "tslice_ms", &l, 0))
-        b_info->sched_params.tslice_ms = l;
-    if (!xlu_cfg_get_long (config, "ratelimit_us", &l, 0))
-        b_info->sched_params.ratelimit_us = l;
     if (!xlu_cfg_get_long (config, "period", &l, 0))
         b_info->sched_params.period = l;
     if (!xlu_cfg_get_long (config, "slice", &l, 0))
-        b_info->sched_params.period = l;
+        b_info->sched_params.slice = l;
     if (!xlu_cfg_get_long (config, "latency", &l, 0))
-        b_info->sched_params.period = l;
+        b_info->sched_params.latency = l;
     if (!xlu_cfg_get_long (config, "extratime", &l, 0))
-        b_info->sched_params.period = l;
+        b_info->sched_params.extratime = l;
 
     if (!xlu_cfg_get_long (config, "vcpus", &l, 0)) {
         b_info->max_vcpus = l;
@@ -4355,7 +4352,7 @@ int main_sharing(int argc, char **argv)
     return 0;
 }
 
-static int sched_credit_domain_get(int domid, libxl_sched_credit_domain *scinfo)
+static int sched_credit_domain_get(int domid, libxl_sched_domain_params *scinfo)
 {
     int rc;
 
@@ -4366,7 +4363,7 @@ static int sched_credit_domain_get(int d
     return rc;
 }
 
-static int sched_credit_domain_set(int domid, libxl_sched_credit_domain *scinfo)
+static int sched_credit_domain_set(int domid, libxl_sched_domain_params *scinfo)
 {
     int rc;
 
@@ -4402,7 +4399,7 @@ static int sched_credit_params_get(int p
 static int sched_credit_domain_output(int domid)
 {
     char *domname;
-    libxl_sched_credit_domain scinfo;
+    libxl_sched_domain_params scinfo;
     int rc;
 
     if (domid < 0) {
@@ -4419,7 +4416,7 @@ static int sched_credit_domain_output(in
         scinfo.weight,
         scinfo.cap);
     free(domname);
-    libxl_sched_credit_domain_dispose(&scinfo);
+    libxl_sched_domain_params_dispose(&scinfo);
     return 0;
 }
 
@@ -4445,7 +4442,7 @@ static int sched_credit_pool_output(uint
 }
 
 static int sched_credit2_domain_get(
-    int domid, libxl_sched_credit2_domain *scinfo)
+    int domid, libxl_sched_domain_params *scinfo)
 {
     int rc;
 
@@ -4457,7 +4454,7 @@ static int sched_credit2_domain_get(
 }
 
 static int sched_credit2_domain_set(
-    int domid, libxl_sched_credit2_domain *scinfo)
+    int domid, libxl_sched_domain_params *scinfo)
 {
     int rc;
 
@@ -4472,7 +4469,7 @@ static int sched_credit2_domain_output(
     int domid)
 {
     char *domname;
-    libxl_sched_credit2_domain scinfo;
+    libxl_sched_domain_params scinfo;
     int rc;
 
     if (domid < 0) {
@@ -4488,12 +4485,12 @@ static int sched_credit2_domain_output(
         domid,
         scinfo.weight);
     free(domname);
-    libxl_sched_credit2_domain_dispose(&scinfo);
+    libxl_sched_domain_params_dispose(&scinfo);
     return 0;
 }
 
 static int sched_sedf_domain_get(
-    int domid, libxl_sched_sedf_domain *scinfo)
+    int domid, libxl_sched_domain_params *scinfo)
 {
     int rc;
 
@@ -4505,7 +4502,7 @@ static int sched_sedf_domain_get(
 }
 
 static int sched_sedf_domain_set(
-    int domid, libxl_sched_sedf_domain *scinfo)
+    int domid, libxl_sched_domain_params *scinfo)
 {
     int rc;
 
@@ -4519,7 +4516,7 @@ static int sched_sedf_domain_output(
     int domid)
 {
     char *domname;
-    libxl_sched_sedf_domain scinfo;
+    libxl_sched_domain_params scinfo;
     int rc;
 
     if (domid < 0) {
@@ -4540,7 +4537,7 @@ static int sched_sedf_domain_output(
         scinfo.extratime,
         scinfo.weight);
     free(domname);
-    libxl_sched_sedf_domain_dispose(&scinfo);
+    libxl_sched_domain_params_dispose(&scinfo);
     return 0;
 }
 
@@ -4620,7 +4617,6 @@ static int sched_domain_output(libxl_sch
  */
 int main_sched_credit(int argc, char **argv)
 {
-    libxl_sched_credit_domain scinfo;
     const char *dom = NULL;
     const char *cpupool = NULL;
     int weight = 256, cap = 0, opt_w = 0, opt_c = 0;
@@ -4695,7 +4691,7 @@ int main_sched_credit(int argc, char **a
     }
 
     if (opt_s) {
-        libxl_sched_credit_params  scparam;
+        libxl_sched_credit_params scparam;
         uint32_t poolid = 0;
 
         if (cpupool) {
@@ -4731,20 +4727,19 @@ int main_sched_credit(int argc, char **a
     } else {
         find_domain(dom);
 
-        rc = sched_credit_domain_get(domid, &scinfo);
-        if (rc)
-            return -rc;
-
         if (!opt_w && !opt_c) { /* output credit scheduler info */
             sched_credit_domain_output(-1);
             return -sched_credit_domain_output(domid);
         } else { /* set credit scheduler paramaters */
+            libxl_sched_domain_params scinfo;
+            libxl_sched_domain_params_init(&scinfo);
+            scinfo.sched = LIBXL_SCHEDULER_CREDIT;
             if (opt_w)
                 scinfo.weight = weight;
             if (opt_c)
                 scinfo.cap = cap;
             rc = sched_credit_domain_set(domid, &scinfo);
-            libxl_sched_credit_domain_dispose(&scinfo);
+            libxl_sched_domain_params_dispose(&scinfo);
             if (rc)
                 return -rc;
         }
@@ -4755,7 +4750,6 @@ int main_sched_credit(int argc, char **a
 
 int main_sched_credit2(int argc, char **argv)
 {
-    libxl_sched_credit2_domain scinfo;
     const char *dom = NULL;
     const char *cpupool = NULL;
     int weight = 256, opt_w = 0;
@@ -4810,18 +4804,17 @@ int main_sched_credit2(int argc, char **
     } else {
         find_domain(dom);
 
-        rc = sched_credit2_domain_get(domid, &scinfo);
-        if (rc)
-            return -rc;
-
         if (!opt_w) { /* output credit2 scheduler info */
             sched_credit2_domain_output(-1);
             return -sched_credit2_domain_output(domid);
         } else { /* set credit2 scheduler paramaters */
+            libxl_sched_domain_params scinfo;
+            libxl_sched_domain_params_init(&scinfo);
+            scinfo.sched = LIBXL_SCHEDULER_CREDIT2;
             if (opt_w)
                 scinfo.weight = weight;
             rc = sched_credit2_domain_set(domid, &scinfo);
-            libxl_sched_credit2_domain_dispose(&scinfo);
+            libxl_sched_domain_params_dispose(&scinfo);
             if (rc)
                 return -rc;
         }
@@ -4832,7 +4825,6 @@ int main_sched_credit2(int argc, char **
 
 int main_sched_sedf(int argc, char **argv)
 {
-    libxl_sched_sedf_domain scinfo;
     const char *dom = NULL;
     const char *cpupool = NULL;
     int period = 0, opt_p = 0;
@@ -4915,15 +4907,15 @@ int main_sched_sedf(int argc, char **arg
     } else {
         find_domain(dom);
 
-        rc = sched_sedf_domain_get(domid, &scinfo);
-        if (rc)
-            return -rc;
-
         if (!opt_p && !opt_s && !opt_l && !opt_e && !opt_w) {
             /* output sedf scheduler info */
             sched_sedf_domain_output(-1);
             return -sched_sedf_domain_output(domid);
         } else { /* set sedf scheduler paramaters */
+            libxl_sched_domain_params scinfo;
+            libxl_sched_domain_params_init(&scinfo);
+            scinfo.sched = LIBXL_SCHEDULER_SEDF;
+
             if (opt_p) {
                 scinfo.period = period;
                 scinfo.weight = 0;
@@ -4942,7 +4934,7 @@ int main_sched_sedf(int argc, char **arg
                 scinfo.slice = 0;
             }
             rc = sched_sedf_domain_set(domid, &scinfo);
-            libxl_sched_sedf_domain_dispose(&scinfo);
+            libxl_sched_domain_params_dispose(&scinfo);
             if (rc)
                 return -rc;
         }

Re: [PATCH 1 of 3] Support of getting scheduler defaults

[Dario Faggioli]

[-- Attachment #1.1: Type: text/plain, Size: 6052 bytes --]

On Tue, 2012-05-22 at 15:59 +0100, Ian Campbell wrote:
> Like the below. Lightly tested with the credit scheduler.
> 
> I think CAP is the only one for which 0 is a real valid value, but I'm
> not sure (especially with the SEDF ones which didn't have existing
> limits checks in the set function I could crib from...).
> 
Yep, that's because they're mostly time values. xen/common/sched_sedf.c
hosts some ranges for some of them, but I'm not sure we want to
propagate those:

#define PERIOD_MAX MILLISECS(10000) /* 10s  */
#define PERIOD_MIN (MICROSECS(10))  /* 10us */
#define SLICE_MIN (MICROSECS(5))    /*  5us */

Also, extratime is a flag, so I think 0 and 1 are both meaningful
values, maybe we can go for -1 as for cap (I'll try and let you know).

> I'm pretty sure that libxl__sched_set_params needs to get the correct
> scheduler for the particular domain, but I've no idea how to get that...
> 
Again, I was thinking something like what Juergen did here could help
(go getting the scheduler of the cpupool the domain belongs to)... O am
I misunderstanding the issue?

+    poolinfo = libxl_list_cpupool(ctx, &n_pools);
+    if (!poolinfo)
+        return ERROR_NOMEM;
+
+    ret = ERROR_INVAL;
+    for (p = 0; p < n_pools; p++) {
+        if (poolinfo[p].poolid == poolid) {
+            scparams->sched = poolinfo[p].sched;
+            ret = 0;
+        }
+        libxl_cpupoolinfo_dispose(poolinfo + p);
+    }
+

> 8<---------------------------
> 
> # HG changeset patch
> # User Ian Campbell <ian.campbell@citrix.com>
> # Date 1337698727 -3600
> # Node ID 355030f95eb313605a0e43aa7328e731b28a28b3
> # Parent  426bbf58cea4559464b6e5d3ff0f65324a5f5926
> libxl: make it possible to explicitly specify default sched params
> 
> To do so we define a descriminating value which is never a valid real value for
> each parameter.
> 
> While there:
> 
>  - remove tslice_ms and ratelimit_us from libxl_sched_params and from the xl
>    domain config parser. These are global scheduler properties, not per-domain
>    ones (and were never read in any case).
>  - removed libxl_sched_*_domain in favour of libxl_sched_params.
>  - rename libxl_sched_params to libxl_sched_domain_params for clarity.
>  - use this new functionality for the various xl commands which set sched
>    parameters, which saves an explicit read-modify-write in xl.
>  - removed call of xc_domain_getinfolist from a few functions which weren't
>    actually using the result (looks like a cut and paste error)
>  - fix xl which was setting period for a variety of different config keys.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> 
> diff -r 426bbf58cea4 -r 355030f95eb3 tools/libxl/libxl.c
> --- a/tools/libxl/libxl.c	Tue May 22 14:19:07 2012 +0100
> +++ b/tools/libxl/libxl.c	Tue May 22 15:58:47 2012 +0100
> @@ -3168,19 +3168,19 @@ libxl_scheduler libxl_get_scheduler(libx
>  }
>
<snip>
>  
>  int libxl_sched_sedf_domain_set(libxl_ctx *ctx, uint32_t domid,
> -                                libxl_sched_sedf_domain *scinfo)
> +                                libxl_sched_domain_params *scinfo)
>  {
> -    xc_domaininfo_t domaininfo;
> -    int rc;
> -
> -    rc = xc_domain_getinfolist(ctx->xch, domid, 1, &domaininfo);
> -    if (rc < 0) {
> -        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain info list");
> +    uint64_t period;
> +    uint64_t slice;
> +    uint64_t latency;
> +    uint16_t extratime;
> +    uint16_t weight;
> +
> +    int ret;
> +
> +    ret = xc_sedf_domain_get(ctx->xch, domid, &period, &slice, &latency,
> +                            &extratime, &weight);
> +    if (ret != 0) {
> +        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain sched sedf");
>          return ERROR_FAIL;
>      }
> -    if (rc != 1 || domaininfo.domain != domid)
> -        return ERROR_INVAL;
> -
> -
> -    rc = xc_sedf_domain_set(ctx->xch, domid, scinfo->period * 1000000,
> -                            scinfo->slice * 1000000, scinfo->latency * 1000000,
> -                            scinfo->extratime, scinfo->weight);
> -    if ( rc < 0 ) {
> +
> +    if (scinfo->period != LIBXL_SCHED_DOMAIN_PARAM_PERIOD_DEFAULT)
> +        period = scinfo->period * 1000000;
> +    if (scinfo->slice != LIBXL_SCHED_DOMAIN_PARAM_SLICE_DEFAULT)
> +        period = scinfo->slice * 1000000;
> +    if (scinfo->latency != LIBXL_SCHED_DOMAIN_PARAM_LATENCY_DEFAULT)
> +        period = scinfo->latency * 1000000;
> +    if (scinfo->extratime != LIBXL_SCHED_DOMAIN_PARAM_EXTRATIME_DEFAULT)
> +        period = scinfo->extratime;
> +    if (scinfo->weight != LIBXL_SCHED_DOMAIN_PARAM_WEIGHT_DEFAULT)
> +        period = scinfo->weight;
> +
> +    ret = xc_sedf_domain_set(ctx->xch, domid, period, slice, latency,
> +                            extratime, weight);
> +    if ( ret < 0 ) {
>          LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "setting domain sched sedf");
>          return ERROR_FAIL;
>      }
>
# xl create vm1.cfg
Parsing config from vm1.cfg
libxl: error: libxl.c:3417:libxl_sched_sedf_domain_set: setting domain sched sedf: Invalid argument

And I'm getting the above independently on what I put in the config file
(valid params, no params at all, etc.). I also can't change the
scheduling parameters of a domain on-line anymore:

# xl sched-sedf -d 3 -p 100 -s 50
libxl: error: libxl.c:3417:libxl_sched_sedf_domain_set: setting domain sched sedf: Invalid argument
libxl_sched_sedf_domain_set failed.

I'll try digging a bit more into this ASAP.

On more thing, are we ok with the _set command failing and he domain
being created anyway? Or perhaps the whole process should just abort?

Regards,
Dario

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


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

Re: [PATCH 1 of 3] Support of getting scheduler defaults

[Juergen Gross]

On 05/23/2012 01:46 AM, Dario Faggioli wrote:
> On Tue, 2012-05-22 at 15:59 +0100, Ian Campbell wrote:
>> Like the below. Lightly tested with the credit scheduler.
>>
>> I think CAP is the only one for which 0 is a real valid value, but I'm
>> not sure (especially with the SEDF ones which didn't have existing
>> limits checks in the set function I could crib from...).
>>
> Yep, that's because they're mostly time values. xen/common/sched_sedf.c
> hosts some ranges for some of them, but I'm not sure we want to
> propagate those:
>
> #define PERIOD_MAX MILLISECS(10000) /* 10s  */
> #define PERIOD_MIN (MICROSECS(10))  /* 10us */
> #define SLICE_MIN (MICROSECS(5))    /*  5us */

I think this should remain in the hypervisor only.

> Also, extratime is a flag, so I think 0 and 1 are both meaningful
> values, maybe we can go for -1 as for cap (I'll try and let you know).

Why not -1 for all values?

>> I'm pretty sure that libxl__sched_set_params needs to get the correct
>> scheduler for the particular domain, but I've no idea how to get that...
>>
> Again, I was thinking something like what Juergen did here could help
> (go getting the scheduler of the cpupool the domain belongs to)... O am
> I misunderstanding the issue?
>
> +    poolinfo = libxl_list_cpupool(ctx,&n_pools);
> +    if (!poolinfo)
> +        return ERROR_NOMEM;
> +
> +    ret = ERROR_INVAL;
> +    for (p = 0; p<  n_pools; p++) {
> +        if (poolinfo[p].poolid == poolid) {
> +            scparams->sched = poolinfo[p].sched;
> +            ret = 0;
> +        }
> +        libxl_cpupoolinfo_dispose(poolinfo + p);
> +    }
> +

This was exactly the purpose of the sniplet.

>> 8<---------------------------
>>
>> # HG changeset patch
>> # User Ian Campbell<ian.campbell@citrix.com>
>> # Date 1337698727 -3600
>> # Node ID 355030f95eb313605a0e43aa7328e731b28a28b3
>> # Parent  426bbf58cea4559464b6e5d3ff0f65324a5f5926
>> libxl: make it possible to explicitly specify default sched params
>>
>> To do so we define a descriminating value which is never a valid real value for
>> each parameter.
>>
>> While there:
>>
>>   - remove tslice_ms and ratelimit_us from libxl_sched_params and from the xl
>>     domain config parser. These are global scheduler properties, not per-domain
>>     ones (and were never read in any case).
>>   - removed libxl_sched_*_domain in favour of libxl_sched_params.
>>   - rename libxl_sched_params to libxl_sched_domain_params for clarity.
>>   - use this new functionality for the various xl commands which set sched
>>     parameters, which saves an explicit read-modify-write in xl.
>>   - removed call of xc_domain_getinfolist from a few functions which weren't
>>     actually using the result (looks like a cut and paste error)
>>   - fix xl which was setting period for a variety of different config keys.
>>
>> Signed-off-by: Ian Campbell<ian.campbell@citrix.com>
>>
>> diff -r 426bbf58cea4 -r 355030f95eb3 tools/libxl/libxl.c
>> --- a/tools/libxl/libxl.c	Tue May 22 14:19:07 2012 +0100
>> +++ b/tools/libxl/libxl.c	Tue May 22 15:58:47 2012 +0100
>> @@ -3168,19 +3168,19 @@ libxl_scheduler libxl_get_scheduler(libx
>>   }
>>
> <snip>
>>
>>   int libxl_sched_sedf_domain_set(libxl_ctx *ctx, uint32_t domid,
>> -                                libxl_sched_sedf_domain *scinfo)
>> +                                libxl_sched_domain_params *scinfo)
>>   {
>> -    xc_domaininfo_t domaininfo;
>> -    int rc;
>> -
>> -    rc = xc_domain_getinfolist(ctx->xch, domid, 1,&domaininfo);
>> -    if (rc<  0) {
>> -        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain info list");
>> +    uint64_t period;
>> +    uint64_t slice;
>> +    uint64_t latency;
>> +    uint16_t extratime;
>> +    uint16_t weight;
>> +
>> +    int ret;
>> +
>> +    ret = xc_sedf_domain_get(ctx->xch, domid,&period,&slice,&latency,
>> +&extratime,&weight);
>> +    if (ret != 0) {
>> +        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain sched sedf");
>>           return ERROR_FAIL;
>>       }
>> -    if (rc != 1 || domaininfo.domain != domid)
>> -        return ERROR_INVAL;
>> -
>> -
>> -    rc = xc_sedf_domain_set(ctx->xch, domid, scinfo->period * 1000000,
>> -                            scinfo->slice * 1000000, scinfo->latency * 1000000,
>> -                            scinfo->extratime, scinfo->weight);
>> -    if ( rc<  0 ) {
>> +
>> +    if (scinfo->period != LIBXL_SCHED_DOMAIN_PARAM_PERIOD_DEFAULT)
>> +        period = scinfo->period * 1000000;
>> +    if (scinfo->slice != LIBXL_SCHED_DOMAIN_PARAM_SLICE_DEFAULT)
>> +        period = scinfo->slice * 1000000;
>> +    if (scinfo->latency != LIBXL_SCHED_DOMAIN_PARAM_LATENCY_DEFAULT)
>> +        period = scinfo->latency * 1000000;
>> +    if (scinfo->extratime != LIBXL_SCHED_DOMAIN_PARAM_EXTRATIME_DEFAULT)
>> +        period = scinfo->extratime;
>> +    if (scinfo->weight != LIBXL_SCHED_DOMAIN_PARAM_WEIGHT_DEFAULT)
>> +        period = scinfo->weight;
>> +
>> +    ret = xc_sedf_domain_set(ctx->xch, domid, period, slice, latency,
>> +                            extratime, weight);
>> +    if ( ret<  0 ) {
>>           LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "setting domain sched sedf");
>>           return ERROR_FAIL;
>>       }
>>
> # xl create vm1.cfg
> Parsing config from vm1.cfg
> libxl: error: libxl.c:3417:libxl_sched_sedf_domain_set: setting domain sched sedf: Invalid argument
>
> And I'm getting the above independently on what I put in the config file
> (valid params, no params at all, etc.). I also can't change the
> scheduling parameters of a domain on-line anymore:
>
> # xl sched-sedf -d 3 -p 100 -s 50
> libxl: error: libxl.c:3417:libxl_sched_sedf_domain_set: setting domain sched sedf: Invalid argument
> libxl_sched_sedf_domain_set failed.
>
> I'll try digging a bit more into this ASAP.

It's easy: Ian repeated an error he (and I) corrected elsewhere:
He's always setting period, regardless of the changed parameter...

> On more thing, are we ok with the _set command failing and he domain
> being created anyway? Or perhaps the whole process should just abort?

Perhaps a warning might be okay.


Juergen

-- 
Juergen Gross                 Principal Developer Operating Systems
PDG ES&S SWE OS6                       Telephone: +49 (0) 89 3222 2967
Fujitsu Technology Solutions              e-mail: juergen.gross@ts.fujitsu.com
Domagkstr. 28                           Internet: ts.fujitsu.com
D-80807 Muenchen                 Company details: ts.fujitsu.com/imprint.html

Re: [PATCH 1 of 3] Support of getting scheduler defaults

[Dario Faggioli]

[-- Attachment #1.1: Type: text/plain, Size: 4052 bytes --]

On Wed, 2012-05-23 at 07:34 +0200, Juergen Gross wrote:
> > #define PERIOD_MAX MILLISECS(10000) /* 10s  */
> > #define PERIOD_MIN (MICROSECS(10))  /* 10us */
> > #define SLICE_MIN (MICROSECS(5))    /*  5us */
> 
> I think this should remain in the hypervisor only.
> 
Me too.

> > Also, extratime is a flag, so I think 0 and 1 are both meaningful
> > values, maybe we can go for -1 as for cap (I'll try and let you know).
> 
> Why not -1 for all values?
> 
Would work, I guess, unless there's some collision with big weights
represented on short unsigned value (not sure it's like that, and I've
always been bad at this kind of math! :-P).

> >> I'm pretty sure that libxl__sched_set_params needs to get the correct
> >> scheduler for the particular domain, but I've no idea how to get that...
> >>
> > Again, I was thinking something like what Juergen did here could help
> > (go getting the scheduler of the cpupool the domain belongs to)... O am
> > I misunderstanding the issue?
> >
> > +    poolinfo = libxl_list_cpupool(ctx,&n_pools);
> > +    if (!poolinfo)
> > +        return ERROR_NOMEM;
> > +
> > +    ret = ERROR_INVAL;
> > +    for (p = 0; p<  n_pools; p++) {
> > +        if (poolinfo[p].poolid == poolid) {
> > +            scparams->sched = poolinfo[p].sched;
> > +            ret = 0;
> > +        }
> > +        libxl_cpupoolinfo_dispose(poolinfo + p);
> > +    }
> > +
> 
> This was exactly the purpose of the sniplet.
> 
Good to hear that. :-)

> > # xl create vm1.cfg
> > Parsing config from vm1.cfg
> > libxl: error: libxl.c:3417:libxl_sched_sedf_domain_set: setting domain sched sedf: Invalid argument
> >
> > And I'm getting the above independently on what I put in the config file
> > (valid params, no params at all, etc.). I also can't change the
> > scheduling parameters of a domain on-line anymore:
> >
> > # xl sched-sedf -d 3 -p 100 -s 50
> > libxl: error: libxl.c:3417:libxl_sched_sedf_domain_set: setting domain sched sedf: Invalid argument
> > libxl_sched_sedf_domain_set failed.
> >
> > I'll try digging a bit more into this ASAP.
> 
> It's easy: Ian repeated an error he (and I) corrected elsewhere:
> He's always setting period, regardless of the changed parameter...
> 
Not sure, I think Ian's patch fixes that:

--- a/tools/libxl/xl_cmdimpl.c  Tue May 22 14:19:07 2012 +0100
+++ b/tools/libxl/xl_cmdimpl.c  Tue May 22 15:58:47 2012 +0100
@@ -627,23 +627,20 @@ static void parse_config_data(const char
 
     libxl_domain_build_info_init_type(b_info, c_info->type);
 
-    /* the following is the actual config parsing with overriding values in the structures */
+    /* the following is the actual config parsing with overriding
+     * values in the structures */
     if (!xlu_cfg_get_long (config, "cpu_weight", &l, 0))
         b_info->sched_params.weight = l;
     if (!xlu_cfg_get_long (config, "cap", &l, 0))
         b_info->sched_params.cap = l;
-    if (!xlu_cfg_get_long (config, "tslice_ms", &l, 0))
-        b_info->sched_params.tslice_ms = l;
-    if (!xlu_cfg_get_long (config, "ratelimit_us", &l, 0))
-        b_info->sched_params.ratelimit_us = l;
     if (!xlu_cfg_get_long (config, "period", &l, 0))
         b_info->sched_params.period = l;
     if (!xlu_cfg_get_long (config, "slice", &l, 0))
-        b_info->sched_params.period = l;
+        b_info->sched_params.slice = l;
     if (!xlu_cfg_get_long (config, "latency", &l, 0))
-        b_info->sched_params.period = l;
+        b_info->sched_params.latency = l;
     if (!xlu_cfg_get_long (config, "extratime", &l, 0))
-        b_info->sched_params.period = l;
+        b_info->sched_params.extratime = l;
 

So it has to be something else...

Thanks and Regards,
Dario

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


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

Re: [PATCH 1 of 3] Support of getting scheduler defaults

[Ian Campbell]

On Wed, 2012-05-23 at 08:22 +0100, Dario Faggioli wrote:
> On Wed, 2012-05-23 at 07:34 +0200, Juergen Gross wrote:
> > > #define PERIOD_MAX MILLISECS(10000) /* 10s  */
> > > #define PERIOD_MIN (MICROSECS(10))  /* 10us */
> > > #define SLICE_MIN (MICROSECS(5))    /*  5us */
> > 
> > I think this should remain in the hypervisor only.
> > 
> Me too.

Agreed.

> > > Also, extratime is a flag, so I think 0 and 1 are both meaningful
> > > values, maybe we can go for -1 as for cap (I'll try and let you know).
> > 
> > Why not -1 for all values?
> > 
> Would work, I guess, unless there's some collision with big weights
> represented on short unsigned value (not sure it's like that, and I've
> always been bad at this kind of math! :-P).

In general in libxl where we can use 0 for the default we do so. It's
not a strong preference though and it's mainly historical from before
the IDL supported init_val.

> > >> I'm pretty sure that libxl__sched_set_params needs to get the correct
> > >> scheduler for the particular domain, but I've no idea how to get that...
> > >>
> > > Again, I was thinking something like what Juergen did here could help
> > > (go getting the scheduler of the cpupool the domain belongs to)... O am
> > > I misunderstanding the issue?
> > >
> > > +    poolinfo = libxl_list_cpupool(ctx,&n_pools);
> > > +    if (!poolinfo)
> > > +        return ERROR_NOMEM;
> > > +
> > > +    ret = ERROR_INVAL;
> > > +    for (p = 0; p<  n_pools; p++) {
> > > +        if (poolinfo[p].poolid == poolid) {
> > > +            scparams->sched = poolinfo[p].sched;
> > > +            ret = 0;
> > > +        }
> > > +        libxl_cpupoolinfo_dispose(poolinfo + p);
> > > +    }
> > > +
> > 
> > This was exactly the purpose of the sniplet.
> > 
> Good to hear that. :-)

Great, I'll make a suitably named function out of it.

> > > # xl create vm1.cfg
> > > Parsing config from vm1.cfg
> > > libxl: error: libxl.c:3417:libxl_sched_sedf_domain_set: setting domain sched sedf: Invalid argument
> > >
> > > And I'm getting the above independently on what I put in the config file
> > > (valid params, no params at all, etc.). I also can't change the
> > > scheduling parameters of a domain on-line anymore:
> > >
> > > # xl sched-sedf -d 3 -p 100 -s 50
> > > libxl: error: libxl.c:3417:libxl_sched_sedf_domain_set: setting domain sched sedf: Invalid argument
> > > libxl_sched_sedf_domain_set failed.
> > >
> > > I'll try digging a bit more into this ASAP.
> > 
> > It's easy: Ian repeated an error he (and I) corrected elsewhere:
> > He's always setting period, regardless of the changed parameter...
> > 
> Not sure, I think Ian's patch fixes that:

I thought so to!


> --- a/tools/libxl/xl_cmdimpl.c  Tue May 22 14:19:07 2012 +0100
> +++ b/tools/libxl/xl_cmdimpl.c  Tue May 22 15:58:47 2012 +0100
> @@ -627,23 +627,20 @@ static void parse_config_data(const char
>  
>      libxl_domain_build_info_init_type(b_info, c_info->type);
>  
> -    /* the following is the actual config parsing with overriding values in the structures */
> +    /* the following is the actual config parsing with overriding
> +     * values in the structures */
>      if (!xlu_cfg_get_long (config, "cpu_weight", &l, 0))
>          b_info->sched_params.weight = l;
>      if (!xlu_cfg_get_long (config, "cap", &l, 0))
>          b_info->sched_params.cap = l;
> -    if (!xlu_cfg_get_long (config, "tslice_ms", &l, 0))
> -        b_info->sched_params.tslice_ms = l;
> -    if (!xlu_cfg_get_long (config, "ratelimit_us", &l, 0))
> -        b_info->sched_params.ratelimit_us = l;
>      if (!xlu_cfg_get_long (config, "period", &l, 0))
>          b_info->sched_params.period = l;
>      if (!xlu_cfg_get_long (config, "slice", &l, 0))
> -        b_info->sched_params.period = l;
> +        b_info->sched_params.slice = l;
>      if (!xlu_cfg_get_long (config, "latency", &l, 0))
> -        b_info->sched_params.period = l;
> +        b_info->sched_params.latency = l;
>      if (!xlu_cfg_get_long (config, "extratime", &l, 0))
> -        b_info->sched_params.period = l;
> +        b_info->sched_params.extratime = l;
>  
> 
> So it has to be something else...
> 
> Thanks and Regards,
> Dario
> 

Re: [PATCH 1 of 3] Support of getting scheduler defaults

[Juergen Gross]

On 05/23/2012 09:41 AM, Ian Campbell wrote:
> # xl create vm1.cfg
> Parsing config from vm1.cfg
> libxl: error: libxl.c:3417:libxl_sched_sedf_domain_set: setting domain sched sedf: Invalid argument
>
> And I'm getting the above independently on what I put in the config file
> (valid params, no params at all, etc.). I also can't change the
> scheduling parameters of a domain on-line anymore:
>
> # xl sched-sedf -d 3 -p 100 -s 50
> libxl: error: libxl.c:3417:libxl_sched_sedf_domain_set: setting domain sched sedf: Invalid argument
> libxl_sched_sedf_domain_set failed.
>
> I'll try digging a bit more into this ASAP.
>>> It's easy: Ian repeated an error he (and I) corrected elsewhere:
>>> He's always setting period, regardless of the changed parameter...
>>>
>> Not sure, I think Ian's patch fixes that:
> I thought so to!

It fixes the original error.

>> --- a/tools/libxl/xl_cmdimpl.c  Tue May 22 14:19:07 2012 +0100
>> +++ b/tools/libxl/xl_cmdimpl.c  Tue May 22 15:58:47 2012 +0100
>> @@ -627,23 +627,20 @@ static void parse_config_data(const char
>>
>>       libxl_domain_build_info_init_type(b_info, c_info->type);
>>
>> -    /* the following is the actual config parsing with overriding values in the structures */
>> +    /* the following is the actual config parsing with overriding
>> +     * values in the structures */
>>       if (!xlu_cfg_get_long (config, "cpu_weight",&l, 0))
>>           b_info->sched_params.weight = l;
>>       if (!xlu_cfg_get_long (config, "cap",&l, 0))
>>           b_info->sched_params.cap = l;
>> -    if (!xlu_cfg_get_long (config, "tslice_ms",&l, 0))
>> -        b_info->sched_params.tslice_ms = l;
>> -    if (!xlu_cfg_get_long (config, "ratelimit_us",&l, 0))
>> -        b_info->sched_params.ratelimit_us = l;
>>       if (!xlu_cfg_get_long (config, "period",&l, 0))
>>           b_info->sched_params.period = l;
>>       if (!xlu_cfg_get_long (config, "slice",&l, 0))
>> -        b_info->sched_params.period = l;
>> +        b_info->sched_params.slice = l;
>>       if (!xlu_cfg_get_long (config, "latency",&l, 0))
>> -        b_info->sched_params.period = l;
>> +        b_info->sched_params.latency = l;
>>       if (!xlu_cfg_get_long (config, "extratime",&l, 0))
>> -        b_info->sched_params.period = l;
>> +        b_info->sched_params.extratime = l;
>>
>>
>> So it has to be something else...

Oh yes:

+
+    if (scinfo->period != LIBXL_SCHED_DOMAIN_PARAM_PERIOD_DEFAULT)
+        period = scinfo->period * 1000000;
+    if (scinfo->slice != LIBXL_SCHED_DOMAIN_PARAM_SLICE_DEFAULT)
+        period = scinfo->slice * 1000000;
+    if (scinfo->latency != LIBXL_SCHED_DOMAIN_PARAM_LATENCY_DEFAULT)
+        period = scinfo->latency * 1000000;
+    if (scinfo->extratime != LIBXL_SCHED_DOMAIN_PARAM_EXTRATIME_DEFAULT)
+        period = scinfo->extratime;
+    if (scinfo->weight != LIBXL_SCHED_DOMAIN_PARAM_WEIGHT_DEFAULT)
+        period = scinfo->weight;

Regardless which parameter was changed, it is always 'period' which is set
to the changed value.


Juergen

-- 
Juergen Gross                 Principal Developer Operating Systems
PDG ES&S SWE OS6                       Telephone: +49 (0) 89 3222 2967
Fujitsu Technology Solutions              e-mail: juergen.gross@ts.fujitsu.com
Domagkstr. 28                           Internet: ts.fujitsu.com
D-80807 Muenchen                 Company details: ts.fujitsu.com/imprint.html

Re: [PATCH 1 of 3] Support of getting scheduler defaults

[Ian Campbell]

On Wed, 2012-05-23 at 09:45 +0100, Juergen Gross wrote:

> Oh yes:
> 
> +
> +    if (scinfo->period != LIBXL_SCHED_DOMAIN_PARAM_PERIOD_DEFAULT)
> +        period = scinfo->period * 1000000;
> +    if (scinfo->slice != LIBXL_SCHED_DOMAIN_PARAM_SLICE_DEFAULT)
> +        period = scinfo->slice * 1000000;
> +    if (scinfo->latency != LIBXL_SCHED_DOMAIN_PARAM_LATENCY_DEFAULT)
> +        period = scinfo->latency * 1000000;
> +    if (scinfo->extratime != LIBXL_SCHED_DOMAIN_PARAM_EXTRATIME_DEFAULT)
> +        period = scinfo->extratime;
> +    if (scinfo->weight != LIBXL_SCHED_DOMAIN_PARAM_WEIGHT_DEFAULT)
> +        period = scinfo->weight;
> 
> Regardless which parameter was changed, it is always 'period' which is set
> to the changed value.

Oh balls! Thanks for noticing, I'll fix this and resend...

Ian.

Re: [PATCH 1 of 3] Support of getting scheduler defaults

[Dario Faggioli]

[-- Attachment #1.1: Type: text/plain, Size: 1453 bytes --]

On Wed, 2012-05-23 at 10:45 +0200, Juergen Gross wrote:
> >> Not sure, I think Ian's patch fixes that:
> > I thought so to!
> 
> It fixes the original error.
> 
Yep, that was what I was looking at ...

> >> So it has to be something else...
> 
> Oh yes:
> 
> +
> +    if (scinfo->period != LIBXL_SCHED_DOMAIN_PARAM_PERIOD_DEFAULT)
> +        period = scinfo->period * 1000000;
> +    if (scinfo->slice != LIBXL_SCHED_DOMAIN_PARAM_SLICE_DEFAULT)
> +        period = scinfo->slice * 1000000;
> +    if (scinfo->latency != LIBXL_SCHED_DOMAIN_PARAM_LATENCY_DEFAULT)
> +        period = scinfo->latency * 1000000;
> +    if (scinfo->extratime != LIBXL_SCHED_DOMAIN_PARAM_EXTRATIME_DEFAULT)
> +        period = scinfo->extratime;
> +    if (scinfo->weight != LIBXL_SCHED_DOMAIN_PARAM_WEIGHT_DEFAULT)
> +        period = scinfo->weight;
> 
> Regardless which parameter was changed, it is always 'period' which is set
> to the changed value.
> 
 ... And completely ignored this! :-P

However, I'm still having some issues (even after fixing this)!

I'll keep looking at this, testing the series Ian just sent and let you
know.

Thanks and Regards,
Dario

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


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

Re: [PATCH 1 of 3] Support of getting scheduler defaults

[Juergen Gross]

On 05/23/2012 09:22 AM, Dario Faggioli wrote:
> On Wed, 2012-05-23 at 07:34 +0200, Juergen Gross wrote:
>>> #define PERIOD_MAX MILLISECS(10000) /* 10s  */
>>> #define PERIOD_MIN (MICROSECS(10))  /* 10us */
>>> #define SLICE_MIN (MICROSECS(5))    /*  5us */
>> I think this should remain in the hypervisor only.
>>
> Me too.
>
>>> Also, extratime is a flag, so I think 0 and 1 are both meaningful
>>> values, maybe we can go for -1 as for cap (I'll try and let you know).
>> Why not -1 for all values?
>>
> Would work, I guess, unless there's some collision with big weights
> represented on short unsigned value (not sure it's like that, and I've
> always been bad at this kind of math! :-P).

The -1 would be set in scparams only, and there it is an integer. It should
never be written to the "real" scheduler parameters passed to the hypervisor.


Juergen

-- 
Juergen Gross                 Principal Developer Operating Systems
PDG ES&S SWE OS6                       Telephone: +49 (0) 89 3222 2967
Fujitsu Technology Solutions              e-mail: juergen.gross@ts.fujitsu.com
Domagkstr. 28                           Internet: ts.fujitsu.com
D-80807 Muenchen                 Company details: ts.fujitsu.com/imprint.html

[PATCH 2 of 3] Support getting scheduler defaults in libxc

[Juergen Gross]

[-- Attachment #1: Type: text/plain, Size: 386 bytes --]

Add scheduler specific interfaces to get scheduling defaults from the
hypervisor.

Signed-off-by: Juergen Gross <juergen.gross@ts.fujitsu.com>


4 files changed, 73 insertions(+)
tools/libxc/xc_csched.c  |   21 +++++++++++++++++++++
tools/libxc/xc_csched2.c |   21 +++++++++++++++++++++
tools/libxc/xc_sedf.c    |   21 +++++++++++++++++++++
tools/libxc/xenctrl.h    |   10 ++++++++++



[-- Attachment #2: xen-staging.hg-3.patch --]
[-- Type: text/x-patch, Size: 4372 bytes --]

# HG changeset patch
# User Juergen Gross <juergen.gross@ts.fujitsu.com>
# Date 1337675490 -7200
# Node ID 19aaa30d7fdce2f1b56cb13399d603d955a61fb8
# Parent  56c50b3f6cc3eb1de8b86024d0e41e65345d9a79
Support getting scheduler defaults in libxc

Add scheduler specific interfaces to get scheduling defaults from the
hypervisor.

Signed-off-by: Juergen Gross <juergen.gross@ts.fujitsu.com>

diff -r 56c50b3f6cc3 -r 19aaa30d7fdc tools/libxc/xc_csched.c
--- a/tools/libxc/xc_csched.c	Tue May 22 10:14:32 2012 +0200
+++ b/tools/libxc/xc_csched.c	Tue May 22 10:31:30 2012 +0200
@@ -105,3 +105,24 @@ xc_sched_credit_params_get(
 
     return rc;
 }
+
+int
+xc_sched_credit_defaults_get(
+    xc_interface *xch,
+    uint32_t cpupool_id,
+    struct xen_domctl_sched_credit *sdom)
+{
+    int rc;
+    DECLARE_SYSCTL;
+
+    sysctl.cmd = XEN_SYSCTL_scheduler_op;
+    sysctl.u.scheduler_op.cpupool_id = cpupool_id;
+    sysctl.u.scheduler_op.sched_id = XEN_SCHEDULER_CREDIT;
+    sysctl.u.scheduler_op.cmd = XEN_SYSCTL_SCHEDOP_getdefaults;
+
+    rc = do_sysctl(xch, &sysctl);
+    if ( rc == 0 )
+        *sdom = sysctl.u.scheduler_op.u.defaults.credit;
+
+    return rc;
+}
diff -r 56c50b3f6cc3 -r 19aaa30d7fdc tools/libxc/xc_csched2.c
--- a/tools/libxc/xc_csched2.c	Tue May 22 10:14:32 2012 +0200
+++ b/tools/libxc/xc_csched2.c	Tue May 22 10:31:30 2012 +0200
@@ -61,3 +61,24 @@ xc_sched_credit2_domain_get(
 
     return err;
 }
+
+int
+xc_sched_credit2_defaults_get(
+    xc_interface *xch,
+    uint32_t cpupool_id,
+    struct xen_domctl_sched_credit2 *sdom)
+{
+    int rc;
+    DECLARE_SYSCTL;
+
+    sysctl.cmd = XEN_SYSCTL_scheduler_op;
+    sysctl.u.scheduler_op.cpupool_id = cpupool_id;
+    sysctl.u.scheduler_op.sched_id = XEN_SCHEDULER_CREDIT2;
+    sysctl.u.scheduler_op.cmd = XEN_SYSCTL_SCHEDOP_getdefaults;
+
+    rc = do_sysctl(xch, &sysctl);
+    if ( rc == 0 )
+        *sdom = sysctl.u.scheduler_op.u.defaults.credit2;
+
+    return rc;
+}
diff -r 56c50b3f6cc3 -r 19aaa30d7fdc tools/libxc/xc_sedf.c
--- a/tools/libxc/xc_sedf.c	Tue May 22 10:14:32 2012 +0200
+++ b/tools/libxc/xc_sedf.c	Tue May 22 10:31:30 2012 +0200
@@ -76,3 +76,24 @@ int xc_sedf_domain_get(
     *weight    = p->weight;
     return ret;
 }
+
+int
+xc_sched_sedf_defaults_get(
+    xc_interface *xch,
+    uint32_t cpupool_id,
+    struct xen_domctl_sched_sedf *sdom)
+{
+    int rc;
+    DECLARE_SYSCTL;
+
+    sysctl.cmd = XEN_SYSCTL_scheduler_op;
+    sysctl.u.scheduler_op.cpupool_id = cpupool_id;
+    sysctl.u.scheduler_op.sched_id = XEN_SCHEDULER_SEDF;
+    sysctl.u.scheduler_op.cmd = XEN_SYSCTL_SCHEDOP_getdefaults;
+
+    rc = do_sysctl(xch, &sysctl);
+    if ( rc == 0 )
+        *sdom = sysctl.u.scheduler_op.u.defaults.sedf;
+
+    return rc;
+}
diff -r 56c50b3f6cc3 -r 19aaa30d7fdc tools/libxc/xenctrl.h
--- a/tools/libxc/xenctrl.h	Tue May 22 10:14:32 2012 +0200
+++ b/tools/libxc/xenctrl.h	Tue May 22 10:31:30 2012 +0200
@@ -664,6 +664,9 @@ int xc_sedf_domain_get(xc_interface *xch
                        uint64_t* period, uint64_t *slice,
                        uint64_t *latency, uint16_t *extratime,
                        uint16_t *weight);
+int xc_sched_sedf_defaults_get(xc_interface *xch,
+                               uint32_t cpupool_id,
+                               struct xen_domctl_sched_sedf *sdom);
 
 int xc_sched_credit_domain_set(xc_interface *xch,
                                uint32_t domid,
@@ -678,12 +681,19 @@ int xc_sched_credit_params_get(xc_interf
 int xc_sched_credit_params_get(xc_interface *xch,
                               uint32_t cpupool_id,
                               struct xen_sysctl_credit_schedule *schedule);
+int xc_sched_credit_defaults_get(xc_interface *xch,
+                               uint32_t cpupool_id,
+                               struct xen_domctl_sched_credit *sdom);
+
 int xc_sched_credit2_domain_set(xc_interface *xch,
                                uint32_t domid,
                                struct xen_domctl_sched_credit2 *sdom);
 
 int xc_sched_credit2_domain_get(xc_interface *xch,
                                uint32_t domid,
+                               struct xen_domctl_sched_credit2 *sdom);
+int xc_sched_credit2_defaults_get(xc_interface *xch,
+                               uint32_t cpupool_id,
                                struct xen_domctl_sched_credit2 *sdom);
 
 int

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

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

[PATCH 3 of 3] full support of setting scheduler parameters on domain creation

[Juergen Gross]

[-- Attachment #1: Type: text/plain, Size: 487 bytes --]

Obtains default scheduler parameters before modifying any settings specified
via domain config.

Corrected an error for setting sedf parameters (setting .period multiple
times).

Signed-off-by: Juergen Gross <juergen.gross@ts.fujitsu.com>


4 files changed, 72 insertions(+), 6 deletions(-)
tools/libxl/libxl.h         |    2 +
tools/libxl/libxl_dom.c     |   65 +++++++++++++++++++++++++++++++++++++++++--
tools/libxl/libxl_types.idl |    1 
tools/libxl/xl_cmdimpl.c    |   10 ++++--



[-- Attachment #2: xen-staging.hg-3.patch --]
[-- Type: text/x-patch, Size: 6064 bytes --]

# HG changeset patch
# User Juergen Gross <juergen.gross@ts.fujitsu.com>
# Date 1337677423 -7200
# Node ID 953383741ff44d97587c2e75da79b092523d6e83
# Parent  19aaa30d7fdce2f1b56cb13399d603d955a61fb8
full support of setting scheduler parameters on domain creation

Obtains default scheduler parameters before modifying any settings specified
via domain config.

Corrected an error for setting sedf parameters (setting .period multiple
times).

Signed-off-by: Juergen Gross <juergen.gross@ts.fujitsu.com>

diff -r 19aaa30d7fdc -r 953383741ff4 tools/libxl/libxl.h
--- a/tools/libxl/libxl.h	Tue May 22 10:31:30 2012 +0200
+++ b/tools/libxl/libxl.h	Tue May 22 11:03:43 2012 +0200
@@ -605,6 +605,8 @@ int libxl_primary_console_exec(libxl_ctx
 /* May be called with info_r == NULL to check for domain's existance */
 int libxl_domain_info(libxl_ctx*, libxl_dominfo *info_r,
                       uint32_t domid);
+int libxl_sched_set_defaults(libxl_ctx*, uint32_t poolid,
+                             libxl_sched_params *scparams);
 libxl_dominfo * libxl_list_domain(libxl_ctx*, int *nb_domain);
 void libxl_dominfo_list_free(libxl_dominfo *list, int nr);
 libxl_cpupoolinfo * libxl_list_cpupool(libxl_ctx*, int *nb_pool);
diff -r 19aaa30d7fdc -r 953383741ff4 tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c	Tue May 22 10:31:30 2012 +0200
+++ b/tools/libxl/libxl_dom.c	Tue May 22 11:03:43 2012 +0200
@@ -42,17 +42,76 @@ libxl_domain_type libxl__domain_type(lib
         return LIBXL_DOMAIN_TYPE_PV;
 }
 
+int libxl_sched_set_defaults(libxl_ctx *ctx,
+                             uint32_t poolid, libxl_sched_params *scparams)
+{
+    libxl_cpupoolinfo *poolinfo;
+    struct xen_domctl_sched_credit credit_info;
+    struct xen_domctl_sched_credit2 credit2_info;
+    struct xen_domctl_sched_sedf sedf_info;
+    int n_pools, p;
+    int ret;
+
+    poolinfo = libxl_list_cpupool(ctx, &n_pools);
+    if (!poolinfo)
+        return ERROR_NOMEM;
+
+    ret = ERROR_INVAL;
+    for (p = 0; p < n_pools; p++) {
+        if (poolinfo[p].poolid == poolid) {
+            scparams->sched = poolinfo[p].sched;
+            ret = 0;
+        }
+        libxl_cpupoolinfo_dispose(poolinfo + p);
+    }
+
+    if (!ret) {
+        switch (scparams->sched) {
+            case LIBXL_SCHEDULER_SEDF:
+                if (xc_sched_sedf_defaults_get(ctx->xch, poolid,
+                                               &sedf_info))
+                    ret = ERROR_FAIL;
+                else {
+                    scparams->period = sedf_info.period;
+                    scparams->slice = sedf_info.slice;
+                    scparams->latency = sedf_info.latency;
+                    scparams->extratime = sedf_info.extratime;
+                    scparams->weight = sedf_info.weight;
+                }
+                break;
+            case LIBXL_SCHEDULER_CREDIT:
+                if (xc_sched_credit_defaults_get(ctx->xch, poolid,
+                                                 &credit_info))
+                    ret = ERROR_FAIL;
+                else {
+                    scparams->weight = credit_info.weight;
+                    scparams->cap = credit_info.cap;
+                }
+                break;
+            case LIBXL_SCHEDULER_CREDIT2:
+                if (xc_sched_credit2_defaults_get(ctx->xch, poolid,
+                                                  &credit2_info))
+                    ret = ERROR_FAIL;
+                else {
+                    scparams->weight = credit2_info.weight;
+                }
+                break;
+            default: ret = ERROR_INVAL;
+        }
+    }
+
+    return ret;
+}
+
 int libxl__sched_set_params(libxl__gc *gc, uint32_t domid, libxl_sched_params *scparams)
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
-    libxl_scheduler sched;
     libxl_sched_sedf_domain sedf_info;
     libxl_sched_credit_domain credit_info;
     libxl_sched_credit2_domain credit2_info;
     int ret;
 
-    sched = libxl_get_scheduler (ctx);
-    switch (sched) {
+    switch (scparams->sched) {
     case LIBXL_SCHEDULER_SEDF:
       sedf_info.period = scparams->period;
       sedf_info.slice = scparams->slice;
diff -r 19aaa30d7fdc -r 953383741ff4 tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl	Tue May 22 10:31:30 2012 +0200
+++ b/tools/libxl/libxl_types.idl	Tue May 22 11:03:43 2012 +0200
@@ -225,6 +225,7 @@ MemKB = UInt(64, init_val = "LIBXL_MEMKB
 MemKB = UInt(64, init_val = "LIBXL_MEMKB_DEFAULT")
 
 libxl_sched_params = Struct("sched_params",[
+    ("sched",        libxl_scheduler),
     ("weight",       integer),
     ("cap",          integer),
     ("tslice_ms",    integer),
diff -r 19aaa30d7fdc -r 953383741ff4 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Tue May 22 10:31:30 2012 +0200
+++ b/tools/libxl/xl_cmdimpl.c	Tue May 22 11:03:43 2012 +0200
@@ -628,6 +628,10 @@ static void parse_config_data(const char
     libxl_domain_build_info_init_type(b_info, c_info->type);
 
     /* the following is the actual config parsing with overriding values in the structures */
+    if (libxl_sched_set_defaults(ctx, c_info->poolid, &b_info->sched_params)) {
+        fprintf(stderr, "Failed to set default scheduling parameters\n");
+        exit(1);
+    }
     if (!xlu_cfg_get_long (config, "cpu_weight", &l, 0))
         b_info->sched_params.weight = l;
     if (!xlu_cfg_get_long (config, "cap", &l, 0))
@@ -639,11 +643,11 @@ static void parse_config_data(const char
     if (!xlu_cfg_get_long (config, "period", &l, 0))
         b_info->sched_params.period = l;
     if (!xlu_cfg_get_long (config, "slice", &l, 0))
-        b_info->sched_params.period = l;
+        b_info->sched_params.slice = l;
     if (!xlu_cfg_get_long (config, "latency", &l, 0))
-        b_info->sched_params.period = l;
+        b_info->sched_params.latency = l;
     if (!xlu_cfg_get_long (config, "extratime", &l, 0))
-        b_info->sched_params.period = l;
+        b_info->sched_params.extratime = l;
 
     if (!xlu_cfg_get_long (config, "vcpus", &l, 0)) {
         b_info->max_vcpus = l;

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

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

Re: [PATCH 3 of 3] full support of setting scheduler parameters on domain creation

[Ian Campbell]

> # HG changeset patch
> # User Juergen Gross <juergen.gross@ts.fujitsu.com>
> # Date 1337677423 -7200
> # Node ID 953383741ff44d97587c2e75da79b092523d6e83
> # Parent  19aaa30d7fdce2f1b56cb13399d603d955a61fb8
> full support of setting scheduler parameters on domain creation
> 
> Obtains default scheduler parameters before modifying any settings
> specified
> via domain config.
> 
> Corrected an error for setting sedf parameters (setting .period
> multiple
> times).
> 
> Signed-off-by: Juergen Gross <juergen.gross@ts.fujitsu.com>
> 
> diff -r 19aaa30d7fdc -r 953383741ff4 tools/libxl/libxl.h
> --- a/tools/libxl/libxl.h       Tue May 22 10:31:30 2012 +0200
> +++ b/tools/libxl/libxl.h       Tue May 22 11:03:43 2012 +0200
> @@ -605,6 +605,8 @@ int libxl_primary_console_exec(libxl_ctx
>  /* May be called with info_r == NULL to check for domain's existance
> */
>  int libxl_domain_info(libxl_ctx*, libxl_dominfo *info_r,
>                        uint32_t domid);
> +int libxl_sched_set_defaults(libxl_ctx*, uint32_t poolid,
> +                             libxl_sched_params *scparams);

This interface really makes libxl_sched_params differ from all the other
libxl structs (which have a public _init function and an internal
setdefaults function). I'm not really sure its justified either, I was
under the impression that you'd found that there were useful
discriminating values?

If this function was called libxl_sched_init (replacing the
autogenerated one) then it might be ok. Although I'm still not really
sure what the issue is with having a discriminating value meaning
default is, doing that keeps the _init function cheap too.

>  int libxl__sched_set_params(libxl__gc *gc, uint32_t domid,
> libxl_sched_params *scparams)
>  {
>      libxl_ctx *ctx = libxl__gc_owner(gc);
> -    libxl_scheduler sched;
>      libxl_sched_sedf_domain sedf_info;
>      libxl_sched_credit_domain credit_info;
>      libxl_sched_credit2_domain credit2_info;
>      int ret;
>  
> -    sched = libxl_get_scheduler (ctx);
> -    switch (sched) {
> +    switch (scparams->sched) {

What happens if scparams->sched is not the scheduler used for this
domain? Should it either be checked or set somewhere?
> 
> Ian.
> 

Re: [PATCH 3 of 3] full support of setting scheduler parameters on domain creation

[Juergen Gross]

On 05/22/2012 02:30 PM, Ian Campbell wrote:
>> # HG changeset patch
>> # User Juergen Gross<juergen.gross@ts.fujitsu.com>
>> # Date 1337677423 -7200
>> # Node ID 953383741ff44d97587c2e75da79b092523d6e83
>> # Parent  19aaa30d7fdce2f1b56cb13399d603d955a61fb8
>> full support of setting scheduler parameters on domain creation
>>
>> Obtains default scheduler parameters before modifying any settings
>> specified
>> via domain config.
>>
>> Corrected an error for setting sedf parameters (setting .period
>> multiple
>> times).
>>
>> Signed-off-by: Juergen Gross<juergen.gross@ts.fujitsu.com>
>>
>> diff -r 19aaa30d7fdc -r 953383741ff4 tools/libxl/libxl.h
>> --- a/tools/libxl/libxl.h       Tue May 22 10:31:30 2012 +0200
>> +++ b/tools/libxl/libxl.h       Tue May 22 11:03:43 2012 +0200
>> @@ -605,6 +605,8 @@ int libxl_primary_console_exec(libxl_ctx
>>   /* May be called with info_r == NULL to check for domain's existance
>> */
>>   int libxl_domain_info(libxl_ctx*, libxl_dominfo *info_r,
>>                         uint32_t domid);
>> +int libxl_sched_set_defaults(libxl_ctx*, uint32_t poolid,
>> +                             libxl_sched_params *scparams);
> This interface really makes libxl_sched_params differ from all the other
> libxl structs (which have a public _init function and an internal
> setdefaults function). I'm not really sure its justified either, I was
> under the impression that you'd found that there were useful
> discriminating values?

Dario opted for this solution, so I proposed a patch implementing it.
I prefer this solution, too, as it isn't exporting scheduler internals to
the tools.

> If this function was called libxl_sched_init (replacing the
> autogenerated one) then it might be ok. Although I'm still not really
> sure what the issue is with having a discriminating value meaning
> default is, doing that keeps the _init function cheap too.

Okay, I can rename it.

>
>>   int libxl__sched_set_params(libxl__gc *gc, uint32_t domid,
>> libxl_sched_params *scparams)
>>   {
>>       libxl_ctx *ctx = libxl__gc_owner(gc);
>> -    libxl_scheduler sched;
>>       libxl_sched_sedf_domain sedf_info;
>>       libxl_sched_credit_domain credit_info;
>>       libxl_sched_credit2_domain credit2_info;
>>       int ret;
>>
>> -    sched = libxl_get_scheduler (ctx);
>> -    switch (sched) {
>> +    switch (scparams->sched) {
> What happens if scparams->sched is not the scheduler used for this
> domain? Should it either be checked or set somewhere?

The check would be the same as the original setting of scparams->sched.
Setting of the scheduler parameters will be rejected by the hypervisor if the
scheduler does not match.


Juergen

-- 
Juergen Gross                 Principal Developer Operating Systems
PDG ES&S SWE OS6                       Telephone: +49 (0) 89 3222 2967
Fujitsu Technology Solutions              e-mail: juergen.gross@ts.fujitsu.com
Domagkstr. 28                           Internet: ts.fujitsu.com
D-80807 Muenchen                 Company details: ts.fujitsu.com/imprint.html

Re: [PATCH 3 of 3] full support of setting scheduler parameters on domain creation

[Ian Campbell]

On Tue, 2012-05-22 at 13:39 +0100, Juergen Gross wrote:
> On 05/22/2012 02:30 PM, Ian Campbell wrote:
> >> # HG changeset patch
> >> # User Juergen Gross<juergen.gross@ts.fujitsu.com>
> >> # Date 1337677423 -7200
> >> # Node ID 953383741ff44d97587c2e75da79b092523d6e83
> >> # Parent  19aaa30d7fdce2f1b56cb13399d603d955a61fb8
> >> full support of setting scheduler parameters on domain creation
> >>
> >> Obtains default scheduler parameters before modifying any settings
> >> specified
> >> via domain config.
> >>
> >> Corrected an error for setting sedf parameters (setting .period
> >> multiple
> >> times).
> >>
> >> Signed-off-by: Juergen Gross<juergen.gross@ts.fujitsu.com>
> >>
> >> diff -r 19aaa30d7fdc -r 953383741ff4 tools/libxl/libxl.h
> >> --- a/tools/libxl/libxl.h       Tue May 22 10:31:30 2012 +0200
> >> +++ b/tools/libxl/libxl.h       Tue May 22 11:03:43 2012 +0200
> >> @@ -605,6 +605,8 @@ int libxl_primary_console_exec(libxl_ctx
> >>   /* May be called with info_r == NULL to check for domain's existance
> >> */
> >>   int libxl_domain_info(libxl_ctx*, libxl_dominfo *info_r,
> >>                         uint32_t domid);
> >> +int libxl_sched_set_defaults(libxl_ctx*, uint32_t poolid,
> >> +                             libxl_sched_params *scparams);
> > This interface really makes libxl_sched_params differ from all the other
> > libxl structs (which have a public _init function and an internal
> > setdefaults function). I'm not really sure its justified either, I was
> > under the impression that you'd found that there were useful
> > discriminating values?
> 
> Dario opted for this solution, so I proposed a patch implementing it.
> I prefer this solution, too, as it isn't exporting scheduler internals to
> the tools.

I don't think "-1 is not a valid weight" is really "exporting scheduler
internals".

Anyway, I am far more concerned that the libxl API is useful to users
than I am about the API between libxl/libxc and the hypervisor (which is
not set in stone and can be fixed etc). The libxl API is the one we want
to expose and keep stable going forward etc.

> 
> > If this function was called libxl_sched_init (replacing the
> > autogenerated one) then it might be ok. Although I'm still not really
> > sure what the issue is with having a discriminating value meaning
> > default is, doing that keeps the _init function cheap too.
> 
> Okay, I can rename it.
> 
> >
> >>   int libxl__sched_set_params(libxl__gc *gc, uint32_t domid,
> >> libxl_sched_params *scparams)
> >>   {
> >>       libxl_ctx *ctx = libxl__gc_owner(gc);
> >> -    libxl_scheduler sched;
> >>       libxl_sched_sedf_domain sedf_info;
> >>       libxl_sched_credit_domain credit_info;
> >>       libxl_sched_credit2_domain credit2_info;
> >>       int ret;
> >>
> >> -    sched = libxl_get_scheduler (ctx);
> >> -    switch (sched) {
> >> +    switch (scparams->sched) {
> > What happens if scparams->sched is not the scheduler used for this
> > domain? Should it either be checked or set somewhere?
> 
> The check would be the same as the original setting of scparams->sched.
> Setting of the scheduler parameters will be rejected by the hypervisor if the
> scheduler does not match.
> 
> 
> Juergen
> 

Re: [PATCH 3 of 3] full support of setting scheduler parameters on domain creation

[Dario Faggioli]

[-- Attachment #1.1: Type: text/plain, Size: 2430 bytes --]

On Tue, 2012-05-22 at 14:39 +0200, Juergen Gross wrote:
> > This interface really makes libxl_sched_params differ from all the other
> > libxl structs (which have a public _init function and an internal
> > setdefaults function). I'm not really sure its justified either, I was
> > under the impression that you'd found that there were useful
> > discriminating values?
> 
> Dario opted for this solution, so I proposed a patch implementing it.
> I prefer this solution, too, as it isn't exporting scheduler internals to
> the tools.
> 
Yep. I agree with Ian that having two different procedures for the first
as compared to the subsequent operations on the scheduling parameters is
bad, but still, as a matter of my personal taste, I'd prefer no to have
things belonging to the hypervisor/scheduler replicated in the
toolstack, although they're just simple default values. It's still
something you need to always remember to check for consistency, or bad
things will happen! :-/

However, Ian's approach seems clean and nice as well, and I'm not really
sure which one I like most, so don't count me when deciding, I'm fine
with both.

> >>   int libxl__sched_set_params(libxl__gc *gc, uint32_t domid,
> >> libxl_sched_params *scparams)
> >>   {
> >>       libxl_ctx *ctx = libxl__gc_owner(gc);
> >> -    libxl_scheduler sched;
> >>       libxl_sched_sedf_domain sedf_info;
> >>       libxl_sched_credit_domain credit_info;
> >>       libxl_sched_credit2_domain credit2_info;
> >>       int ret;
> >>
> >> -    sched = libxl_get_scheduler (ctx);
> >> -    switch (sched) {
> >> +    switch (scparams->sched) {
> > What happens if scparams->sched is not the scheduler used for this
> > domain? Should it either be checked or set somewhere?
> 
> The check would be the same as the original setting of scparams->sched.
> Setting of the scheduler parameters will be rejected by the hypervisor if the
> scheduler does not match.
> 
I was thinking this should go through finding out what domid's cpupool
is and then checking that scheduler, as it is being done somewhere
else... Isn't this the case?

Regards,
Dario

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


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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