On 05/13/2011 02:01 PM, Michal Hocko wrote:
> Let's add file permission for tasks and file and directory permissions
> for control files into cgroup so that we can add them into configuration
> files.
> Permissions are initialized to NO_PERMS (unsigned -1 which doesn't
> represent any valid permissions) to reflect that no value is set. Let's
> also add a common initialization functions for both cgroup table and
> single cgroup.
>
> Signed-off-by: Michal Hocko <mho...@suse.cz>
I know, probably you do not know, but we use kernel's checkpatch script
for code style checking of libcgroup sources. And the script complains a
bit:
WARNING: externs should be avoided in .c files
#45: FILE: src/config.c:75:
+void init_cgroup_table(struct cgroup *cgroups, size_t count);
WARNING: return of an errno should typically be -ve (return -ECGFAIL)
#63: FILE: src/config.c:726:
+ return ECGFAIL;
WARNING: line over 80 characters
#96: FILE: src/wrapper.c:28:
+ cgroup->task_fperm = cgroup->control_fperm =
cgroup->control_dperm = NO_PERMS;
(well, you can ignore -ECGFAIL warning, I wonder how to turn it off)
Apart from few checkpatch problems, the patch looks fine.
> ---
> include/libcgroup/groups.h | 4 ++++
> src/config.c | 8 ++++++++
> src/libcgroup-internal.h | 3 +++
> src/wrapper.c | 15 ++++++++++++++-
> 4 files changed, 29 insertions(+), 1 deletions(-)
>
> diff --git a/include/libcgroup/groups.h b/include/libcgroup/groups.h
> index 1060641..61f18a0 100644
> --- a/include/libcgroup/groups.h
> +++ b/include/libcgroup/groups.h
> @@ -109,6 +109,10 @@ struct cgroup;
> */
> struct cgroup_controller;
>
> +/**
> + * Uninitialized file/directory permissions used for task/control files.
> + */
> +#define NO_PERMS (-1U)
>
> /**
> * Allocate new cgroup structure. This function itself does not create new
> diff --git a/src/config.c b/src/config.c
> index f1873ea..4f5d04a 100644
> --- a/src/config.c
> +++ b/src/config.c
> @@ -72,6 +72,8 @@ static int cgroup_table_index;
> */
> #define CGROUP_FILESYSTEM "cgroup"
>
> +void init_cgroup_table(struct cgroup *cgroups, size_t count);
> +
> /*
> * NOTE: All these functions return 1 on success
> * and not 0 as is the library convention
> @@ -105,6 +107,7 @@ int cgroup_config_insert_cgroup(char *cg_name)
>
> memset(newblk + oldlen, 0, (MAX_CGROUPS - oldlen) *
> sizeof(struct cgroup));
> + init_cgroup_table(newblk + oldlen, MAX_CGROUPS - oldlen);
> config_cgroup_table = newblk;
> cgroup_dbg("MAX_CGROUPS %d\n", MAX_CGROUPS);
> cgroup_dbg("reallocated config_cgroup_table to %p\n",
> config_cgroup_table);
> @@ -712,6 +715,11 @@ int cgroup_config_load_config(const char *pathname)
> }
>
> config_cgroup_table = calloc(MAX_CGROUPS, sizeof(struct cgroup));
> + if (!config_cgroup_table)
> + return ECGFAIL;
> +
> + init_cgroup_table(config_cgroup_table, MAX_CGROUPS);
> +
> if (yyparse() != 0) {
> cgroup_dbg("Failed to parse file %s\n", pathname);
> fclose(yyin);
> diff --git a/src/libcgroup-internal.h b/src/libcgroup-internal.h
> index 45c1ded..e79c038 100644
> --- a/src/libcgroup-internal.h
> +++ b/src/libcgroup-internal.h
> @@ -84,8 +84,11 @@ struct cgroup {
> int index;
> uid_t tasks_uid;
> gid_t tasks_gid;
> + mode_t task_fperm;
> uid_t control_uid;
> gid_t control_gid;
> + mode_t control_fperm;
> + mode_t control_dperm;
> };
>
> struct cg_mount_point {
> diff --git a/src/wrapper.c b/src/wrapper.c
> index 4828ea8..90c8bc3 100644
> --- a/src/wrapper.c
> +++ b/src/wrapper.c
> @@ -23,13 +23,26 @@
> #include <string.h>
> #include <unistd.h>
>
> +static void init_cgroup(struct cgroup *cgroup)
> +{
> + cgroup->task_fperm = cgroup->control_fperm = cgroup->control_dperm =
> NO_PERMS;
> +}
> +
> +void init_cgroup_table(struct cgroup *cgroups, size_t count)
> +{
> + size_t i;
> +
> + for (i = 0; i < count; ++i)
> + init_cgroup(&cgroups[i]);
> +}
> +
> struct cgroup *cgroup_new_cgroup(const char *name)
> {
> struct cgroup *cgroup = calloc(1, sizeof(struct cgroup));
> -
> if (!cgroup)
> return NULL;
>
> + init_cgroup(cgroup);
> strncpy(cgroup->name, name, sizeof(cgroup->name));
>
> return cgroup;
------------------------------------------------------------------------------
vRanger cuts backup time in half-while increasing security.
With the market-leading solution for virtual backup and recovery,
you get blazing-fast, flexible, and affordable data protection.
Download your free trial now.
http://p.sf.net/sfu/quest-d2dcopy1
_______________________________________________
Libcg-devel mailing list
Libcg-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libcg-devel
