On Wed, 29 May 2013 00:01:16 -0500
Serge Hallyn <serge.hal...@ubuntu.com> wrote:
>
> Those go through commands.c and are already mutex'ed that way.
>
> Also remove a unmatched container_disk_unlock in lxcapi_create.
>
> Since is_stopped uses getstate which is no longer locked, rename
> it to drop the _locked suffix.
>
> And convert save_config to taking the disk lock. This way the
> save_ and load_config are mutexing each other, as they should.
>
> Currently (after this patch) disk_lock is only taken around
> save_config and load_config. I think we will want to tighten
> some of this back down, but really we can't lock out external
> editors acting on config files anyway - so it's more important
> (after ensuring we don't have deadlocks) to make sure the API is
> thread-safe, which means taking the process_lock() around open/close
> etc. So I'll focus on that in the next bit. Then work on a
> fn by fn locking rationale.
>
> Signed-off-by: Serge Hallyn <serge.hal...@ubuntu.com>
> ---
> src/lxc/lxccontainer.c | 29 ++++++++---------------------
> 1 file changed, 8 insertions(+), 21 deletions(-)
>
> diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
> index b34b8e8..9eb431c 100644
> --- a/src/lxc/lxccontainer.c
> +++ b/src/lxc/lxccontainer.c
> @@ -287,21 +287,15 @@ out:
>
> static const char *lxcapi_state(struct lxc_container *c)
> {
> - const char *ret;
> lxc_state_t s;
>
> if (!c)
> return NULL;
> - if (container_disk_lock(c))
> - return NULL;
> s = lxc_getstate(c->name, c->config_path);
> - ret = lxc_state2str(s);
> - container_disk_unlock(c);
> -
> - return ret;
> + return lxc_state2str(s);
> }
>
> -static bool is_stopped_locked(struct lxc_container *c)
> +static bool is_stopped(struct lxc_container *c)
> {
> lxc_state_t s;
> s = lxc_getstate(c->name, c->config_path);
> @@ -326,10 +320,7 @@ static bool lxcapi_freeze(struct lxc_container
> *c) if (!c)
> return false;
>
> - if (container_disk_lock(c))
> - return false;
> ret = lxc_freeze(c->name, c->config_path);
> - container_disk_unlock(c);
> if (ret)
> return false;
> return true;
> @@ -341,10 +332,7 @@ static bool lxcapi_unfreeze(struct lxc_container
> *c) if (!c)
> return false;
>
> - if (container_disk_lock(c))
> - return false;
> ret = lxc_unfreeze(c->name, c->config_path);
> - container_disk_unlock(c);
> if (ret)
> return false;
> return true;
> @@ -888,7 +876,6 @@ static bool lxcapi_create(struct lxc_container
> *c, const char *t, out_unlock:
> if (partial_fd >= 0)
> remove_partial(c, partial_fd);
> - container_disk_unlock(c);
> out:
> if (tpath)
> free(tpath);
> @@ -1159,13 +1146,13 @@ static bool lxcapi_save_config(struct
> lxc_container *c, const char *alt_file) FILE *fout = fopen(alt_file,
> "w"); if (!fout)
> return false;
> - if (container_mem_lock(c)) {
> + if (container_disk_lock(c)) {
> fclose(fout);
> return false;
> }
This isn't new with your change, but shouldn't we try to get the
disk_lock before doing the fopen() since fopen() is going to truncate
the file and if the getting the lock fails now we have an empty config
file?
> write_config(fout, c->lxc_conf);
> fclose(fout);
> - container_mem_unlock(c);
> + container_disk_unlock(c);
> return true;
> }
>
> @@ -1185,7 +1172,7 @@ static bool lxcapi_destroy(struct lxc_container
> *c) return false;
> }
>
> - if (!is_stopped_locked(c)) {
> + if (!is_stopped(c)) {
> // we should queue some sort of error - in
> c->error_string? ERROR("container %s is not stopped", c->name);
> goto out;
> @@ -1342,7 +1329,7 @@ static bool lxcapi_set_cgroup_item(struct
> lxc_container *c, const char *subsys, if (container_mem_lock(c))
> return false;
>
> - if (is_stopped_locked(c))
> + if (is_stopped(c))
> goto err;
>
> ret = lxc_cgroup_set(c->name, subsys, value, c->config_path);
> @@ -1363,7 +1350,7 @@ static int lxcapi_get_cgroup_item(struct
> lxc_container *c, const char *subsys, c if (container_mem_lock(c))
> return -1;
>
> - if (is_stopped_locked(c))
> + if (is_stopped(c))
> goto out;
>
> ret = lxc_cgroup_get(c->name, subsys, retv, inlen,
> c->config_path); @@ -1827,7 +1814,7 @@ struct lxc_container
> *lxcapi_clone(struct lxc_container *c, const char *newname, if
> (container_mem_lock(c)) return NULL;
>
> - if (!is_stopped_locked(c)) {
> + if (!is_stopped(c)) {
> ERROR("error: Original container (%s) is running",
> c->name); goto out;
> }
------------------------------------------------------------------------------
Introducing AppDynamics Lite, a free troubleshooting tool for Java/.NET
Get 100% visibility into your production application - at no cost.
Code-level diagnostics for performance bottlenecks with <2% overhead
Download for free and get started troubleshooting in minutes.
http://p.sf.net/sfu/appdyn_d2d_ap1
_______________________________________________
Lxc-devel mailing list
Lxc-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/lxc-devel
