Re: [Xen-users] xl doesn't honour the parameter cpu_weight from my config file while xm does honour it

Re: [Xen-users] xl doesn't honour the parameter cpu_weight from my config file while xm does honour it

[Ian Campbell]

Hi Dieter, 

thanks for the report.

It seems that support for this config file variable is not present in xl
at the moment. We should try and add this for 4.2 IMHO.

If you know a little bit of C (or are interested in learning) then this
should be a pretty simple thing to implement -- please let me know if
you want more details.

Ian.

On Fri, 2012-04-20 at 16:00 +0100, Dieter Bloms wrote:
> Hi,
> 
> I've installed xen-unstable 4.2 from actual git (last commit was
> 4dc7dbef5400f0608321d579aebb57f933e8f707).
> 
> When I start a domU with xm all is fine include the cpu_weight I
> configured in my domU config.
> 
> When I start the domU with xl then all my domU have the default
> cpu_weight of 256 instead of the configured one.
> 
> Was the name of cpu_weight being changed for xl command ?
> 
> My domU config looks like this:
> 
> --snip--
> name="vdrserver"
> description="vdrserver for my clients"
> memory=768
> maxmem=2048
> vcpus=1
> cpus="1"
> cpu_weight = 128
> on_poweroff="destroy"
> on_reboot="restart"
> on_crash="destroy"
> 
> localtime=0
> keymap="de"
> 
> builder="linux"
> bootloader="/usr/bin/pygrub"
> bootargs=""
> extra="console=hvc0 tmem cgroup_disable=memory independent_wallclock=1 iommu=soft"
> nographic=1
> keymap = 'de'
> 
> disk=[ 
>   'phy:/dev/mapper/xenimages-vdrserver,xvda1,w',
>   'phy:/dev/mapper/xenimages-swap_vdrserver,xvda2,w',
> ]
> vif=[ 'mac=00:00:00:00:00:80,bridge=br0', ]
> 
> pci = [ '06:00.0', '06:01.0', '00:12.2', '00:13.2']
> --snip--
> 
> 

Re: [Xen-users] xl doesn't honour the parameter cpu_weight from my config file while xm does honour it

[Dieter Bloms]

Hi,

I'am not a programmer, but will have a look at it the next days and let
you know if my skills are good enough.

On Fri, Apr 20, Ian Campbell wrote:

> Hi Dieter, 
> 
> thanks for the report.
> 
> It seems that support for this config file variable is not present in xl
> at the moment. We should try and add this for 4.2 IMHO.
> 
> If you know a little bit of C (or are interested in learning) then this
> should be a pretty simple thing to implement -- please let me know if
> you want more details.
> 
> Ian.
> 
> On Fri, 2012-04-20 at 16:00 +0100, Dieter Bloms wrote:
> > Hi,
> > 
> > I've installed xen-unstable 4.2 from actual git (last commit was
> > 4dc7dbef5400f0608321d579aebb57f933e8f707).
> > 
> > When I start a domU with xm all is fine include the cpu_weight I
> > configured in my domU config.
> > 
> > When I start the domU with xl then all my domU have the default
> > cpu_weight of 256 instead of the configured one.
> > 
> > Was the name of cpu_weight being changed for xl command ?
> > 
> > My domU config looks like this:
> > 
> > --snip--
> > name="vdrserver"
> > description="vdrserver for my clients"
> > memory=768
> > maxmem=2048
> > vcpus=1
> > cpus="1"
> > cpu_weight = 128
> > on_poweroff="destroy"
> > on_reboot="restart"
> > on_crash="destroy"
> > 
> > localtime=0
> > keymap="de"
> > 
> > builder="linux"
> > bootloader="/usr/bin/pygrub"
> > bootargs=""
> > extra="console=hvc0 tmem cgroup_disable=memory independent_wallclock=1 iommu=soft"
> > nographic=1
> > keymap = 'de'
> > 
> > disk=[ 
> >   'phy:/dev/mapper/xenimages-vdrserver,xvda1,w',
> >   'phy:/dev/mapper/xenimages-swap_vdrserver,xvda2,w',
> > ]
> > vif=[ 'mac=00:00:00:00:00:80,bridge=br0', ]
> > 
> > pci = [ '06:00.0', '06:01.0', '00:12.2', '00:13.2']
> > --snip--
> > 
> > 
> 
> 

-- 
Gruß

  Dieter

--
I do not get viruses because I do not use MS software.
If you use Outlook then please do not put my email address in your
address-book so that WHEN you get a virus it won't use my address in the
>From field.

Re: [Xen-users] xl doesn't honour the parameter cpu_weight from my config file while xm does honour it

[Dieter Bloms]

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

Hello Ian,

I've made a little patch to set the cpu_weight for credit, credit2 and
sedf scheduler from the config file.

Btw.: for the sedf scheduler there seems to be some type mismatch.  
The functions xc_sedf_domain_set and xc_sedf_domain_get expect the type
'uint16_t' for variables 'extratime' and 'weight' while the structure
'xen_domctl_sched_sedf' defines the type uint32_t for them.
I think they should be the same, or not ?

Anyway, I've tested this patch for the credit scheduler and it works so
far.


On Fri, Apr 20, Ian Campbell wrote:

> Hi Dieter, 
> 
> thanks for the report.
> 
> It seems that support for this config file variable is not present in xl
> at the moment. We should try and add this for 4.2 IMHO.
> 
> If you know a little bit of C (or are interested in learning) then this
> should be a pretty simple thing to implement -- please let me know if
> you want more details.
> 
> Ian.
> 
> On Fri, 2012-04-20 at 16:00 +0100, Dieter Bloms wrote:
> > Hi,
> > 
> > I've installed xen-unstable 4.2 from actual git (last commit was
> > 4dc7dbef5400f0608321d579aebb57f933e8f707).
> > 
> > When I start a domU with xm all is fine include the cpu_weight I
> > configured in my domU config.
> > 
> > When I start the domU with xl then all my domU have the default
> > cpu_weight of 256 instead of the configured one.
> > 
> > Was the name of cpu_weight being changed for xl command ?
> > 
> > My domU config looks like this:
> > 
> > --snip--
> > name="vdrserver"
> > description="vdrserver for my clients"
> > memory=768
> > maxmem=2048
> > vcpus=1
> > cpus="1"
> > cpu_weight = 128
> > on_poweroff="destroy"
> > on_reboot="restart"
> > on_crash="destroy"
> > 
> > localtime=0
> > keymap="de"
> > 
> > builder="linux"
> > bootloader="/usr/bin/pygrub"
> > bootargs=""
> > extra="console=hvc0 tmem cgroup_disable=memory independent_wallclock=1 iommu=soft"
> > nographic=1
> > keymap = 'de'
> > 
> > disk=[ 
> >   'phy:/dev/mapper/xenimages-vdrserver,xvda1,w',
> >   'phy:/dev/mapper/xenimages-swap_vdrserver,xvda2,w',
> > ]
> > vif=[ 'mac=00:00:00:00:00:80,bridge=br0', ]
> > 
> > pci = [ '06:00.0', '06:01.0', '00:12.2', '00:13.2']
> > --snip--
> > 
> > 
> 
> 
> 
> _______________________________________________
> Xen-users mailing list
> Xen-users@lists.xen.org
> http://lists.xen.org/xen-users

-- 
Gruß

  Dieter

--
I do not get viruses because I do not use MS software.
If you use Outlook then please do not put my email address in your
address-book so that WHEN you get a virus it won't use my address in the
From field.

[-- Attachment #2: add_support_for_cpu_weight_config_in_xl.diff --]
[-- Type: text/x-diff, Size: 3621 bytes --]

diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index e63c7bd..706e282 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -110,6 +110,10 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
         return ERROR_INVAL;
     }
 
+    if (!b_info->weight)
+        b_info->weight = 256;
+    if (!b_info->cap)
+        b_info->cap = 0;
     if (!b_info->max_vcpus)
         b_info->max_vcpus = 1;
     if (!b_info->cur_vcpus)
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 0bdd654..f858a42 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -65,9 +65,38 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid,
     libxl_ctx *ctx = libxl__gc_owner(gc);
     int tsc_mode;
     char *xs_domid, *con_domid;
+    libxl_scheduler sched;
+    struct xen_domctl_scheduler_op sched_op;
+
     xc_domain_max_vcpus(ctx->xch, domid, info->max_vcpus);
     libxl_set_vcpuaffinity_all(ctx, domid, info->max_vcpus, &info->cpumap);
     xc_domain_setmaxmem(ctx->xch, domid, info->target_memkb + LIBXL_MAXMEM_CONSTANT);
