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@xxxxxxxxxx>
> > > ---
> > > 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 = ¶ms[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@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list