[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 6/8] Implement schedular tunables API using cgroups



On Thu, Jul 23, 2009 at 04:53:56PM +0100, Daniel P. Berrange wrote:
> On Thu, Jul 23, 2009 at 05:38:45PM +0200, Daniel Veillard wrote:
> > On Wed, Jul 22, 2009 at 04:23:45PM +0100, Daniel P. Berrange wrote:
> > > * src/qemu_driver.c:  Add driver methods qemuGetSchedulerType,
> > >   qemuGetSchedulerParameters, qemuSetSchedulerParameters
> > > * src/lxc_driver.c: Fix to use unsigned long long consistently
> > >   for schedular parameters
> > > * src/cgroup.h, src/cgroup.c: Fix cpu_shares to take unsigned
> > >   long long
> > > * src/util.c, src/util.h, src/libvirt_private.syms: Add a
> > >   virStrToDouble helper
> > > * src/virsh.c: Fix handling of --set arg to schedinfo command
> > >   to honour the designated data type of each schedular tunable
> > >   as declared by the driver
> > > 
> > > Signed-off-by: Daniel P. Berrange <berrange redhat com>
> > > ---
> > >  src/cgroup.c             |   10 +-
> > >  src/cgroup.h             |    4 +-
> > >  src/libvirt_private.syms |    1 +
> > >  src/lxc_driver.c         |    9 ++-
> > >  src/qemu_driver.c        |  151 +++++++++++++++++++++++++++++-
> > >  src/util.c               |   20 ++++
> > >  src/util.h               |    3 +
> > [...]
> > > @@ -1355,9 +1355,14 @@ static int lxcSetSchedulerParameters(virDomainPtr domain,
> > >  
> > >      for (i = 0; i < nparams; i++) {
> > >          virSchedParameterPtr param = &params[i];
> > > +        if (param->type != VIR_DOMAIN_SCHED_FIELD_ULLONG) {
> > > +            lxcError(NULL, domain, VIR_ERR_INVALID_ARG,
> > > +                     _("invalid type for cpu_shares tunable, expected a 'ullong'"));
> > > +            goto cleanup;
> > > +        }
> > 
> >   Humpf, we are actually breaking the API in some ways there, what is
> >   the argument ? Consistency with qemu scheduling parameters ?
> 
> Yes & no. It was essentially broken already.
> 
>   - The lxcGetSchedulerParameters method returned cpu_shares
>      with type ULLONG, and filled the '.ul'  field.
>   - The lxcSetSchedulerParameters method did not check the type
>     at all, and blindly read the '.ui' field instead.
> 
> The language bindings need to know the data types for each parameter in
> order to covert from the languages' type to the parameter type. The Perl
> and Python both do this by calling virGetSchedulerParameters to fetch
> current values, and then updating the values in place, and then calling
> virSetSchedulerParameters to update them.  So the fact that the LXC
> driver 'get' method returned ULLONG, means it should be also accepting
> ULLONG in the set method, otherwise it is impossible for the language
> bindings to work. A similar situation exists for virsh with its --set
> parameter, which can only work by calling 'get' to discover the existing
> type and then updating it. So this code is just validating what apps
> should have already been doing, and fixing the mistaken reading of .ui
> to be .ul

  Okay :-) thanks !

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]