[lxc-devel] [PATCH 1/1] cgroup: recursively delete cgroups when asked
Michael H. Warfield
mhw at WittsEnd.com
Mon Jan 13 19:53:40 UTC 2014
On Mon, 2014-01-13 at 12:38 -0500, Dwight Engen wrote:
> On Mon, 13 Jan 2014 12:19:25 -0500
> S.Çağlar Onur <caglar at 10ur.org> wrote:
> > 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));
> Hi Çağlar, sorry for messing that one up. It looks like autodev is
> initialized to -1 which must be why the if check is catching. Other
> places in conf.c are comparing for > 0 so maybe we should use that
> instead of == 1?
That's some of my stuff from months ago. That's to enable autodetection
of certain conditions where we need to set the autodev default to be
enabled. Serge, Stéphane, and I had been discussing how to reasonable
detect when we should enable autodev by default and that settled in on
whether systemd was in the readlink dereference for init. But we (I)
also wanted an explicit enable / disable which resulted in the autodev
option being a trinary of -1/0/1.
Regards,
Mike
> > 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>
> >
> >
> >
>
> _______________________________________________
> lxc-devel mailing list
> lxc-devel at lists.linuxcontainers.org
> http://lists.linuxcontainers.org/listinfo/lxc-devel
>
--
Michael H. Warfield (AI4NB) | (770) 978-7061 | mhw at WittsEnd.com
/\/\|=mhw=|\/\/ | (678) 463-0932 | http://www.wittsend.com/mhw/
NIC whois: MHW9 | An optimist believes we live in the best of all
PGP Key: 0x674627FF | possible worlds. A pessimist is sure of it!
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 465 bytes
Desc: This is a digitally signed message part
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20140113/2d039f61/attachment.pgp>
More information about the lxc-devel
mailing list