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. > +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. > + } > + > + (*pos)++; > + *handle = pos; Is this line necessary? pos already equals to *handle from above. > + > +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? > +int cgroup_get_controller_begin(void **handle, char **name, char **path); > +int cgroup_get_controller_next(void **handle, char **name, char **path); > +int cgroup_get_controller_end(void **handle); > /* The wrappers for filling libcg structures */ > > struct cgroup *cgroup_new_cgroup(const char *name); > Index: libcg/src/libcgroup.map > =================================================================== > --- libcg.orig/src/libcgroup.map > +++ libcg/src/libcgroup.map > @@ -63,4 +63,7 @@ global: > cgroup_read_stats_next; > cgroup_read_stats_end; > cgroup_walk_tree_set_flags; > + cgroup_get_controller_end; > + cgroup_get_controller_next; > + cgroup_get_controller_begin; > } CGROUP_0.33; > Index: libcg/tests/Makefile.am > =================================================================== > --- libcg.orig/tests/Makefile.am > +++ libcg/tests/Makefile.am > @@ -2,7 +2,7 @@ INCLUDES = -I$(top_srcdir)/include > LDADD = $(top_srcdir)/src/.libs/libcgroup.la > > # compile the tests, but do not install them > -noinst_PROGRAMS = libcgrouptest01 libcg_ba setuid pathtest walk_test > read_stats walk_task > +noinst_PROGRAMS = libcgrouptest01 libcg_ba setuid pathtest walk_test > read_stats walk_task get_controller > > libcgrouptest01_SOURCES=libcgrouptest01.c test_functions.c libcgrouptest.h > libcg_ba_SOURCES=libcg_ba.cpp > @@ -11,6 +11,7 @@ pathtest_SOURCES=pathtest.c > walk_test_SOURCES=walk_test.c > read_stats_SOURCES=read_stats.c > walk_task_SOURCES=walk_task.c > +get_controller_SOURCES=get_controller.c > > EXTRA_DIST = pathtest.sh runlibcgrouptest.sh > > Index: libcg/tests/get_controller.c > =================================================================== > --- /dev/null > +++ libcg/tests/get_controller.c > @@ -0,0 +1,35 @@ > +#include <libcgroup.h> > +#include <stdio.h> > +#include <stdlib.h> > + > +int main() > +{ > + int error; > + char *path, *name; > + void *handle; > + > + error = cgroup_init(); > + > + if (error) { > + printf("cgroup_init failed with %s\n", cgroup_strerror(error)); > + exit(1); > + } > + > + error = cgroup_get_controller_begin(&handle, &name, &path); > + > + while (error != ECGEOF) { > + printf("Controller %s is mounted at %s\n", name, path); > + free(path); > + free(name); > + error = cgroup_get_controller_next(&handle, &name, &path); > + if (error && error != ECGEOF) { > + printf("cgroup_get_contrller_next failed with %s", > + cgroup_strerror(error)); > + exit(1); > + } > + } > + > + error = cgroup_get_controller_end(&handle); > + > + return 0; > +} > > ------------------------------------------------------------------------------ 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
