[lxc-devel] Network interface - netns migration bug
Hello,
I've encountered this bug with lxc-0.9.0, using the lxc-phys scenario -
assigning a physical interface to a container.
In my understanding, this is what happens when a container starts (well, the
relevant parts):
- the configuration file is parsed into a config structure - the kernel
interface index is found using if_nametoindex(). This ifindex will be further
used to move the interface from the host netns to the container's.
- the child process is cloned in a new netns;
- the parent issues a RTM_NEWLINK message to create a new interface in the
netns associated with the pid of the child, having the known ifindex;
- it is implied that the interface will be removed from the host namespace;
- the child process will further use the same ifindex in order to rename the
interface as needed; it is assumed that the interface will keep the same
ifindex in the container network namespaces.
Here's my scenario - I'm using a small binary [1] that lists all the known
interfaces and their respective ifindexes.
- the following interfaces are available on host:
Interface 1 : lo
Interface 2 : fm1-mac5
Interface 3 : fm2-mac5
Interface 4 : eth2
Interface 5 : tunl0
Interface 6 : sit0
- the following virtual interfaces are created when lxc-unsharing a process in
a new netns:
Interface 1 : lo
Interface 2 : tunl0
Interface 3 : sit0
At this point, I want to create a container that has fm1-mac5 physically
assigned. lxc-start fails with this message:
lxc-start: failed to rename tunl0->fm1-mac5 : File exists
lxc-start: failed to setup netdev
lxc-start: failed to setup the network for 'foo'
lxc-start: failed to setup the container
lxc-start: invalid sequence number 1. expected 2
lxc-start: failed to spawn 'foo'
By adding a few printf()s in the code, I was able to find the following facts:
- fm1-mac5 will have "2" as associated ifindex
- the new child process will be cloned with a new netns, with interfaces lo,
tunl0 and sit0
- when issuing the RTM_NEWLINK netlink message, ifindex 2 (fm2-mac5) will be
moved to the new netns, _however_ it will receive ifindex 4, since all previous
ones are occupied
- when trying to rename the interface represented by ifindex 2 (tunl0) to
"fm1-mac5", it will fail, because the interface is already there and it has
ifindex 4
After container start fails, here are the interfaces on the host again:
Interface 1 : lo
Interface 3 : fm2-mac5
Interface 4 : eth2
Interface 5 : tunl0
Interface 6 : sit0
Interface 7 : fm1-mac5
fm1-mac5 now has ifindex 7 - when moving fm1-mac5 (having ifindex 4 in the
container netns) back to the host netns, it will receive a new ifindex, since 4
is occupied (eth2).
Running lxc-start again will succeed, and the following interfaces will be
present in the container:
Interface 1 : lo
Interface 2 : tunl0
Interface 3 : sit0
Interface 7 : fm1-mac5
There was no problem assigning ifindex 7 to fm1-mac5 in the new netns, since it
was used by no other interface.
I'm assuming that this doesn't manifest itself with virtual interfaces, since
they receive high ifindexes (after all other interfaces) at creation time, with
no possibility to clash with a new netns' default interfaces. This should only
appear in the LXC_NET_PHYS case.
In this case, one thing that can be done is to force re-reading the ifindex,
based on netdev->link:
---
src/lxc/conf.c | 8
1 file changed, 8 insertions(+)
diff --git a/src/lxc/conf.c b/src/lxc/conf.c
index 6b3f318..ff6b30b 100644
--- a/src/lxc/conf.c
+++ b/src/lxc/conf.c
@@ -1846,6 +1846,14 @@ static int setup_netdev(struct lxc_netdev *netdev)
return 0;
}
+ /* if LXC_NET_PHYS, update the ifindex */
+ if (netdev->type == LXC_NET_PHYS)
+ if (!(netdev->ifindex = if_nametoindex(netdev->link))) {
+ ERROR("unable to retrieve index for interface %s",
+ netdev->link);
+ return -1;
+ }
+
/* retrieve the name of the interface */
if (!if_indextoname(netdev->ifindex, current_ifname)) {
ERROR("no interface corresponding to index '%d'",
--
1.7.11.7
However, this does not solve a main issue: assuming that the ifindex will
remain the same, in all situations, after an interface has been moved to a new
netns. It _should_ not appear with virtual interfaces, but that depends on the
ifindexes that are allocated to new interfaces.
How do you think this should be handled?
Thank you,
Bogdan P.
[1]
http://iijean.blogspot.com/2010/03/howto-get-list-of-network-interfaces-in.html
--
DreamFactory - Open Source REST & JSON Services for HTML5 & Native Apps
OAuth, Users, Roles, SQL, NoSQL, BLOB Storage and External API Access
Free app hosting. Or install the open source package on any LAMP server.
Sign up and see examples for AngularJS, jQuery, Sencha Touch and Native!
http://pubads.g.doubleclick.net/gampad/clk?id=63469471&iu=/4140/ostg.clktrk
_
Re: [lxc-devel] [PATCH] introduce lxcapi_add_device_node and lxcapi_remove_device_node to API
Quoting S.Çağlar Onur (cag...@10ur.org):
> Adding block/char devices to running container is a common operation so
> provide a common
> implementation for users to consume.
>
> Signed-off-by: S.Çağlar Onur
Acked-by: Serge E. Hallyn
> ---
> src/lxc/lxccontainer.c | 100
> +
> src/lxc/lxccontainer.h | 9 +
> 2 files changed, 109 insertions(+)
>
> diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
> index ede0113..58eebb0 100644
> --- a/src/lxc/lxccontainer.c
> +++ b/src/lxc/lxccontainer.c
> @@ -2912,6 +2912,104 @@ static bool lxcapi_may_control(struct lxc_container
> *c)
> return lxc_try_cmd(c->name, c->config_path) == 0;
> }
>
> +static bool lxcapi_add_device_node(struct lxc_container *c, char *path)
> +{
> + int ret, size = 32;
> + struct stat st;
> + char *value = alloca(size);
> + char dest_path[MAXPATHLEN];
> +
> + /* make sure container is running */
> + if (!c->is_running(c)) {
> + ERROR("container is not running");
> + return false;
> + }
> +
> + /* make sure that we can access given path */
> + if(access(path, F_OK) < 0 || lstat(path, &st) < 0)
> + return false;
> +
> + /* continue if path is character device or block device */
> + if S_ISCHR(st.st_mode)
> + ret = snprintf(value, size, "c %d:%d rwm", major(st.st_rdev),
> minor(st.st_rdev));
> + else if S_ISBLK(st.st_mode)
> + ret = snprintf(value, size, "b %d:%d rwm", major(st.st_rdev),
> minor(st.st_rdev));
> + else
> + return false;
> +
> + /* check snprintf return code */
> + if (ret < 0 || ret >= size)
> + return false;
> +
> + /* prepare dest_path */
> + ret = snprintf(dest_path, MAXPATHLEN, "/proc/%d/root%s",
> c->init_pid(c), path);
> + if (ret < 0 || ret >= MAXPATHLEN)
> + return false;
> +
> + /* remove dest_path if exists in the container */
> + if(access(dest_path, F_OK) == 0)
> + unlink(dest_path);
> +
> + /* create the device node */
> + if (mknod(dest_path, st.st_mode, st.st_rdev) < 0)
> + return false;
> +
> + /* add to device list */
> + if (!c->set_cgroup_item(c, "devices.allow", value)) {
> + ERROR("set_cgroup_item failed");
> + return false;
> + }
> +
> + return true;
> +}
> +
> +static bool lxcapi_remove_device_node(struct lxc_container *c, char *path)
> +{
> + int ret, size = 32;
> + struct stat st;
> + char *value = alloca(size);
> + char dest_path[MAXPATHLEN];
> +
> + /* make sure container is running */
> + if (!c->is_running(c)) {
> + ERROR("container is not running");
> + return false;
> + }
> +
> + /* prepare dest_path */
> + ret = snprintf(dest_path, MAXPATHLEN, "/proc/%d/root%s",
> c->init_pid(c), path);
> + if (ret < 0 || ret >= MAXPATHLEN)
> + return false;
> +
> + /* make sure that we can access dest_path */
> + if(access(dest_path, F_OK) < 0 || lstat(dest_path, &st) < 0)
> + return false;
> +
> + /* continue if path is character device or block device */
> + if S_ISCHR(st.st_mode)
> + ret = snprintf(value, size, "c %d:%d rwm", major(st.st_rdev),
> minor(st.st_rdev));
> + else if S_ISBLK(st.st_mode)
> + ret = snprintf(value, size, "b %d:%d rwm", major(st.st_rdev),
> minor(st.st_rdev));
> + else
> + return false;
> +
> + /* check snprintf return code */
> + if (ret < 0 || ret >= size)
> + return false;
> +
> + /* remove dest_path if exists in the container */
> + if(access(dest_path, F_OK) == 0)
> + unlink(dest_path);
> +
> + /* remove from device list */
> + if (!c->set_cgroup_item(c, "devices.deny", value)) {
> + ERROR("set_cgroup_item failed");
> + return false;
> + }
> +
> + return true;
> +}
> +
> static int lxcapi_attach_run_waitl(struct lxc_container *c,
> lxc_attach_options_t *options, const char *program, const char *arg, ...)
> {
> va_list ap;
> @@ -3033,6 +3131,8 @@ struct lxc_container *lxc_container_new(const char
> *name, const char *configpath
> c->snapshot_restore = lxcapi_snapshot_restore;
> c->snapshot_destroy = lxcapi_snapshot_destroy;
> c->may_control = lxcapi_may_control;
> + c->add_device_node = lxcapi_add_device_node;
> + c->remove_device_node = lxcapi_remove_device_node;
>
> /* we'll allow the caller to update these later */
> if (lxc_log_init(NULL, "none", NULL, "lxc_container", 0,
> c->config_path)) {
> diff --git a/src/lxc/lxccontainer.h b/src/lxc/lxccontainer.h
> index 486035a..a94de9a 100644
> --- a/src/lxc/lxccontainer.h
> +++ b/src/lxc/lxccontainer.h
> @@ -236,6 +236,15 @@ struct lxc_container {
>* and the caller may not access it. Return true otherwise.
>*/
> b
[lxc-devel] [lxc/lxc] ff4bac: introduce lxcapi_add_device_node and lxcapi_remove...
Branch: refs/heads/master Home: https://github.com/lxc/lxc Commit: ff4bacf71a3078cb2b926bfb248266236da9b1eb https://github.com/lxc/lxc/commit/ff4bacf71a3078cb2b926bfb248266236da9b1eb Author: S.Çağlar Onur Date: 2013-11-13 (Wed, 13 Nov 2013) Changed paths: M src/lxc/lxccontainer.c M src/lxc/lxccontainer.h Log Message: --- introduce lxcapi_add_device_node and lxcapi_remove_device_node to API Adding block/char devices to running container is a common operation so provide a common implementation for users to consume. Signed-off-by: S.Çağlar Onur Signed-off-by: Serge Hallyn -- DreamFactory - Open Source REST & JSON Services for HTML5 & Native Apps OAuth, Users, Roles, SQL, NoSQL, BLOB Storage and External API Access Free app hosting. Or install the open source package on any LAMP server. Sign up and see examples for AngularJS, jQuery, Sencha Touch and Native! http://pubads.g.doubleclick.net/gampad/clk?id=63469471&iu=/4140/ostg.clktrk___ Lxc-devel mailing list Lxc-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/lxc-devel
[lxc-devel] [lxc/lxc]
Branch: refs/heads/master Home: https://github.com/lxc/lxc -- DreamFactory - Open Source REST & JSON Services for HTML5 & Native Apps OAuth, Users, Roles, SQL, NoSQL, BLOB Storage and External API Access Free app hosting. Or install the open source package on any LAMP server. Sign up and see examples for AngularJS, jQuery, Sencha Touch and Native! http://pubads.g.doubleclick.net/gampad/clk?id=63469471&iu=/4140/ostg.clktrk___ Lxc-devel mailing list Lxc-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/lxc-devel
Re: [lxc-devel] [PATCH] introduce lxcapi_add_device_node and lxcapi_remove_device_node to API (v3)
On Wed, Nov 13, 2013 at 12:39:00AM -0500, S.Çağlar Onur wrote:
> Adding block/char devices to running container is a common operation so
> provide a common implementation for users to consume.
>
> changes since v2;
> * lets the user set an alternate path inside the container as Stéphane
> suggested
>
> changes since v1;
> * removed duplicated code
>
> Signed-off-by: S.Çağlar Onur
Hi,
So at first glance the reason why the remove function also take both src
and dest path wasn't very obvious though after thinking about it some
more, I guess it makes sense to always look for type/major/minor of the
source device, so passing only the dest path wouldn't work. And it's
possible that at some point we may want to do something again the dest
path (like removing it) so it doesn't hurt to have it passed too (though
at this point, passing it or not shouldn't make any difference).
Acked-by: Stéphane Graber
Note that Serge pushed v0 by accident but he then reverted it, so I'll
push v3 now.
> ---
> src/lxc/lxccontainer.c | 100
> +
> src/lxc/lxccontainer.h | 19 ++
> 2 files changed, 119 insertions(+)
>
> diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
> index 05ca643..2a70bc7 100644
> --- a/src/lxc/lxccontainer.c
> +++ b/src/lxc/lxccontainer.c
> @@ -49,6 +49,7 @@
> #include
> #include
> #include
> +#include
>
> #if HAVE_IFADDRS_H
> #include
> @@ -62,6 +63,8 @@
> #endif
> #endif
>
> +#define MAX_BUFFER 4096
> +
> lxc_log_define(lxc_container, lxc);
>
> static bool file_exists(char *f)
> @@ -2920,6 +2923,101 @@ static bool lxcapi_may_control(struct lxc_container
> *c)
> return lxc_try_cmd(c->name, c->config_path) == 0;
> }
>
> +static bool add_remove_device_node(struct lxc_container *c, char *src_path,
> char *dest_path, bool add)
> +{
> + int ret;
> + struct stat st;
> + char path[MAXPATHLEN];
> + char value[MAX_BUFFER];
> + char *directory_path = NULL, *p;
> +
> + /* make sure container is running */
> + if (!c->is_running(c)) {
> + ERROR("container is not running");
> + goto out;
> + }
> +
> + /* use src_path if dest_path is NULL otherwise use dest_path */
> + p = dest_path ? dest_path : src_path;
> +
> + /* prepare the path */
> + ret = snprintf(path, MAXPATHLEN, "/proc/%d/root/%s", c->init_pid(c), p);
> + if (ret < 0 || ret >= MAXPATHLEN)
> + goto out;
> + remove_trailing_slashes(path);
> +
> + p = add ? src_path : path;
> + /* make sure we can access p */
> + if(access(p, F_OK) < 0 || stat(p, &st) < 0)
> + goto out;
> +
> + /* continue if path is character device or block device */
> + if S_ISCHR(st.st_mode)
> + ret = snprintf(value, MAX_BUFFER, "c %d:%d rwm",
> major(st.st_rdev), minor(st.st_rdev));
> + else if S_ISBLK(st.st_mode)
> + ret = snprintf(value, MAX_BUFFER, "b %d:%d rwm",
> major(st.st_rdev), minor(st.st_rdev));
> + else
> + goto out;
> +
> + /* check snprintf return code */
> + if (ret < 0 || ret >= MAX_BUFFER)
> + goto out;
> +
> + directory_path = dirname(strdup(path));
> + /* remove path and directory_path (if empty) */
> + if(access(path, F_OK) == 0) {
> + if (unlink(path) < 0) {
> + ERROR("unlink failed");
> + goto out;
> + }
> + if (rmdir(directory_path) < 0 && errno != ENOTEMPTY) {
> + ERROR("rmdir failed");
> + goto out;
> + }
> + }
> +
> + if (add) {
> + /* create the missing directories */
> + if (mkdir_p(directory_path, 0755) < 0) {
> + ERROR("failed to create directory");
> + goto out;
> + }
> +
> + /* create the device node */
> + if (mknod(path, st.st_mode, st.st_rdev) < 0) {
> + ERROR("mknod failed");
> +goto out;
> + }
> +
> + /* add device node to device list */
> + if (!c->set_cgroup_item(c, "devices.allow", value)) {
> + ERROR("set_cgroup_item failed while adding the device
> node");
> + goto out;
> + }
> + } else {
> + /* remove device node from device list */
> + if (!c->set_cgroup_item(c, "devices.deny", value)) {
> + ERROR("set_cgroup_item failed while removing the device
> node");
> + goto out;
> + }
> + }
> + return true;
> +out:
> + if (directory_path)
> + free(directory_path);
> + return false;
> +}
> +
> +static bool lxcapi_add_device_node(struct lxc_container *c, char *src_path,
> char *dest_path)
> +{
> + return add_remove_device_node(c, src_path, dest_path, true);
> +}
> +
> +static bool lxcapi_remov
[lxc-devel] [lxc/lxc] a9a0ed: introduce lxcapi_add_device_node and lxcapi_remove...
Branch: refs/heads/master Home: https://github.com/lxc/lxc Commit: a9a0ed90dd1cdadd412576a45af16419efc0e939 https://github.com/lxc/lxc/commit/a9a0ed90dd1cdadd412576a45af16419efc0e939 Author: S.Çağlar Onur Date: 2013-11-13 (Wed, 13 Nov 2013) Changed paths: M src/lxc/lxccontainer.c M src/lxc/lxccontainer.h Log Message: --- introduce lxcapi_add_device_node and lxcapi_remove_device_node to API (v3) Adding block/char devices to running container is a common operation so provide a common implementation for users to consume. changes since v2; * lets the user set an alternate path inside the container as Stéphane suggested changes since v1; * removed duplicated code Signed-off-by: S.Çağlar Onur Acked-by: Stéphane Graber -- DreamFactory - Open Source REST & JSON Services for HTML5 & Native Apps OAuth, Users, Roles, SQL, NoSQL, BLOB Storage and External API Access Free app hosting. Or install the open source package on any LAMP server. Sign up and see examples for AngularJS, jQuery, Sencha Touch and Native! http://pubads.g.doubleclick.net/gampad/clk?id=63469471&iu=/4140/ostg.clktrk___ Lxc-devel mailing list Lxc-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/lxc-devel
[lxc-devel] Rethinking lxc-info a bit
Hello, We recently got reports of the recent changes to lxc-info breaking existing scripts. While discusing those issues, I noticed a few points that I think are worth discussing and addressing, I'm going to postpone alpha3 until that's done as the current state of things would break quite a bunch of scripts. == confusing -n behaviour == Since Dwight's last change, -n now accepts a regular expression, which I believe is the only case where it does. That seems fairly unintuitive and redundant with what lxc-list for example provides. This also brought on the next problem. == change of behaviour when one of the filter is passed == In the past, someone could do "lxc-info -n p1 -p" and trivially retrieve the PID. The new behaviour instead returns: Name: p1 Pid:19446 Even though I didn't ask for the container's name. "pid" was also renamed to "Pid", breaking anyone attempting to grep for the entry. == --state-is option is redundant == The state-is option always seemed a bit odd to me, in fact, it's absolutely identical to "lxc-wait -t 0 -n -s " and I don't really think it has its place in lxc-info. I'd suggest we just remove it entirely (yes, that'll break some scripts). I'm sorry I didn't think about those problems when reviewing the recent changes to lxc-info, but hopefully it's not too late to correct some of that. So my suggestion for lxc-info in LXC 1.0 are: - Only support one container and make -n mandatory, fail with an error if the container can't be found. - Drop --state-is entirely and tell anyone who used it to use lxc-wait instead. - Only print Name: if none of the filters are passed - Make the combination of -H + a single filter only return that value, so that "lxc-info -n p1 -P -H" will just return 19446 without any formatting. Recommend doing that to anyone parsing lxc-info's output. - Have -H also apply to the general formatting, simply printing : when passed. With those done, there will still be breakage for users of alpha2 upgrading to alpha3, but that should at least ensure no more surprises after that point and a more script friendly command. Thoughts? -- Stéphane Graber Ubuntu developer http://www.ubuntu.com signature.asc Description: Digital signature -- DreamFactory - Open Source REST & JSON Services for HTML5 & Native Apps OAuth, Users, Roles, SQL, NoSQL, BLOB Storage and External API Access Free app hosting. Or install the open source package on any LAMP server. Sign up and see examples for AngularJS, jQuery, Sencha Touch and Native! http://pubads.g.doubleclick.net/gampad/clk?id=63469471&iu=/4140/ostg.clktrk___ Lxc-devel mailing list Lxc-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/lxc-devel
Re: [lxc-devel] Rethinking lxc-info a bit
Quoting Stéphane Graber (stgra...@ubuntu.com): > Hello, > > We recently got reports of the recent changes to lxc-info breaking > existing scripts. In my own case, I have a host with several containers where a backup script uses 'lxc-info -n $container -p | awk ...' to get the init pid, then rsyncs from /proc/$pid/root/$path to /decrypted_backup/$name/$path. The fix was trivial (once diagnosed), but I don't know how many people have scripts built into their infrastructure depending on this. In the past, lxc-info -n www -p would have shown pid: $pid I always thought the 'pid:\t' was silly in that case. OTOH, getting rid of it now would, again, break existing scripts. > While discusing those issues, I noticed a few points that I think are > worth discussing and addressing, I'm going to postpone alpha3 until > that's done as the current state of things would break quite a bunch of > scripts. > > == confusing -n behaviour == > Since Dwight's last change, -n now accepts a regular expression, which I > believe is the only case where it does. That seems fairly unintuitive > and redundant with what lxc-list for example provides. Is there anything which lxc-list would not suffice for? > This also brought on the next problem. > > == change of behaviour when one of the filter is passed == > In the past, someone could do "lxc-info -n p1 -p" and trivially retrieve > the PID. > > The new behaviour instead returns: > Name: p1 > Pid:19446 > > Even though I didn't ask for the container's name. "pid" was also > renamed to "Pid", breaking anyone attempting to grep for the entry. > > == --state-is option is redundant == > The state-is option always seemed a bit odd to me, in fact, it's > absolutely identical to "lxc-wait -t 0 -n -s " and I don't > really think it has its place in lxc-info. I'd suggest we just remove it > entirely (yes, that'll break some scripts). > > > I'm sorry I didn't think about those problems when reviewing the recent > changes to lxc-info, but hopefully it's not too late to correct some of > that. > > > So my suggestion for lxc-info in LXC 1.0 are: > - Only support one container and make -n mandatory, fail with an error >if the container can't be found. > - Drop --state-is entirely and tell anyone who used it to use lxc-wait >instead. > - Only print Name: if none of the filters are passed > - Make the combination of -H + a single filter only return that value, >so that "lxc-info -n p1 -P -H" will just return 19446 without any >formatting. Recommend doing that to anyone parsing lxc-info's output. Sounds good to me. Perhaps we should have a transition guide for 1.0? Where would that belong? > - Have -H also apply to the general formatting, simply printing : > when passed. > > > With those done, there will still be breakage for users of alpha2 > upgrading to alpha3, but that should at least ensure no more surprises > after that point and a more script friendly command. > > > Thoughts? > > -- > Stéphane Graber > Ubuntu developer > http://www.ubuntu.com > -- > DreamFactory - Open Source REST & JSON Services for HTML5 & Native Apps > OAuth, Users, Roles, SQL, NoSQL, BLOB Storage and External API Access > Free app hosting. Or install the open source package on any LAMP server. > Sign up and see examples for AngularJS, jQuery, Sencha Touch and Native! > http://pubads.g.doubleclick.net/gampad/clk?id=63469471&iu=/4140/ostg.clktrk > ___ > Lxc-devel mailing list > Lxc-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/lxc-devel -- DreamFactory - Open Source REST & JSON Services for HTML5 & Native Apps OAuth, Users, Roles, SQL, NoSQL, BLOB Storage and External API Access Free app hosting. Or install the open source package on any LAMP server. Sign up and see examples for AngularJS, jQuery, Sencha Touch and Native! http://pubads.g.doubleclick.net/gampad/clk?id=63469471&iu=/4140/ostg.clktrk ___ Lxc-devel mailing list Lxc-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/lxc-devel
[lxc-devel] [PATCH] gather all locking related code into src/lxc/lxclock.c
Signed-off-by: S.Çağlar Onur
---
src/lxc/lxclock.c | 74 +--
src/lxc/lxclock.h | 3 +++
src/lxc/utils.c | 57 +-
3 files changed, 65 insertions(+), 69 deletions(-)
diff --git a/src/lxc/lxclock.c b/src/lxc/lxclock.c
index 3857ff0..64823d2 100644
--- a/src/lxc/lxclock.c
+++ b/src/lxc/lxclock.c
@@ -31,6 +31,10 @@
#include
#include
+#ifdef MUTEX_DEBUGGING
+#include
+#endif
+
#define OFLAG (O_CREAT | O_RDWR)
#define SEMMODE 0660
#define SEMVALUE 1
@@ -40,10 +44,55 @@ lxc_log_define(lxc_lock, lxc);
#ifdef MUTEX_DEBUGGING
pthread_mutex_t thread_mutex = PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP;
+pthread_mutex_t static_mutex = PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP;
+
+inline void dump_stacktrace(void)
+{
+ void *array[MAX_STACKDEPTH];
+ size_t size;
+ char **strings;
+ size_t i;
+
+ size = backtrace(array, MAX_STACKDEPTH);
+ strings = backtrace_symbols(array, size);
+
+ // Using fprintf here as our logging module is not thread safe
+ fprintf(stderr, "\tObtained %zd stack frames.\n", size);
+
+ for (i = 0; i < size; i++)
+ fprintf(stderr, "\t\t%s\n", strings[i]);
+
+ free (strings);
+}
#else
pthread_mutex_t thread_mutex = PTHREAD_MUTEX_INITIALIZER;
+pthread_mutex_t static_mutex = PTHREAD_MUTEX_INITIALIZER;
+
+inline void dump_stacktrace(void) {;}
#endif
+void lock_mutex(pthread_mutex_t *l)
+{
+ int ret;
+
+ if ((ret = pthread_mutex_lock(l)) != 0) {
+ fprintf(stderr, "pthread_mutex_lock returned:%d %s", ret,
strerror(ret));
+ dump_stacktrace();
+ exit(1);
+ }
+}
+
+void unlock_mutex(pthread_mutex_t *l)
+{
+ int ret;
+
+ if ((ret = pthread_mutex_unlock(l)) != 0) {
+ fprintf(stderr, "pthread_mutex_lock returned:%d %s", ret,
strerror(ret));
+ dump_stacktrace();
+ exit(1);
+ }
+}
+
static char *lxclock_name(const char *p, const char *n)
{
int ret;
@@ -267,24 +316,23 @@ void lxc_putlock(struct lxc_lock *l)
void process_lock(void)
{
- int ret;
-
- if ((ret = pthread_mutex_lock(&thread_mutex)) != 0) {
- ERROR("pthread_mutex_lock returned:%d %s", ret, strerror(ret));
- dump_stacktrace();
- exit(1);
- }
+ lock_mutex(&thread_mutex);
}
void process_unlock(void)
{
- int ret;
+ unlock_mutex(&thread_mutex);
+}
- if ((ret = pthread_mutex_unlock(&thread_mutex)) != 0) {
- ERROR("pthread_mutex_unlock returned:%d %s", ret,
strerror(ret));
- dump_stacktrace();
- exit(1);
- }
+/* Protects static const values inside the lxc_global_config_value funtion */
+void static_lock(void)
+{
+ lock_mutex(&static_mutex);
+}
+
+void static_unlock(void)
+{
+ unlock_mutex(&static_mutex);
}
int container_mem_lock(struct lxc_container *c)
diff --git a/src/lxc/lxclock.h b/src/lxc/lxclock.h
index dcdf79d..12ba827 100644
--- a/src/lxc/lxclock.h
+++ b/src/lxc/lxclock.h
@@ -87,6 +87,9 @@ extern void lxc_putlock(struct lxc_lock *l);
extern void process_lock(void);
extern void process_unlock(void);
+extern void static_lock(void);
+extern void static_unlock(void);
+
struct lxc_container;
extern int container_mem_lock(struct lxc_container *c);
extern void container_mem_unlock(struct lxc_container *c);
diff --git a/src/lxc/utils.c b/src/lxc/utils.c
index 4bc2c35..3fab9ae 100644
--- a/src/lxc/utils.c
+++ b/src/lxc/utils.c
@@ -39,11 +39,6 @@
#include
#include
#include
-#include
-
-#ifdef MUTEX_DEBUGGING
-#include
-#endif
#ifndef HAVE_GETLINE
#ifdef HAVE_FGETLN
@@ -59,57 +54,6 @@
lxc_log_define(lxc_utils, lxc);
-
-#ifdef MUTEX_DEBUGGING
-static pthread_mutex_t static_mutex = PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP;
-
-inline void dump_stacktrace(void)
-{
- void *array[MAX_STACKDEPTH];
- size_t size;
- char **strings;
- size_t i;
-
- size = backtrace(array, MAX_STACKDEPTH);
- strings = backtrace_symbols(array, size);
-
- // Using fprintf here as our logging module is not thread safe
- fprintf(stderr, "\tObtained %zd stack frames.\n", size);
-
- for (i = 0; i < size; i++)
- fprintf(stderr, "\t\t%s\n", strings[i]);
-
- free (strings);
-}
-#else
-static pthread_mutex_t static_mutex = PTHREAD_MUTEX_INITIALIZER;
-
-inline void dump_stacktrace(void) {;}
-#endif
-
-/* Protects static const values inside the lxc_global_config_value funtion */
-static void static_lock(void)
-{
- int ret;
-
- if ((ret = pthread_mutex_lock(&static_mutex)) != 0) {
- ERROR("pthread_mutex_lock returned:%d %s", ret, strerror(ret));
- dump_stacktrace();
- exit(1);
- }
-}
-
-static void static_unlock(void)
-{
- int ret;
-
- if ((ret = pthread_mutex_unlock(&stat
Re: [lxc-devel] Rethinking lxc-info a bit
On Wed, 13 Nov 2013 14:41:55 -0500 Stéphane Graber wrote: > Hello, > > We recently got reports of the recent changes to lxc-info breaking > existing scripts. > > While discusing those issues, I noticed a few points that I think are > worth discussing and addressing, I'm going to postpone alpha3 until > that's done as the current state of things would break quite a bunch > of scripts. > > == confusing -n behaviour == > Since Dwight's last change, -n now accepts a regular expression, > which I believe is the only case where it does. That seems fairly > unintuitive and redundant with what lxc-list for example provides. > This also brought on the next problem. I did this because I wanted to be able to get info on all/some containers and the previous precedent for that: lxc-wait, even before I messed with it ;) used regex matching. I think Serge mentioned that he didn't like regex's there, and would prefer glob fnmatch style matching. I'm not particularly tied to lxc-info being able to handle multiples though, so I'm fine if we want to rip that out :) Also I don't think lxc-ls has a way to list all containers, (ie. both defined and active) but it would be easy to have it do that now. > == change of behaviour when one of the filter is passed == > In the past, someone could do "lxc-info -n p1 -p" and trivially > retrieve the PID. > > The new behaviour instead returns: > Name: p1 > Pid:19446 > > Even though I didn't ask for the container's name. "pid" was also > renamed to "Pid", breaking anyone attempting to grep for the entry. I agree name shouldn't be printed in that case, or any time a specific thing is asked for. I actually looked for trouble the name changing might cause, and the only potential case I saw in the shipped lxc stuff was lxc-test-ubuntu, but it was okay because it called lxc-info -i and awked out the second column, so renaming "IP" didn't break it. You are correct though that external scripts might've broken though. If we're going to say that lxc-info's output is "stable" though so folks can grep out which parts they want, then maybe we don't need options like -i and -p ? > == --state-is option is redundant == > The state-is option always seemed a bit odd to me, in fact, it's > absolutely identical to "lxc-wait -t 0 -n -s " and I > don't really think it has its place in lxc-info. I'd suggest we just > remove it entirely (yes, that'll break some scripts). Agree, especially considering it doesn't even seem to work right now. $? seems to always be 0 for me. > I'm sorry I didn't think about those problems when reviewing the > recent changes to lxc-info, but hopefully it's not too late to > correct some of that. Sorry for introducing these problems :( > > So my suggestion for lxc-info in LXC 1.0 are: > - Only support one container and make -n mandatory, fail with an > error if the container can't be found. > - Drop --state-is entirely and tell anyone who used it to use > lxc-wait instead. > - Only print Name: if none of the filters are passed > - Make the combination of -H + a single filter only return that > value, so that "lxc-info -n p1 -P -H" will just return 19446 without > any formatting. Recommend doing that to anyone parsing lxc-info's > output. > - Have -H also apply to the general formatting, simply printing > : when passed. That all sounds fine to me. > With those done, there will still be breakage for users of alpha2 > upgrading to alpha3, but that should at least ensure no more surprises > after that point and a more script friendly command. > > > Thoughts? > -- DreamFactory - Open Source REST & JSON Services for HTML5 & Native Apps OAuth, Users, Roles, SQL, NoSQL, BLOB Storage and External API Access Free app hosting. Or install the open source package on any LAMP server. Sign up and see examples for AngularJS, jQuery, Sencha Touch and Native! http://pubads.g.doubleclick.net/gampad/clk?id=63469471&iu=/4140/ostg.clktrk ___ Lxc-devel mailing list Lxc-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/lxc-devel
