[libvirt] Re: [discuss] The new cgroup patches for libvirt

[Balbir Singh]

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