On 04/06/2011 10:37 AM, Jan Safranek wrote:
> Current libcgroup design handles each hierarchy only once. If a hierarchy
> is mounted twice or more times, only the first mount point is taken into
> account and the others are 'invisible' to libcgroup.
>
> This causes cgsnapshot and lssubsys to show only one mount point for a
> hierarchy and especially in case of cgsnapshot it's not what user expects.
> The patch below adds a list of all mount points to cg_mount_table_s structure.
>
> Signed-off-by: Jan Safranek<[email protected]>
Patch is ok, the only problem I see with it is strcpy in 
cgroup_init(api.c) function, I will sent separate patch for that.

Acked-by: Ivana Hutarova Varekova<[email protected]>

> ---
>
>   src/api.c                |   58 
> ++++++++++++++++++++++++++++++++++++++--------
>   src/config.c             |   29 +++++++++++++----------
>   src/libcgroup-internal.h |   10 +++++++-
>   3 files changed, 74 insertions(+), 23 deletions(-)
>
> diff --git a/src/api.c b/src/api.c
> index c3dba98..0dc135a 100644
> --- a/src/api.c
> +++ b/src/api.c
> @@ -718,6 +718,29 @@ unlock:
>       return ret;
>   }
>
> +int cg_add_duplicate_mount(struct cg_mount_table_s *item, const char *path)
> +{
> +     struct cg_mount_point *mount, *it;
> +
> +     mount = malloc(sizeof(struct cg_mount_point));
> +     if (!mount)
> +             return ECGFAIL;
> +     mount->next = NULL;
> +     strncpy(mount->path, path, sizeof(mount->path));
> +     mount->path[sizeof(mount->path)-1] = '\0';
> +
> +     /*
> +      * Add the mount point to the end of the list.
> +      * Assuming the list is short, no optimization is done.
> +      */
> +     it =&item->mount;
> +     while (it->next)
> +             it = it->next;
> +
> +     it->next = mount;
> +     return 0;
> +}
> +
>   /**
>    * cgroup_init(), initializes the MOUNT_POINT.
>    *
> @@ -826,12 +849,19 @@ int cgroup_init(void)
>                       }
>                       if (duplicate) {
>                               cgroup_dbg("controller %s is already mounted on 
> %s\n",
> -                                     mntopt, cg_mount_table[j].path);
> -                             break;
> +                                     mntopt, cg_mount_table[j].mount.path);
> +                             ret = cg_add_duplicate_mount(&cg_mount_table[j],
> +                                             ent->mnt_dir);
> +                             if (ret)
> +                                     goto unlock_exit;
> +                             /* continue with next controller */
> +                             continue;
>                       }
>
>                       strcpy(cg_mount_table[found_mnt].name, controllers[i]);
> -                     strcpy(cg_mount_table[found_mnt].path, ent->mnt_dir);
> +                     strcpy(cg_mount_table[found_mnt].mount.path,
> +                                     ent->mnt_dir);
> +                     cg_mount_table[found_mnt].mount.next = NULL;
>                       cgroup_dbg("Found cgroup option %s, count %d\n",
>                               ent->mnt_opts, found_mnt);
>                       found_mnt++;
> @@ -859,12 +889,18 @@ int cgroup_init(void)
>
>                       if (duplicate) {
>                               cgroup_dbg("controller %s is already mounted on 
> %s\n",
> -                                     mntopt, cg_mount_table[j].path);
> +                                     mntopt, cg_mount_table[j].mount.path);
> +                             ret = cg_add_duplicate_mount(&cg_mount_table[j],
> +                                             ent->mnt_dir);
> +                             if (ret)
> +                                     goto unlock_exit;
>                               continue;
>                       }
>
>                       strcpy(cg_mount_table[found_mnt].name, mntopt);
> -                     strcpy(cg_mount_table[found_mnt].path, ent->mnt_dir);
> +                     strcpy(cg_mount_table[found_mnt].mount.path,
> +                                     ent->mnt_dir);
> +                     cg_mount_table[found_mnt].mount.next = NULL;
>                       cgroup_dbg("Found cgroup option %s, count %d\n",
>                               ent->mnt_opts, found_mnt);
>                       found_mnt++;
> @@ -959,10 +995,12 @@ static char *cg_build_path_locked(const char *name, 
> char *path,
>                */
>               if (strcmp(cg_mount_table[i].name, type) == 0) {
>                       if (cg_namespace_table[i]) {
> -                             sprintf(path, "%s/%s/", cg_mount_table[i].path,
> -                                                     cg_namespace_table[i]);
> +                             sprintf(path, "%s/%s/",
> +                                             cg_mount_table[i].mount.path,
> +                                             cg_namespace_table[i]);
>                       } else {
> -                             sprintf(path, "%s/", cg_mount_table[i].path);
> +                             sprintf(path, "%s/",
> +                                             cg_mount_table[i].mount.path);
>                       }
>
>                       if (name) {
> @@ -3282,7 +3320,7 @@ int cgroup_get_controller_next(void **handle, struct 
> cgroup_mount_point *info)
>
>       strncpy(info->name, cg_mount_table[*pos].name, FILENAME_MAX);
>
> -     strncpy(info->path, cg_mount_table[*pos].path, FILENAME_MAX);
> +     strncpy(info->path, cg_mount_table[*pos].mount.path, FILENAME_MAX);
>
>       (*pos)++;
>       *handle = pos;
> @@ -3608,7 +3646,7 @@ int cgroup_get_subsys_mount_point(const char 
> *controller, char **mount_point)
>               if (strncmp(cg_mount_table[i].name, controller, FILENAME_MAX))
>                       continue;
>
> -             *mount_point = strdup(cg_mount_table[i].path);
> +             *mount_point = strdup(cg_mount_table[i].mount.path);
>
>               if (!*mount_point) {
>                       last_errno = errno;
> diff --git a/src/config.c b/src/config.c
> index 0f71003..63c5946 100644
> --- a/src/config.c
> +++ b/src/config.c
> @@ -328,7 +328,8 @@ int cgroup_config_insert_into_mount_table(char *name, 
> char *mount_point)
>        * Merge controller names with the same mount point
>        */
>       for (i = 0; i<  config_table_index; i++) {
> -             if (strcmp(config_mount_table[i].path, mount_point) == 0) {
> +             if (strcmp(config_mount_table[i].mount.path,
> +                             mount_point) == 0) {
>                       char *cname = config_mount_table[i].name;
>                       strncat(cname, ",", FILENAME_MAX - strlen(cname) - 1);
>                       strncat(cname, name,
> @@ -338,7 +339,8 @@ int cgroup_config_insert_into_mount_table(char *name, 
> char *mount_point)
>       }
>
>       strcpy(config_mount_table[config_table_index].name, name);
> -     strcpy(config_mount_table[config_table_index].path, mount_point);
> +     strcpy(config_mount_table[config_table_index].mount.path, mount_point);
> +     config_mount_table[config_table_index].mount.next = NULL;
>       config_table_index++;
>   done:
>       pthread_rwlock_unlock(&config_table_lock);
> @@ -368,7 +370,9 @@ int cgroup_config_insert_into_namespace_table(char *name, 
> char *nspath)
>       pthread_rwlock_wrlock(&namespace_table_lock);
>
>       strcpy(config_namespace_table[namespace_table_index].name, name);
> -     strcpy(config_namespace_table[namespace_table_index].path, nspath);
> +     strcpy(config_namespace_table[namespace_table_index].mount.path,
> +                     nspath);
> +     config_namespace_table[namespace_table_index].mount.next = NULL;
>       namespace_table_index++;
>
>       pthread_rwlock_unlock(&namespace_table_lock);
> @@ -398,7 +402,7 @@ static int cgroup_config_mount_fs(void)
>       for (i = 0; i<  config_table_index; i++) {
>               struct cg_mount_table_s *curr = &(config_mount_table[i]);
>
> -             ret = stat(curr->path,&buff);
> +             ret = stat(curr->mount.path,&buff);
>
>               if (ret<  0&&  errno != ENOENT) {
>                       last_errno = errno;
> @@ -406,7 +410,7 @@ static int cgroup_config_mount_fs(void)
>               }
>
>               if (errno == ENOENT) {
> -                     ret = cg_mkdir_p(curr->path);
> +                     ret = cg_mkdir_p(curr->mount.path);
>                       if (ret)
>                               return ret;
>               } else if (!S_ISDIR(buff.st_mode)) {
> @@ -415,8 +419,8 @@ static int cgroup_config_mount_fs(void)
>                       return ECGOTHER;
>               }
>
> -             ret = mount(CGROUP_FILESYSTEM, curr->path, CGROUP_FILESYSTEM,
> -                                                             0, curr->name);
> +             ret = mount(CGROUP_FILESYSTEM, curr->mount.path,
> +                             CGROUP_FILESYSTEM, 0, curr->name);
>
>               if (ret<  0)
>                       return ECGMOUNTFAIL;
> @@ -477,10 +481,10 @@ static int cgroup_config_unmount_controllers(void)
>                * We ignore failures and ensure that all mounted
>                * containers are unmounted
>                */
> -             error = umount(config_mount_table[i].path);
> +             error = umount(config_mount_table[i].mount.path);
>               if (error<  0)
>                       cgroup_dbg("Unmount failed\n");
> -             error = rmdir(config_mount_table[i].path);
> +             error = rmdir(config_mount_table[i].mount.path);
>               if (error<  0)
>                       cgroup_dbg("rmdir failed\n");
>       }
> @@ -503,7 +507,7 @@ static int config_validate_namespaces(void)
>                * are good, else we will need to go for two
>                * loops. This should be optimized in the future
>                */
> -             mount_path = cg_mount_table[i].path;
> +             mount_path = cg_mount_table[i].mount.path;
>
>               if (!mount_path) {
>                       last_errno = errno;
> @@ -538,7 +542,7 @@ static int config_validate_namespaces(void)
>                * Search through the mount table to locate which subsystems
>                * are mounted together.
>                */
> -             while (!strncmp(cg_mount_table[j].path, mount_path,
> +             while (!strncmp(cg_mount_table[j].mount.path, mount_path,
>                                                       FILENAME_MAX)) {
>                       if (!namespace&&  cg_namespace_table[j]) {
>                               /* In case namespace is not setup, set it up */
> @@ -636,7 +640,8 @@ static int config_order_namespace_table(void)
>                                       goto error_out;
>                               }
>
> -                             cg_namespace_table[j] = 
> strdup(config_namespace_table[i].path);
> +                             cg_namespace_table[j] = strdup(
> +                                     config_namespace_table[i].mount.path);
>                               if (!cg_namespace_table[j]) {
>                                       last_errno = errno;
>                                       error = ECGOTHER;
> diff --git a/src/libcgroup-internal.h b/src/libcgroup-internal.h
> index 801b35e..45c1ded 100644
> --- a/src/libcgroup-internal.h
> +++ b/src/libcgroup-internal.h
> @@ -88,10 +88,18 @@ struct cgroup {
>       gid_t control_gid;
>   };
>
> +struct cg_mount_point {
> +     char path[FILENAME_MAX];
> +     struct cg_mount_point *next;
> +};
>
>   struct cg_mount_table_s {
> +     /** Controller name. */
>       char name[FILENAME_MAX];
> -     char path[FILENAME_MAX];
> +     /**
> +      * List of mount points, at least one mount point is there for sure.
> +      */
> +     struct cg_mount_point mount;
>       int index;
>   };
>
>
>
> ------------------------------------------------------------------------------
> Xperia(TM) PLAY
> It's a major breakthrough. An authentic gaming
> smartphone on the nation's most reliable network.
> And it wants your games.
> http://p.sf.net/sfu/verizon-sfdev
> _______________________________________________
> Libcg-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/libcg-devel


------------------------------------------------------------------------------
Xperia(TM) PLAY
It's a major breakthrough. An authentic gaming
smartphone on the nation's most reliable network.
And it wants your games.
http://p.sf.net/sfu/verizon-sfdev
_______________________________________________
Libcg-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/libcg-devel

Reply via email to