Hi Dhaval,
Sorry for my delayed response.
Dhaval Giani wrote:
>> The cgconfig service succeeds even if there are something wrong
>> in /etc/cgconfig.conf. For example, it succeeds even if specifying
>> not-existing user (nouser) as a perm.task.uid like the following:
>>
>> # cat /etc/cgconfig.conf
>> mount {
>> cpuset = /mnt/cgroups/cpuset;
>> memory = /mnt/cgroups/memory;
>> }
>> group root {
>> perm {
>> task {
>> uid = nouser;
>> gid = root;
>> }
>> admin {
>> uid = root;
>> gid = root;
>> }
>> }
>> cpuset {
>> cpuset.cpus = 0;
>> cpuset.mems = 0;
>> }
>> memory {
>> memory.use_hierarchy = 1;
>> memory.limit_in_bytes = 1000000000000;
>> }
>> }
>> #
>> # service cgconfig start
>> Starting cgconfig service: parsing failed at line number 10
>> [ OK ]
>> # echo $?
>> 0
>> #
>>
>>
>> I feel that is not a good behavior and the cgconfig service should
>> fail if there are something wrong in /etc/cgconfig.conf.
>> This patch fixes the behavior. In the above case, the behavior is
>> changed like the following by this patch:
>>
>> # service cgconfig start
>> Starting cgconfig service: parsing failed at line number 10
>> Loading configuration file /etc/cgconfig.conf failed
>> Cgroup parsing failed
>> Failed to parse /etc/cgconfig.conf [FAILED]
>> #
>>
>> Any comments are welcome.
>>
>
> Ah. OK, I need to go back and see why we returned a zero return value
> even on failure. One main comment, let's make an entry in the error list
> (something like EINVALCONFIG) and use that instead of a (seemingly
> random) return value like 1.
I feel it is better that ECGROUPPARSEFAIL is renamed to ECGRULESPARSEFAIL
and ECGCONFIGPARSEFAIL is added instead of EINVALCONFIG for the above
error.
Current error list is defined as the following:
enum cgroup_errors {
[snip]
ECGROUPPARSEFAIL, /* Failed to parse rules configuration file. */
[snip]
};
I think the error in /etc/cgconfig.conf should be distinguished from
the one in /etc/cgrules.conf and these errors should be different values.
For the readability, I will change the error list like the following:
enum cgroup_errors {
[snip]
ECGRULESPARSEFAIL, /* Failed to parse rules file (/etc/cgrules.conf). */
ECGCONFIGPARSEFAIL, /* Failed to parse config file (/etc/cgconfig.conf). */
[snip]
};
How about this ?
If OK, I will create a new patch according to the above.
Thanks
Ken'ichi Ohmichi
------------------------------------------------------------------------------
Come build with us! The BlackBerry(R) Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9 - 12, 2009. Register now!
http://p.sf.net/sfu/devconference
_______________________________________________
Libcg-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/libcg-devel
