Re: [libvirt] [PATCH 6/8] Implement schedular tunables API using cgroups
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(a)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 = ¶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
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|