Balbir Singh wrote: > Feature: Provide new libcgroup walk tree API > > From: Balbir Singh <[email protected]> > > This patch adds the capability to walk cgroups by providing a new API > called cgroup_walk_tree. The API accepts the controller to walk and the > order in which the directories and files must be visited. The code is > implemented as an iterator, the begin function starts the walk and > we have depth control. The next function gets the following node > and returns ECGEOF when done.
Thanks a lot! It looks much better now, see my comments inline. <presonal feeling> The usual way to iterate through something on FS is open/read/close pattern, where open does not read anything, it just opens a stream (or iterator), for example fts_open/fts_read/fts_close and opendir/readdir/closedir. People are used to it and they know the drill. You invent something different, your cgroup_walk_tree_begin() not only opens the iterator, it also reads the first element. This is of course possible, it just does not respect the pattern I am used to. </personal feeling> > libcgroup.map has been updated to reflect the same change and the prototype > is exported in libcgroup.h. > > I've also added test cases (tests/walk_test.c). Sample output is show > > root is /cgroup/cpu/// > path , parent , relative /, full /cgroup/cpu/// > path a, parent , relative /a, full /cgroup/cpu///a > path e, parent a, relative /a/e, full /cgroup/cpu///a/e > path f, parent e, relative /a/e/f, full /cgroup/cpu///a/e/f > path f, parent a, relative /a/f, full /cgroup/cpu///a/f > path x, parent a, relative /a/x, full /cgroup/cpu///a/x > path b, parent a, relative /a/b, full /cgroup/cpu///a/b > path c, parent b, relative /a/b/c, full /cgroup/cpu///a/b/c > path d, parent c, relative /a/b/c/d, full /cgroup/cpu///a/b/c/d > path default, parent , relative /default, full /cgroup/cpu///default > root is /cgroup/cpu/// > path , parent , relative /, full /cgroup/cpu/// > path a, parent , relative /a, full /cgroup/cpu///a > path e, parent a, relative /a/e, full /cgroup/cpu///a/e > path f, parent a, relative /a/f, full /cgroup/cpu///a/f > path x, parent a, relative /a/x, full /cgroup/cpu///a/x > path b, parent a, relative /a/b, full /cgroup/cpu///a/b > path default, parent , relative /default, full /cgroup/cpu///default > > NOTE: Parent directory is represented by an empty (not NULL) string "". > The length of the string is 0. > > Signed-off-by: Balbir Singh <[email protected]> > --- > > api.c | 97 > +++++++++++++++++++++++++++++++++++++++++++++++++++++ > libcgroup.h | 49 +++++++++++++++++++++++++++ > libcgroup.map | 2 + > tests/Makefile | 6 +++ > tests/walk_test.c | 66 ++++++++++++++++++++++++++++++++++++ > 5 files changed, 219 insertions(+), 1 deletions(-) > create mode 100644 tests/walk_test.c > > > diff --git a/api.c b/api.c > index 968f9e1..ae03f02 100644 > --- a/api.c > +++ b/api.c > @@ -105,6 +105,7 @@ char *cgroup_strerror_codes[] = { > "Cgroup, rules file does not exist", > "Cgroup mounting failed", > "The config file can not be opend", > + "End of File or iterator", > }; > > static int cg_chown_file(FTS *fts, FTSENT *ent, uid_t owner, gid_t group) > @@ -2242,3 +2243,99 @@ int cgroup_get_last_errno() > { > return last_errno; > } > + > + > +static int cg_walk_node(FTS *fts, FTSENT *ent, const int depth, > + struct cgroup_file_info *info) > +{ > + int ret = 0; > + int base_level; > + > + cgroup_dbg("seeing file %s\n", ent->fts_path); > + > + info->path = ent->fts_name; > + info->parent = ent->fts_parent->fts_name; > + info->full_path = ent->fts_path; > + info->depth = ent->fts_level; > + info->type = CGROUP_FILE_TYPE_OTHER; > + > + if (depth && (info->depth > depth)) > + return 0; > + > + switch (ent->fts_info) { > + case FTS_DNR: > + case FTS_ERR: > + errno = ent->fts_errno; > + break; > + case FTS_D: > + info->type = CGROUP_FILE_TYPE_DIR; > + break; > + case FTS_DC: > + case FTS_NSOK: > + case FTS_NS: > + case FTS_DP: > + break; > + case FTS_F: > + info->type = CGROUP_FILE_TYPE_FILE; > + break; > + case FTS_DEFAULT: > + break; > + } > + return ret; > +} > + > +int cgroup_walk_tree_next(const int depth, void **handle, > + struct cgroup_file_info *info, int base_level) > +{ > + int ret = 0; > + FTS *fts = *(FTS **)handle; > + FTSENT *ent; > + > + if (!handle) > + return ECGINVAL; > + ent = fts_read(fts); > + if (!ent) { > + fts_close(fts); > + return ECGEOF; I wouldn't close the fts here, I'd add new cgroup__walk_tree_end(), which would free everything (i.e. close fts). Imagine a situation when application walks through a tree and either finds what it was looking for or an error occurs - the application does not walk through the rest and wants to end the walk in the middle. > + } > + if (!base_level && depth) > + base_level = ent->fts_level + depth; > + ret = cg_walk_node(fts, ent, base_level, info); > + *handle = fts; > + return ret; > +} > + > +/* > + * TODO: Need to decide a better place to put this function. > + */ > +int cgroup_walk_tree_begin(char *controller, char *base_path, const int > depth, > + void **handle, struct cgroup_file_info *info, > + int *base_level) > +{ > + int ret = 0; > + cgroup_dbg("path is %s\n", base_path); > + char *cg_path[2]; > + char full_path[FILENAME_MAX]; > + FTSENT *ent; > + FTS *fts; > + > + if (!cg_build_path(base_path, full_path, controller)) > + return ECGOTHER; > + > + *base_level = 0; > + cg_path[0] = full_path; > + cg_path[1] = NULL; > + > + fts = fts_open(cg_path, FTS_LOGICAL | FTS_NOCHDIR | > + FTS_NOSTAT, NULL); > + ent = fts_read(fts); > + if (!ent) { > + cgroup_dbg("fts_read failed\n"); > + return ECGINVAL; > + } > + if (!*base_level && depth) > + *base_level = ent->fts_level + depth; > + ret = cg_walk_node(fts, ent, *base_level, info); > + *handle = fts; > + return ret; > +} > diff --git a/libcgroup.h b/libcgroup.h > index 08613cf..c08b216 100644 > --- a/libcgroup.h > +++ b/libcgroup.h > @@ -94,6 +94,31 @@ enum cgroup_errors { > ECGROUPNORULES, /* Rules list does not exist. */ > ECGMOUNTFAIL, > ECGSENTINEL, /* Please insert further error codes above this */ > + ECGEOF, /* End of file, iterator */ > +}; > + > +/* > + * Don't use CGROUP_WALK_TYPE_FILE right now. It is added here for > + * later refactoring and better implementation. Most users *should* > + * use CGROUP_WALK_TYPE_PRE_DIR. > + */ > +enum cgroup_walk_type { > + CGROUP_WALK_TYPE_PRE_DIR = 0x1, /* Pre Order Directory */ > + CGROUP_WALK_TYPE_POST_DIR = 0x2, /* Post Order Directory */ > +}; > + > +enum cgroup_file_type { > + CGROUP_FILE_TYPE_FILE, /* File */ > + CGROUP_FILE_TYPE_DIR, /* Directory */ > + CGROUP_FILE_TYPE_OTHER, /* Directory */ > +}; > + > +struct cgroup_file_info { > + enum cgroup_file_type type; > + const char *path; > + const char *parent; > + const char *full_path; > + short depth; > }; > > #define CG_NV_MAX 100 > @@ -199,6 +224,30 @@ char *cgroup_strerror(int code); > */ > int cgroup_get_last_errno(); > > +/** > + * Walk through the directory tree for the specified controller. > + * @controller: Name of the controller, for which we want to walk > + * the directory tree > + * @base_path: Begin walking from this path > + * @depth: The maximum depth to which the function should walk, 0 > + * implies all the way down > + * @handle: Handle to be used during iteration > + * @info: info filled and returned about directory information I'd appreciate some info who is supposed to free the strings inside @info (the libcgroup/fts is) and for how long the strings are available (until next call to cgroup_walk_tree_* with the same handle, if I get it right). > + */ > +int cgroup_walk_tree_begin(char *controller, char *base_path, const int > depth, > + void **handle, struct cgroup_file_info *info, > + int *base_level); > +/** > + * Get the next element during the walk > + * @depth: The maximum depth to which the function should walk, 0 > + * implies all the way down > + * @handle: Handle to be used during iteration > + * @info: info filled and returned about directory information > + * > + * Returns ECGEOF when we are done walking through the nodes. > + */ > +int cgroup_walk_tree_next(const int depth, void **handle, > + struct cgroup_file_info *info, int base_level); What is base_level good for? Shouldn't be depth and base_level stored inside the **handle? It doesn't need to be FTS*, it can be allocated struct containing all this info. > > /* The wrappers for filling libcg structures */ > > diff --git a/libcgroup.map b/libcgroup.map > index fffe448..58e2f62 100644 > --- a/libcgroup.map > +++ b/libcgroup.map > @@ -49,5 +49,7 @@ global: > CGROUP_0.33 { > global: > cgroup_get_last_errno; > + cgroup_walk_tree_begin; > + cgroup_walk_tree_next; > } CGROUP_0.32.1; > > diff --git a/tests/Makefile b/tests/Makefile > index 00bfe3d..9ebadac 100644 > --- a/tests/Makefile > +++ b/tests/Makefile > @@ -8,7 +8,8 @@ TARGET= libtest_functions.a \ > libcgrouptest01 \ > libcg_ba \ > setuid \ > - pathtest > + pathtest \ > + walk_test > > all: $(TARGET) > > @@ -30,5 +31,8 @@ setuid: setuid.c > pathtest: pathtest.c > $(CC) $(CFLAGS) -o $@ $< $(LDFLAGS) $(LIBS) > > +walk_test: walk_test.c > + $(CC) $(CFLAGS) -o $@ $< $(LDFLAGS) $(LIBS) > + > clean: > \rm -f $(TARGET) test_functions.o > diff --git a/tests/walk_test.c b/tests/walk_test.c > new file mode 100644 > index 0000000..1e97a17 > --- /dev/null > +++ b/tests/walk_test.c > @@ -0,0 +1,66 @@ > +#include <stdio.h> > +#include <stdlib.h> > +#include <sys/types.h> > +#include <unistd.h> > +#include <string.h> > +#include <libcgroup.h> > + > +void visit_node(struct cgroup_file_info *info, char *root) > +{ > + if (info->type == CGROUP_FILE_TYPE_DIR) { > + printf("path %s, parent %s, relative %s, full %s\n", > + info->path, info->parent, info->full_path + > + + strlen(root) - 1, > + info->full_path); > + } > +} > + > +int main(int argc, char *argv[]) > +{ > + int ret; > + char *controller; > + void *handle; > + struct cgroup_file_info info; > + char root[FILENAME_MAX]; > + int lvl; > + > + if (argc < 2) { > + fprintf(stderr, "Usage %s: <controller name>\n", > + argv[0]); > + exit(EXIT_FAILURE); > + } > + > + controller = argv[1]; > + > + cgroup_init(); > + > + ret = cgroup_walk_tree_begin(controller, "/", 0, &handle, &info, &lvl); > + > + if (ret != 0) { > + fprintf(stderr, "Walk failed\n"); > + exit(EXIT_FAILURE); > + } > + strncpy(root, info.full_path, strlen(info.full_path)); > + printf("root is %s\n", root); > + visit_node(&info, root); > + while ((ret = cgroup_walk_tree_next(0, &handle, &info, lvl)) != > + ECGEOF) { > + visit_node(&info, root); > + } > + > + ret = cgroup_walk_tree_begin(controller, "/", 2, &handle, &info, &lvl); > + > + if (ret != 0) { > + fprintf(stderr, "Walk failed\n"); > + exit(EXIT_FAILURE); > + } > + strncpy(root, info.full_path, strlen(info.full_path)); > + printf("root is %s\n", root); > + visit_node(&info, root); > + while ((ret = cgroup_walk_tree_next(2, &handle, &info, lvl)) != > + ECGEOF) { > + visit_node(&info, root); > + } > + > + return EXIT_SUCCESS; > +} > ------------------------------------------------------------------------------ Open Source Business Conference (OSBC), March 24-25, 2009, San Francisco, CA -OSBC tackles the biggest issue in open source: Open Sourcing the Enterprise -Strategies to boost innovation and cut costs with open source participation -Receive a $600 discount off the registration fee with the source code: SFAD http://p.sf.net/sfu/XcvMzF8H _______________________________________________ Libcg-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/libcg-devel
