On Tue, May 19, 2009 at 03:31:34PM +0200, Jan Safranek wrote:
> Thanks for the API! In general it looks very well, I just found few
> nits, see below.
>
> Jan
>
> Dhaval Giani wrote:
>> This set of APIs will allow the caller to query the mount table
>> and find out what controller is mounted at what path.
>>
>> Test program has been included in the patch. Running the test program
>> results in
>>
>> [dha...@gondor tests]$ ../libtool --mode=execute ./get_controller
>> Controller cpu is mounted at /cgroup
>> Controller cpuacct is mounted at /cgroup
>> Controller memory is mounted at /cgroup1
>> [dha...@gondor tests]$
>>
>> Which is the setup on this system.
>>
>> Signed-off-by: Dhaval Giani <[email protected]>
>> Cc: Jan Safranek <[email protected]>
>>
>> ---
>> include/libcgroup.h | 12 ++++++
>> src/api.c | 85
>> +++++++++++++++++++++++++++++++++++++++++++++++++
>> src/libcgroup.map | 3 +
>> tests/Makefile.am | 3 +
>> tests/get_controller.c | 35 ++++++++++++++++++++
>> 5 files changed, 137 insertions(+), 1 deletion(-)
>>
>> Index: libcg/src/api.c
>> ===================================================================
>> --- libcg.orig/src/api.c
>> +++ libcg/src/api.c
>> @@ -2525,3 +2525,88 @@ int cgroup_get_task_begin(char *cgroup,
>> return ret;
>> }
>> +
>> +
>> +int cgroup_get_controller_end(void **handle)
>> +{
>> + int *key = (int *) *handle;
>> +
>> + if (!cgroup_initialized)
>> + return ECGROUPNOTINITIALIZED;
>> +
>> + if (!key)
>> + return ECGINVAL;
>> +
>> + free(key);
>> + *handle = NULL;
>> +
>> + return 0;
>> +}
>> +/*
>> + * name and path will be allocated by the API. The caller *must* free
>> + * name and path before calling into the API again else there will be
>> + * leaks.
>> + */
> This note should be in the public header (something less strict is
> already there), it's more or less useless here.
>
good point. I will move the comment there.
>> +int cgroup_get_controller_next(void **handle, char **name, char **path)
>> +{
>> + int error = 0;
>> + int *pos = (int *) *handle;
>> + int ret = 0;
>> +
>> + if (!cgroup_initialized)
>> + return ECGROUPNOTINITIALIZED;
>> +
>> + if (!pos)
>> + return ECGINVAL;
>> +
>> + pthread_rwlock_rdlock(&cg_mount_table_lock);
>> +
>> + if (cg_mount_table[*pos].name[0] == '\0') {
>> + ret = ECGEOF;
>> + goto out_unlock;
>> + }
>> +
>> + *name = strdup(cg_mount_table[*pos].name);
>> +
>> + if (!*name) {
>> + last_errno = errno;
>> + ret = ECGOTHER;
>> + goto out_unlock;
>> + }
>> +
>> + *path = strdup(cg_mount_table[*pos].path);
>> +
>> + if (!*path) {
>> + last_errno = errno;
>> + ret = ECGOTHER;
>> + goto out_unlock;
>
> Should be *name freed? It's really matter of point of view if partial
> results should be returned.
>
It should be freed. I missed it.
>> + }
>> +
>> + (*pos)++;
>> + *handle = pos;
>
> Is this line necessary? pos already equals to *handle from above.
>
I had some issues not doing that earlier. Will test and confirm again.
>> +
>> +out_unlock:
>> + pthread_rwlock_unlock(&cg_mount_table_lock);
>> + return ret;
>> +}
>> +
>> +int cgroup_get_controller_begin(void **handle, char **name, char **path)
>> +{
>> + int *pos;
>> +
>> + if (!cgroup_initialized)
>> + return ECGROUPNOTINITIALIZED;
>> +
>> + pos = malloc(sizeof(int));
>> +
>> + if (!pos) {
>> + last_errno = errno;
>> + return ECGOTHER;
>> + }
>> +
>> + *pos = 0;
>> +
>> + *handle = pos;
>> +
>> + return cgroup_get_controller_next(handle, name, path);
>> +}
>> Index: libcg/include/libcgroup.h
>> ===================================================================
>> --- libcg.orig/include/libcgroup.h
>> +++ libcg/include/libcgroup.h
>> @@ -303,6 +303,18 @@ int cgroup_get_task_begin(char *cgroup, */
>> int cgroup_get_task_next(void **handle, pid_t *pid);
>> int cgroup_get_task_end(void **handle);
>> +
>> +/**
>> + * Read the mount table to give a list where each controller is
>> + * mounted
>> + * @handle: Handle to be used for iteration.
>> + * @name: The variable where the name is stored. Should be freed by caller.
>> + * @path: Te variable where the path to the controller is stored. Should be
>> + * freed by the caller.
>> + */
>
> Should there be a note in which order are the controllers returned? It
> seems to me it's grouped by mount point (which is good), is this order
> guaranteed?
>
This order is guaranteed. Balbir has pointed out that I shuold document
that property, but I sort of missed it in this posting. I shall do so in
the next iteration.
thanks,
--
regards,
Dhaval
------------------------------------------------------------------------------
Crystal Reports - New Free Runtime and 30 Day Trial
Check out the new simplified licensing option that enables
unlimited royalty-free distribution of the report engine
for externally facing server and web deployment.
http://p.sf.net/sfu/businessobjects
_______________________________________________
Libcg-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/libcg-devel
