[lxc-devel] [PATCH] Eliminate duplicate entries from list_active_containers
S.Çağlar Onur
caglar at 10ur.org
Tue Oct 22 02:41:29 UTC 2013
Hi,
On Mon, Oct 21, 2013 at 8:22 PM, Serge Hallyn <serge.hallyn at ubuntu.com>wrote:
> Quoting S.Çağlar Onur (caglar at 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)
OK, I was just trying to make sure both cases return same result :) I'll
iterate one more time with that change.
> > Signed-off-by: S.Çağlar Onur <caglar at 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 at lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/lxc-devel
>
--
S.Çağlar Onur <caglar at 10ur.org>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20131021/a0137257/attachment.html>
More information about the lxc-devel
mailing list