+
+    sched = libxl_get_scheduler (ctx);
+    
+    switch (sched) {
+    case LIBXL_SCHEDULER_SEDF:
+      xc_sedf_domain_get (ctx->xch, domid, &(sched_op.u.sedf.period), &(sched_op.u.sedf.slice), &(sched_op.u.sedf.latency), (uint16_t *) &(sched_op.u.sedf.extratime), (uint16_t *) &(sched_op.u.sedf.weight));
+      sched_op.u.sedf.weight = info->weight;
+      xc_sedf_domain_set (ctx->xch, domid, sched_op.u.sedf.period, sched_op.u.sedf.slice, sched_op.u.sedf.latency, (uint16_t) sched_op.u.sedf.extratime, (uint16_t) sched_op.u.sedf.weight);
+      break;
+    case LIBXL_SCHEDULER_CREDIT:
+//      struct xen_domctl_sched_credit sdom;
+      sched_op.u.credit.weight = info->weight;
+      sched_op.u.credit.cap = info->cap;
+      xc_sched_credit_domain_set(ctx->xch, domid, &(sched_op.u.credit));
+      break;
+    case LIBXL_SCHEDULER_CREDIT2:
+      sched_op.u.credit2.weight = info->weight;
+      xc_sched_credit2_domain_set(ctx->xch, domid, &(sched_op.u.credit2));
+      break;
+    case LIBXL_SCHEDULER_ARINC653:
+      /* not implemented */
+      break;
+    default:
+        abort();
+    }
+
     if (info->type == LIBXL_DOMAIN_TYPE_PV)
         xc_domain_set_memmap_limit(ctx->xch, domid,
                 (info->max_memkb + info->u.pv.slack_memkb));
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 5cf9708..f185d4c 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -232,6 +232,8 @@ MemKB = UInt(64, init_val = "LIBXL_MEMKB_DEFAULT")
 libxl_domain_build_info = Struct("domain_build_info",[
     ("max_vcpus",       integer),
     ("cur_vcpus",       integer),
+    ("weight",          integer),
+    ("cap",             integer),
     ("cpumap",          libxl_cpumap),
     ("tsc_mode",        libxl_tsc_mode),
     ("max_memkb",       MemKB),
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 5703512..d7dcb84 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -587,6 +587,11 @@ static void parse_config_data(const char *configfile_filename_report,
     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 (!xlu_cfg_get_long (config, "cpu_weight", &l, 0))
+        b_info->weight = l;
+    if (!xlu_cfg_get_long (config, "cap", &l, 0))
+        b_info->cap = l;
+
     if (!xlu_cfg_get_long (config, "vcpus", &l, 0)) {
         b_info->max_vcpus = l;
         b_info->cur_vcpus = (1 << l) - 1;

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

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

Re: [Xen-users] xl doesn't honour the parameter cpu_weight from my config file while xm does honour it

[Ian Campbell]

(xen-users to BCC, this seems like a devel thing for now)

On Mon, 2012-04-23 at 10:46 +0100, Dieter Bloms wrote:
> Hello Ian,
> 
> I've made a little patch to set the cpu_weight for credit, credit2 and
> sedf scheduler from the config file.

Awesome, thank you!

> Btw.: for the sedf scheduler there seems to be some type mismatch.  
> The functions xc_sedf_domain_set and xc_sedf_domain_get expect the type
> 'uint16_t' for variables 'extratime' and 'weight' while the structure
> 'xen_domctl_sched_sedf' defines the type uint32_t for them.
> I think they should be the same, or not ?

I'm not clear enough on the scheduling stuff to be able to say one way
or another. I'd be inclined to suggest that if this needs to change for
some reason then we should leave it for post 4.2.

> Anyway, I've tested this patch for the credit scheduler and it works so
> far.

Cool. One thing which we need in order to able to apply a patch is a
"Signed-off-by" per the DCO as described in
http://wiki.xen.org/wiki/SubmittingXenPatches. 

I've also a few minor comments on the patch itself below.

> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index e63c7bd..706e282 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -110,6 +110,10 @@ int libxl__domain_build_info_setdefault(libxl__gc
> *gc,
>          return ERROR_INVAL;
>      }
>  
> +    if (!b_info->weight)
> +        b_info->weight = 256;

I suppose that weight=0 does not make sense and therefore 0 is available
as a discriminating value meaning "the default".

> +    if (!b_info->cap)
> +        b_info->cap = 0;

!b_info->cap means that b_info_cap is already 0, although I guess this
serves as a more explicit indication of the default.

>      if (!b_info->max_vcpus)
>          b_info->max_vcpus = 1;
>      if (!b_info->cur_vcpus)
> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> index 0bdd654..f858a42 100644
> --- a/tools/libxl/libxl_dom.c
> +++ b/tools/libxl/libxl_dom.c
> @@ -65,9 +65,38 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid,
>      libxl_ctx *ctx = libxl__gc_owner(gc);
>      int tsc_mode;
>      char *xs_domid, *con_domid;
> +    libxl_scheduler sched;
> +    struct xen_domctl_scheduler_op sched_op;
> +
>      xc_domain_max_vcpus(ctx->xch, domid, info->max_vcpus);
>      libxl_set_vcpuaffinity_all(ctx, domid, info->max_vcpus,
> &info->cpumap);
>      xc_domain_setmaxmem(ctx->xch, domid, info->target_memkb +
> LIBXL_MAXMEM_CONSTANT);
> +
> +    sched = libxl_get_scheduler (ctx);
> +    
> +    switch (sched) {
> +    case LIBXL_SCHEDULER_SEDF:
> +      xc_sedf_domain_get (ctx->xch, domid, &(sched_op.u.sedf.period),
> &(sched_op.u.sedf.slice), &(sched_op.u.sedf.latency), (uint16_t *)
> &(sched_op.u.sedf.extratime), (uint16_t *) &(sched_op.u.sedf.weight));
> +      sched_op.u.sedf.weight = info->weight;
> +      xc_sedf_domain_set (ctx->xch, domid, sched_op.u.sedf.period,
> sched_op.u.sedf.slice, sched_op.u.sedf.latency, (uint16_t)
> sched_op.u.sedf.extratime, (uint16_t) sched_op.u.sedf.weight);

Wow, that's a pretty sucky interface at the libxc level! Anyway no need
for you to fix it here but please do wrap your lines to <80 columns for
readability.

> +      break;
> +    case LIBXL_SCHEDULER_CREDIT:
> +//      struct xen_domctl_sched_credit sdom;

Please don't leave commented out code in place.

> +      sched_op.u.credit.weight = info->weight;
> +      sched_op.u.credit.cap = info->cap;
> +      xc_sched_credit_domain_set(ctx->xch, domid,
> &(sched_op.u.credit));
> +      break;
> +    case LIBXL_SCHEDULER_CREDIT2:
> +      sched_op.u.credit2.weight = info->weight;
> +      xc_sched_credit2_domain_set(ctx->xch, domid,
> &(sched_op.u.credit2));
> +      break;
> +    case LIBXL_SCHEDULER_ARINC653:
> +      /* not implemented */
> +      break;
> +    default:
> +        abort();
> +    }
> +
>      if (info->type == LIBXL_DOMAIN_TYPE_PV)
>          xc_domain_set_memmap_limit(ctx->xch, domid,
>                  (info->max_memkb + info->u.pv.slack_memkb));
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index 5cf9708..f185d4c 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -232,6 +232,8 @@ MemKB = UInt(64, init_val = "LIBXL_MEMKB_DEFAULT")
>  libxl_domain_build_info = Struct("domain_build_info",[
>      ("max_vcpus",       integer),
>      ("cur_vcpus",       integer),
> +    ("weight",          integer),
> +    ("cap",             integer),

Are these values common parameters to all schedulers? Do they always
have the same meaning/units/etc?

I wonder if perhaps including each of libxl_sched_*_params in build_info
might be a preferable interface? I would probably cleanup the above code
in build_pre too since you could just call the appropriate
libxl_sched_*_params_set.

Ideally libxl_sched_*_params would be in a union in
libxl_domain_build_info but that would require that xl could easily
determine which scheduler was going to be used for the domain, having a
non-union here would keep things somewhat simpler from that PoV.

I've CC'd George and Dario for their thoughts on this interface. We
should try and keep it simple so that we can get this into 4.2. 

>      ("cpumap",          libxl_cpumap),
>      ("tsc_mode",        libxl_tsc_mode),
>      ("max_memkb",       MemKB),
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 5703512..d7dcb84 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -587,6 +587,11 @@ static void parse_config_data(const char
> *configfile_filename_report,
>      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 (!xlu_cfg_get_long (config, "cpu_weight", &l, 0))
> +        b_info->weight = l;
> +    if (!xlu_cfg_get_long (config, "cap", &l, 0))
> +        b_info->cap = l;
> +
>      if (!xlu_cfg_get_long (config, "vcpus", &l, 0)) {
>          b_info->max_vcpus = l;
>          b_info->cur_vcpus = (1 << l) - 1;

Re: [Xen-users] xl doesn't honour the parameter cpu_weight from my config file while xm does honour it

[Dario Faggioli]

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

On Mon, 2012-04-23 at 13:04 +0100, Ian Campbell wrote: 
> > I've made a little patch to set the cpu_weight for credit, credit2 and
> > sedf scheduler from the config file.
> 
> Awesome, thank you!
> 
Agreed, nice work and thank you for doing it!

> > Btw.: for the sedf scheduler there seems to be some type mismatch.  
> > The functions xc_sedf_domain_set and xc_sedf_domain_get expect the type
> > 'uint16_t' for variables 'extratime' and 'weight' while the structure
> > 'xen_domctl_sched_sedf' defines the type uint32_t for them.
> > I think they should be the same, or not ?
> 
> I'm not clear enough on the scheduling stuff to be able to say one way
> or another. I'd be inclined to suggest that if this needs to change for
> some reason then we should leave it for post 4.2.
> 
In Xen's SEDF code weight is a 'short', while extratime is nothing more
than a boolean flag (it can store only the EXTRA_AWARE flag). So I think
it would be worth looking at this, but I agree we ca defer that for now,
especially considering the whole SEDF interface might need some
restructuring! :-/

> >      xc_domain_max_vcpus(ctx->xch, domid, info->max_vcpus);
> >      libxl_set_vcpuaffinity_all(ctx, domid, info->max_vcpus,
> > &info->cpumap);
> >      xc_domain_setmaxmem(ctx->xch, domid, info->target_memkb +
> > LIBXL_MAXMEM_CONSTANT);
> > +
> > +    sched = libxl_get_scheduler (ctx);
> > +    
> > +    switch (sched) {
> > +    case LIBXL_SCHEDULER_SEDF:
> > +      xc_sedf_domain_get (ctx->xch, domid, &(sched_op.u.sedf.period),
> > &(sched_op.u.sedf.slice), &(sched_op.u.sedf.latency), (uint16_t *)
> > &(sched_op.u.sedf.extratime), (uint16_t *) &(sched_op.u.sedf.weight));
> > +      sched_op.u.sedf.weight = info->weight;
> > +      xc_sedf_domain_set (ctx->xch, domid, sched_op.u.sedf.period,
> > sched_op.u.sedf.slice, sched_op.u.sedf.latency, (uint16_t)
> > sched_op.u.sedf.extratime, (uint16_t) sched_op.u.sedf.weight);
> 
> Wow, that's a pretty sucky interface at the libxc level! Anyway no need
> for you to fix it here but please do wrap your lines to <80 columns for
> readability.
> 
Indeed! I really think we have to do something about this, but that
definitely will happen post-release. :-)

> > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> > index 5cf9708..f185d4c 100644
> > --- a/tools/libxl/libxl_types.idl
> > +++ b/tools/libxl/libxl_types.idl
> > @@ -232,6 +232,8 @@ MemKB = UInt(64, init_val = "LIBXL_MEMKB_DEFAULT")
> >  libxl_domain_build_info = Struct("domain_build_info",[
> >      ("max_vcpus",       integer),
> >      ("cur_vcpus",       integer),
> > +    ("weight",          integer),
> > +    ("cap",             integer),
> 
> Are these values common parameters to all schedulers? Do they always
> have the same meaning/units/etc?
> 
Maybe weight is, but still, I think having some mechanism for specifying
the full set of parameter of a specific scheduler is to be preferred...

> I wonder if perhaps including each of libxl_sched_*_params in build_info
> might be a preferable interface? I would probably cleanup the above code
> in build_pre too since you could just call the appropriate
> libxl_sched_*_params_set.
> 
... Exactly, that's much better looking to me.

> Ideally libxl_sched_*_params would be in a union in
> libxl_domain_build_info but that would require that xl could easily
> determine which scheduler was going to be used for the domain, having a
> non-union here would keep things somewhat simpler from that PoV.
> 
Yes, again, I agree that a union will be even better, but maybe not so
much a big deal for now (we can turn it into an union later, right? Or
you think there will be some API implications?)

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: [Xen-users] xl doesn't honour the parameter cpu_weight from my config file while xm does honour it

[Dieter Bloms]

Hi,

> Maybe weight is, but still, I think having some mechanism for specifying
> the full set of parameter of a specific scheduler is to be preferred...
> 
> > I wonder if perhaps including each of libxl_sched_*_params in build_info
> > might be a preferable interface? I would probably cleanup the above code
> > in build_pre too since you could just call the appropriate
> > libxl_sched_*_params_set.
> > 
> ... Exactly, that's much better looking to me.

that's a good hint.

> > Ideally libxl_sched_*_params would be in a union in
> > libxl_domain_build_info but that would require that xl could easily
> > determine which scheduler was going to be used for the domain, having a
> > non-union here would keep things somewhat simpler from that PoV.
> > 
> Yes, again, I agree that a union will be even better, but maybe not so
> much a big deal for now (we can turn it into an union later, right? Or
> you think there will be some API implications?)

ok, I see the point and will change it to a union.
I checked out the source with git not hg, may I produce a diff with git
and send it via mail ?


-- 
Best regards

  Dieter Bloms

--
I do not get viruses because I do not use MS software.
If you use Outlook then please do not put my email address in your
address-book so that WHEN you get a virus it won't use my address in the
>From field.

Re: [Xen-users] xl doesn't honour the parameter cpu_weight from my config file while xm does honour it

[Dario Faggioli]

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

On Mon, 2012-04-23 at 16:41 +0100, Dieter Bloms wrote:
> ok, I see the point and will change it to a union.
>
Cool!

> I checked out the source with git not hg, may I produce a diff with git
> and send it via mail ?
> 
I think you should. Either use something automatic like git `send-email'
or just produce the diff and then inline or attach, following also the
other instructions Ian pointed you to (on the wiki)... Was it this you
were asking?

Regads,
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: [Xen-users] xl doesn't honour the parameter cpu_weight from my config file while xm does honour it

[Dieter Bloms]

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

Hi,


On Mon, Apr 23, Dario Faggioli wrote:

> > I checked out the source with git not hg, may I produce a diff with git
> > and send it via mail ?
> > 
> I think you should. Either use something automatic like git `send-email'
> or just produce the diff and then inline or attach, following also the
> other instructions Ian pointed you to (on the wiki)... Was it this you
> were asking?

yes.

ok, second try ;)
I hope this fit more your needs.

I've defined a new union in the libxl_domain_build_info structure.
For this I had to move some structure definition like
libxl_sched_credit_domain more to the top.
I named the union 'us' for union scheduler, because the letter 'u' was
already used for the type of domain.


-- 
Best regards

  Dieter Bloms

--
I do not get viruses because I do not use MS software.
If you use Outlook then please do not put my email address in your
address-book so that WHEN you get a virus it won't use my address in the
From field.

[-- Attachment #2: add_support_for_cpu_weight_config_in_xl.diff --]
[-- Type: text/x-diff, Size: 5166 bytes --]

libxl: set domain scheduling parameters while creating the dom

the domain specific scheduling parameters like cpu_weight, cap, slice, ...
will be set during creating the domain, so this parameters can be defined
in the domain config file

Signed-off-by: Dieter Bloms <dieter@bloms.de>

diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 0bdd654..38acff4 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -124,8 +124,27 @@ int libxl__build_post(libxl__gc *gc, uint32_t domid,
     char *dom_path, *vm_path;
     xs_transaction_t t;
     char **ents, **hvm_ents;
+    libxl_scheduler sched;
     int i;
 
+    sched = libxl_get_scheduler (ctx);
+    switch (sched) {
+    case LIBXL_SCHEDULER_SEDF:
+      libxl_sched_sedf_domain_set(ctx, domid, &(info->us.sedf));
+      break;
+    case LIBXL_SCHEDULER_CREDIT:
+      libxl_sched_credit_domain_set(ctx, domid, &(info->us.credit));
+      break;
+    case LIBXL_SCHEDULER_CREDIT2:
+      libxl_sched_credit2_domain_set(ctx, domid, &(info->us.credit2));
+      break;
+    case LIBXL_SCHEDULER_ARINC653:
+      /* not implemented */
+      break;
+    default:
+        abort();
+    }
+
     libxl_cpuid_apply_policy(ctx, domid);
     if (info->cpuid != NULL)
         libxl_cpuid_set(ctx, domid, info->cpuid);
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 5cf9708..c1cdc3c 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -224,6 +224,27 @@ libxl_domain_create_info = Struct("domain_create_info",[
 
 MemKB = UInt(64, init_val = "LIBXL_MEMKB_DEFAULT")
 
+libxl_sched_credit_domain = Struct("sched_credit_domain", [
+    ("weight", integer),
+    ("cap", integer),
+    ])
+
+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_sched_arinc653_domain = Struct("sched_arinc653_domain", [
+    ("weight", integer),
+    ])
+
 # Instances of libxl_file_reference contained in this struct which
 # have been mapped (with libxl_file_reference_map) will be unmapped
 # by libxl_domain_build/restore. If either of these are never called
@@ -256,6 +277,13 @@ libxl_domain_build_info = Struct("domain_build_info",[
     # extra parameters pass directly to qemu for HVM guest, NULL terminated
     ("extra_hvm",        libxl_string_list),
 
+    ("us", KeyedUnion(None, libxl_scheduler, "sched",
+                 [("credit", libxl_sched_credit_domain),
+                 ("credit2", libxl_sched_credit2_domain),
+                 ("sedf", libxl_sched_sedf_domain),
+                 ("arinc653", libxl_sched_arinc653_domain),
+                 ], keyvar_init_val = "-1")),
+
     ("u", KeyedUnion(None, libxl_domain_type, "type",
                 [("hvm", Struct(None, [("firmware",         string),
                                        ("bios",             libxl_bios_type),
@@ -417,28 +445,12 @@ 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_event_type = Enumeration("event_type", [
     (1, "DOMAIN_SHUTDOWN"),
     (2, "DOMAIN_DEATH"),
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 5703512..db51ca6 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -587,6 +587,23 @@ static void parse_config_data(const char *configfile_filename_report,
     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 (!xlu_cfg_get_long (config, "cpu_weight", &l, 0))
+        b_info->us.credit.weight = l;
+        b_info->us.credit2.weight = l;
+        b_info->us.sedf.weight = l;
+
+    if (!xlu_cfg_get_long (config, "cap", &l, 0))
+        b_info->us.credit.cap = l;
+
+    if (!xlu_cfg_get_long (config, "period", &l, 0))
+        b_info->us.sedf.period = l;
+    if (!xlu_cfg_get_long (config, "slice", &l, 0))
+        b_info->us.sedf.period = l;
+    if (!xlu_cfg_get_long (config, "latency", &l, 0))
+        b_info->us.sedf.period = l;
+    if (!xlu_cfg_get_long (config, "extratime", &l, 0))
+        b_info->us.sedf.period = l;
+
     if (!xlu_cfg_get_long (config, "vcpus", &l, 0)) {
         b_info->max_vcpus = l;
         b_info->cur_vcpus = (1 << l) - 1;

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

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

Re: [Xen-users] xl doesn't honour the parameter cpu_weight from my config file while xm does honour it

[Dario Faggioli]

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

On Mon, 2012-04-23 at 20:35 +0100, Dieter Bloms wrote: 
> ok, second try ;)
> I hope this fit more your needs.
> 
This really looks much better to me!

> I've defined a new union in the libxl_domain_build_info structure.
> For this I had to move some structure definition like
> libxl_sched_credit_domain more to the top.
>
That's fine.

What might be missing is some documentation in docs/man/xl.cfg.pod.5,
explaining about the new options... :-)

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: [Xen-users] xl doesn't honour the parameter cpu_weight from my config file while xm does honour it

[Dieter Bloms]

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

Hi,


On Tue, Apr 24, Dario Faggioli wrote:

> What might be missing is some documentation in docs/man/xl.cfg.pod.5,
> explaining about the new options... :-)

I've added a little documentation, so now I hope it is ok.


-- 
best regards

  Dieter Bloms

--
I do not get viruses because I do not use MS software.
If you use Outlook then please do not put my email address in your
address-book so that WHEN you get a virus it won't use my address in the
>From field.

[-- Attachment #2: add_support_for_cpu_weight_config_in_xl.diff --]
[-- Type: text/x-diff, Size: 6742 bytes --]

libxl: set domain scheduling parameters while creating the dom

the domain specific scheduling parameters like cpu_weight, cap, slice, ...
will be set during creating the domain, so this parameters can be defined
in the domain config file

Signed-off-by: Dieter Bloms <dieter@bloms.de>

diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index e2cd251..4060933 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -112,6 +112,44 @@ List of which cpus the guest is allowed to use. Default behavior is
 (all vcpus will run on cpus 0,2,3,5), or `cpus=["2", "3"]` (all vcpus
 will run on cpus 2 and 3).
 
+=item B<cpu-weight="weight of cpu (default 256)">
+
+A domain with a weight of 512 will get twice as much CPU as a domain
+with a weight of 256 on a contended host.
+Legal weights range from 1 to 65535 and the default is 256.
+Can be set for credit, credit2 and sedf scheduler.
+
+=item B<cap="max amount of cpu (default 0)">
+
+The cap optionally fixes the maximum amount of CPU a domain will be
+able to consume, even if the host system has idle CPU cycles.
+The cap is expressed in percentage of one physical CPU:
+100 is 1 physical CPU, 50 is half a CPU, 400 is 4 CPUs, etc.
+The default, 0, means there is no upper cap.
+Can be set for the credit and credit2 scheduler.
+
+=item B<period="time in nanoseconds (default 0)">
+
+The normal EDF scheduling usage in nanoseconds. This means every period
+the domain gets cpu time defined in slice.
+Can be set for sedf scheduler.
+
+=item B<slice="time in nanoseconds (default 0)">
+
+The normal EDF scheduling usage in nanoseconds. it defines the time 
+a domain get every period time.
+Can be set for sedf scheduler.
+
+=item B<latency=" (default 0)">
+
+Scaled period if domain is doing heavy I/O.
+Can be set for sedf scheduler.
+
+=item B<extratime="BOOLEAN (default 0)">
+
+Flag for allowing domain to run in extra time.
+Can be set for sedf scheduler.
+
 =item B<memory=MBYTES>
 
 Start the guest with MBYTES megabytes of RAM.
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 0bdd654..38acff4 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -124,8 +124,27 @@ int libxl__build_post(libxl__gc *gc, uint32_t domid,
     char *dom_path, *vm_path;
     xs_transaction_t t;
     char **ents, **hvm_ents;
+    libxl_scheduler sched;
     int i;
 
+    sched = libxl_get_scheduler (ctx);
+    switch (sched) {
+    case LIBXL_SCHEDULER_SEDF:
+      libxl_sched_sedf_domain_set(ctx, domid, &(info->us.sedf));
+      break;
+    case LIBXL_SCHEDULER_CREDIT:
+      libxl_sched_credit_domain_set(ctx, domid, &(info->us.credit));
+      break;
+    case LIBXL_SCHEDULER_CREDIT2:
+      libxl_sched_credit2_domain_set(ctx, domid, &(info->us.credit2));
+      break;
+    case LIBXL_SCHEDULER_ARINC653:
+      /* not implemented */
+      break;
+    default:
+        abort();
+    }
+
     libxl_cpuid_apply_policy(ctx, domid);
     if (info->cpuid != NULL)
         libxl_cpuid_set(ctx, domid, info->cpuid);
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 5cf9708..c1cdc3c 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -224,6 +224,27 @@ libxl_domain_create_info = Struct("domain_create_info",[
 
 MemKB = UInt(64, init_val = "LIBXL_MEMKB_DEFAULT")
 
+libxl_sched_credit_domain = Struct("sched_credit_domain", [
+    ("weight", integer),
+    ("cap", integer),
+    ])
+
+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_sched_arinc653_domain = Struct("sched_arinc653_domain", [
+    ("weight", integer),
+    ])
+
 # Instances of libxl_file_reference contained in this struct which
 # have been mapped (with libxl_file_reference_map) will be unmapped
 # by libxl_domain_build/restore. If either of these are never called
@@ -256,6 +277,13 @@ libxl_domain_build_info = Struct("domain_build_info",[
     # extra parameters pass directly to qemu for HVM guest, NULL terminated
     ("extra_hvm",        libxl_string_list),
 
+    ("us", KeyedUnion(None, libxl_scheduler, "sched",
+                 [("credit", libxl_sched_credit_domain),
+                 ("credit2", libxl_sched_credit2_domain),
+                 ("sedf", libxl_sched_sedf_domain),
+                 ("arinc653", libxl_sched_arinc653_domain),
+                 ], keyvar_init_val = "-1")),
+
     ("u", KeyedUnion(None, libxl_domain_type, "type",
                 [("hvm", Struct(None, [("firmware",         string),
                                        ("bios",             libxl_bios_type),
@@ -417,28 +445,12 @@ 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_event_type = Enumeration("event_type", [
     (1, "DOMAIN_SHUTDOWN"),
     (2, "DOMAIN_DEATH"),
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 5703512..db51ca6 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -587,6 +587,23 @@ static void parse_config_data(const char *configfile_filename_report,
     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 (!xlu_cfg_get_long (config, "cpu_weight", &l, 0))
+        b_info->us.credit.weight = l;
+        b_info->us.credit2.weight = l;
+        b_info->us.sedf.weight = l;
+
+    if (!xlu_cfg_get_long (config, "cap", &l, 0))
+        b_info->us.credit.cap = l;
+
+    if (!xlu_cfg_get_long (config, "period", &l, 0))
+        b_info->us.sedf.period = l;
+    if (!xlu_cfg_get_long (config, "slice", &l, 0))
+        b_info->us.sedf.period = l;
+    if (!xlu_cfg_get_long (config, "latency", &l, 0))
+        b_info->us.sedf.period = l;
+    if (!xlu_cfg_get_long (config, "extratime", &l, 0))
+        b_info->us.sedf.period = l;
+
     if (!xlu_cfg_get_long (config, "vcpus", &l, 0)) {
         b_info->max_vcpus = l;
         b_info->cur_vcpus = (1 << l) - 1;

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

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

Re: [Xen-users] xl doesn't honour the parameter cpu_weight from my config file while xm does honour it

[Ian Campbell]

On Tue, 2012-04-24 at 13:14 +0100, Dieter Bloms wrote:
> libxl: set domain scheduling parameters while creating the dom
> 
> the domain specific scheduling parameters like cpu_weight, cap,
> slice, ...
> will be set during creating the domain, so this parameters can be
> defined
> in the domain config file
> 
> Signed-off-by: Dieter Bloms <dieter@bloms.de>
> 
> diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> index e2cd251..4060933 100644
> --- a/docs/man/xl.cfg.pod.5
> +++ b/docs/man/xl.cfg.pod.5
> @@ -112,6 +112,44 @@ List of which cpus the guest is allowed to use.
> Default behavior is
>  (all vcpus will run on cpus 0,2,3,5), or `cpus=["2", "3"]` (all vcpus
>  will run on cpus 2 and 3).
>  
> +=item B<cpu-weight="weight of cpu (default 256)">

I think you mean cpu_weight rather than cpu-weight.

The typography in this file isn't exactly strict but it seems that we
normally put the default in the text rather than in the header and use a
SHOUTY name for the value. e.g.

        =item B<cpu_weight=WEIGHT>
        
        Specifies the weight of the domain. A domain with a weight of
        512 will get twice as much CPU as a domain with a weight of 256
        on a contended host. Legal weights range from 1 to 65535 and the
        default is 256. Can be set for credit, credit2 and sedf
        scheduler.

This also avoids the ambiguous use of " in the title, since depending on
the item the quotes may be a required part of the syntax but not in this
case.

[...]
> +=item B<latency=" (default 0)">

=item B<latency=N>

I think you missed the value here.

[...]
> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> index 0bdd654..38acff4 100644
> --- a/tools/libxl/libxl_dom.c
> +++ b/tools/libxl/libxl_dom.c
> @@ -124,8 +124,27 @@ int libxl__build_post(libxl__gc *gc, uint32_t
> domid,
>      char *dom_path, *vm_path;
>      xs_transaction_t t;
>      char **ents, **hvm_ents;
> +    libxl_scheduler sched;
>      int i;
>  
> +    sched = libxl_get_scheduler (ctx);
> +    switch (sched) {
> +    case LIBXL_SCHEDULER_SEDF:
> +      libxl_sched_sedf_domain_set(ctx, domid, &(info->us.sedf));
> +      break;
> +    case LIBXL_SCHEDULER_CREDIT:
> +      libxl_sched_credit_domain_set(ctx, domid, &(info->us.credit));
> +      break;
> +    case LIBXL_SCHEDULER_CREDIT2:
> +      libxl_sched_credit2_domain_set(ctx, domid,
> &(info->us.credit2));
> +      break;
> +    case LIBXL_SCHEDULER_ARINC653:
> +      /* not implemented */
> +      break;
> +    default:
> +        abort();
> +    }
> +
>      libxl_cpuid_apply_policy(ctx, domid);
>      if (info->cpuid != NULL)
>          libxl_cpuid_set(ctx, domid, info->cpuid);
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index 5cf9708..c1cdc3c 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -224,6 +224,27 @@ libxl_domain_create_info =
> Struct("domain_create_info",[
>  
>  MemKB = UInt(64, init_val = "LIBXL_MEMKB_DEFAULT")
>  
> +libxl_sched_credit_domain = Struct("sched_credit_domain", [
> +    ("weight", integer),
> +    ("cap", integer),
> +    ])
> +
> +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_sched_arinc653_domain = Struct("sched_arinc653_domain", [
> +    ("weight", integer),
> +    ])
> +
>  # Instances of libxl_file_reference contained in this struct which
>  # have been mapped (with libxl_file_reference_map) will be unmapped
>  # by libxl_domain_build/restore. If either of these are never called
> @@ -256,6 +277,13 @@ libxl_domain_build_info =
> Struct("domain_build_info",[
>      # extra parameters pass directly to qemu for HVM guest, NULL
> terminated
>      ("extra_hvm",        libxl_string_list),
>  
> +    ("us", KeyedUnion(None, libxl_scheduler, "sched",
> +                 [("credit", libxl_sched_credit_domain),
> +                 ("credit2", libxl_sched_credit2_domain),
> +                 ("sedf", libxl_sched_sedf_domain),
> +                 ("arinc653", libxl_sched_arinc653_domain),
> +                 ], keyvar_init_val = "-1")),

I don't think a KeyedUnion is right here, since the user of libxl
doesn't really have a choice about which scheduler is in user (that's
set at boot time or via cpupool interfaces).

You could use a Union, but below I'll make an argument that perhaps a
Struct would be better.

[...]
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 5703512..db51ca6 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -587,6 +587,23 @@ static void parse_config_data(const char
> *configfile_filename_report,
>      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 (!xlu_cfg_get_long (config, "cpu_weight", &l, 0))
> +        b_info->us.credit.weight = l;
> +        b_info->us.credit2.weight = l;
> +        b_info->us.sedf.weight = l;

You need {} here otherwise only the credit setting is associated with
the if.

However, since b_info->us is a union this is actually setting the same
value into different places in overlapping memory (since all member of a
union occupy the same storage). One solution would be to use
libxl_get_scheduler to get the scheduler and fill in only the correct
option, but then that gets complex when you have to deal with cpupools
etc.

I think you should make "us" a Struct and call it sched_params or
something along those lines. It's just simpler all around, libxl can
internally figure out which set of parms to use (as you do correctly in
this patch).

Does the above make sense?

Ian.


> +    if (!xlu_cfg_get_long (config, "cap", &l, 0))
> +        b_info->us.credit.cap = l;
> +
> +    if (!xlu_cfg_get_long (config, "period", &l, 0))
> +        b_info->us.sedf.period = l;
> +    if (!xlu_cfg_get_long (config, "slice", &l, 0))
> +        b_info->us.sedf.period = l;
> +    if (!xlu_cfg_get_long (config, "latency", &l, 0))
> +        b_info->us.sedf.period = l;
> +    if (!xlu_cfg_get_long (config, "extratime", &l, 0))
> +        b_info->us.sedf.period = l;
> +
>      if (!xlu_cfg_get_long (config, "vcpus", &l, 0)) {
>          b_info->max_vcpus = l;
>          b_info->cur_vcpus = (1 << l) - 1;
> 

Re: [Xen-users] xl doesn't honour the parameter cpu_weight from my config file while xm does honour it

[Ian Jackson]

Dieter Bloms writes ("Re: [Xen-devel] [Xen-users] xl doesn't honour the parameter cpu_weight from my config file while xm does honour it"):
> On Tue, Apr 24, Dario Faggioli wrote:
> > What might be missing is some documentation in docs/man/xl.cfg.pod.5,
> > explaining about the new options... :-)
> 
> I've added a little documentation, so now I hope it is ok.

This is better, thanks.  I have some comments.

> +=item B<cpu-weight="weight of cpu (default 256)">

I'm not qualified to review the documented semantics here.

> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> index 0bdd654..38acff4 100644
> --- a/tools/libxl/libxl_dom.c
> +++ b/tools/libxl/libxl_dom.c
> @@ -124,8 +124,27 @@ int libxl__build_post(libxl__gc *gc, uint32_t domid,
> +    sched = libxl_get_scheduler (ctx);
> +    switch (sched) {
> +    case LIBXL_SCHEDULER_SEDF:
> +      libxl_sched_sedf_domain_set(ctx, domid, &(info->us.sedf));
> +      break;
> +    case LIBXL_SCHEDULER_CREDIT:
> +      libxl_sched_credit_domain_set(ctx, domid, &(info->us.credit));
> +      break;
> +    case LIBXL_SCHEDULER_CREDIT2:
> +      libxl_sched_credit2_domain_set(ctx, domid, &(info->us.credit2));
> +      break;
> +    case LIBXL_SCHEDULER_ARINC653:
> +      /* not implemented */
> +      break;
> +    default:
> +        abort();
> +    }

This is a very repetitive piece of code.  Can't it be autogenerated
somehow by the idl compiler ?  Ian C ?

Also, we use 4-character indents and you have used 2.  (If this were
the only thing that needed changing I would fix it when I committed.)

> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index 5cf9708..c1cdc3c 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -224,6 +224,27 @@ libxl_domain_create_info = Struct("domain_create_info",[
> +libxl_sched_credit_domain = Struct("sched_credit_domain", [
> +    ("weight", integer),
> +    ("cap", integer),
> +    ])

You seem to have just moved many of these about ?  That's just because
the idl file doesn't support forward declarations, right ?

Thanks,
Ian.

Re: [Xen-users] xl doesn't honour the parameter cpu_weight from my config file while xm does honour it

[Ian Campbell]

On Tue, 2012-04-24 at 14:24 +0100, Ian Jackson wrote:
> Dieter Bloms writes ("Re: [Xen-devel] [Xen-users] xl doesn't honour the parameter cpu_weight from my config file while xm does honour it"):
> > On Tue, Apr 24, Dario Faggioli wrote:
> > > What might be missing is some documentation in docs/man/xl.cfg.pod.5,
> > > explaining about the new options... :-)
> > 
> > I've added a little documentation, so now I hope it is ok.
> 
> This is better, thanks.  I have some comments.
> 
> > +=item B<cpu-weight="weight of cpu (default 256)">
> 
> I'm not qualified to review the documented semantics here.
> 
> > diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> > index 0bdd654..38acff4 100644
> > --- a/tools/libxl/libxl_dom.c
> > +++ b/tools/libxl/libxl_dom.c
> > @@ -124,8 +124,27 @@ int libxl__build_post(libxl__gc *gc, uint32_t domid,
> > +    sched = libxl_get_scheduler (ctx);
> > +    switch (sched) {
> > +    case LIBXL_SCHEDULER_SEDF:
> > +      libxl_sched_sedf_domain_set(ctx, domid, &(info->us.sedf));
> > +      break;
> > +    case LIBXL_SCHEDULER_CREDIT:
> > +      libxl_sched_credit_domain_set(ctx, domid, &(info->us.credit));
> > +      break;
> > +    case LIBXL_SCHEDULER_CREDIT2:
> > +      libxl_sched_credit2_domain_set(ctx, domid, &(info->us.credit2));
> > +      break;
> > +    case LIBXL_SCHEDULER_ARINC653:
> > +      /* not implemented */
> > +      break;
> > +    default:
> > +        abort();
> > +    }
> 
> This is a very repetitive piece of code.  Can't it be autogenerated
> somehow by the idl compiler ?  Ian C ?

Not without a bunch more plumbing in the infrastructure, which would
probably heavily outweigh the code here...

A compromise might be to make this a libxl__sched_set_params(gc, domid,
&info->us), so at least it wouldn't be repeated again somewhere.

> Also, we use 4-character indents and you have used 2.  (If this were
> the only thing that needed changing I would fix it when I committed.)
> 
> > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> > index 5cf9708..c1cdc3c 100644
> > --- a/tools/libxl/libxl_types.idl
> > +++ b/tools/libxl/libxl_types.idl
> > @@ -224,6 +224,27 @@ libxl_domain_create_info = Struct("domain_create_info",[
> > +libxl_sched_credit_domain = Struct("sched_credit_domain", [
> > +    ("weight", integer),
> > +    ("cap", integer),
> > +    ])
> 
> You seem to have just moved many of these about ?

I think he said as much in his commit message? Oh, it was actually in
the comment of hte previous posting:
> I've defined a new union in the libxl_domain_build_info structure.
> For this I had to move some structure definition like
> libxl_sched_credit_domain more to the top.


>   That's just because
> the idl file doesn't support forward declarations, right ?
> 
> Thanks,
> Ian.

Re: [Xen-users] xl doesn't honour the parameter cpu_weight from my config file while xm does honour it

[Ian Jackson]

Ian Campbell writes ("Re: [Xen-devel] [Xen-users] xl doesn't honour the parameter cpu_weight from my config file while xm does honour it"):
> On Tue, 2012-04-24 at 14:24 +0100, Ian Jackson wrote:
> > This is a very repetitive piece of code.  Can't it be autogenerated
> > somehow by the idl compiler ?  Ian C ?
> 
> Not without a bunch more plumbing in the infrastructure, which would
> probably heavily outweigh the code here...

Fair enough.

> A compromise might be to make this a libxl__sched_set_params(gc, domid,
> &info->us), so at least it wouldn't be repeated again somewhere.

That would be nice but I wouldn't insist on it until it was necessary
to avoid a second copy.

Ian.

Re: [Xen-users] xl doesn't honour the parameter cpu_weight from my config file while xm does honour it

[Dieter Bloms]

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

Hi,

On Tue, Apr 24, Ian Campbell wrote:

> I think you mean cpu_weight rather than cpu-weight.

yes of course, fixed.

> =item B<latency=N>
> 
> I think you missed the value here.

I don't know the value for it, so I added N as you suggested.

> > +    ("us", KeyedUnion(None, libxl_scheduler, "sched",
> > +                 [("credit", libxl_sched_credit_domain),
> > +                 ("credit2", libxl_sched_credit2_domain),
> > +                 ("sedf", libxl_sched_sedf_domain),
> > +                 ("arinc653", libxl_sched_arinc653_domain),
> > +                 ], keyvar_init_val = "-1")),
> 
> I don't think a KeyedUnion is right here, since the user of libxl
> doesn't really have a choice about which scheduler is in user (that's
> set at boot time or via cpupool interfaces).
> 
> You could use a Union, but below I'll make an argument that perhaps a
> Struct would be better.

Hm, but when I don't use a union with the structure for each scheduler I
have to create a structure depend of scheduler at runtime, because the
functions libxl_sched_XXXXXX_domain_set use there own structure for
their scheduler.

I've create a new function libxl__sched_set_params, which set the params
depend on the scheduler as you suggested.

I'am not a software developer, so please be forbear with me.

Here my next try.

-- 
Best regards

  Dieter

--
I do not get viruses because I do not use MS software.
If you use Outlook then please do not put my email address in your
address-book so that WHEN you get a virus it won't use my address in the
From field.

[-- Attachment #2: add_support_for_cpu_weight_config_in_xl.diff --]
[-- Type: text/x-diff, Size: 7918 bytes --]

libxl: set domain scheduling parameters while creating the dom

the domain specific scheduling parameters like cpu_weight, cap, slice, ...
will be set during creating the domain, so this parameters can be defined
in the domain config file

Signed-off-by: Dieter Bloms <dieter@bloms.de>

diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index e2cd251..b0c8064 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -112,6 +112,44 @@ List of which cpus the guest is allowed to use. Default behavior is
 (all vcpus will run on cpus 0,2,3,5), or `cpus=["2", "3"]` (all vcpus
 will run on cpus 2 and 3).
 
+=item B<cpu_weight=WEIGHT>
+
+A domain with a weight of 512 will get twice as much CPU as a domain
+with a weight of 256 on a contended host.
+Legal weights range from 1 to 65535 and the default is 256.
+Can be set for credit, credit2 and sedf scheduler.
+
+=item B<cap=N>
+
+The cap optionally fixes the maximum amount of CPU a domain will be
+able to consume, even if the host system has idle CPU cycles.
+The cap is expressed in percentage of one physical CPU:
+100 is 1 physical CPU, 50 is half a CPU, 400 is 4 CPUs, etc.
+The default, 0, means there is no upper cap.
+Can be set for the credit and credit2 scheduler.
+
+=item B<period=NANOSECONDS>
+
+The normal EDF scheduling usage in nanoseconds. This means every period
+the domain gets cpu time defined in slice.
+Can be set for sedf scheduler.
+
+=item B<slice=NANOSECONDS>
+
+The normal EDF scheduling usage in nanoseconds. it defines the time 
+a domain get every period time.
+Can be set for sedf scheduler.
+
+=item B<latency=N>
+
+Scaled period if domain is doing heavy I/O.
+Can be set for sedf scheduler.
+
+=item B<extratime=BOOLEAN>
+
+Flag for allowing domain to run in extra time.
+Can be set for sedf scheduler.
+
 =item B<memory=MBYTES>
 
 Start the guest with MBYTES megabytes of RAM.
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 0bdd654..654da78 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -42,6 +42,33 @@ libxl_domain_type libxl__domain_type(libxl__gc *gc, uint32_t domid)
         return LIBXL_DOMAIN_TYPE_PV;
 }
 
+int libxl__sched_set_params(libxl__gc *gc, uint32_t domid, libxl_domain_build_info *info)
+{
+    libxl_ctx *ctx = libxl__gc_owner(gc);
+    libxl_scheduler sched;
+    int ret;
+
+    sched = libxl_get_scheduler (ctx);
+    switch (sched) {
+    case LIBXL_SCHEDULER_SEDF:
+      ret=libxl_sched_sedf_domain_set(ctx, domid, &(info->us.sedf));
+      break;
+    case LIBXL_SCHEDULER_CREDIT:
+      ret=libxl_sched_credit_domain_set(ctx, domid, &(info->us.credit));
+      break;
+    case LIBXL_SCHEDULER_CREDIT2:
+      ret=libxl_sched_credit2_domain_set(ctx, domid, &(info->us.credit2));
+      break;
+    case LIBXL_SCHEDULER_ARINC653:
+      /* not implemented */
+      ret=0;
+      break;
+    default:
+      ret=-1;
+    }
+    return ret;
+}
+
 int libxl__domain_shutdown_reason(libxl__gc *gc, uint32_t domid)
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
@@ -126,6 +153,8 @@ int libxl__build_post(libxl__gc *gc, uint32_t domid,
     char **ents, **hvm_ents;
     int i;
 
+    libxl__sched_set_params (gc, domid, info);
+
     libxl_cpuid_apply_policy(ctx, domid);
     if (info->cpuid != NULL)
         libxl_cpuid_set(ctx, domid, info->cpuid);
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index a4b933b..bfcdff4 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -617,6 +617,7 @@ int libxl__atfork_init(libxl_ctx *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_domain_build_info *info);
 #define LIBXL__DOMAIN_IS_TYPE(gc, domid, type) \
     libxl__domain_type((gc), (domid)) == LIBXL_DOMAIN_TYPE_##type
 typedef struct {
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 5cf9708..c1cdc3c 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -224,6 +224,27 @@ libxl_domain_create_info = Struct("domain_create_info",[
 
 MemKB = UInt(64, init_val = "LIBXL_MEMKB_DEFAULT")
 
+libxl_sched_credit_domain = Struct("sched_credit_domain", [
+    ("weight", integer),
+    ("cap", integer),
+    ])
+
+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_sched_arinc653_domain = Struct("sched_arinc653_domain", [
+    ("weight", integer),
+    ])
+
 # Instances of libxl_file_reference contained in this struct which
 # have been mapped (with libxl_file_reference_map) will be unmapped
 # by libxl_domain_build/restore. If either of these are never called
@@ -256,6 +277,13 @@ libxl_domain_build_info = Struct("domain_build_info",[
     # extra parameters pass directly to qemu for HVM guest, NULL terminated
     ("extra_hvm",        libxl_string_list),
 
+    ("us", KeyedUnion(None, libxl_scheduler, "sched",
+                 [("credit", libxl_sched_credit_domain),
+                 ("credit2", libxl_sched_credit2_domain),
+                 ("sedf", libxl_sched_sedf_domain),
+                 ("arinc653", libxl_sched_arinc653_domain),
+                 ], keyvar_init_val = "-1")),
+
     ("u", KeyedUnion(None, libxl_domain_type, "type",
                 [("hvm", Struct(None, [("firmware",         string),
                                        ("bios",             libxl_bios_type),
@@ -417,28 +445,12 @@ 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_event_type = Enumeration("event_type", [
     (1, "DOMAIN_SHUTDOWN"),
     (2, "DOMAIN_DEATH"),
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 5703512..811f1ac 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -587,6 +587,24 @@ static void parse_config_data(const char *configfile_filename_report,
     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 (!xlu_cfg_get_long (config, "cpu_weight", &l, 0)) {
+        b_info->us.credit.weight = l;
+        b_info->us.credit2.weight = l;
+        b_info->us.sedf.weight = l;
+    }
+
+    if (!xlu_cfg_get_long (config, "cap", &l, 0))
+        b_info->us.credit.cap = l;
+
+    if (!xlu_cfg_get_long (config, "period", &l, 0))
+        b_info->us.sedf.period = l;
+    if (!xlu_cfg_get_long (config, "slice", &l, 0))
+        b_info->us.sedf.period = l;
+    if (!xlu_cfg_get_long (config, "latency", &l, 0))
+        b_info->us.sedf.period = l;
+    if (!xlu_cfg_get_long (config, "extratime", &l, 0))
+        b_info->us.sedf.period = l;
+
     if (!xlu_cfg_get_long (config, "vcpus", &l, 0)) {
         b_info->max_vcpus = l;
         b_info->cur_vcpus = (1 << l) - 1;

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

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

Re: [Xen-users] xl doesn't honour the parameter cpu_weight from my config file while xm does honour it

[Ian Campbell]

On Tue, 2012-04-24 at 15:33 +0100, Dieter Bloms wrote:
> Hi,
> 
> On Tue, Apr 24, Ian Campbell wrote:
> 
> > I think you mean cpu_weight rather than cpu-weight.
> 
> yes of course, fixed.
> 
> > =item B<latency=N>
> > 
> > I think you missed the value here.
> 
> I don't know the value for it, so I added N as you suggested.
> 
> > > +    ("us", KeyedUnion(None, libxl_scheduler, "sched",
> > > +                 [("credit", libxl_sched_credit_domain),
> > > +                 ("credit2", libxl_sched_credit2_domain),
> > > +                 ("sedf", libxl_sched_sedf_domain),
> > > +                 ("arinc653", libxl_sched_arinc653_domain),
> > > +                 ], keyvar_init_val = "-1")),
> > 
> > I don't think a KeyedUnion is right here, since the user of libxl
> > doesn't really have a choice about which scheduler is in user (that's
> > set at boot time or via cpupool interfaces).
> > 
> > You could use a Union, but below I'll make an argument that perhaps a
> > Struct would be better.
> 
> Hm, but when I don't use a union with the structure for each scheduler I
> have to create a structure depend of scheduler at runtime, because the
> functions libxl_sched_XXXXXX_domain_set use there own structure for
> their scheduler.

I mean create a single struct with all of the options in it, from which
libxl will select the appropriate set of parameters (much like the code
you have now does. You don't need to decide the content of that struct
at runtime.

The IDL patch is below, other than changing "us" to "sched_params" I
think most of the rest of your patch remains the same.

> I've create a new function libxl__sched_set_params, which set the params
> depend on the scheduler as you suggested.
> 
> I'am not a software developer, so please be forbear with me.

That's ok, thanks for doing this!

You add a struct for arinc635 but don't actually do anything with it,
you may as well leave it out for now?

Ian.

Re: [Xen-users] xl doesn't honour the parameter cpu_weight from my config file while xm does honour it

[Ian Jackson]

Ian Campbell writes ("Re: [Xen-devel] [Xen-users] xl doesn't honour the parameter cpu_weight from my config file while xm does honour it"):
> I mean create a single struct with all of the options in it, from which
> libxl will select the appropriate set of parameters (much like the code
> you have now does. You don't need to decide the content of that struct
> at runtime.
> 
> The IDL patch is below, other than changing "us" to "sched_params" I
> think most of the rest of your patch remains the same.

Missing attachment ?

Ian.

Re: [Xen-users] xl doesn't honour the parameter cpu_weight from my config file while xm does honour it

[Ian Campbell]

On Tue, 2012-04-24 at 17:03 +0100, Ian Jackson wrote:
> Ian Campbell writes ("Re: [Xen-devel] [Xen-users] xl doesn't honour the parameter cpu_weight from my config file while xm does honour it"):
> > I mean create a single struct with all of the options in it, from which
> > libxl will select the appropriate set of parameters (much like the code
> > you have now does. You don't need to decide the content of that struct
> > at runtime.
> > 
> > The IDL patch is below, other than changing "us" to "sched_params" I
> > think most of the rest of your patch remains the same.
> 
> Missing attachment ?

Yes, sorry.

Here it is:

diff -r aef90d90eb3b tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl	Tue Apr 24 16:53:00 2012 +0100
+++ b/tools/libxl/libxl_types.idl	Tue Apr 24 17:15:12 2012 +0100
@@ -224,6 +224,32 @@ libxl_domain_create_info = Struct("domai
 
 MemKB = UInt(64, init_val = "LIBXL_MEMKB_DEFAULT")
 
+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_sched_arinc653_domain = Struct("sched_arinc653_domain", [
+    ("weight", integer),
+    ])
+
 # Instances of libxl_file_reference contained in this struct which
 # have been mapped (with libxl_file_reference_map) will be unmapped
 # by libxl_domain_build/restore. If either of these are never called
@@ -256,6 +282,12 @@ libxl_domain_build_info = Struct("domain
     # extra parameters pass directly to qemu for HVM guest, NULL terminated
     ("extra_hvm",        libxl_string_list),
 
+    ("sched_params",     Struct(None, [("credit", libxl_sched_credit_domain),
+                                       ("credit2", libxl_sched_credit2_domain),
+                                       ("sedf", libxl_sched_sedf_domain),
+                                       ("arinc653", libxl_sched_arinc653_domain),
+                                       ]))
+    ,    
     ("u", KeyedUnion(None, libxl_domain_type, "type",
                 [("hvm", Struct(None, [("firmware",         string),
                                        ("bios",             libxl_bios_type),
@@ -417,28 +449,6 @@ 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_event_type = Enumeration("event_type", [
     (1, "DOMAIN_SHUTDOWN"),
     (2, "DOMAIN_DEATH"),

Re: [Xen-users] xl doesn't honour the parameter cpu_weight from my config file while xm does honour it

[Ian Jackson]

Ian Campbell writes ("Re: [Xen-devel] [Xen-users] xl doesn't honour the parameter cpu_weight from my config file while xm does honour it"):
> diff -r aef90d90eb3b tools/libxl/libxl_types.idl
> --- a/tools/libxl/libxl_types.idl	Tue Apr 24 16:53:00 2012 +0100
> +++ b/tools/libxl/libxl_types.idl	Tue Apr 24 17:15:12 2012 +0100
...
> +libxl_sched_credit_domain = Struct("sched_credit_domain", [
> +    ("weight", integer),
...
...
> +libxl_sched_credit2_domain = Struct("sched_credit2_domain", [
> +    ("weight", integer),
...
> +libxl_sched_sedf_domain = Struct("sched_sedf_domain", [
...
> +    ("weight", integer),
> +    ])
> +
> +libxl_sched_arinc653_domain = Struct("sched_arinc653_domain", [
> +    ("weight", integer),
> +    ])
...
> +    ("sched_params",     Struct(None, [("credit", libxl_sched_credit_domain),
> +                                       ("credit2", libxl_sched_credit2_domain),
> +                                       ("sedf", libxl_sched_sedf_domain),
> +                                       ("arinc653", libxl_sched_arinc653_domain),
> +                                       ]))

The resulting sched_params structure contains four subfields called
"weight", all of which mean (roughly, obviously) the same thing and
all of which are to be set from the same "weight" xl configuration
parameter.  Is this really the most sensible way to do things ?

Perhaps it would be better to have a single sched_params struct which
contained all the parameters needed for any scheduler, and simply have
them ignored by libxl for schedulers we're not using.

Ian.

Re: [Xen-users] xl doesn't honour the parameter cpu_weight from my config file while xm does honour it

[Ian Campbell]

On Tue, 2012-04-24 at 17:20 +0100, Ian Jackson wrote:
> Ian Campbell writes ("Re: [Xen-devel] [Xen-users] xl doesn't honour the parameter cpu_weight from my config file while xm does honour it"):
> > diff -r aef90d90eb3b tools/libxl/libxl_types.idl
> > --- a/tools/libxl/libxl_types.idl	Tue Apr 24 16:53:00 2012 +0100
> > +++ b/tools/libxl/libxl_types.idl	Tue Apr 24 17:15:12 2012 +0100
> ...
> > +libxl_sched_credit_domain = Struct("sched_credit_domain", [
> > +    ("weight", integer),
> ...
> ...
> > +libxl_sched_credit2_domain = Struct("sched_credit2_domain", [
> > +    ("weight", integer),
> ...
> > +libxl_sched_sedf_domain = Struct("sched_sedf_domain", [
> ...
> > +    ("weight", integer),
> > +    ])
> > +
> > +libxl_sched_arinc653_domain = Struct("sched_arinc653_domain", [
> > +    ("weight", integer),
> > +    ])
> ...
> > +    ("sched_params",     Struct(None, [("credit", libxl_sched_credit_domain),
> > +                                       ("credit2", libxl_sched_credit2_domain),
> > +                                       ("sedf", libxl_sched_sedf_domain),
> > +                                       ("arinc653", libxl_sched_arinc653_domain),
> > +                                       ]))
> 
> The resulting sched_params structure contains four subfields called
> "weight", all of which mean (roughly, obviously) the same thing and
> all of which are to be set from the same "weight" xl configuration
> parameter.  Is this really the most sensible way to do things ?

Perhaps not, but it would imply major surgery to the other scheduler
interfaces to do it some other way (since these same structs are used
for libxl_sched_*_{set,get}.

I'm not 100% sure that "weight" has the same meaning for each scheduler
either. It's at least plausible that I might want one weight for a
doamin if the scheduler is credit and something else if it is sedf.

> Perhaps it would be better to have a single sched_params struct which
> contained all the parameters needed for any scheduler, and simply have
> them ignored by libxl for schedulers we're not using.

You'd need to reconstitute each of the split structs in order to call
libxl_sched_FOO_set internally, I don't think I want to ask Dieter to
take on that much bigger job.

Modulo this IDL bit Dieter's patch looks quite nice.

Ian.

Re: [Xen-users] xl doesn't honour the parameter cpu_weight from my config file while xm does honour it

[Dieter Bloms]

Hi,

On Tue, Apr 24, Ian Jackson wrote:

> Perhaps it would be better to have a single sched_params struct which
> contained all the parameters needed for any scheduler, and simply have
> them ignored by libxl for schedulers we're not using.

my first version has this type of design and then someone said that this
was not a good design and I have to use union with libxl_sched_*_params.

Anyway I think it is a good design to have one struct with all parameters
and I'am willing to implement it.


-- 
Gruß

  Dieter

--
I do not get viruses because I do not use MS software.
If you use Outlook then please do not put my email address in your
address-book so that WHEN you get a virus it won't use my address in the
>From field.

Re: [Xen-users] xl doesn't honour the parameter cpu_weight from my config file while xm does honour it

[Dieter Bloms]

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

Hi,

On Tue, Apr 24, Dieter Bloms wrote:

> Hi,
> 
> On Tue, Apr 24, Ian Jackson wrote:
> 
> > Perhaps it would be better to have a single sched_params struct which
> > contained all the parameters needed for any scheduler, and simply have
> > them ignored by libxl for schedulers we're not using.

...

> Anyway I think it is a good design to have one struct with all parameters
> and I'am willing to implement it.

I've made a new patch.
Maybe it fit all your needs.


-- 
best regards

  Dieter Bloms

--
I do not get viruses because I do not use MS software.
If you use Outlook then please do not put my email address in your
address-book so that WHEN you get a virus it won't use my address in the
>From field.

[-- Attachment #2: add_support_for_cpu_weight_config_in_xl.diff --]
[-- Type: text/x-diff, Size: 6982 bytes --]

libxl: set domain scheduling parameters while creating the domU

the domain specific scheduling parameters like cpu_weight, cap, slice, ...
will be set during creating the domain, so this parameters can be defined
in the domain config file

Signed-off-by: Dieter Bloms <dieter@bloms.de>

diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index e2cd251..b0c8064 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -112,6 +112,44 @@ List of which cpus the guest is allowed to use. Default behavior is
 (all vcpus will run on cpus 0,2,3,5), or `cpus=["2", "3"]` (all vcpus
 will run on cpus 2 and 3).
 
+=item B<cpu_weight=WEIGHT>
+
+A domain with a weight of 512 will get twice as much CPU as a domain
+with a weight of 256 on a contended host.
+Legal weights range from 1 to 65535 and the default is 256.
+Can be set for credit, credit2 and sedf scheduler.
+
+=item B<cap=N>
+
+The cap optionally fixes the maximum amount of CPU a domain will be
+able to consume, even if the host system has idle CPU cycles.
+The cap is expressed in percentage of one physical CPU:
+100 is 1 physical CPU, 50 is half a CPU, 400 is 4 CPUs, etc.
+The default, 0, means there is no upper cap.
+Can be set for the credit and credit2 scheduler.
+
+=item B<period=NANOSECONDS>
+
+The normal EDF scheduling usage in nanoseconds. This means every period
+the domain gets cpu time defined in slice.
+Can be set for sedf scheduler.
+
+=item B<slice=NANOSECONDS>
+
+The normal EDF scheduling usage in nanoseconds. it defines the time 
+a domain get every period time.
+Can be set for sedf scheduler.
+
+=item B<latency=N>
+
+Scaled period if domain is doing heavy I/O.
+Can be set for sedf scheduler.
+
+=item B<extratime=BOOLEAN>
+
+Flag for allowing domain to run in extra time.
+Can be set for sedf scheduler.
+
 =item B<memory=MBYTES>
 
 Start the guest with MBYTES megabytes of RAM.
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 0bdd654..55b033a 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -42,6 +42,40 @@ libxl_domain_type libxl__domain_type(libxl__gc *gc, uint32_t domid)
         return LIBXL_DOMAIN_TYPE_PV;
 }
 
+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) {
+    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;
+    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;
+    case LIBXL_SCHEDULER_CREDIT2:
+      credit2_info.weight = scparams->weight;
+      ret=libxl_sched_credit2_domain_set(ctx, domid, &credit2_info);
+      break;
+    default:
+      ret=-1;
+    }
+    return ret;
+}
+
 int libxl__domain_shutdown_reason(libxl__gc *gc, uint32_t domid)
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
@@ -126,6 +160,8 @@ int libxl__build_post(libxl__gc *gc, uint32_t domid,
     char **ents, **hvm_ents;
     int i;
 
+    libxl__sched_set_params (gc, domid, &(info->sched_params));
+
     libxl_cpuid_apply_policy(ctx, domid);
     if (info->cpuid != NULL)
         libxl_cpuid_set(ctx, domid, info->cpuid);
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index a4b933b..2b76b0e 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -617,6 +617,7 @@ int libxl__atfork_init(libxl_ctx *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);
 #define LIBXL__DOMAIN_IS_TYPE(gc, domid, type) \
     libxl__domain_type((gc), (domid)) == LIBXL_DOMAIN_TYPE_##type
 typedef struct {
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 5cf9708..7789327 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -224,6 +224,17 @@ libxl_domain_create_info = Struct("domain_create_info",[
 
 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),
+    ], dir=DIR_IN)
+
 # Instances of libxl_file_reference contained in this struct which
 # have been mapped (with libxl_file_reference_map) will be unmapped
 # by libxl_domain_build/restore. If either of these are never called
@@ -255,6 +266,8 @@ libxl_domain_build_info = Struct("domain_build_info",[
     ("extra_pv",         libxl_string_list),
     # 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),
 
     ("u", KeyedUnion(None, libxl_domain_type, "type",
                 [("hvm", Struct(None, [("firmware",         string),
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 5703512..8e67307 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -587,6 +587,23 @@ static void parse_config_data(const char *configfile_filename_report,
     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 (!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;
+    if (!xlu_cfg_get_long (config, "latency", &l, 0))
+        b_info->sched_params.period = l;
+    if (!xlu_cfg_get_long (config, "extratime", &l, 0))
+        b_info->sched_params.period = l;
+
     if (!xlu_cfg_get_long (config, "vcpus", &l, 0)) {
         b_info->max_vcpus = l;
         b_info->cur_vcpus = (1 << l) - 1;

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

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

Re: [Xen-users] xl doesn't honour the parameter cpu_weight from my config file while xm does honour it

[Ian Campbell]

On Tue, 2012-04-24 at 20:35 +0100, Dieter Bloms wrote:
> Hi,
> 
> On Tue, Apr 24, Dieter Bloms wrote:
> 
> > Hi,
> > 
> > On Tue, Apr 24, Ian Jackson wrote:
> > 
> > > Perhaps it would be better to have a single sched_params struct which
> > > contained all the parameters needed for any scheduler, and simply have
> > > them ignored by libxl for schedulers we're not using.
> 
> ...
> 
> > Anyway I think it is a good design to have one struct with all parameters
> > and I'am willing to implement it.
> 
> I've made a new patch.
> Maybe it fit all your needs.

Thanks, this patch look good to me, I'll leave it to IanJ to decide
which approach he prefers.

Ian.

Re: [Xen-users] xl doesn't honour the parameter cpu_weight from my config file while xm does honour it

[Ian Jackson]

Ian Campbell writes ("Re: [Xen-devel] [Xen-users] xl doesn't honour the parameter cpu_weight from my config file while xm does honour it"):
> On Tue, 2012-04-24 at 20:35 +0100, Dieter Bloms wrote:
> > I've made a new patch.
> > Maybe it fit all your needs.
> 
> Thanks, this patch look good to me, I'll leave it to IanJ to decide
> which approach he prefers.

Thanks, I'll take that as an ack.  I've applied Dieter's latest
version, which I like.

I edited the docs message to change "can be set for" to "honoured
by", because variables can be set which will then be ignored.  Eg:

 +=item B<cpu_weight=WEIGHT>
 +
 +A domain with a weight of 512 will get twice as much CPU as a domain
 +with a weight of 256 on a contended host.
 +Legal weights range from 1 to 65535 and the default is 256.
-+Can be set for credit, credit2 and sedf scheduler.
++Honoured by the credit, credit2 and sedf schedulers.

Ian.