6709366 After changing the iSNS server name, the iSCSI Target requires a restart for it to be used

2 files changed, 266 insertions, 153 deletions

diff --git a/usr/src/cmd/iscsi/iscsitgtd/isns_client.c b/usr/src/cmd/iscsi/iscsitgtd/isns_client.c
index 9f260d9bee..c16f59adb9 100644
--- a/usr/src/cmd/iscsi/iscsitgtd/isns_client.c
+++ b/usr/src/cmd/iscsi/iscsitgtd/isns_client.c
@@ -70,14 +70,14 @@ extern target_queue_t	*mgmtq;
  * eid_ip: Entity IP info
  */
 static	int scn_port = 0;
-static	esi_scn_arg_t	isns_args;
+static	esi_scn_arg_t	isns_args = {{0}, {0}, 0};
 static	ip_t	eid_ip;
 static	int	num_reg = 0;
 static	pthread_t	scn_tid = 0;
+static	pthread_t	isns_tid = 0;
 static	Boolean_t	isns_shutdown = False;
-
-Boolean_t	isns_initialized = False;
-sema_t		isns_sema;
+static int ISNS_SLEEP_SECS = 20;
+Boolean_t	isns_server_connection_thr_running = False;
 target_queue_t	*mgmtq = NULL;
 
 static	int	get_ip_addr(char *node, ip_t *sa);
@@ -93,6 +93,8 @@ static	int	isns_scn_reg(int, char *);
 static	int	isns_scn_dereg(int so, char *node);
 static	tgt_node_t	*find_tgt_by_name(char *, char **);
 static	tgt_node_t	*find_next_tgt(tgt_node_t *, char **);
