OS-4589 zoneadm list slow with a lot of zones

Reviewed by: Patrick Mooney <patrick.mooney@joyent.com>
Reviewed by: Dave Eddy <dave.eddy@joyent.com>

4 files changed, 186 insertions, 211 deletions

diff --git a/usr/src/cmd/zoneadm/zoneadm.c b/usr/src/cmd/zoneadm/zoneadm.c
index 62b709ff77..312159cabd 100644
--- a/usr/src/cmd/zoneadm/zoneadm.c
+++ b/usr/src/cmd/zoneadm/zoneadm.c
@@ -107,9 +107,6 @@ typedef struct zone_entry {
 
 #define	CLUSTER_BRAND_NAME	"cluster"
 
-static zone_entry_t *zents;
-static size_t nzents;
-
 #define	LOOPBACK_IF	"lo0"
 #define	SOCKET_AF(af)	(((af) == AF_UNSPEC) ? AF_INET : (af))
 
@@ -408,19 +405,6 @@ zerror(const char *fmt, ...)
 	va_end(alist);
 }
 
-static void *
-safe_calloc(size_t nelem, size_t elsize)
-{
-	void *r = calloc(nelem, elsize);
-
-	if (r == NULL) {
-		zerror(gettext("failed to allocate %lu bytes: %s"),
-		    (ulong_t)nelem * elsize, strerror(errno));
-		exit(Z_ERR);
-	}
-	return (r);
-}
-
 static void
 zone_print(zone_entry_t *zent, boolean_t verbose, boolean_t parsable)
 {
@@ -492,6 +476,9 @@ lookup_zone_info(const char *zone_name, zoneid_t zid, zone_entry_t *zent)
 	(void) strlcpy(zent->zbrand, "???", sizeof (zent->zbrand));
 	zent->zstate_str = "???";
 
+	if (strcmp(zone_name, GLOBAL_ZONENAME) == 0)
+		zid = zent->zdid = GLOBAL_ZONEID;
+
 	zent->zid = zid;
 
 	if (zonecfg_get_uuid(zone_name, uuid) == Z_OK &&
@@ -536,8 +523,8 @@ lookup_zone_info(const char *zone_name, zoneid_t zid, zone_entry_t *zent)
 	zent->zstate_str = zone_state_str(zent->zstate_num);
 
 	/*
-	 * A zone's brand is only available in the .xml file describing it,
-	 * which is only visible to the global zone.  This causes
+	 * A zone's brand might only be available in the .xml file describing
+	 * it, which is only visible to the global zone.  This causes
 	 * zone_get_brand() to fail when called from within a non-global
 	 * zone.  Fortunately we only do this on labeled systems, where we
 	 * know all zones are native.
@@ -607,165 +594,63 @@ lookup_zone_info(const char *zone_name, zoneid_t zid, zone_entry_t *zent)
 	return (Z_OK);
 }
 
-/*
- * fetch_zents() calls zone_list(2) to find out how many zones are running
- * (which is stored in the global nzents), then calls zone_list(2) again
- * to fetch the list of running zones (stored in the global zents).  This
- * function may be called multiple times, so if zents is already set, we
- * return immediately to save work.
- *
- * Note that the data about running zones can change while this function
- * is running, so its possible that the list of zones will have empty slots
- * at the end.
- */
-
-static int
-fetch_zents(void)
-{
-	zoneid_t *zids = NULL;
-	uint_t nzents_saved;
-	int i, retv;
-	FILE *fp;
-	boolean_t inaltroot;
-	zone_entry_t *zentp;
-	const char *altroot;
-
-	if (nzents > 0)
-		return (Z_OK);
-
-	if (zone_list(NULL, &nzents) != 0) {
-		zperror(gettext("failed to get zoneid list"), B_FALSE);
-		return (Z_ERR);
-	}
-
-again:
-	if (nzents == 0)
-		return (Z_OK);
-
-	zids = safe_calloc(nzents, sizeof (zoneid_t));
-	nzents_saved = nzents;
-
-	if (zone_list(zids, &nzents) != 0) {
-		zperror(gettext("failed to get zone list"), B_FALSE);
-		free(zids);
-		return (Z_ERR);
-	}
-	if (nzents != nzents_saved) {
-		/* list changed, try again */
-		free(zids);
-		goto again;
-	}
-
-	zents = safe_calloc(nzents, sizeof (zone_entry_t));
-
-	inaltroot = zonecfg_in_alt_root();
-	if (inaltroot) {
-		fp = zonecfg_open_scratch("", B_FALSE);
-		altroot = zonecfg_get_root();
-	} else {
-		fp = NULL;
-	}
-	zentp = zents;
-	retv = Z_OK;
-	for (i = 0; i < nzents; i++) {
-		char name[ZONENAME_MAX];
-		char altname[ZONENAME_MAX];
-		char rev_altroot[MAXPATHLEN];
-
-		if (getzonenamebyid(zids[i], name, sizeof (name)) < 0) {
-			/*
-			 * There is a race condition where the zone may have
-			 * shutdown since we retrieved the number of running
-			 * zones above.  This is not an error, there will be
-			 * an empty slot at the end of the list.
-			 */
-			continue;
-		}
-		if (zonecfg_is_scratch(name)) {
-			/* Ignore scratch zones by default */
-			if (!inaltroot)
-				continue;
-			if (fp == NULL ||
-			    zonecfg_reverse_scratch(fp, name, altname,
-			    sizeof (altname), rev_altroot,
-			    sizeof (rev_altroot)) == -1) {
-				zerror(gettext("could not resolve scratch "
-				    "zone %s"), name);
-				retv = Z_ERR;
-				continue;
-			}
-			/* Ignore zones in other alternate roots */
-			if (strcmp(rev_altroot, altroot) != 0)
-				continue;
-			(void) strcpy(name, altname);
-		} else {
-			/* Ignore non-scratch when in an alternate root */
-			if (inaltroot && strcmp(name, GLOBAL_ZONENAME) != 0)
-				continue;
-		}
-		if (lookup_zone_info(name, zids[i], zentp) != Z_OK) {
-			/*
-			 * There is a race condition where the zone may have
-			 * shutdown since we retrieved the number of running
-			 * zones above.  This is not an error, there will be
-			 * an empty slot at the end of the list.
-			 */
-			continue;
-		}
-		zentp++;
-	}
-	nzents = zentp - zents;
-	if (fp != NULL)
-		zonecfg_close_scratch(fp);
-
-	free(zids);
-	return (retv);
-}
-
 static int
 zone_print_list(zone_state_t min_state, boolean_t verbose, boolean_t parsable)
 {
-	int i;
 	zone_entry_t zent;
 	FILE *cookie;
-	char *name;
+	struct zoneent *ze;
 
 	/*
-	 * First get the list of running zones from the kernel and print them.
-	 * If that is all we need, then return.
-	 */
-	if ((i = fetch_zents()) != Z_OK) {
-		/*
-		 * No need for error messages; fetch_zents() has already taken
-		 * care of this.
-		 */
-		return (i);
-	}
-	for (i = 0; i < nzents; i++)
-		zone_print(&zents[i], verbose, parsable);
-	if (min_state >= ZONE_STATE_RUNNING)
-		return (Z_OK);
-	/*
-	 * Next, get the full list of zones from the configuration, skipping
-	 * any we have already printed.
+	 * Get the full list of zones from the configuration.
 	 */
 	cookie = setzoneent();
-	while ((name = getzoneent(cookie)) != NULL) {
-		for (i = 0; i < nzents; i++) {
-			if (strcmp(zents[i].zname, name) == 0)
-				break;
-		}
-		if (i < nzents) {
-			free(name);
-			continue;
-		}
-		if (lookup_zone_info(name, ZONE_ID_UNDEFINED, &zent) != Z_OK) {
-			free(name);
-			continue;
+	while ((ze = getzoneent_private(cookie)) != NULL) {
+		char *name = ze->zone_name;
+		zoneid_t zid;
+
+		zid = getzoneidbyname(name);
+
+		if (ze->zone_brand[0] == '\0') {
+			/* old, incomplete index entry */
+			if (lookup_zone_info(name, zid, &zent) != Z_OK) {
+				free(ze);
+				continue;
+			}
+		} else {
+			/* new, full index entry */
+			(void) strlcpy(zent.zname, name, sizeof (zent.zname));
+			(void) strlcpy(zent.zroot, ze->zone_path,
+			    sizeof (zent.zroot));
+			uuid_unparse(ze->zone_uuid, zent.zuuid);
+			(void) strlcpy(zent.zbrand, ze->zone_brand,
+			    sizeof (zent.zbrand));
+			zent.ziptype = ze->zone_iptype;
+			zent.zdid = ze->zone_did;
+			zent.zid = zid;
+
+			if (zid != -1) {
+				int err;
+
+				err = zone_get_state(name,
+				    (zone_state_t *)&ze->zone_state);
+				if (err != Z_OK) {
+					errno = err;
+					zperror2(name, gettext("could not get "
+					    "state"));
+					free(ze);
+					continue;
+				}
+			}
+
+			zent.zstate_num = ze->zone_state;
+			zent.zstate_str = zone_state_str(zent.zstate_num);
 		}
-		free(name);
+
 		if (zent.zstate_num >= min_state)
 			zone_print(&zent, verbose, parsable);
+
+		free(ze);
 	}
 	endzoneent(cookie);
 	return (Z_OK);
@@ -1641,10 +1526,10 @@ auth_check(char *user, char *zone, int cmd_num)
  *     not already running (or ready).
  */
 static int
-sanity_check(char *zone, int cmd_num, boolean_t running,
+sanity_check(char *zone, int cmd_num, boolean_t need_running,
     boolean_t unsafe_when_running, boolean_t force)
 {
-	zone_entry_t *zent;
+	boolean_t is_running = B_FALSE;
 	priv_set_t *privset;
 	zone_state_t state, min_state;
 	char kernzone[ZONENAME_MAX];
@@ -1715,51 +1600,54 @@ sanity_check(char *zone, int cmd_num, boolean_t running,
 	}
 
 	if (!zonecfg_in_alt_root()) {
-		zent = lookup_running_zone(zone);
-	} else if ((fp = zonecfg_open_scratch("", B_FALSE)) == NULL) {
-		zent = NULL;
-	} else {
-		if (zonecfg_find_scratch(fp, zone, zonecfg_get_root(),
-		    kernzone, sizeof (kernzone)) == 0)
-			zent = lookup_running_zone(kernzone);
-		else
-			zent = NULL;
+		/* Avoid the xml read overhead of lookup_running_zone */
+		if (getzoneidbyname(zone) != -1)
+			is_running = B_TRUE;
+
+	} else if ((fp = zonecfg_open_scratch("", B_FALSE)) != NULL) {
+		if (zonecfg_find_scratch(fp, zone, zonecfg_get_root(), kernzone,
+		    sizeof (kernzone)) == 0 && getzoneidbyname(kernzone) != -1)
+			is_running = B_TRUE;
+
 		zonecfg_close_scratch(fp);
 	}
 
 	/*
 	 * Look up from the kernel for 'running' zones.
 	 */
-	if (running && !force) {
-		if (zent == NULL) {
+	if (need_running && !force) {
+		if (!is_running) {
 			zerror(gettext("not running"));
 			return (Z_ERR);
 		}
 	} else {
 		int err;
 
-		if (unsafe_when_running && zent != NULL) {
+		err = zone_get_state(zone, &state);
+
+		if (unsafe_when_running && is_running) {
 			/* check whether the zone is ready or running */
-			if ((err = zone_get_state(zent->zname,
-			    &zent->zstate_num)) != Z_OK) {
+			char *zstate_str;
+
+			if (err != Z_OK) {
 				errno = err;
-				zperror2(zent->zname,
-				    gettext("could not get state"));
+				zperror2(zone, gettext("could not get state"));
 				/* can't tell, so hedge */
-				zent->zstate_str = "ready/running";
+				zstate_str = "ready/running";
 			} else {
-				zent->zstate_str =
-				    zone_state_str(zent->zstate_num);
+				zstate_str = zone_state_str(state);
 			}
 			zerror(gettext("%s operation is invalid for %s zones."),
-			    cmd_to_str(cmd_num), zent->zstate_str);
+			    cmd_to_str(cmd_num), zstate_str);
 			return (Z_ERR);
 		}
-		if ((err = zone_get_state(zone, &state)) != Z_OK) {
+
+		if (err != Z_OK) {
 			errno = err;
 			zperror2(zone, gettext("could not get state"));
 			return (Z_ERR);
 		}
+
 		switch (cmd_num) {
 		case CMD_UNINSTALL:
 			if (state == ZONE_STATE_CONFIGURED) {
@@ -2832,9 +2720,8 @@ static boolean_t
 verify_fix_did(zone_dochandle_t handle)
 {
 	zoneid_t mydid;
-	zone_entry_t zent;
+	struct zoneent *ze;
 	FILE *cookie;
-	char *name;
 	boolean_t fix = B_FALSE;
 
 	mydid = zonecfg_get_did(handle);
@@ -2845,20 +2732,34 @@ verify_fix_did(zone_dochandle_t handle)
 
 	/* Get the full list of zones from the configuration. */
 	cookie = setzoneent();
-	while ((name = getzoneent(cookie)) != NULL) {
-		if (strcmp(target_zone, name) == 0) {
-			free(name);
-			break;	/* Once we find our entry, stop. */
-		}
+	while ((ze = getzoneent_private(cookie)) != NULL) {
+		char *name;
+		zoneid_t did;
 
-		if (strcmp(name, "global") == 0 ||
-		    lookup_zone_info(name, ZONE_ID_UNDEFINED, &zent) != Z_OK) {
-			free(name);
+		name = ze->zone_name;
+		if (strcmp(name, GLOBAL_ZONENAME) == 0 ||
+		    strcmp(name, target_zone) == 0) {
+			free(ze);
 			continue;
 		}
 
-		free(name);
-		if (zent.zdid == mydid) {
+		if (ze->zone_brand[0] == '\0') {
+			/* old, incomplete index entry */
+			zone_entry_t zent;
+
+			if (lookup_zone_info(name, ZONE_ID_UNDEFINED,
+			    &zent) != Z_OK) {
+				free(ze);
+				continue;
+			}
+			did = zent.zdid;
+		} else {
+			/* new, full index entry */
+			did = ze->zone_did;
+		}
+		free(ze);
+
+		if (did == mydid) {
 			fix = B_TRUE;
 			break;
 		}
diff --git a/usr/src/head/libzonecfg.h b/usr/src/head/libzonecfg.h
index 21ab4093a8..6adc07badc 100644
--- a/usr/src/head/libzonecfg.h
+++ b/usr/src/head/libzonecfg.h
@@ -168,6 +168,11 @@ extern "C" {
  */
 #define	ZONE_DRY_RUN		0x01
 
+typedef enum zone_iptype {
+	ZS_SHARED,
+	ZS_EXCLUSIVE
+} zone_iptype_t;
+
 /*
  * The integer field expresses the current values on a get.
  * On a put, it represents the new values if >= 0 or "don't change" if < 0.
@@ -178,6 +183,9 @@ struct zoneent {
 	char	zone_path[MAXPATHLEN];		/* path to zone storage */
 	uuid_t	zone_uuid;			/* unique ID for zone */
 	char	zone_newname[ZONENAME_MAX];	/* for doing renames */
+	char	zone_brand[MAXNAMELEN];		/* zone's brand */
+	zone_iptype_t zone_iptype;		/* zone's IP type */
+	zoneid_t zone_did;			/* persistent debug ID */
 };
 
 typedef struct zone_dochandle *zone_dochandle_t;	/* opaque handle */
@@ -281,11 +289,6 @@ typedef struct {
 	char		*zpe_vers;
 } zone_pkg_entry_t;
 
-typedef enum zone_iptype {
-	ZS_SHARED,
-	ZS_EXCLUSIVE
-} zone_iptype_t;
-
 /*
  * Basic configuration management routines.
  */
diff --git a/usr/src/lib/libzonecfg/common/getzoneent.c b/usr/src/lib/libzonecfg/common/getzoneent.c
index 2113ecd44b..c9f1c12bcf 100644
--- a/usr/src/lib/libzonecfg/common/getzoneent.c
+++ b/usr/src/lib/libzonecfg/common/getzoneent.c
@@ -128,6 +128,8 @@ getzoneent_private(FILE *cookie)
 			/* skip comment lines */
 			continue;
 		}
+
+		/* zonename */
 		p = gettok(&cp);
 		if (*p == '\0' || strlen(p) >= ZONENAME_MAX) {
 			/*
@@ -137,6 +139,7 @@ getzoneent_private(FILE *cookie)
 		}
 		(void) strlcpy(ze->zone_name, p, ZONENAME_MAX);
 
+		/* state */
 		p = gettok(&cp);
 		if (*p == '\0') {
 			/* state field should not be empty */
@@ -153,6 +156,7 @@ getzoneent_private(FILE *cookie)
 			continue;
 		}
 
+		/* zonepath */
 		p = gettok(&cp);
 		if (strlen(p) >= MAXPATHLEN) {
 			/* very long paths are not allowed */
@@ -160,10 +164,35 @@ getzoneent_private(FILE *cookie)
 		}
 		(void) strlcpy(ze->zone_path, p, MAXPATHLEN);
 
+		/* uuid */
 		p = gettok(&cp);
 		if (uuid_parse(p, ze->zone_uuid) == -1)
 			uuid_clear(ze->zone_uuid);
 
+		/* brand [optional] */
+		p = gettok(&cp);
+		if (strlen(p) >= MAXNAMELEN) {
+			/* very long names are not allowed */
+			continue;
+		}
+		(void) strlcpy(ze->zone_brand, p, MAXNAMELEN);
+
+		/* IP type [optional] */
+		p = gettok(&cp);
+		if (strlen(p) >= MAXNAMELEN) {
+			/* very long names are not allowed */
+			continue;
+		}
+		ze->zone_iptype = ZS_SHARED;
+		if (*p == 'e') {
+			ze->zone_iptype = ZS_EXCLUSIVE;
+		}
+
+		/* debug ID [optional] */
+		p = gettok(&cp);
+		if (*p != '\0')
+			ze->zone_did = atoi(p);
+
 		break;
 	}
 
@@ -294,12 +323,14 @@ putzoneent(struct zoneent *ze, zoneent_op_t operation)
 	char buf[MAX_INDEX_LEN];
 	int tmp_file_desc, lock_fd, err;
 	boolean_t exist, need_quotes;
-	char *cp;
+	char *cp, *tmpp;
 	char tmp_path[MAXPATHLEN];
 	char path[MAXPATHLEN];
 	char uuidstr[UUID_PRINTABLE_STRING_LENGTH];
 	size_t namelen;
-	const char *zone_name, *zone_state, *zone_path, *zone_uuid;
+	const char *zone_name, *zone_state, *zone_path, *zone_uuid,
+	    *zone_brand = "", *zone_iptype;
+	zoneid_t zone_did;
 
 	assert(ze != NULL);
 
@@ -352,6 +383,9 @@ putzoneent(struct zoneent *ze, zoneent_op_t operation)
 	exist = B_FALSE;
 	zone_name = ze->zone_name;
 	namelen = strlen(zone_name);
+	zone_brand = ze->zone_brand;
+	zone_iptype = (ze->zone_iptype == ZS_SHARED ? "sh" : "ex");
+	zone_did = ze->zone_did;
 	for (;;) {
 		if (fgets(buf, sizeof (buf), index_file) == NULL) {
 			if (operation == PZE_ADD && !exist) {
@@ -406,6 +440,11 @@ putzoneent(struct zoneent *ze, zoneent_op_t operation)
 		}
 		zone_path = gettok(&cp);
 		zone_uuid = gettok(&cp);
+		zone_brand = gettok(&cp);
+		zone_iptype = gettok(&cp);
+		tmpp = gettok(&cp);
+		if (*tmpp != '\0')
+			zone_did = atoi(tmpp);
 
 		switch (operation) {
 		case PZE_ADD:
@@ -434,6 +473,21 @@ putzoneent(struct zoneent *ze, zoneent_op_t operation)
 				uuid_unparse(ze->zone_uuid, uuidstr);
 				zone_uuid = uuidstr;
 			}
+
+			/* If a brand is supplied, use it. */
+			if (ze->zone_brand[0] != '\0') {
+				zone_brand = ze->zone_brand;
+
+				/*
+				 * Since the brand, iptype and did are optional,
+				 * we we only reset the iptype and did if the
+				 * brand is provided.
+				 */
+				zone_iptype = (ze->zone_iptype == ZS_SHARED ?
+				    "sh" : "ex");
+				zone_did = ze->zone_did;
+			}
+
 			break;
 
 		case PZE_REMOVE:
@@ -465,9 +519,17 @@ putzoneent(struct zoneent *ze, zoneent_op_t operation)
 		 * method for escaping them.
 		 */
 		need_quotes = (strchr(zone_path, ':') != NULL);
-		(void) fprintf(tmp_file, "%s:%s:%s%s%s:%s\n", zone_name,
-		    zone_state, need_quotes ? "\"" : "", zone_path,
-		    need_quotes ? "\"" : "", zone_uuid);
+
+		if (*zone_brand != '\0') {
+			(void) fprintf(tmp_file, "%s:%s:%s%s%s:%s:%s:%s:%d\n",
+			    zone_name, zone_state, need_quotes ? "\"" : "",
+			    zone_path, need_quotes ? "\"" : "", zone_uuid,
+			    zone_brand, zone_iptype, zone_did);
+		} else {
+			(void) fprintf(tmp_file, "%s:%s:%s%s%s:%s\n", zone_name,
+			    zone_state, need_quotes ? "\"" : "", zone_path,
+			    need_quotes ? "\"" : "", zone_uuid);
+		}
 		exist = B_TRUE;
 	}
 
diff --git a/usr/src/lib/libzonecfg/common/libzonecfg.c b/usr/src/lib/libzonecfg/common/libzonecfg.c
index f0a16df5b4..39421ff952 100644
--- a/usr/src/lib/libzonecfg/common/libzonecfg.c
+++ b/usr/src/lib/libzonecfg/common/libzonecfg.c
@@ -1130,7 +1130,7 @@ zonecfg_set_sched(zone_dochandle_t handle, char *sched)
  * In general, the operation of this function should succeed or fail as
  * a unit.
  */
-int
+static int
 zonecfg_refresh_index_file(zone_dochandle_t handle)
 {
 	char name[ZONENAME_MAX], zonepath[MAXPATHLEN];
@@ -1152,6 +1152,15 @@ zonecfg_refresh_index_file(zone_dochandle_t handle)
 	(void) strlcpy(ze.zone_path, zonepath + strlen(zonecfg_root),
 	    sizeof (ze.zone_path));
 
+	if ((err = zonecfg_get_brand(handle, ze.zone_brand,
+	    sizeof (ze.zone_brand))) != 0)
+		return (err);
+
+	if ((err = zonecfg_get_iptype(handle, &ze.zone_iptype)) != Z_OK)
+		return (err);
+
+	ze.zone_did = zonecfg_get_did(handle);
+
 	if (is_renaming(handle)) {
 		opcode = PZE_MODIFY;
 		(void) strlcpy(ze.zone_name, handle->zone_dh_delete_name,