Update the LOCKING comment. Take mem_lock in want_daemonize.
convert lxcapi_destroy to not use privlock/slock by hand. Fix a coverity-found potential dereference of NULL c->lxc_conf. api_cgroup_get_item() and api_cgroup_set_item(): use disklock, not memlock, since the values are set through the cgroup fs on the running container. Signed-off-by: Serge Hallyn <serge.hal...@ubuntu.com> --- src/lxc/lxccontainer.c | 56 +++++++++++++++++++++++--------------------------- 1 file changed, 26 insertions(+), 30 deletions(-) diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c index 9bc1caf..24b6008 100644 --- a/src/lxc/lxccontainer.c +++ b/src/lxc/lxccontainer.c @@ -145,11 +145,11 @@ void remove_partial(struct lxc_container *c, int fd) } /* LOCKING - * 1. c->privlock protects the struct lxc_container from multiple threads. - * 2. c->slock protects the on-disk container data - * 3. thread_mutex protects process data (ex: fd table) from multiple threads - * slock is an flock, which does not exclude threads. Therefore slock should - * always be wrapped inside privlock. + * 1. container_mem_lock(c) protects the struct lxc_container from multiple threads. + * 2. container_disk_lock(c) protects the on-disk container data - in particular the + * container configuration file. + * The container_disk_lock also takes the container_mem_lock. + * 3. thread_mutex protects process data (ex: fd table) from multiple threads. * NOTHING mutexes two independent programs with their own struct * lxc_container for the same c->name, between API calls. For instance, * c->config_read(); c->start(); Between those calls, data on disk @@ -160,7 +160,7 @@ void remove_partial(struct lxc_container *c, int fd) * due to hung callers. So I prefer to keep the locks only within our own * functions, not across functions. * - * If you're going to fork while holding a lxccontainer, increment + * If you're going to clone while holding a lxccontainer, increment * c->numthreads (under privlock) before forking. When deleting, * decrement numthreads under privlock, then if it hits 0 you can delete. * Do not ever use a lxccontainer whose numthreads you did not bump. @@ -406,7 +406,12 @@ static void lxcapi_want_daemonize(struct lxc_container *c) { if (!c) return; + if (!container_mem_lock(c)) { + ERROR("Error getting mem lock"); + return; + } c->daemonize = 1; + container_mem_unlock(c); } static bool lxcapi_wait(struct lxc_container *c, const char *state, int timeout) @@ -1218,12 +1223,8 @@ static bool lxcapi_destroy(struct lxc_container *c) if (!c || !lxcapi_is_defined(c)) return false; - if (lxclock(c->privlock, 0)) - return false; - if (lxclock(c->slock, 0)) { - lxcunlock(c->privlock); + if (container_disk_lock(c)) return false; - } if (!is_stopped(c)) { // we should queue some sort of error - in c->error_string? @@ -1231,7 +1232,7 @@ static bool lxcapi_destroy(struct lxc_container *c) goto out; } - if (c->lxc_conf->rootfs.path && c->lxc_conf->rootfs.mount) + if (c->lxc_conf && c->lxc_conf->rootfs.path && c->lxc_conf->rootfs.mount) r = bdev_init(c->lxc_conf->rootfs.path, c->lxc_conf->rootfs.mount, NULL); if (r) { if (r->ops->destroy(r) < 0) { @@ -1250,8 +1251,7 @@ static bool lxcapi_destroy(struct lxc_container *c) ret = true; out: - lxcunlock(c->privlock); - lxcunlock(c->slock); + container_disk_unlock(c); return ret; } @@ -1374,42 +1374,38 @@ err: static bool lxcapi_set_cgroup_item(struct lxc_container *c, const char *subsys, const char *value) { int ret; - bool b = false; if (!c) return false; - if (container_mem_lock(c)) + if (is_stopped(c)) return false; - if (is_stopped(c)) - goto err; + if (container_disk_lock(c)) + return false; - ret = lxc_cgroup_set(c->name, subsys, value, c->config_path); - if (!ret) - b = true; -err: - container_mem_unlock(c); - return b; + ret = lxc_cgroup_set(c->name, subsys, value, c->config_path) == 0; + + container_disk_unlock(c); + return ret == 0; } static int lxcapi_get_cgroup_item(struct lxc_container *c, const char *subsys, char *retv, int inlen) { - int ret = -1; + int ret; if (!c || !c->lxc_conf) return -1; - if (container_mem_lock(c)) + if (is_stopped(c)) return -1; - if (is_stopped(c)) - goto out; + if (container_disk_lock(c)) + return -1; ret = lxc_cgroup_get(c->name, subsys, retv, inlen, c->config_path); -out: - container_mem_unlock(c); + container_disk_unlock(c); return ret; } -- 1.8.1.2 ------------------------------------------------------------------------------ Get 100% visibility into Java/.NET code with AppDynamics Lite It's a free troubleshooting tool designed for production Get down to code-level detail for bottlenecks, with <2% overhead. Download for free and get started troubleshooting in minutes. http://p.sf.net/sfu/appdyn_d2d_ap2 _______________________________________________ Lxc-devel mailing list Lxc-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/lxc-devel