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

S.Çağlar Onur caglar at 10ur.org
Mon Jan 13 17:19:25 UTC 2014


OK I think we need following

diff --git a/src/lxc/confile.c b/src/lxc/confile.c
index 50d09da..4208b0e 100644
--- a/src/lxc/confile.c
+++ b/src/lxc/confile.c
@@ -2209,7 +2209,7 @@ void write_config(FILE *fout, struct lxc_conf *c)
                fprintf(fout, "lxc.seccomp = %s\n", c->seccomp);
        if (c->kmsg == 0)
                fprintf(fout, "lxc.kmsg = 0\n");
-       if (c->autodev)
+       if (c->autodev == 1)
                fprintf(fout, "lxc.autodev = 1\n");
        if (c->loglevel != LXC_LOG_PRIORITY_NOTSET)
                fprintf(fout, "lxc.loglevel = %s\n",
lxc_log_priority_to_string(c->loglevel));

On Mon, Jan 13, 2014 at 12:09 PM, S.Çağlar Onur <caglar at 10ur.org> wrote:
> On Mon, Jan 13, 2014 at 12:05 PM, S.Çağlar Onur <caglar at 10ur.org> wrote:
>> Hi Serge,
>>
>> On Mon, Jan 13, 2014 at 11:24 AM, Serge Hallyn <serge.hallyn at ubuntu.com> wrote:
>>> Quoting S.Çağlar Onur (caglar at 10ur.org):
>>>> 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?
>>>
>>> Are you sure it was that patch?  You can drop my patch and your
>>> test succeeds again?
>>
>> Hmm you are right, after a git bisect it pointed following
>
> [hit send too early]
>
> and yes reverting that commit fixes the issue
>
> [caglar at qp:~] for i in seq 100; do sudo lxc-test-device-add-remove; done
> lxc_container: rmdir failed
> Removing /dev/network_latency from the container
> (device_add_remove_test) failed...
> lxc_container: Container /var/lib/lxc:device_add_remove_test already exists
> Creating the container (device_add_remove_test) failed...
> [caglar at qp:~] sudo lxc-stop -n device_add_remove_test
> [caglar at qp:~] sudo lxc-destroy -n device_add_remove_test
>
> [revert/install etc.]
>
> [caglar at qp:~] for i in seq 100; do sudo lxc-test-device-add-remove; done
> [caglar at qp:~]
>
>
>> df2d4205073d3f57543951ca7ffabf891b230634 is the first bad commit
>> commit df2d4205073d3f57543951ca7ffabf891b230634
>> Author: Dwight Engen <dwight.engen at oracle.com>
>> Date:   Thu Jan 9 15:36:13 2014 -0500
>>
>>     ensure all config items are duplicated on clone/write_config
>>
>>     Since previously I had found a config item that wasn't being propagated
>>     by lxc-clone, I went through all the config items and made sure that:
>>      a) Each item is documented in lxc.conf
>>      b) Each item is written out by write_config
>>
>>     The only one that isn't is lxc.include, which by its nature only pulls
>>     in other config item types.
>>
>>     Signed-off-by: Dwight Engen <dwight.engen at oracle.com>
>>     Acked-by: Serge E. Hallyn <serge.hallyn at ubuntu.com>
>>
>>>> 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>
>>>> _______________________________________________
>>>> lxc-devel mailing list
>>>> lxc-devel at lists.linuxcontainers.org
>>>> http://lists.linuxcontainers.org/listinfo/lxc-devel
>>> _______________________________________________
>>> 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>



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


More information about the lxc-devel mailing list