On 03/21/2011 11:54 AM, Balbir Singh wrote:
> * Jan Safranek <jsafr...@redhat.com> [2011-03-15 14:40:24]:
>
>> Current libcgroup design handles each hierarchy only once. If a hierarchy
>> is mounted twice or more times, only the first mount point is taken into
>> account and the others are 'invisible' to libcgroup.
>>
>> This causes cgsnapshot and lssubsys to show only one mount point for a
>> hierarchy and especially in case of cgsnapshot it's not what user expects.
>> The patch below enhances internal table of mount points with duplicate
>> mounts of the same hierarchy.
>>
>> In addition. cgroup_get_controller_begin is enhanced too. This introduces
>> incompatible ABI change, sizeof() public structure changes and all apps
>> needs to be recompiled - how does it sound to you? Or should I make new set
>> of begin/next/end functions to get all mount points of a hierarchy?
>>
>>
>> Please note that the patch is not intended to be accepted, I need to test
>> it and also some changes in lssubsys and cgsnapshot are needed.
>>
>
> I was going to ask about the test case and how this was tested.
It was tested only very very lightly, nothing to talk about :)
>> diff --git a/src/api.c b/src/api.c
>> index b76b793..508fc1f 100644
>> --- a/src/api.c
>> +++ b/src/api.c
>> @@ -823,16 +823,12 @@ int cgroup_init(void)
>> break;
>> }
>> }
>> - if (duplicate) {
>> - cgroup_dbg("controller %s is already mounted on
>> %s\n",
>> - mntopt, cg_mount_table[j].path);
>> - break;
>> - }
>> -
>> strcpy(cg_mount_table[found_mnt].name, controllers[i]);
>> strcpy(cg_mount_table[found_mnt].path, ent->mnt_dir);
>> - cgroup_dbg("Found cgroup option %s, count %d\n",
>> - ent->mnt_opts, found_mnt);
>> + cg_mount_table[found_mnt].duplicate = duplicate;
>
> I presume you have this under the if (duplicate) part, but the diff
> seems to indicate otherwise.
No, it diff shows it corretly, if (duplicate) is removed completely,
it's not needed at all (the debug message just below was modified to
indicate the duplicate status).
>
>> + cgroup_dbg("Found cgroup option %s, count %d, " \
>> + "duplicate %d\n", ent->mnt_opts, found_mnt,
>> + duplicate);
>> found_mnt++;
>> }
>>
>> diff --git a/src/libcgroup-internal.h b/src/libcgroup-internal.h
>> index 801b35e..5cf8fbe 100644
>> --- a/src/libcgroup-internal.h
>> +++ b/src/libcgroup-internal.h
>> @@ -93,6 +93,12 @@ struct cg_mount_table_s {
>> char name[FILENAME_MAX];
>> char path[FILENAME_MAX];
>> int index;
>> +
>> + /**
>> + * When set, the same controller is already in mount table with lower
>> + * index - used when one hierarchy is mounted multiple times.
>> + */
>> + int duplicate;
>> };
>>
>
> There are quite a lot of checks for if
> (cg_mount_table[idx].duplicate), is there an easier way to fix this?
> Should we have a unique list as well for use by these larger
> functions?
Yeah, I know, lot of checks... to be honest, I was lazy to make new
list. Now that I think about it, I don't like the code now.
So, with this feedback and also the feedback on IRC, I'll rework it:
- it won't change ABI -> separate set of
get_controllers_ext?_begin/next/end functions. I'll try to find better
name :)
- with separate list of duplicate mount points so cg_mount_table lists
only the first mount point as it is now
Jan
------------------------------------------------------------------------------
Colocation vs. Managed Hosting
A question and answer guide to determining the best fit
for your organization - today and in the future.
http://p.sf.net/sfu/internap-sfd2d
_______________________________________________
Libcg-devel mailing list
Libcg-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libcg-devel