+static int isns_populate_and_update_server_info(Boolean_t state);
+static int get_addr_family(char *node);
 
 /*
  * find_tgt_by_name searches DB by iscsi name or local name, if found
@@ -351,60 +353,120 @@ process_esi(int so, isns_pdu_t *esi)
 	isns_free_pdu(cmd);
 }
 
-/*
- * esi_scn_thr() is the thread creates an end point to receive and process
- * ESI & SCN messages.  This thread is created when isns_access is enabled
- * and for the duration of the iscsi daemon
- */
-static void
-*esi_scn_thr(void *arg)
-{
-	struct sockaddr		sa, *ai;
-	struct sockaddr_in	sin;
-	struct sockaddr_in6	sin6;
-	int			so, fd;
+static int
+is_isns_server_up(char *server) {
+	int			so;
 	socklen_t		len;
-	char			strport[NI_MAXSERV];
-	int	pf;
-	esi_scn_arg_t		*args = (esi_scn_arg_t *)arg;
-	isns_pdu_t		*scn = NULL;
+	struct sockaddr		sa;
 
+	/* no server specified */
+	if (server == NULL) {
+		return (-1);
+	}
 	/*
-	 * open isns server connect and determine which PF_INET
-	 * to use
+	 * open isns server connect and determine which PF_INET to use
 	 */
-	if ((so = isns_open(args->server)) < 0) {
+	if ((so = isns_open(server)) < 0) {
 		syslog(LOG_ERR,
-		    "esi_scn_thr: isns server %s not found", args->server);
-		(void) sema_post(&isns_sema);
-		return (NULL);
+		    "isns server %s not found",
+			server);
+		return (-1);
 	}
 	len = sizeof (sa);
 	if (getsockname(so, &sa, &len) < 0) {
 		isns_close(so);
-		syslog(LOG_ALERT, "getsockname failed");
-		(void) sema_post(&isns_sema);
-		return (NULL);
+		syslog(LOG_ALERT,
+			"isns getsockname failed");
+		return (-1);
 	}
-	pf = sa.sa_family;
 	isns_close(so);
 
-	if (pf != PF_INET && pf != PF_INET6) {
-		syslog(LOG_ERR, "esi_scn_thr: unknown domain type");
-		(void) sema_post(&isns_sema);
-		return (NULL);
+	if (sa.sa_family != PF_INET &&
+		sa.sa_family != PF_INET6) {
+		syslog(LOG_ERR,
+			"isns unknown domain type");
+		return (-1);
 	}
+	return (0);
+}
+
+/*
+ * This thread sit's in a loop and ensures that it keeps checking for
+ * connection to isns_server. Once the connection works it registers
+ * with the isns and bails out.
+ * We expect the isns server to be fault taulerant and has persistence
+ * for the registered entries.
+ */
+static void *
+isns_server_connection_thr(void *arg)
+{
+	char *server = isns_args.server;
+	Boolean_t registered_targets = False;
+
+	while (isns_shutdown == False) {
+
+		if (is_isns_server_up(server) == 0) {
+			if (registered_targets == False) {
+				/*
+				 * register all targets, what happens if
+				 * no targets are created yet? this should
+				 * not be a failure, when new target gets
+				 * created, update gets call. what if SCN
+				 * register fails?
+				 */
+				if (isns_reg_all() == 0) {
+					/* scn register all targets */
+					if (isns_op_all(ISNS_SCN_REG) != 0) {
+						syslog(LOG_ERR,
+						    "SCN registrations"
+						    " failed\n");
+						(void) isns_op_all(
+						    ISNS_DEV_DEREG);
+						registered_targets = False;
+					} else {
+						registered_targets = True;
+						break;
+					}
+				}
+			}
+		} else {
+			syslog(LOG_INFO,
+			    "isns server %s is not reachable", server);
+			registered_targets = False;
+		}
+		(void) sleep(ISNS_SLEEP_SECS);
+		/* If isns was disabled, deregister and close the thread */
+		if (isns_enabled() == False) {
+			syslog(LOG_INFO,
+			    "isns server is disabled, dergister target");
+			isns_fini();
+			return (NULL);
+		}
 
-	/*
-	 * create and bind SCN socket
-	 * save the scn port info
-	 */
-	if ((so = socket(pf, SOCK_STREAM, 0)) == -1) {
-		syslog(LOG_ALERT, "socket failed");
-		(void) sema_post(&isns_sema);
-		return (NULL);
 	}
+	return (NULL);
+}
+
+/*
+ * esi_scn_thr() is the thread creates an end point to receive and process
+ * ESI & SCN messages.  This thread is created when isns_access is enabled
+ * and for the duration of the iscsi daemon
+ */
+static void *
+esi_scn_thr(void *arg)
+{
+	struct sockaddr		sa, *ai;
+	struct sockaddr_in	sin;
+	struct sockaddr_in6	sin6;
+	int			so, fd, pf;
+	socklen_t		len;
+	char			strport[NI_MAXSERV];
+	isns_pdu_t		*scn = NULL;
+	struct timeval timeout;
+	fd_set fdset;
+	int socket_ready = 0;
 
+	pf = get_addr_family(isns_args.entity);
 	if (pf == PF_INET) {
 		bzero(&sin, sizeof (sin));
 		sin.sin_family = PF_INET;
@@ -412,52 +474,89 @@ static void
 		sin.sin_addr.s_addr = INADDR_ANY;
 		ai = (struct sockaddr *)&sin;
 		len = sizeof (sin);
-	} else {
+	} else if (pf == PF_INET6) {
 		bzero(&sin6, sizeof (sin6));
 		sin6.sin6_family = PF_INET6;
 		sin6.sin6_port = htons(0);
 		sin6.sin6_addr = in6addr_any;
 		ai = (struct sockaddr *)&sin6;
 		len = sizeof (sin6);
+	} else {
+		syslog(LOG_ERR, "Bad address family. Exit esi_scn_thr");
+		return (NULL);
+	}
+
+	/*
+	 * create and bind SCN socket
+	 * save the scn port info
+	 */
+	if ((so = socket(pf, SOCK_STREAM, 0)) == -1) {
+		syslog(LOG_ALERT, "create isns socket failed");
+		return (NULL);
 	}
 
 	(void) setsockopt(so, SOL_SOCKET, SO_REUSEADDR, 0, 0);
 
 	if (bind(so, ai, len) < 0) {
 		syslog(LOG_ALERT, "esi_scn_thr: bind failed");
-		(void) sema_post(&isns_sema);
+		(void) close(so);
 		return (NULL);
 	}
 
 	/* get scn port info */
 	len = sizeof (sa);
 	if (getsockname(so, &sa, &len) < 0) {
-		syslog(LOG_ALERT, "getsockname failed");
-		(void) sema_post(&isns_sema);
+		syslog(LOG_ALERT, "isns getsockname failed");
+		(void) close(so);
 		return (NULL);
 	}
 	if (getnameinfo(&sa, len, NULL, 0, strport, NI_MAXSERV,
 	    NI_NUMERICSERV) != 0) {
-		syslog(LOG_ALERT, "getnameinfo failed");
-		(void) sema_post(&isns_sema);
+		syslog(LOG_ALERT, "isns getnameinfo failed");
+		(void) close(so);
 		return (NULL);
 	}
 	scn_port = atoi(strport);
 
-	(void) sema_post(&isns_sema);
 
 	if (listen(so, 5) < 0) {
 		syslog(LOG_ALERT, "esi_scn_thr: failed listen");
+		(void) close(so);
 		return (NULL);
 	}
 
 	/* listen for esi or scn messages */
 	while (isns_shutdown == False) {
-		if ((fd = accept(so, &sa, &len)) < 0) {
-			syslog(LOG_ALERT, "esi_scn_thr: failed accept");
-			continue;
+		/* ISNS_ESI_INTERVAL_ATTR_ID is set to 10s */
+		timeout.tv_sec = 10;
+		timeout.tv_usec = 0;
+		FD_ZERO(&fdset);
+		FD_SET(so, &fdset);
+
+		socket_ready = select(so + 1, &fdset, NULL, NULL, &timeout);
+
+		/* If disabled bail out, dont care about packets */
+		if (isns_enabled() == False) {
+			syslog(LOG_INFO,
+			    "isns server is disabled, dergister target");
+			isns_fini();
+			(void) close(so);
+			return (NULL);
 		}
 
+		if (socket_ready < 0) {
+			syslog(LOG_ERR,
+			    "esi_scn_thr: select failed, retrying.");
+			continue;
+		} else if (socket_ready == 0) { /* timeout */
+			continue;
+		} else {
+			/* Socket is ready */
+			if ((fd = accept(so, &sa, &len)) < 0) {
+				syslog(LOG_ALERT, "esi_scn_thr: failed accept");
+				continue;
+			}
+		}
 		if (isns_recv(fd, (isns_rsp_t **)&scn) == 0) {
 			/* Just return success for ESI */
 			switch (scn->func_id) {
@@ -482,6 +581,7 @@ static void
 
 		(void) close(fd);
 	}
+	(void) close(so);
 	return (NULL);
 }
 
@@ -536,32 +636,24 @@ isns_op_all(uint16_t op)
 
 		free(iname);
 	}
-
 	isns_close(so);
 	return (0);
 }
 
-/*
- * isns_init() needs to be call before all ISNS operations.
- * Save the isns_server & entity name.
- * Start esi_scn_thr to receive ESI & SCN messages
- */
-int
-isns_init(target_queue_t *q)
-{
+static int
+isns_populate_and_update_server_info(Boolean_t update) {
 	char		*isns_srv, *isns_port;
-
-	if (q != NULL)
-		mgmtq = q;
-
-	if (isns_enabled() == False)
-		return (0);
-
-	/* initialize */
-	(void) sema_init(&isns_sema, 0, USYNC_THREAD, NULL);
+	int retcode = 0;
 
 	/* get isns server info */
 	tgt_find_value_str(main_config, XML_ELEMENT_ISNS_SERV, &isns_srv);
+	if (isns_srv == NULL) {
+		syslog(LOG_INFO,
+		    "The server has not been setup, "
+		    "but enabling the isns access");
+		retcode = -1;
+		return (retcode);
+	}
 	isns_port = strchr(isns_srv, ':');
 	if (isns_port == NULL) {
 		isns_args.isns_port = ISNS_DEFAULT_SERVER_PORT;
@@ -572,43 +664,72 @@ isns_init(target_queue_t *q)
 		}
 		*isns_port = '\0';
 	}
-	bcopy(isns_srv, isns_args.server, MAXHOSTNAMELEN);
+
+	if (update == True) {
+		/* isns_server changed */
+		if (strcmp(isns_srv, isns_args.server) != 0) {
+			/* de-reg from old iSNS server if it is setup */
+			syslog(LOG_INFO,
+			    "Detected a new isns server, deregistering"
+			    " %s", isns_args.server);
+			(void) isns_dereg_all();
+			bcopy(isns_srv, isns_args.server, MAXHOSTNAMELEN);
+			/* Register with the new server */
+			if (isns_reg_all() == 0) {
+				/* scn register all targets */
+				if (isns_op_all(ISNS_SCN_REG) != 0) {
+					syslog(LOG_ERR,
+					    "SCN registrations failed\n");
+					(void) isns_op_all(ISNS_DEV_DEREG);
+					retcode = -1;
+				}
+			}
+		}
+	} else {
+		bcopy(isns_srv, isns_args.server, MAXHOSTNAMELEN);
+	}
 	free(isns_srv);
+	return (retcode);
+}
+
+/*
+ * isns_init() needs to be call before all ISNS operations.
+ * Save the isns_server & entity name.
+ * Start esi_scn_thr to receive ESI & SCN messages
+ */
+int
+isns_init(target_queue_t *q)
+{
+	if (q != NULL)
+		mgmtq = q;
+
+	if (isns_enabled() == False)
+		return (0);
 
 	/* get local hostname for entity usage */
 	if ((gethostname(isns_args.entity, MAXHOSTNAMELEN) < 0) ||
 	    (get_ip_addr(isns_args.entity, &eid_ip) < 0)) {
-		syslog(LOG_ERR, "ISNS fails to get ENTITY properties");
-		return (-1);
+			syslog(LOG_ERR, "ISNS fails to get ENTITY properties");
+			return (-1);
 	}
-
-	if (pthread_create(&scn_tid, NULL, esi_scn_thr, (void *)&isns_args) !=
+	(void) isns_populate_and_update_server_info(False);
+	if (pthread_create(&scn_tid, NULL,
+	    esi_scn_thr, (void *)&isns_args) !=
 	    0) {
 		syslog(LOG_ALERT, "isns_init failed to pthread_create");
+		pthread_kill(isns_tid, SIGKILL);
 		return (-1);
 	}
 
-	if (sema_wait(&isns_sema) != 0) {
-		syslog(LOG_ERR, "sema_wait error\n");
-	}
-
-	isns_initialized = True;
-
-	/*
-	 * register all targets, what happens if no targets are created yet?
-	 * this should not be a failure, when new target gets created, update
-	 * gets call.
-	 * what if SCN register fails?
-	 */
-	if (isns_reg_all() == 0) {
-		/* scn register all targets */
-		if (isns_op_all(ISNS_SCN_REG) != 0) {
-			syslog(LOG_ERR, "SCN registrations failed\n");
-			(void) isns_op_all(ISNS_DEV_DEREG);
-			return (-1);
-		}
+	if (pthread_create(&isns_tid, NULL, isns_server_connection_thr,
+	    (void *)NULL) != 0) {
+		syslog(LOG_ALERT,
+		    "isns_init failed to create the "
+		    "isns connection thr");
+		return (-1);
 	}
 
+	isns_server_connection_thr_running = True;
 	return (0);
 }
 
@@ -619,69 +740,44 @@ isns_init(target_queue_t *q)
 int
 isns_update()
 {
-	char	*isns_srv = NULL, *isns_port;
-
-	if (isns_initialized == False) {
-		/* isns enabled after iscsi started */
-		if (isns_enabled() == True) {
-			(void) isns_init(NULL);
-		}
-		return (0);
-	}
-
+	Boolean_t is_isns_enabled = isns_enabled();
 	/*
-	 * isns is disabled after enabled,
-	 * log off all targets and fini isns service
+	 * If the isns thread was not started before and we are going
+	 * enabled from disabled start the threads.
 	 */
-	if (isns_initialized == True && isns_enabled() == False) {
-		isns_fini();
-		return (0);
-	}
-
-	/*
-	 * isns is already initialized and isns_access is still enabled,
-	 * let's update the isns_server name
-	 */
-	tgt_find_value_str(main_config, XML_ELEMENT_ISNS_SERV, &isns_srv);
-	isns_port = strchr(isns_srv, ':');
-	if (isns_port == NULL) {
-		isns_args.isns_port = ISNS_DEFAULT_SERVER_PORT;
-	} else {
-		isns_args.isns_port = strtoul(isns_port + 1, NULL, 0);
-		if (isns_args.isns_port == 0) {
-			isns_args.isns_port = ISNS_DEFAULT_SERVER_PORT;
+	if (isns_server_connection_thr_running == False) {
+		if (is_isns_enabled == True) {
+			(void) isns_init(NULL);
+			return (0);
+		} else {
+			syslog(LOG_INFO,
+			    "isns_update: isns is disabled");
 		}
-		*isns_port = '\0';
-	}
-
-	/*
-	 * If the iSNS server is different, then dregister all from
-	 * the old iSNS server, and register all to the new iSNS server
-	 */
-
-	if (strcmp(isns_srv, isns_args.server) != 0) {
-		/* de-reg from old iSNS server */
-		(void) isns_dereg_all();
-
-		bcopy(isns_srv, isns_args.server, MAXHOSTNAMELEN);
-
-		if (isns_reg_all() == 0) {
-			/* scn register all targets */
-			if (isns_op_all(ISNS_SCN_REG) != 0) {
-				syslog(LOG_ERR, "SCN registrations failed\n");
-				(void) isns_op_all(ISNS_DEV_DEREG);
+	} else {
+		/*
+		 * isns is disabled after enabled,
+		 * log off all targets and fini isns service
+		 */
+		if (is_isns_enabled == False) {
+			/* pthread_join for the isns thread */
+			pthread_join(isns_tid, NULL);
+			pthread_join(scn_tid, NULL);
+			isns_server_connection_thr_running = False;
+		} else {
+			/*
+			 * Read the configuration file incase the server
+			 * has changed.
+			 */
+			if (isns_populate_and_update_server_info(True) == -1) {
 				return (-1);
 			}
 		}
 	}
-	if (isns_srv != NULL)
-		free(isns_srv);
-
 	return (0);
 }
 
 /*
- * isns_fini is called when isns access is disable
+ * isns_fini is called when isns access is disabled
  */
 void
 isns_fini()
@@ -695,8 +791,17 @@ isns_fini()
 
 	/* log off all targets */
 	(void) isns_op_all(ISNS_TGT_LOGOUT);
+}
 
-	isns_initialized = False;
+static int
+get_addr_family(char *node) {
+	struct addrinfo		*ai = NULL;
+
+	if (getaddrinfo(node, NULL, NULL, &ai) != 0) {
+		syslog(LOG_ALERT, "get_addr_family: server %s not found", node);
+		return (-1);
+	}
+	return (ai->ai_family);
 }
 
 static int
@@ -707,7 +812,7 @@ get_ip_addr(char *node, ip_t *sa)
 	struct sockaddr_in6	*sin6;
 
 	if (getaddrinfo(node, NULL, NULL, &ai) != 0) {
-		syslog(LOG_ALERT, "ISNS server not found");
+		syslog(LOG_ALERT, "get_ip_addr: server %s not found", node);
 		return (-1);
 	}
 
@@ -1245,7 +1350,8 @@ isns_reg(char *targ)
 	char		*iqn;
 
 	if ((so = isns_open(isns_args.server)) == -1) {
-		syslog(LOG_ERR, "isns_reg failed");
+		syslog(LOG_ERR, "isns_reg failed with server: %s",
+		    isns_args.server);
 		return (-1);
 	}
 
@@ -1305,7 +1411,6 @@ isns_reg_all()
 	}
 	(void) strcpy(iname, n);
 	free(n);
-
 	if ((so = isns_open(isns_args.server)) == -1) {
 		syslog(LOG_ALERT, "ISNS: fails to connect to %s\n",
 		    isns_args.server);
diff --git a/usr/src/cmd/iscsi/iscsitgtd/mgmt_modify.c b/usr/src/cmd/iscsi/iscsitgtd/mgmt_modify.c
index c0d6058578..71678cf55d 100644
--- a/usr/src/cmd/iscsi/iscsitgtd/mgmt_modify.c
+++ b/usr/src/cmd/iscsi/iscsitgtd/mgmt_modify.c
@@ -567,10 +567,12 @@ modify_admin(tgt_node_t *x)
 	char		*msg	= NULL;
 	char		*prop;
 	Boolean_t	changes_made = False;
+	Boolean_t	update_isns = False;
 	admin_table_t	*ap;
 
 	for (ap = admin_prop_list; ap->name; ap++) {
 		if (tgt_find_value_str(x, ap->name, &prop) == True) {
+
 			if ((prop == NULL) || (strlen(prop) == 0))
 				break;
 
@@ -607,6 +609,10 @@ modify_admin(tgt_node_t *x)
 					return (msg);
 				}
 			}
+			if (0 == strcmp(ap->name, XML_ELEMENT_ISNS_ACCESS) ||
+			    0 == strcmp(ap->name, XML_ELEMENT_ISNS_SERV)) {
+				update_isns = True;
+			}
 			free(prop);
 			changes_made = True;
 		}
@@ -614,9 +620,11 @@ modify_admin(tgt_node_t *x)
 
 	if (changes_made == True) {
 		/* isns_update updates isns_access & isns server name */
-		if (isns_update() != 0) {
-			xml_rtn_msg(&msg, ERR_ISNS_ERROR);
-			return (msg);
+		if (update_isns == True) {
+			if (isns_update() != 0) {
+				xml_rtn_msg(&msg, ERR_ISNS_ERROR);
+				return (msg);
+			}
 		}
 		if (mgmt_config_save2scf() == True) {
 			xml_rtn_msg(&msg, ERR_SUCCESS);