Quoting S.Çağlar Onur (cag...@10ur.org):
> Hey there,
>
> On Sun, Jan 12, 2014 at 7:24 PM, Stéphane Graber <stgra...@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.hal...@ubuntu.com>
> >
> > Acked-by: Stéphane Graber <stgra...@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?
> 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@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@lists.linuxcontainers.org
> > http://lists.linuxcontainers.org/listinfo/lxc-devel
> >
>
>
>
> --
> S.Çağlar Onur <cag...@10ur.org>
> _______________________________________________
> lxc-devel mailing list
> lxc-devel@lists.linuxcontainers.org
> http://lists.linuxcontainers.org/listinfo/lxc-devel
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel
