Quoting S.Çağlar Onur (cag...@10ur.org):
> list_active_containers parses /proc/net/unix which can contain multiple
> entries for the same container;
>
> 0000000000000000: 00000002 00000000 00010000 0001 01 273672
> @/var/lib/lxc/6/command
> 0000000000000000: 00000002 00000000 00010000 0001 01 274395
> @/var/lib/lxc/5/command
> 0000000000000000: 00000002 00000000 00010000 0001 01 273890
> @/var/lib/lxc/4/command
> 0000000000000000: 00000002 00000000 00010000 0001 01 273141
> @/var/lib/lxc/3/command
> 0000000000000000: 00000002 00000000 00010000 0001 01 273915
> @/var/lib/lxc/2/command
> 0000000000000000: 00000002 00000000 00010000 0001 01 273683
> @/var/lib/lxc/1/command
> 0000000000000000: 00000002 00000000 00010000 0001 01 273074
> @/var/lib/lxc/0/command
> 0000000000000000: 00000002 00000000 00010000 0001 01 273931
> @/var/lib/lxc/9/command
> 0000000000000000: 00000002 00000000 00010000 0001 01 273110
> @/var/lib/lxc/8/command
> 0000000000000000: 00000002 00000000 00010000 0001 01 273390
> @/var/lib/lxc/7/command
> 0000000000000000: 00000003 00000000 00000000 0001 03 275903
> @/var/lib/lxc/8/command
> 0000000000000000: 00000003 00000000 00000000 0001 03 276043
> @/var/lib/lxc/1/command
> 0000000000000000: 00000003 00000000 00000000 0001 03 273301
> @/var/lib/lxc/0/command
> 0000000000000000: 00000003 00000000 00000000 0001 03 275650
> @/var/lib/lxc/4/command
>
> On this system list_active_containers returns 14 containers while only 10
> containers are running.
>
> Following patch;
>
> * Introduces array_contains function to do a binary search on given array,
> * Starts to sort arrays inside the add_to_clist and add_to_names functions,
> * Consumes array_contains in list_active_containers to eliminate duplicates,
> * Replaces the linear search code in lxcapi_get_interfaces with the new
> function.
Thanks - that patch on the whole is good, except that you move the
adding to *names in list_active_containers() to after the attempt to
load the container. Not loading the container if a container list is
not passed in is deliberately done to avoid a potentially large
amount of work. (The very low potential for differing results when
passing in *cret and not is deemed worthwhile)
> Signed-off-by: S.Çağlar Onur <cag...@10ur.org>
> ---
> src/lxc/lxccontainer.c | 161
> ++++++++++++++++++++++++++-----------------------
> 1 file changed, 86 insertions(+), 75 deletions(-)
>
> diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
> index c8ecef3..46389ab 100644
> --- a/src/lxc/lxccontainer.c
> +++ b/src/lxc/lxccontainer.c
> @@ -1240,12 +1240,67 @@ out:
> return false;
> }
>
> +// used by qsort and bsearch functions
> +static inline int string_cmp(char **first, char **second)
> +{
> + return strcmp(*first, *second);
> +}
> +
> +static bool add_to_names(char ***names, char *cname, int pos)
> +{
> + char **newnames = realloc(*names, (pos+1) * sizeof(char *));
> + if (!newnames) {
> + ERROR("Out of memory");
> + return false;
> + }
> +
> + *names = newnames;
> + newnames[pos] = strdup(cname);
> + if (!newnames[pos])
> + return false;
> +
> + // sort the arrray as we will use binary search on it
> + qsort(newnames, pos + 1, sizeof(char *), (int (*)(const void *,const
> void *))string_cmp);
> +
> + return true;
> +}
> +
> +// used by qsort and bsearch functions
> +static inline int container_cmp(struct lxc_container **first, struct
> lxc_container **second)
> +{
> + return strcmp((*first)->name, (*second)->name);
> +}
> +
> +static bool add_to_clist(struct lxc_container ***list, struct lxc_container
> *c, int pos)
> +{
> + struct lxc_container **newlist = realloc(*list, (pos+1) * sizeof(struct
> lxc_container *));
> + if (!newlist) {
> + ERROR("Out of memory");
> + return false;
> + }
> +
> + *list = newlist;
> + newlist[pos] = c;
> +
> + // sort the arrray as we will use binary search on it
> + qsort(newlist, pos + 1, sizeof(struct lxc_container *), (int (*)(const
> void *,const void *))container_cmp);
> +
> + return true;
> +}
> +
> +static bool array_contains(char **names, char *cname, int size) {
> + char *result = NULL;
> + result = (char *)bsearch((char *)&cname, (char *)names, size,
> sizeof(char *), (int (*)(const void *,const void *))string_cmp);
> + if (result != NULL)
> + return true;
> + return false;
> +}
> +
> static char** lxcapi_get_interfaces(struct lxc_container *c)
> {
> - int count = 0, i;
> - bool found = false;
> + int count = 0;
> struct ifaddrs *interfaceArray = NULL, *tempIfAddr = NULL;
> - char **interfaces = NULL, **temp;
> + char **interfaces = NULL;
> int old_netns = -1, new_netns = -1;
>
> if (!enter_to_ns(c, &old_netns, &new_netns))
> @@ -1259,30 +1314,10 @@ static char** lxcapi_get_interfaces(struct
> lxc_container *c)
>
> /* Iterate through the interfaces */
> for (tempIfAddr = interfaceArray; tempIfAddr != NULL; tempIfAddr =
> tempIfAddr->ifa_next) {
> - /*
> - * WARNING: Following "for" loop does a linear search over the
> interfaces array
> - * For the containers with lots of interfaces this may be
> problematic.
> - * I'm not expecting this to be the common usage but if it
> turns out to be
> - * than using binary search or a hash table could be more
> elegant solution.
> - */
> - for (i = 0; i < count; i++) {
> - if (strcmp(interfaces[i], tempIfAddr->ifa_name) == 0) {
> - found = true;
> - break;
> - }
> - }
> -
> - if (!found) {
> - count += 1;
> - temp = realloc(interfaces, count * sizeof(*interfaces));
> - if (!temp) {
> - count -= 1;
> - goto out;
> - }
> - interfaces = temp;
> - interfaces[count - 1] = strdup(tempIfAddr->ifa_name);
> + if (!array_contains(interfaces, tempIfAddr->ifa_name, count)) {
> + add_to_names(&interfaces, tempIfAddr->ifa_name, count);
> + count++;
> }
> - found = false;
> }
>
> out:
> @@ -2796,34 +2831,6 @@ int lxc_get_wait_states(const char **states)
> return MAX_STATE;
> }
>
> -
> -static bool add_to_names(char ***names, char *cname, int pos)
> -{
> - char **newnames = realloc(*names, (pos+1) * sizeof(char *));
> - if (!newnames) {
> - ERROR("Out of memory");
> - return false;
> - }
> - *names = newnames;
> - newnames[pos] = strdup(cname);
> - if (!newnames[pos])
> - return false;
> - return true;
> -}
> -
> -static bool add_to_clist(struct lxc_container ***list, struct lxc_container
> *c, int pos)
> -{
> - struct lxc_container **newlist = realloc(*list, (pos+1) * sizeof(struct
> lxc_container *));
> - if (!newlist) {
> - ERROR("Out of memory");
> - return false;
> - }
> -
> - *list = newlist;
> - newlist[pos] = c;
> - return true;
> -}
> -
> /*
> * These next two could probably be done smarter with reusing a common
> function
> * with different iterators and tests...
> @@ -2922,9 +2929,10 @@ free_bad:
>
> int list_active_containers(const char *lxcpath, char ***names, struct
> lxc_container ***cret)
> {
> - int i, cfound = 0, nfound = 0;
> + int i, found = 0;
> int lxcpath_len;
> char *line = NULL;
> + char **unique_names = NULL;
> size_t len = 0;
> struct lxc_container *c;
>
> @@ -2963,52 +2971,55 @@ int list_active_containers(const char *lxcpath, char
> ***names, struct lxc_contai
> continue;
> *p2 = '\0';
>
> - if (names) {
> - if (!add_to_names(names, p, nfound))
> - goto free_bad;
> - }
> - cfound++;
> -
> - if (!cret) {
> - nfound++;
> + /*
> + * /proc/net/unix may contain multiple entries for command
> socket
> + * Check the existence before adding to the list
> + */
> + if (array_contains(unique_names, p, found)) {
> + INFO("Container %s:%s is seen before, skipping",
> lxcpath, p);
> continue;
> }
>
> + // try loading the container
> c = lxc_container_new(p, lxcpath);
> if (!c) {
> - INFO("Container %s:%s is running but could not be
> loaded",
> - lxcpath, p);
> - if (names)
> - free((*names)[cfound--]);
> + INFO("Container %s:%s is running but could not be
> loaded, skipping", lxcpath, p);
> continue;
> }
>
> + // all tests passed, we now can add this to names list
> + if(!add_to_names(&unique_names, p, found))
> + goto free_bad;
> +
> /*
> * If this is an anonymous container, then is_defined *can*
> * return false. So we don't do that check. Count on the
> * fact that the command socket exists.
> */
>
> - if (!add_to_clist(cret, c, nfound)) {
> + if (cret && !add_to_clist(cret, c, found)) {
> lxc_container_put(c);
> goto free_bad;
> }
> - nfound++;
> + found++;
> }
>
> + if (names)
> + *names = unique_names;
> +
> process_lock();
> fclose(f);
> process_unlock();
> - return nfound;
> + return found;
>
> free_bad:
> - if (names && *names) {
> - for (i=0; i<cfound; i++)
> - free((*names)[i]);
> - free(*names);
> + if(unique_names) {
> + for (i=0; i<found; i++)
> + free(unique_names[i]);
> + free(unique_names);
> }
> if (cret && *cret) {
> - for (i=0; i<nfound; i++)
> + for (i=0; i<found; i++)
> lxc_container_put((*cret)[i]);
> free(*cret);
> }
> --
> 1.8.3.2
>
>
> ------------------------------------------------------------------------------
> October Webinars: Code for Performance
> Free Intel webinars can help you accelerate application performance.
> Explore tips for MPI, OpenMP, advanced profiling, and more. Get the most from
> the latest Intel processors and coprocessors. See abstracts and register >
> http://pubads.g.doubleclick.net/gampad/clk?id=60135991&iu=/4140/ostg.clktrk
> _______________________________________________
> Lxc-devel mailing list
> Lxc-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/lxc-devel
------------------------------------------------------------------------------
October Webinars: Code for Performance
Free Intel webinars can help you accelerate application performance.
Explore tips for MPI, OpenMP, advanced profiling, and more. Get the most from
the latest Intel processors and coprocessors. See abstracts and register >
http://pubads.g.doubleclick.net/gampad/clk?id=60135991&iu=/4140/ostg.clktrk
_______________________________________________
Lxc-devel mailing list
Lxc-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/lxc-devel
