[lxc-devel] [PATCH 1/1] cgroup: recursively delete cgroups when asked

S.Çağlar Onur caglar at 10ur.org
Mon Jan 13 05:39:13 UTC 2014


On Sun, Jan 12, 2014 at 11:58 PM, S.Çağlar Onur <caglar at 10ur.org> wrote:
> Hey there,
>
> On Sun, Jan 12, 2014 at 7:24 PM, Stéphane Graber <stgraber at ubuntu.com> wrote:
>> On Sat, Jan 11, 2014 at 12:14:26AM -0600, Serge Hallyn wrote:
>>> Currently when a container is shut down, lxc walks the set of all
>>> cgroup paths it created, in reverse order, and tries to remove them.
>>> This doesn't suffice if the container has also created new cgroups.
>>>
>>> It'd be impolite to recursively remove all the cgroup paths we created,
>>> since this can include '/lxc' and thereunder all other containers
>>> started since.
>>>
>>> This patch changes container shutdown to only delete the container's own
>>> path, but do so recursively.  Note that if we fail during startup,
>>> the container won't have created any cgroup paths so it the old
>>> way works fine.
>>>
>>> Signed-off-by: Serge Hallyn <serge.hallyn at ubuntu.com>
>>
>> Acked-by: Stéphane Graber <stgraber at ubuntu.com>
>
> This patch started to cause my add_remove_device_node tests to fail
> (rmdir started to return EBUSY), following seems to solve it but I'm
> not sure whether it's a fix or just a workaround. Any ideas what might
> be triggering that?

Converted my go test to C API and sent to list as I thought having a
test for that won't hurt. On the other hand, I'll be waiting your
answer to submit the other patch that ignores the EBUSY.

> diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
> index 48fef74..80e68fb 100644
> --- a/src/lxc/lxccontainer.c
> +++ b/src/lxc/lxccontainer.c
> @@ -2934,7 +2934,7 @@ static bool add_remove_device_node(struct
> lxc_container *c, const char *src_path
>                         ERROR("unlink failed");
>                         goto out;
>                 }
> -               if (rmdir(directory_path) < 0 && errno != ENOTEMPTY) {
> +               if (rmdir(directory_path) < 0 && errno != ENOTEMPTY &&
> errno != EBUSY) {
>                         ERROR("rmdir failed");
>                         goto out;
>                 }
>
>
>>> ---
>>>  src/lxc/cgroup.c | 101 ++++++++++++++++++++++++++++++++++++++++++++++++-------
>>>  1 file changed, 88 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/src/lxc/cgroup.c b/src/lxc/cgroup.c
>>> index b1196b4..be43c73 100644
>>> --- a/src/lxc/cgroup.c
>>> +++ b/src/lxc/cgroup.c
>>> @@ -68,9 +68,9 @@ static char **subsystems_from_mount_options(const char *mount_options, char **ke
>>>  static void lxc_cgroup_mount_point_free(struct cgroup_mount_point *mp);
>>>  static void lxc_cgroup_hierarchy_free(struct cgroup_hierarchy *h);
>>>  static bool is_valid_cgroup(const char *name);
>>> -static int create_or_remove_cgroup(bool remove, struct cgroup_mount_point *mp, const char *path);
>>> +static int create_or_remove_cgroup(bool remove, struct cgroup_mount_point *mp, const char *path, int recurse);
>>>  static int create_cgroup(struct cgroup_mount_point *mp, const char *path);
>>> -static int remove_cgroup(struct cgroup_mount_point *mp, const char *path);
>>> +static int remove_cgroup(struct cgroup_mount_point *mp, const char *path, bool recurse);
>>>  static char *cgroup_to_absolute_path(struct cgroup_mount_point *mp, const char *path, const char *suffix);
>>>  static struct cgroup_process_info *find_info_for_subsystem(struct cgroup_process_info *info, const char *subsystem);
>>>  static int do_cgroup_get(const char *cgroup_path, const char *sub_filename, char *value, size_t len);
>>> @@ -81,6 +81,75 @@ static int cgroup_recursive_task_count(const char *cgroup_path);
>>>  static int count_lines(const char *fn);
>>>  static int handle_cgroup_settings(struct cgroup_mount_point *mp, char *cgroup_path);
>>>
>>> +static int cgroup_rmdir(char *dirname)
>>> +{
>>> +     struct dirent dirent, *direntp;
>>> +     int saved_errno = 0;
>>> +     DIR *dir;
>>> +     int ret, failed=0;
>>> +     char pathname[MAXPATHLEN];
>>> +
>>> +     dir = opendir(dirname);
>>> +     if (!dir) {
>>> +             ERROR("%s: failed to open %s", __func__, dirname);
>>> +             return -1;
>>> +     }
>>> +
>>> +     while (!readdir_r(dir, &dirent, &direntp)) {
>>> +             struct stat mystat;
>>> +             int rc;
>>> +
>>> +             if (!direntp)
>>> +                     break;
>>> +
>>> +             if (!strcmp(direntp->d_name, ".") ||
>>> +                 !strcmp(direntp->d_name, ".."))
>>> +                     continue;
>>> +
>>> +             rc = snprintf(pathname, MAXPATHLEN, "%s/%s", dirname, direntp->d_name);
>>> +             if (rc < 0 || rc >= MAXPATHLEN) {
>>> +                     ERROR("pathname too long");
>>> +                     failed=1;
>>> +                     if (!saved_errno)
>>> +                             saved_errno = -ENOMEM;
>>> +                     continue;
>>> +             }
>>> +             ret = lstat(pathname, &mystat);
>>> +             if (ret) {
>>> +                     SYSERROR("%s: failed to stat %s", __func__, pathname);
>>> +                     failed=1;
>>> +                     if (!saved_errno)
>>> +                             saved_errno = errno;
>>> +                     continue;
>>> +             }
>>> +             if (S_ISDIR(mystat.st_mode)) {
>>> +                     if (cgroup_rmdir(pathname) < 0) {
>>> +                             if (!saved_errno)
>>> +                                     saved_errno = errno;
>>> +                             failed=1;
>>> +                     }
>>> +             }
>>> +     }
>>> +
>>> +     if (rmdir(dirname) < 0) {
>>> +             SYSERROR("%s: failed to delete %s", __func__, dirname);
>>> +             if (!saved_errno)
>>> +                     saved_errno = errno;
>>> +             failed=1;
>>> +     }
>>> +
>>> +     ret = closedir(dir);
>>> +     if (ret) {
>>> +             SYSERROR("%s: failed to close directory %s", __func__, dirname);
>>> +             if (!saved_errno)
>>> +                     saved_errno = errno;
>>> +             failed=1;
>>> +     }
>>> +
>>> +     errno = saved_errno;
>>> +     return failed ? -1 : 0;
>>> +}
>>> +
>>>  struct cgroup_meta_data *lxc_cgroup_load_meta()
>>>  {
>>>       const char *cgroup_use = NULL;
>>> @@ -745,7 +814,7 @@ extern struct cgroup_process_info *lxc_cgroup_create(const char *name, const cha
>>>                * In that case, remove the cgroup from all previous hierarchies
>>>                */
>>>               for (j = 0, info_ptr = base_info; j < i && info_ptr; info_ptr = info_ptr->next, j++) {
>>> -                     r = remove_cgroup(info_ptr->designated_mount_point, info_ptr->created_paths[info_ptr->created_paths_count - 1]);
>>> +                     r = remove_cgroup(info_ptr->designated_mount_point, info_ptr->created_paths[info_ptr->created_paths_count - 1], false);
>>>                       if (r < 0)
>>>                               WARN("could not clean up cgroup we created when trying to create container");
>>>                       free(info_ptr->created_paths[info_ptr->created_paths_count - 1]);
>>> @@ -1039,8 +1108,7 @@ void lxc_cgroup_process_info_free_and_remove(struct cgroup_process_info *info)
>>>       if (!info)
>>>               return;
>>>       next = info->next;
>>> -     for (pp = info->created_paths; pp && *pp; pp++);
>>> -     for ((void)(pp && --pp); info->created_paths && pp >= info->created_paths; --pp) {
>>> +     {
>>>               struct cgroup_mount_point *mp = info->designated_mount_point;
>>>               if (!mp)
>>>                       mp = lxc_cgroup_find_mount_point(info->hierarchy, info->cgroup_path, true);
>>> @@ -1049,7 +1117,10 @@ void lxc_cgroup_process_info_free_and_remove(struct cgroup_process_info *info)
>>>                        * '/lxc' cgroup in this container but another container
>>>                        * is still running (for example)
>>>                        */
>>> -                     (void)remove_cgroup(mp, *pp);
>>> +                     (void)remove_cgroup(mp, info->cgroup_path, true);
>>> +     }
>>> +     for (pp = info->created_paths; pp && *pp; pp++);
>>> +     for ((void)(pp && --pp); info->created_paths && pp >= info->created_paths; --pp) {
>>>               free(*pp);
>>>       }
>>>       free(info->created_paths);
>>> @@ -1609,7 +1680,7 @@ bool is_valid_cgroup(const char *name)
>>>       return strcmp(name, ".") != 0 && strcmp(name, "..") != 0;
>>>  }
>>>
>>> -int create_or_remove_cgroup(bool do_remove, struct cgroup_mount_point *mp, const char *path)
>>> +int create_or_remove_cgroup(bool do_remove, struct cgroup_mount_point *mp, const char *path, int recurse)
>>>  {
>>>       int r, saved_errno = 0;
>>>       char *buf = cgroup_to_absolute_path(mp, path, NULL);
>>> @@ -1617,9 +1688,13 @@ int create_or_remove_cgroup(bool do_remove, struct cgroup_mount_point *mp, const
>>>               return -1;
>>>
>>>       /* create or remove directory */
>>> -     r = do_remove ?
>>> -             rmdir(buf) :
>>> -             mkdir(buf, 0777);
>>> +     if (do_remove) {
>>> +             if (recurse)
>>> +                     r = cgroup_rmdir(buf);
>>> +             else
>>> +                     r = rmdir(buf);
>>> +     } else
>>> +             r = mkdir(buf, 0777);
>>>       saved_errno = errno;
>>>       free(buf);
>>>       errno = saved_errno;
>>> @@ -1628,12 +1703,12 @@ int create_or_remove_cgroup(bool do_remove, struct cgroup_mount_point *mp, const
>>>
>>>  int create_cgroup(struct cgroup_mount_point *mp, const char *path)
>>>  {
>>> -     return create_or_remove_cgroup(false, mp, path);
>>> +     return create_or_remove_cgroup(false, mp, path, false);
>>>  }
>>>
>>> -int remove_cgroup(struct cgroup_mount_point *mp, const char *path)
>>> +int remove_cgroup(struct cgroup_mount_point *mp, const char *path, bool recurse)
>>>  {
>>> -     return create_or_remove_cgroup(true, mp, path);
>>> +     return create_or_remove_cgroup(true, mp, path, recurse);
>>>  }
>>>
>>>  char *cgroup_to_absolute_path(struct cgroup_mount_point *mp, const char *path, const char *suffix)
>>> --
>>> 1.8.5.2
>>>
>>> _______________________________________________
>>> lxc-devel mailing list
>>> lxc-devel at lists.linuxcontainers.org
>>> http://lists.linuxcontainers.org/listinfo/lxc-devel
>>
>> --
>> Stéphane Graber
>> Ubuntu developer
>> http://www.ubuntu.com
>>
>> _______________________________________________
>> lxc-devel mailing list
>> lxc-devel at lists.linuxcontainers.org
>> http://lists.linuxcontainers.org/listinfo/lxc-devel
>>
>
>
>
> --
> S.Çağlar Onur <caglar at 10ur.org>



-- 
S.Çağlar Onur <caglar at 10ur.org>


More information about the lxc-devel mailing list