Daniel P. Berrange wrote:
On Fri, Oct 03, 2008 at 09:31:52PM +0530, Balbir Singh wrote:
> Hi, Everyone,
>
> I've seen a new set of patches from Dan Smith, which implement cgroup support
> for libvirt. While the patches seem simple, there are some issues that have been
> pointed out in the posting itself.
>
> I hope that libvirt will switch over (may be after your concerns are addressed
> and definitely in the longer run) to using libcgroups rather than having an
> internal implementation of cgroups. The advantages of switching over would be
> using the functionality that libcgroup already provides
>
> libcgroups (
libcg.sf.net) provides
>
> 1. Ability to configure and mount cgroups and controllers via initscripts and a
> configuration file
> 2. An API to control and read cgroups information
> 3. Thread safety around API calls
> 4. Daemons to automatically classify a task based on a certain set of rules
> 5. API to extract current cgroup classification (where is the task currently in
> the cgroup hierarchy)
So from a functional point of view you are addressing essentially three
use cases
1. System configuration for controllers
2. Automatic task classification
3. Application development API for creating groups
If each piece is correctly designed, the choice of implementation for
each of these can be, and in some cases must be, totally independant.
Since the kernel restricts that a single controller can only be attached
to one cgroupsfs mount point, and one attach cannot be changed, the choice
of how / where to mount controllers must remain outside the scope of
applications. If any application using cgroups were to specify mount
points, it would be inflicting its own requirements on every user of
cgroups. This implies that applications must be designed to work with
whatever controller mount configuration the admin has configured, and
not configure stuff themselves. So impl for point 1 (configuration)
must, by neccessity, be completely independant of impl for point 3
(application API).
Considering automatic task classification. The task classification engine
must be able to cope with the fact that applications have some functional
requirements on cgroups setup. Taking libvirt as an example, we have a
specific need to apply some controllers over a group of processes forming
a container. A task classification engine must not re-clasify individual
tasks within a container because that would conflict with the semantics
required by libvirt. It is, however, free to re-classify the libvirtd
daemon itself - whatever cgroup libvirtd is placed in, it will create the
LXC cgroups below this point.
So if libvirt is designed correctly, it will work with whatever cgroup
task classification engine that might be running. Similarly if the task
classification engine has been designed to co-operate with applications
there is no problem running it alonside libvirt. Thus the implementation
of points 2 (task classification) and point 3 (application API) have no
need to be formally tied together. Furthermore tieing them together does
not magically solve the problem that both applications & the cgroups task
classification engine need to be intelligently designed to co-operate.
Agreed!
> While re-implementing might sound like a cool thing to do, here are the drawbacks
>
> 1. It leads to code duplication and reduces code reuse
This is important if the library code is providing significant value add to
the application using it. As it stands, libcgroup is merely a direct interface
to the cgroups filesystem providing weakly typed setters & getters - with the
exception of looking at the mount table to find where a controller lives, this
is not hard / complex code, so the benefits of re-use are not particularly high.
Please see my earlier email on layering of API.
In such a scenario reducing code duplication is not in itself a
benefit, since
there are costs associated with using external libraries. It is more complicated
integrate 2 independant style sof API, particularly with different views on
error reporting, memory management and varying expectations for the semantic
models exposed.
I disagree, I see a lot of code that does the same thing, look through
/proc/mounts, read and parse values to write and read. I see two API's you've
built on top of what libcgroup has (one for setting memory limit and the other
for devices). Please compare the patch sizes as well and you'll see what I mean.
There are a number of 'hard' questions wrt to cgroups usage
by applications,
two of which are outlined above. Simply having all applications use a single
API cannot magically solve any of these problems - no matter what API is used
application developers need to take care to design their usage of cgroups
such that it 'plays nicely' with other applications.
Playing "nicely" is a definite requirement, but not using existing code or
contributing to it if something is broken and re-implementing it, sounds a
little extreme.
> 2. It leads to confused users
The use of cgroups is an internal implementation detail for libvirt's
LXC driver. In comon with all libvirt drivers, the user has no need to
know about the underlying impl details and these can & will change at
will as we discover better ways to achieve things. As such its irrelevant
to a user how we configure the cgroups filesytem for libvirt - whether we
do it directly, or via libcg the end result is identical - a set of
directories in the cgroups filesystem.
Why do you want to invest in maintaining cgroups code? You'll see as time passes
by that the effort spent in maintaining, enhancing the code will increase. For
example, CPU shares are currently unsupported by containers and the cost of
maintenance will continue to increase as newer controllers are developed.
> I understand that in the past there has been a perception that
libcgroups might
> not yet be ready, because we did not have ABI stability built into the library
> and the header file had old comments about things changing. I would urge the
> group to look at the current implementation of libcgroups (look at v0.32) and
> help us
>
> 1. Fix any issues you see or point them to us
> 2. Add new API or request for new API that can help us integrate better with libvirt
Our of the 3 functional areas I outlined earlier on, the ones that can
provide the most value to users of libvirt, are a standardized means to
configure cgroups mounts and have them active at boot time, and the task
classification engine. Both of these are interesting & hard problems for
which there is clear value in only having 1 global implementation. Neither
of these has a strong dependancy on the API that 3rd party applications
use to talk to cgroups filesytem - from a functional point of view they
ought to 'just work' no matter what API the app uses, provided the
application is design to assume presence of other applications using
cgroups.
So I do see value in the work cgroups project is producing, but the
application development API is the least critical part of this - the
task engine and cofiguration policy is the key value add that is useful
to everyone. Since cgroups is an internal implementation detail for
libvirt LXC driver, we can change our impl at any time we like, should
circumstance change & use of libcgroup.so be critically neccessary.
Sounds reasonable to me, I hope we do converge at some point sooner rather than
latter (well time will tell :) )
--
Balbir