On Mon, Sep 16, 2013 at 2:39 PM, Stéphane Graber <stgra...@ubuntu.com>wrote:
> On Mon, Sep 16, 2013 at 02:26:47PM -0400, S.Çağlar Onur wrote:
> > get_ips accepts an interface name as a parameter but there was no
> > way to get the interfaces names from the container. This patch
> > introduces a new get_interfaces call to the API so that users
> > can obtain the name of the interfaces.
> >
> > Signed-off-by: S.Çağlar Onur <cag...@10ur.org>
>
> I think I'm fine with this version:
>
> One quick comment though, wouldn't we expect get_interfaces() to return
> all the interfaces by default, including loopback, including those
> without addresses and those that aren't standard packet based IP
> interfaces?
>
> I still think we don't want to list loopback addresses in get_ips(), at
> least by default, but I think I'd expect get_interfaces to return me all
> the existing interfaces without filtering. Does that make sense?
Guess so. I've no objection including loopback in get_interfaces. I
excluded it thinking that there is no point returning it as get_ips will
ignore it. What about something like below as a replacement to v2 (this
version starts to use if_nameindex instead of getifaddrs)?
+static char** lxcapi_get_interfaces(struct lxc_container *c)
+{
+ int count = 0;
+ struct if_nameindex *if_ni = NULL, *i;
+ char **interfaces = NULL, **temp;
+ int old_netns = -1, new_netns = -1;
+
+ if (!enter_to_ns(c, &old_netns, &new_netns))
+ goto out;
+
+ if_ni = if_nameindex();
+ if (if_ni == NULL) {
+ SYSERROR("failed to get interfaces list");
+ goto out;
+ }
+
+ for (i = if_ni; ! (i->if_index == 0 && i->if_name == NULL); i++) {
+ count += 1;
+ temp = realloc(interfaces, count * sizeof(*interfaces));
+ if (!temp) {
+ count -= 1;
+ goto out;
+ }
+ interfaces = temp;
+ interfaces[count - 1] = strdup(i->if_name);
+ }
+
+out:
+ if_freenameindex(if_ni);
+
+ exit_from_ns(c, &old_netns, &new_netns);
+
+ /* Append NULL to the array */
+ interfaces = (char **)lxc_append_null_to_array((void **)interfaces,
count);
+
+ return interfaces;
+}
And if we need get_ips to return ips then I guess we can always add a flag
to get_ips as you suggested before?
> ---
> > src/lxc/lxccontainer.c | 117
> ++++++++++++++++++++++++++++++++++---------------
> > src/lxc/lxccontainer.h | 1 +
> > src/lxc/utils.c | 22 +++++++++-
> > src/lxc/utils.h | 1 +
> > 4 files changed, 105 insertions(+), 36 deletions(-)
> >
> > diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
> > index 79237df..6aa59fc 100644
> > --- a/src/lxc/lxccontainer.c
> > +++ b/src/lxc/lxccontainer.c
> > @@ -1179,23 +1179,26 @@ static bool lxcapi_clear_config_item(struct
> lxc_container *c, const char *key)
> > return ret == 0;
> > }
> >
> > -char** lxcapi_get_ips(struct lxc_container *c, char* interface, char*
> family, int scope)
> > -{
> > - int count = 0;
> > - struct ifaddrs *interfaceArray = NULL, *tempIfAddr = NULL;
> > - char addressOutputBuffer[INET6_ADDRSTRLEN];
> > - void *tempAddrPtr = NULL;
> > - char **addresses = NULL, **temp;
> > - char *address = NULL;
> > +static inline void exit_from_ns(struct lxc_container *c, int
> *old_netns, int *new_netns) {
> > + /* Switch back to original netns */
> > + if (*old_netns >= 0 && setns(*old_netns, CLONE_NEWNET))
> > + SYSERROR("failed to setns");
> > + if (*new_netns >= 0)
> > + close(*new_netns);
> > + if (*old_netns >= 0)
> > + close(*old_netns);
> > +}
> > +
> > +static inline bool enter_to_ns(struct lxc_container *c, int *old_netns,
> int *new_netns) {
> > + int ret = 0;
> > char new_netns_path[MAXPATHLEN];
> > - int old_netns = -1, new_netns = -1, ret = 0;
> >
> > if (!c->is_running(c))
> > goto out;
> >
> > /* Save reference to old netns */
> > - old_netns = open("/proc/self/ns/net", O_RDONLY);
> > - if (old_netns < 0) {
> > + *old_netns = open("/proc/self/ns/net", O_RDONLY);
> > + if (*old_netns < 0) {
> > SYSERROR("failed to open /proc/self/ns/net");
> > goto out;
> > }
> > @@ -1205,16 +1208,78 @@ char** lxcapi_get_ips(struct lxc_container *c,
> char* interface, char* family, in
> > if (ret < 0 || ret >= MAXPATHLEN)
> > goto out;
> >
> > - new_netns = open(new_netns_path, O_RDONLY);
> > - if (new_netns < 0) {
> > + *new_netns = open(new_netns_path, O_RDONLY);
> > + if (*new_netns < 0) {
> > SYSERROR("failed to open %s", new_netns_path);
> > goto out;
> > }
> >
> > - if (setns(new_netns, CLONE_NEWNET)) {
> > + if (setns(*new_netns, CLONE_NEWNET)) {
> > SYSERROR("failed to setns");
> > goto out;
> > }
> > + return true;
> > +out:
> > + exit_from_ns(c, old_netns, new_netns);
> > + return false;
> > +}
> > +
> > +static char** lxcapi_get_interfaces(struct lxc_container *c)
> > +{
> > + int count = 0;
> > + struct ifaddrs *interfaceArray = NULL, *tempIfAddr = NULL;
> > + char **interfaces = NULL, **temp;
> > + int old_netns = -1, new_netns = -1;
> > +
> > + if (!enter_to_ns(c, &old_netns, &new_netns))
> > + goto out;
> > +
> > + /* Grab the list of interfaces */
> > + if (getifaddrs(&interfaceArray)) {
> > + SYSERROR("failed to get interfaces list");
> > + goto out;
> > + }
> > +
> > + /* Iterate through the interfaces */
> > + for (tempIfAddr = interfaceArray; tempIfAddr != NULL; tempIfAddr =
> tempIfAddr->ifa_next) {
> > + /* filter out loopback interface */
> > + if(tempIfAddr->ifa_addr == NULL ||
> tempIfAddr->ifa_addr->sa_family != AF_PACKET || strcmp("lo",
> tempIfAddr->ifa_name) == 0)
> > + continue;
> > +
> > + count += 1;
> > + temp = realloc(interfaces, count * sizeof(*interfaces));
> > + if (!temp) {
> > + count -= 1;
> > + goto out;
> > + }
> > + interfaces = temp;
> > + interfaces[count - 1] = strdup(tempIfAddr->ifa_name);
> > + }
> > +
> > +out:
> > + if(interfaceArray)
> > + freeifaddrs(interfaceArray);
> > +
> > + exit_from_ns(c, &old_netns, &new_netns);
> > +
> > + /* Append NULL to the array */
> > + interfaces = (char **)lxc_append_null_to_array((void
> **)interfaces, count);
> > +
> > + return interfaces;
> > +}
> > +
> > +static char** lxcapi_get_ips(struct lxc_container *c, char* interface,
> char* family, int scope)
> > +{
> > + int count = 0;
> > + struct ifaddrs *interfaceArray = NULL, *tempIfAddr = NULL;
> > + char addressOutputBuffer[INET6_ADDRSTRLEN];
> > + void *tempAddrPtr = NULL;
> > + char **addresses = NULL, **temp;
> > + char *address = NULL;
> > + int old_netns = -1, new_netns = -1;
> > +
> > + if (!enter_to_ns(c, &old_netns, &new_netns))
> > + goto out;
> >
> > /* Grab the list of interfaces */
> > if (getifaddrs(&interfaceArray)) {
> > @@ -1243,7 +1308,6 @@ char** lxcapi_get_ips(struct lxc_container *c,
> char* interface, char* family, in
> > continue;
> > else if (!interface && strcmp("lo", tempIfAddr->ifa_name)
> == 0)
> > continue;
> > -
> > address = (char
> *)inet_ntop(tempIfAddr->ifa_addr->sa_family,
> > tempAddrPtr,
> > addressOutputBuffer,
> > @@ -1265,28 +1329,10 @@ out:
> > if(interfaceArray)
> > freeifaddrs(interfaceArray);
> >
> > - /* Switch back to original netns */
> > - if (old_netns >= 0 && setns(old_netns, CLONE_NEWNET))
> > - SYSERROR("failed to setns");
> > - if (new_netns >= 0)
> > - close(new_netns);
> > - if (old_netns >= 0)
> > - close(old_netns);
> > + exit_from_ns(c, &old_netns, &new_netns);
> >
> > /* Append NULL to the array */
> > - if (count) {
> > - count++;
> > - temp = realloc(addresses, count * sizeof(*addresses));
> > - if (!temp) {
> > - int i;
> > - for (i = 0; i < count-1; i++)
> > - free(addresses[i]);
> > - free(addresses);
> > - return NULL;
> > - }
> > - addresses = temp;
> > - addresses[count - 1] = NULL;
> > - }
> > + addresses = (char **)lxc_append_null_to_array((void **)addresses,
> count);
> >
> > return addresses;
> > }
> > @@ -2576,6 +2622,7 @@ struct lxc_container *lxc_container_new(const char
> *name, const char *configpath
> > c->get_config_path = lxcapi_get_config_path;
> > c->set_config_path = lxcapi_set_config_path;
> > c->clone = lxcapi_clone;
> > + c->get_interfaces = lxcapi_get_interfaces;
> > c->get_ips = lxcapi_get_ips;
> > c->attach = lxcapi_attach;
> > c->attach_run_wait = lxcapi_attach_run_wait;
> > diff --git a/src/lxc/lxccontainer.h b/src/lxc/lxccontainer.h
> > index f9ae43b..f17d975 100644
> > --- a/src/lxc/lxccontainer.h
> > +++ b/src/lxc/lxccontainer.h
> > @@ -90,6 +90,7 @@ struct lxc_container {
> > * the length which was our would be printed. */
> > int (*get_config_item)(struct lxc_container *c, const char *key,
> char *retv, int inlen);
> > int (*get_keys)(struct lxc_container *c, const char *key, char
> *retv, int inlen);
> > + char** (*get_interfaces)(struct lxc_container *c);
> > char** (*get_ips)(struct lxc_container *c, char* interface, char*
> family, int scope);
> > /*
> > * get_cgroup_item returns the number of bytes read, or an error
> (<0).
> > diff --git a/src/lxc/utils.c b/src/lxc/utils.c
> > index 78b234d..924cc19 100644
> > --- a/src/lxc/utils.c
> > +++ b/src/lxc/utils.c
> > @@ -852,7 +852,7 @@ int lxc_write_to_file(const char *filename, const
> void* buf, size_t count, bool
> > return -1;
> > ret = lxc_write_nointr(fd, buf, count);
> > if (ret < 0)
> > - goto out_error;
> > + goto out_error;
> > if ((size_t)ret != count)
> > goto out_error;
> > if (add_newline) {
> > @@ -899,3 +899,23 @@ int lxc_read_from_file(const char *filename, void*
> buf, size_t count)
> > errno = saved_errno;
> > return ret;
> > }
> > +
> > +void **lxc_append_null_to_array(void **array, size_t count)
> > +{
> > + void **temp;
> > +
> > + /* Append NULL to the array */
> > + if (count) {
> > + temp = realloc(array, (count + 1) * sizeof(*array));
> > + if (!temp) {
> > + int i;
> > + for (i = 0; i < count; i++)
> > + free(array[i]);
> > + free(array);
> > + return NULL;
> > + }
> > + array = temp;
> > + array[count] = NULL;
> > + }
> > + return array;
> > +}
> > diff --git a/src/lxc/utils.h b/src/lxc/utils.h
> > index ba7cfb3..55f98fa 100644
> > --- a/src/lxc/utils.h
> > +++ b/src/lxc/utils.h
> > @@ -236,4 +236,5 @@ extern void lxc_free_array(void **array, lxc_free_fn
> element_free_fn);
> > extern size_t lxc_array_len(void **array);
> > extern void **lxc_dup_array(void **array, lxc_dup_fn element_dup_fn,
> lxc_free_fn element_free_fn);
> >
> > +extern void **lxc_append_null_to_array(void **array, size_t count);
> > #endif
> > --
> > 1.8.1.2
> >
> >
> >
> ------------------------------------------------------------------------------
> > LIMITED TIME SALE - Full Year of Microsoft Training For Just $49.99!
> > 1,500+ hours of tutorials including VisualStudio 2012, Windows 8,
> SharePoint
> > 2013, SQL 2012, MVC 4, more. BEST VALUE: New Multi-Library Power Pack
> includes
> > Mobile, Cloud, Java, and UX Design. Lowest price ever! Ends 9/20/13.
> >
> http://pubads.g.doubleclick.net/gampad/clk?id=58041151&iu=/4140/ostg.clktrk
> > _______________________________________________
> > Lxc-devel mailing list
> > Lxc-devel@lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/lxc-devel
>
> --
> Stéphane Graber
> Ubuntu developer
> http://www.ubuntu.com
>
--
S.Çağlar Onur <cag...@10ur.org>
------------------------------------------------------------------------------
LIMITED TIME SALE - Full Year of Microsoft Training For Just $49.99!
1,500+ hours of tutorials including VisualStudio 2012, Windows 8, SharePoint
2013, SQL 2012, MVC 4, more. BEST VALUE: New Multi-Library Power Pack includes
Mobile, Cloud, Java, and UX Design. Lowest price ever! Ends 9/20/13.
http://pubads.g.doubleclick.net/gampad/clk?id=58041151&iu=/4140/ostg.clktrk
_______________________________________________
Lxc-devel mailing list
Lxc-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/lxc-devel
