[lxc-devel] [PATCH] Eliminate duplicate entries from list_active_containers

S.Çağlar Onur caglar at 10ur.org
Mon Oct 21 20:41:08 UTC 2013


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.

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





More information about the lxc-devel mailing list