Hi Dwight, On Mon, Jan 13, 2014 at 12:38 PM, Dwight Engen <dwight.en...@oracle.com> wrote: > On Mon, 13 Jan 2014 12:19:25 -0500 > S.Çağlar Onur <cag...@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?
No problem at all and > 0 sounds good to me. Thanks for taking care of it! >> On Mon, Jan 13, 2014 at 12:09 PM, S.Çağlar Onur <cag...@10ur.org> >> wrote: >> > On Mon, Jan 13, 2014 at 12:05 PM, S.Çağlar Onur <cag...@10ur.org> >> > wrote: >> >> Hi Serge, >> >> >> >> On Mon, Jan 13, 2014 at 11:24 AM, Serge Hallyn >> >> <serge.hal...@ubuntu.com> wrote: >> >>> 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? >> >> >> >> Hmm you are right, after a git bisect it pointed following >> > >> > [hit send too early] >> > >> > and yes reverting that commit fixes the issue >> > >> > [caglar@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@qp:~] sudo lxc-stop -n device_add_remove_test >> > [caglar@qp:~] sudo lxc-destroy -n device_add_remove_test >> > >> > [revert/install etc.] >> > >> > [caglar@qp:~] for i in seq 100; do sudo lxc-test-device-add-remove; >> > done [caglar@qp:~] >> > >> > >> >> df2d4205073d3f57543951ca7ffabf891b230634 is the first bad commit >> >> commit df2d4205073d3f57543951ca7ffabf891b230634 >> >> Author: Dwight Engen <dwight.en...@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.en...@oracle.com> >> >> Acked-by: Serge E. Hallyn <serge.hal...@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@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 >> >> >> >> >> >> >> >> -- >> >> S.Çağlar Onur <cag...@10ur.org> >> > >> > >> > >> > -- >> > S.Çağlar Onur <cag...@10ur.org> >> >> >> > -- S.Çağlar Onur <cag...@10ur.org> _______________________________________________ lxc-devel mailing list lxc-devel@lists.linuxcontainers.org http://lists.linuxcontainers.org/listinfo/lxc-devel