<div dir="ltr">Hey Stéphane,<br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Sep 13, 2013 at 6:32 PM, Stéphane Graber <span dir="ltr"><<a href="mailto:stgraber@ubuntu.com" target="_blank">stgraber@ubuntu.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div class="im">On Fri, Sep 13, 2013 at 06:21:20PM -0400, S.Çağlar Onur wrote:<br>
> Signed-off-by: S.Çağlar Onur <<a href="mailto:caglar@10ur.org">caglar@10ur.org</a>><br>
<br>
</div>The loopback filtering was there so that all the software using get_ips<br>
to wait for the container to be reachable would work.<br>
Your change will essentially force everyone to do two calls, one for all<br>
interfaces and one just for lo, then substract the second set from the<br>
first, that seems suboptimal.<br>
<br>
I guess your plan is that most API users will iterate through<br>
get_interfaces then call get_ips for the interfaces they want, though<br>
that's just making things harder for little benefit.<br>
<br>
<br>
So I see two ways not to regress in that regard:<br>
1) Make scope filtering work with IPv4, so that get_ips() user can pass<br>
scope=0 and only get global IPv4 and IPv6 addresses.<br>
2) Add a no_loopback argument to get_ips.<br></blockquote><div><br></div><div>To be honest I removed filtering just because I thought get_ips and get_interfaces should return everything in the name of the completeness not because I saw something wrong with them. For that reason I can add filtering back to get_ips and start to filter loopback on get_interfaces. </div>
<div><br></div><div>My real question is what do you think having a get_interfaces method in the API? I added that cause there was no way to get interface names from the container (and get_ips accepts an interface parameter).</div>
<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
As it's:<br>
Nacked-by: Stéphane Graber <<a href="mailto:stgraber@ubuntu.com">stgraber@ubuntu.com</a>><br>
<div><div class="h5"><br>
> ---<br>
> src/lxc/lxccontainer.c | 119 +++++++++++++++++++++++++++++++++++++++----------<br>
> src/lxc/lxccontainer.h | 1 +<br>
> 2 files changed, 97 insertions(+), 23 deletions(-)<br>
><br>
> diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c<br>
> index 79237df..14b6942 100644<br>
> --- a/src/lxc/lxccontainer.c<br>
> +++ b/src/lxc/lxccontainer.c<br>
> @@ -1179,23 +1179,26 @@ static bool lxcapi_clear_config_item(struct lxc_container *c, const char *key)<br>
> return ret == 0;<br>
> }<br>
><br>
> -char** lxcapi_get_ips(struct lxc_container *c, char* interface, char* family, int scope)<br>
> -{<br>
> - int count = 0;<br>
> - struct ifaddrs *interfaceArray = NULL, *tempIfAddr = NULL;<br>
> - char addressOutputBuffer[INET6_ADDRSTRLEN];<br>
> - void *tempAddrPtr = NULL;<br>
> - char **addresses = NULL, **temp;<br>
> - char *address = NULL;<br>
> +static void exit_from_ns(struct lxc_container *c, int *old_netns, int *new_netns) {<br>
> + /* Switch back to original netns */<br>
> + if (*old_netns >= 0 && setns(*old_netns, CLONE_NEWNET))<br>
> + SYSERROR("failed to setns");<br>
> + if (*new_netns >= 0)<br>
> + close(*new_netns);<br>
> + if (*old_netns >= 0)<br>
> + close(*old_netns);<br>
> +}<br>
> +<br>
> +static bool enter_to_ns(struct lxc_container *c, int *old_netns, int *new_netns) {<br>
> + int ret = 0;<br>
> char new_netns_path[MAXPATHLEN];<br>
> - int old_netns = -1, new_netns = -1, ret = 0;<br>
><br>
> if (!c->is_running(c))<br>
> goto out;<br>
><br>
> /* Save reference to old netns */<br>
> - old_netns = open("/proc/self/ns/net", O_RDONLY);<br>
> - if (old_netns < 0) {<br>
> + *old_netns = open("/proc/self/ns/net", O_RDONLY);<br>
> + if (*old_netns < 0) {<br>
> SYSERROR("failed to open /proc/self/ns/net");<br>
> goto out;<br>
> }<br>
> @@ -1205,16 +1208,93 @@ char** lxcapi_get_ips(struct lxc_container *c, char* interface, char* family, in<br>
> if (ret < 0 || ret >= MAXPATHLEN)<br>
> goto out;<br>
><br>
> - new_netns = open(new_netns_path, O_RDONLY);<br>
> - if (new_netns < 0) {<br>
> + *new_netns = open(new_netns_path, O_RDONLY);<br>
> + if (*new_netns < 0) {<br>
> SYSERROR("failed to open %s", new_netns_path);<br>
> goto out;<br>
> }<br>
><br>
> - if (setns(new_netns, CLONE_NEWNET)) {<br>
> + if (setns(*new_netns, CLONE_NEWNET)) {<br>
> SYSERROR("failed to setns");<br>
> goto out;<br>
> }<br>
> + return true;<br>
> +out:<br>
> + exit_from_ns(c, old_netns, new_netns);<br>
> + return false;<br>
> +}<br>
> +<br>
> +char** lxcapi_get_interfaces(struct lxc_container *c)<br>
> +{<br>
> + int count = 0;<br>
> + struct ifaddrs *interfaceArray = NULL, *tempIfAddr = NULL;<br>
> + char **interfaces = NULL, **temp;<br>
> + int old_netns = -1, new_netns = -1;<br>
> +<br>
> + if (!enter_to_ns(c, &old_netns, &new_netns))<br>
> + goto out;<br>
> +<br>
> + /* Grab the list of interfaces */<br>
> + if (getifaddrs(&interfaceArray)) {<br>
> + SYSERROR("failed to get interfaces list");<br>
> + goto out;<br>
> + }<br>
> +<br>
> + /* Iterate through the interfaces */<br>
> + for (tempIfAddr = interfaceArray; tempIfAddr != NULL; tempIfAddr = tempIfAddr->ifa_next) {<br>
> + if (tempIfAddr->ifa_addr == NULL)<br>
> + continue;<br>
> +<br>
> + if(tempIfAddr->ifa_addr->sa_family != AF_PACKET) {<br>
> + continue;<br>
> + }<br>
> +<br>
> + count += 1;<br>
> + temp = realloc(interfaces, count * sizeof(*interfaces));<br>
> + if (!temp) {<br>
> + count -= 1;<br>
> + goto out;<br>
> + }<br>
> + interfaces = temp;<br>
> + interfaces[count - 1] = strdup(tempIfAddr->ifa_name);<br>
> + }<br>
> +<br>
> +out:<br>
> + if(interfaceArray)<br>
> + freeifaddrs(interfaceArray);<br>
> +<br>
> + exit_from_ns(c, &old_netns, &new_netns);<br>
> +<br>
> + /* Append NULL to the array */<br>
> + if (count) {<br>
> + count++;<br>
> + temp = realloc(interfaces, count * sizeof(*interfaces));<br>
> + if (!temp) {<br>
> + int i;<br>
> + for (i = 0; i < count-1; i++)<br>
> + free(interfaces[i]);<br>
> + free(interfaces);<br>
> + return NULL;<br>
> + }<br>
> + interfaces = temp;<br>
> + interfaces[count - 1] = NULL;<br>
> + }<br>
> +<br>
> + return interfaces;<br>
> +}<br>
> +<br>
> +char** lxcapi_get_ips(struct lxc_container *c, char* interface, char* family, int scope)<br>
> +{<br>
> + int count = 0;<br>
> + struct ifaddrs *interfaceArray = NULL, *tempIfAddr = NULL;<br>
> + char addressOutputBuffer[INET6_ADDRSTRLEN];<br>
> + void *tempAddrPtr = NULL;<br>
> + char **addresses = NULL, **temp;<br>
> + char *address = NULL;<br>
> + int old_netns = -1, new_netns = -1;<br>
> +<br>
> + if (!enter_to_ns(c, &old_netns, &new_netns))<br>
> + goto out;<br>
><br>
> /* Grab the list of interfaces */<br>
> if (getifaddrs(&interfaceArray)) {<br>
> @@ -1241,8 +1321,6 @@ char** lxcapi_get_ips(struct lxc_container *c, char* interface, char* family, in<br>
><br>
> if (interface && strcmp(interface, tempIfAddr->ifa_name))<br>
> continue;<br>
> - else if (!interface && strcmp("lo", tempIfAddr->ifa_name) == 0)<br>
> - continue;<br>
><br>
> address = (char *)inet_ntop(tempIfAddr->ifa_addr->sa_family,<br>
> tempAddrPtr,<br>
> @@ -1265,13 +1343,7 @@ out:<br>
> if(interfaceArray)<br>
> freeifaddrs(interfaceArray);<br>
><br>
> - /* Switch back to original netns */<br>
> - if (old_netns >= 0 && setns(old_netns, CLONE_NEWNET))<br>
> - SYSERROR("failed to setns");<br>
> - if (new_netns >= 0)<br>
> - close(new_netns);<br>
> - if (old_netns >= 0)<br>
> - close(old_netns);<br>
> + exit_from_ns(c, &old_netns, &new_netns);<br>
><br>
> /* Append NULL to the array */<br>
> if (count) {<br>
> @@ -2576,6 +2648,7 @@ struct lxc_container *lxc_container_new(const char *name, const char *configpath<br>
> c->get_config_path = lxcapi_get_config_path;<br>
> c->set_config_path = lxcapi_set_config_path;<br>
> c->clone = lxcapi_clone;<br>
> + c->get_interfaces = lxcapi_get_interfaces;<br>
> c->get_ips = lxcapi_get_ips;<br>
> c->attach = lxcapi_attach;<br>
> c->attach_run_wait = lxcapi_attach_run_wait;<br>
> diff --git a/src/lxc/lxccontainer.h b/src/lxc/lxccontainer.h<br>
> index f9ae43b..f17d975 100644<br>
> --- a/src/lxc/lxccontainer.h<br>
> +++ b/src/lxc/lxccontainer.h<br>
> @@ -90,6 +90,7 @@ struct lxc_container {<br>
> * the length which was our would be printed. */<br>
> int (*get_config_item)(struct lxc_container *c, const char *key, char *retv, int inlen);<br>
> int (*get_keys)(struct lxc_container *c, const char *key, char *retv, int inlen);<br>
> + char** (*get_interfaces)(struct lxc_container *c);<br>
> char** (*get_ips)(struct lxc_container *c, char* interface, char* family, int scope);<br>
> /*<br>
> * get_cgroup_item returns the number of bytes read, or an error (<0).<br>
> --<br>
> 1.8.1.2<br>
><br>
><br>
</div></div>> ------------------------------------------------------------------------------<br>
> How ServiceNow helps IT people transform IT departments:<br>
> 1. Consolidate legacy IT systems to a single system of record for IT<br>
> 2. Standardize and globalize service processes across IT<br>
> 3. Implement zero-touch automation to replace manual, redundant tasks<br>
> <a href="http://pubads.g.doubleclick.net/gampad/clk?id=51271111&iu=/4140/ostg.clktrk" target="_blank">http://pubads.g.doubleclick.net/gampad/clk?id=51271111&iu=/4140/ostg.clktrk</a><br>
> _______________________________________________<br>
> Lxc-devel mailing list<br>
> <a href="mailto:Lxc-devel@lists.sourceforge.net">Lxc-devel@lists.sourceforge.net</a><br>
> <a href="https://lists.sourceforge.net/lists/listinfo/lxc-devel" target="_blank">https://lists.sourceforge.net/lists/listinfo/lxc-devel</a><br>
<span class=""><font color="#888888"><br>
--<br>
Stéphane Graber<br>
Ubuntu developer<br>
<a href="http://www.ubuntu.com" target="_blank">http://www.ubuntu.com</a><br>
</font></span></blockquote></div><br><br clear="all"><div><br></div>-- <br>S.Çağlar Onur <<a href="mailto:caglar@10ur.org">caglar@10ur.org</a>>
</div></div>