From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [Xen-users] xl doesn't honour the parameter cpu_weight from my config file while xm does honour it Date: Tue, 24 Apr 2012 14:27:36 +0100 Message-ID: <1335274056.4347.136.camel@zakaz.uk.xensource.com> References: <20120420150012.GB3720@bloms.de> <1334934791.28331.101.camel@zakaz.uk.xensource.com> <20120423094623.GA13565@bloms.de> <1335182682.4347.15.camel@zakaz.uk.xensource.com> <1335190962.3122.10.camel@Abyss> <20120423154112.GA15320@bloms.de> <1335197251.22133.5.camel@Solace> <20120423193518.GA16134@bloms.de> <1335247525.2397.4.camel@Abyss> <20120424121402.GA19331@bloms.de> <20374.43433.379674.977454@mariner.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20374.43433.379674.977454@mariner.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Jackson Cc: George Dunlap , Dario Faggioli , Dieter Bloms , xen-devel List-Id: xen-devel@lists.xenproject.org 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 > > 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.