On 07/26/2010 10:49 AM, Dhaval Giani wrote:
> part of the series to convert to a thread aware as opposed to a
> thread safe model.
>
> Signed-off-by: Dhaval Giani<[email protected]>
> ---
> include/libcgroup/deprecated.h | 12 +++++++
> include/libcgroup/error.h | 1
> include/libcgroup/groups.h | 3 -
> src/api.c | 66
> +++++++++++++++++++++++++----------------
> src/libcgroup.map | 1
> 5 files changed, 55 insertions(+), 28 deletions(-)
>
> Index: libcg/src/api.c
> ===================================================================
> --- libcg.orig/src/api.c
> +++ libcg/src/api.c
> @@ -1331,15 +1331,28 @@ int cgroup_create_cgroup(struct cgroup *
> int error = 0;
> int retval = 0;
> int ret;
> -
> - if (!cgroup_initialized)
> - return ECGROUPNOTINITIALIZED;
> + struct cgroup_context_s *cg_context;
> + int deprecated = 0;
>
> if (!cgroup)
> return ECGROUPNOTALLOWED;
>
> + if (!cgroup->context) {
> + pthread_rwlock_rdlock(&cg_mount_table_lock);
> + cgroup->context = cgroup_context;
> + deprecated = 1;
> + cgroup_dbg("WARNING: This is deprecated. Please utilize the " \
> + "new context aware interfaces\n");
> + }
How does an application set cgroup->context? I have not found any
function for that, did I miss something?
I think that it would be better to have
cgroup_new_cgroup_ctx(struct cgroup_context_s *ctx, char *name), which
creates the structure and puts the context there, and
cgroup_new_cgroup(), which just calls
cgroup_new_cgroup_ctx(the_global_context, name) (and optionally yells at
user about deprecated API, uninitialized library etc.)
cgroup_create_cgroup (and all other functions like delete,
create_from_parent, compare, ...) just assume that the context is always
filled, no extra handling is needed there.
And I am not sure about _context suffix - if it is going to be heavily
used, I would propose _ctx. If it is used only in
cgroup_new_cgroup_context and cgroup_init_context then it's probably fine.
> +
> + if (!cgroup->context->size)
> + return ECGROUPINVALCONTEXT;
> +
> + cg_context = cgroup->context;
> +
> for (i = 0; i< cgroup->index; i++) {
> - if (!cgroup_test_subsys_mounted(cgroup->controller[i]->name))
> + if (!cgroup_test_subsys_context(cg_context,
> + cgroup->controller[i]->name))
> return ECGROUPSUBSYSNOTMOUNTED;
> }
>
> @@ -1348,6 +1361,7 @@ int cgroup_create_cgroup(struct cgroup *
> last_errno = errno;
> return ECGOTHER;
> }
> +
> fts_path[1] = NULL;
> path = fts_path[0];
>
> @@ -1357,7 +1371,7 @@ int cgroup_create_cgroup(struct cgroup *
> * data structure. If not, we fail.
> */
> for (k = 0; k< cgroup->index; k++) {
> - if (!cg_build_path(cgroup->name, path,
> + if (!cg_build_path_context(cg_context, cgroup->name, path,
> cgroup->controller[k]->name))
> continue;
>
> @@ -1366,19 +1380,16 @@ int cgroup_create_cgroup(struct cgroup *
> goto err;
>
> base = strdup(path);
> -
> if (!base) {
> last_errno = errno;
> error = ECGOTHER;
> goto err;
> }
>
> - if (!ignore_ownership) {
> - cgroup_dbg("Changing ownership of %s\n", fts_path[0]);
> - error = cg_chown_recursive(fts_path,
> - cgroup->control_uid, cgroup->control_gid);
> - }
> + cgroup_dbg("Changing ownership of %s\n", fts_path[0]);
>
> + error = cg_chown_recursive(fts_path,
> + cgroup->control_uid, cgroup->control_gid);
> if (error)
> goto err;
>
> @@ -1392,6 +1403,7 @@ int cgroup_create_cgroup(struct cgroup *
> error = ECGOTHER;
> goto err;
> }
> +
> error = cg_set_control_value(path,
> cgroup->controller[k]->values[j]->value);
> /*
> @@ -1409,26 +1421,30 @@ int cgroup_create_cgroup(struct cgroup *
> }
> }
>
> - if (!ignore_ownership) {
> - ret = snprintf(path, FILENAME_MAX, "%s/tasks", base);
> - if (ret< 0 || ret>= FILENAME_MAX) {
> - last_errno = errno;
> - error = ECGOTHER;
> - goto err;
> - }
> - error = chown(path, cgroup->tasks_uid,
> - cgroup->tasks_gid);
> - if (error) {
> - last_errno = errno;
> - error = ECGOTHER;
> - goto err;
> - }
> + ret = snprintf(path, FILENAME_MAX, "%s/tasks", base);
> + if (ret< 0 || ret>= FILENAME_MAX) {
> + last_errno = errno;
> + error = ECGOTHER;
> + goto err;
> }
> +
> + error = chown(path, cgroup->tasks_uid, cgroup->tasks_gid);
> + if (error) {
> + last_errno = errno;
> + error = ECGOTHER;
> + goto err;
> + }
> +
> free(base);
> base = NULL;
> }
>
> err:
> + if(deprecated) {
> + cgroup->context = NULL;
> + pthread_rwlock_unlock(&cg_mount_table_lock);
> + }
> +
> if (path)
> free(path);
> if (base)
> Index: libcg/include/libcgroup/error.h
> ===================================================================
> --- libcg.orig/include/libcgroup/error.h
> +++ libcg/include/libcgroup/error.h
> @@ -71,6 +71,7 @@ enum {
> ECGNAMESPACEPATHS,
> ECGNAMESPACECONTROLLER,
> ECGMOUNTNAMESPACE,
> + ECGROUPINVALCONTEXT,
> };
>
> /**
> Index: libcg/include/libcgroup/deprecated.h
> ===================================================================
> --- libcg.orig/include/libcgroup/deprecated.h
> +++ libcg/include/libcgroup/deprecated.h
> @@ -21,6 +21,18 @@ __BEGIN_DECLS
> */
> int cgroup_init(void);
>
> +/**
> + * Physically create a control group in kernel. The group is created in all
> + * hierarchies, which cover controllers added by cgroup_add_controller().
> + * All parameters set by cgroup_add_value_* functions are written.
> + * The created groups has owner which was set by cgroup_set_uid_gid().
> + * @param cgroup
> + * @param ignore_ownership When nozero, all errors are ignored when setting
> + * owner of the group and/or its tasks file.
> + * @todo what is ignore_ownership good for?
> + */
> +int cgroup_create_cgroup(struct cgroup *cgroup, int ignore_ownership);
> +
> __END_DECLS
>
> #endif
> Index: libcg/include/libcgroup/groups.h
> ===================================================================
> --- libcg.orig/include/libcgroup/groups.h
> +++ libcg/include/libcgroup/groups.h
> @@ -172,9 +172,6 @@ void cgroup_free_controllers(struct cgro
> * All parameters set by cgroup_add_value_* functions are written.
> * The created groups has owner which was set by cgroup_set_uid_gid().
> * @param cgroup
> - * @param ignore_ownership When nozero, all errors are ignored when setting
> - * owner of the group and/or its tasks file.
> - * @todo what is ignore_ownership good for?
> */
> int cgroup_create_cgroup(struct cgroup *cgroup, int ignore_ownership);
>
> Index: libcg/src/libcgroup.map
> ===================================================================
> --- libcg.orig/src/libcgroup.map
> +++ libcg/src/libcgroup.map
> @@ -93,4 +93,5 @@ CGROUP_0.37 {
> cgroup_get_procs;
> cgroup_init_context;
> cgroup_free_context;
> + cgroup_create_cgroup_context;
> } CGROUP_0.36;
>
>
>
> ------------------------------------------------------------------------------
> The Palm PDK Hot Apps Program offers developers who use the
> Plug-In Development Kit to bring their C/C++ apps to Palm for a share
> of $1 Million in cash or HP Products. Visit us here for more details:
> http://ad.doubleclick.net/clk;226879339;13503038;l?
> http://clk.atdmt.com/CRS/go/247765532/direct/01/
> _______________________________________________
> Libcg-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/libcg-devel
------------------------------------------------------------------------------
The Palm PDK Hot Apps Program offers developers who use the
Plug-In Development Kit to bring their C/C++ apps to Palm for a share
of $1 Million in cash or HP Products. Visit us here for more details:
http://ad.doubleclick.net/clk;226879339;13503038;l?
http://clk.atdmt.com/CRS/go/247765532/direct/01/
_______________________________________________
Libcg-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/libcg-devel
