6495328 daemon should use msync on mmap'd files

6496416 iscsitadm modify admin -d <basedir> will report error if there are targets created by zvol directly
6496424 targets can not be seen if created by zvol with shareiscsi property
6499244 libiscsitgt leaks like a sieve
6505266 SUNWiscsitgtu doesn't correctly disable target during package removal

8 files changed, 145 insertions, 24 deletions

diff --git a/usr/src/cmd/iscsi/iscsitgtd/main.c b/usr/src/cmd/iscsi/iscsitgtd/main.c
index 6faa043d1b..526d0a007e 100644
--- a/usr/src/cmd/iscsi/iscsitgtd/main.c
+++ b/usr/src/cmd/iscsi/iscsitgtd/main.c
@@ -250,7 +250,6 @@ process_target_config()
 			return (False);
 		}
 
-		targets_config = node;
 		while ((next = tgt_node_next(node, XML_ELEMENT_TARG,
 		    next)) != NULL) {
 			if (tgt_find_value_str(next, XML_ELEMENT_INAME,
@@ -272,6 +271,21 @@ process_target_config()
 		xmlTextReaderClose(r);
 		xmlFreeTextReader(r);
 		xmlCleanupParser();
+
+		/*
+		 * It's possible that nodes exist, these would be ZFS nodes.
+		 * We to move them to the new configuration tree.
+		 */
+		if (targets_config) {
+			next = NULL;
+			while ((next = tgt_node_next(targets_config,
+			    XML_ELEMENT_TARG, next)) != NULL) {
+				tgt_node_add(node, tgt_node_dup(next));
+			}
+
+			tgt_node_free(targets_config);
+		}
+		targets_config = node;
 		return (True);
 	} else {
 
@@ -821,6 +835,18 @@ main(int argc, char **argv)
 	q = queue_alloc();
 
 	/*
+	 * During the normal corse of events 'target_basedir' will be
+	 * free when the administrator changes the base directory. Instead
+	 * of trying to check for the initial case of this value being set
+	 * to some static string, always allocate that string.
+	 * NOTE: This must be done *after* process_config() is called since
+	 * it's possible and very likely that this value will be set at that
+	 * time.
+	 */
+	if (target_basedir == NULL)
+		target_basedir = strdup(DEFAULT_TARGET_BASEDIR);
+
+	/*
 	 * Initialize the various subsystems. In most cases these are
 	 * just initializing mutexs.
 	 */
diff --git a/usr/src/cmd/iscsi/iscsitgtd/mgmt_modify.c b/usr/src/cmd/iscsi/iscsitgtd/mgmt_modify.c
index 4c26205cd0..5516afda86 100644
--- a/usr/src/cmd/iscsi/iscsitgtd/mgmt_modify.c
+++ b/usr/src/cmd/iscsi/iscsitgtd/mgmt_modify.c
@@ -677,7 +677,8 @@ update_basedir(char *name, char *prop)
 {
 	tgt_node_t	*targ	= NULL;
 	int		count	= 0;
-	char		*msg	= NULL;
+	char		*msg	= NULL,
+			*val	= NULL;
 
 	if ((prop == NULL) || (strlen(prop) == 0) || (prop[0] != '/')) {
 		xml_rtn_msg(&msg, ERR_INVALID_BASEDIR);
@@ -686,7 +687,24 @@ update_basedir(char *name, char *prop)
 
 	while ((targ = tgt_node_next(targets_config, XML_ELEMENT_TARG,
 	    targ)) != NULL) {
-		count++;
+		/*
+		 * If the target does not have the "in-core" attribute, or
+		 * if it does have the attribute, but is set to "false" then
+		 * count the target.
+		 * Targets that are marked as in-core simply mean that some
+		 * other entity is storing the configuration data. Since that's
+		 * the case there's no trouble in change the base directory
+		 * because nothing will be lost.
+		 */
+		if ((tgt_find_attr_str(targ, XML_ELEMENT_INCORE, &val) ==
+		    False) ||
+		    ((val != NULL) && (strcmp(val, XML_VALUE_TRUE) != 0))) {
+			count++;
+		}
+		if (val) {
+			free(val);
+			val = NULL;
+		}
 	}
 
 	if (target_basedir == NULL) {
diff --git a/usr/src/cmd/iscsi/iscsitgtd/t10_sbc.c b/usr/src/cmd/iscsi/iscsitgtd/t10_sbc.c
index aa83824864..d0ec462c92 100644
--- a/usr/src/cmd/iscsi/iscsitgtd/t10_sbc.c
+++ b/usr/src/cmd/iscsi/iscsitgtd/t10_sbc.c
@@ -75,6 +75,7 @@ static char *sense_mode_control(t10_lu_impl_t *lu, char *buf);
 static char *sense_info_ctrl(char *buf);
 static scsi_cmd_table_t lba_table[];
 
+static long sbc_page_size;
 /*
  * []----
  * | sbc_init_common -- Initialize LU data which is common to all I_T_Ls
@@ -86,6 +87,8 @@ sbc_common_init(t10_lu_common_t *lu)
 	disk_params_t	*d;
 	tgt_node_t	*node	= lu->l_root;
 
+	sbc_page_size = sysconf(_SC_PAGESIZE);
+
 	if ((d = (disk_params_t *)calloc(1, sizeof (*d))) == NULL)
 		return (False);
 
@@ -690,6 +693,18 @@ sbc_write_data(t10_cmd_t *cmd, emul_handle_t id, size_t offset, char *data,
 			return;
 
 		if (d->d_fast_write == False) {
+			uint64_t	sa;
+			size_t		len;
+
+			/*
+			 * msync requires the address to be page aligned.
+			 * That means we need to account for any alignment
+			 * loss in the len field and access the full page.
+			 */
+			sa = (uint64_t)(intptr_t)data & ~(sbc_page_size - 1);
+			len = (((size_t)data & (sbc_page_size - 1)) +
+			    data_len + sbc_page_size - 1) &
+			    ~(sbc_page_size -1);
 
 			/*
 			 * We only need to worry about sync'ing the blocks
@@ -697,7 +712,8 @@ sbc_write_data(t10_cmd_t *cmd, emul_handle_t id, size_t offset, char *data,
 			 * enabled for AIO the file will be opened with F_SYNC
 			 * which performs the correct action.
 			 */
-			if (fsync(cmd->c_lu->l_common->l_fd) == -1) {
+			if (msync((char *)(intptr_t)sa, len, MS_SYNC) == -1) {
+				perror("msync");
 				spc_sense_create(cmd, KEY_HARDWARE_ERROR, 0);
 				trans_send_complete(cmd, STATUS_CHECK);
 				return;
@@ -803,10 +819,19 @@ send_error:
 		/*
 		 * Immediate bit is not set, so go ahead a flush things.
 		 */
-		if (fsync(cmd->c_lu->l_common->l_fd) != 0) {
-			spc_sense_create(cmd, KEY_MEDIUM_ERROR, 0);
-			trans_send_complete(cmd, STATUS_CHECK);
-			return;
+		if (cmd->c_lu->l_common->l_mmap == MAP_FAILED) {
+			if (fsync(cmd->c_lu->l_common->l_fd) != 0) {
+				spc_sense_create(cmd, KEY_MEDIUM_ERROR, 0);
+				trans_send_complete(cmd, STATUS_CHECK);
+				return;
+			}
+		} else {
+			if (msync(cmd->c_lu->l_common->l_mmap,
+			    cmd->c_lu->l_common->l_size, MS_SYNC) == -1) {
+				spc_sense_create(cmd, KEY_MEDIUM_ERROR, 0);
+				trans_send_complete(cmd, STATUS_CHECK);
+				return;
+			}
 		}
 	}
 	trans_send_complete(cmd, STATUS_GOOD);
@@ -1086,21 +1111,45 @@ sbc_synccache(t10_cmd_t *cmd, uint8_t *cdb, size_t cdb_len)
 
 			/*
 			 * Immediately return a status of GOOD. If an error
-			 * occurs with the fsync the next command will pick
-			 * up an error.
+			 * occurs with the fsync/msync the next command will
+			 * pick up an error.
 			 */
 			trans_send_complete(cmd, STATUS_GOOD);
-			if (fsync(cmd->c_lu->l_common->l_fd) == -1) {
-				cmd->c_lu->l_status	= KEY_HARDWARE_ERROR;
-				cmd->c_lu->l_asc	= 0x00;
-				cmd->c_lu->l_ascq	= 0x00;
+			if (cmd->c_lu->l_common->l_mmap == MAP_FAILED) {
+				if (fsync(cmd->c_lu->l_common->l_fd) == -1) {
+					cmd->c_lu->l_status =
+					    KEY_HARDWARE_ERROR;
+					cmd->c_lu->l_asc = 0x00;
+					cmd->c_lu->l_ascq = 0x00;
+				}
+			} else {
+				if (msync(cmd->c_lu->l_common->l_mmap,
+				    cmd->c_lu->l_common->l_size, MS_SYNC) ==
+				    -1) {
+					cmd->c_lu->l_status =
+					    KEY_HARDWARE_ERROR;
+					cmd->c_lu->l_asc = 0x00;
+					cmd->c_lu->l_ascq = 0x00;
+				}
 			}
 		} else {
-			if (fsync(cmd->c_lu->l_common->l_fd) == -1) {
-				spc_sense_create(cmd, KEY_HARDWARE_ERROR, 0);
-				trans_send_complete(cmd, STATUS_CHECK);
-			} else
-				trans_send_complete(cmd, STATUS_GOOD);
+			if (cmd->c_lu->l_common->l_mmap == MAP_FAILED) {
+				if (fsync(cmd->c_lu->l_common->l_fd) == -1) {
+					spc_sense_create(cmd,
+					    KEY_HARDWARE_ERROR, 0);
+					trans_send_complete(cmd, STATUS_CHECK);
+				} else
+					trans_send_complete(cmd, STATUS_GOOD);
+			} else {
+				if (msync(cmd->c_lu->l_common->l_mmap,
+				    cmd->c_lu->l_common->l_size, MS_SYNC) ==
+				    -1) {
+					spc_sense_create(cmd,
+					    KEY_HARDWARE_ERROR, 0);
+					trans_send_complete(cmd, STATUS_CHECK);
+				} else
+					trans_send_complete(cmd, STATUS_GOOD);
+			}
 		}
 	}
 }
diff --git a/usr/src/cmd/iscsi/iscsitgtd/t10_ssc.c b/usr/src/cmd/iscsi/iscsitgtd/t10_ssc.c
index c15ce6e14f..dfa83a34a0 100644
--- a/usr/src/cmd/iscsi/iscsitgtd/t10_ssc.c
+++ b/usr/src/cmd/iscsi/iscsitgtd/t10_ssc.c
@@ -63,6 +63,8 @@ static uint32_t find_last_obj_id(char *file_mark, off_t eod);
 static char *sense_dev_config(ssc_params_t *s, char *data);
 static char *sense_compression(ssc_params_t *s, char *data);
 
+static long ssc_page_size;
+
 /*
  * []----
  * | ssc_init_common -- initialize common information that all ITLs will use
@@ -74,6 +76,8 @@ ssc_common_init(t10_lu_common_t *lu)
 	ssc_params_t	*s;
 	ssc_obj_mark_t	mark;
 
+	ssc_page_size = sysconf(_SC_PAGESIZE);
+
 	if (lu->l_mmap == MAP_FAILED)
 		return (False);
 
@@ -474,13 +478,27 @@ ssc_write_data(t10_cmd_t *cmd, emul_handle_t id, size_t offset, char *data,
 		return;
 
 	if (s->s_fast_write_ack == False) {
+		uint64_t	sa;
+		size_t		len;
+
+		/*
+		 * msync requires the address to be page aligned.
+		 * That means we need to account for any alignment
+		 * loss in the len field and access the full page.
+		 */
+		sa = (uint64_t)(intptr_t)data & ~(ssc_page_size - 1);
+		len = (((size_t)data & (ssc_page_size - 1)) +
+		    data_len + ssc_page_size - 1) &
+		    ~(ssc_page_size -1);
+
 		/*
 		 * We only need to worry about sync'ing the blocks
 		 * in the mmap case because if the fast cache isn't
 		 * enabled for AIO the file will be opened with F_SYNC
 		 * which performs the correct action.
 		 */
-		if (fsync(cmd->c_lu->l_common->l_fd) == -1) {
+		if (msync((char *)(intptr_t)sa, len, MS_SYNC) == -1) {
+			perror("msync");
 			spc_sense_create(cmd, KEY_HARDWARE_ERROR, 0);
 			trans_send_complete(cmd, STATUS_CHECK);
 			return;
diff --git a/usr/src/cmd/iscsi/iscsitgtd/target.h b/usr/src/cmd/iscsi/iscsitgtd/target.h
index bdeef64724..7eca879db4 100644
--- a/usr/src/cmd/iscsi/iscsitgtd/target.h
+++ b/usr/src/cmd/iscsi/iscsitgtd/target.h
@@ -37,8 +37,9 @@
 extern "C" {
 #endif
 
-#define	DEFAULT_CONFIG_LOCATION	"/etc/iscsi/target_config.xml"
-#define	DEFAULT_TARGET_BASEDIR	"/iscsi_targets"
+#define	DEFAULT_CONFIG		"/etc/iscsi/"
+#define	DEFAULT_CONFIG_LOCATION	DEFAULT_CONFIG "target_config.xml"
+#define	DEFAULT_TARGET_BASEDIR	DEFAULT_CONFIG
 #define	DEFAULT_TARGET_LOG	"/tmp/target_log"
 
 /*
diff --git a/usr/src/lib/libiscsitgt/common/if_zfs.c b/usr/src/lib/libiscsitgt/common/if_zfs.c
index ef26a8fe7d..b91337292e 100644
--- a/usr/src/lib/libiscsitgt/common/if_zfs.c
+++ b/usr/src/lib/libiscsitgt/common/if_zfs.c
@@ -58,11 +58,13 @@ iscsitgt_zfs_share(const char *dataset)
 	if (strcmp(n->x_name, XML_ELEMENT_ERROR) == 0 &&
 	    tgt_find_value_int(n, XML_ELEMENT_CODE, &code) == True &&
 	    code == 1000) {
+		free(str);
 		tgt_node_free(n);
 		return (0);
 	}
 
 	errno = EINVAL;
+	free(str);
 	tgt_node_free(n);
 	return (-1);
 }
@@ -87,10 +89,12 @@ iscsitgt_zfs_unshare(const char *dataset)
 	if (strcmp(n->x_name, XML_ELEMENT_ERROR) == 0 &&
 	    tgt_find_value_int(n, XML_ELEMENT_CODE, &code) == True &&
 	    code == 1000) {
+		free(str);
 		tgt_node_free(n);
 		return (0);
 	}
 
+	free(str);
 	tgt_node_free(n);
 	errno = EINVAL;
 	return (-1);
@@ -114,10 +118,12 @@ iscsitgt_zfs_is_shared(const char *dataset)
 	if (strcmp(n->x_name, XML_ELEMENT_ERROR) == 0 &&
 	    tgt_find_value_int(n, XML_ELEMENT_CODE, &code) == True &&
 	    code == 1000) {
+		free(str);
 		tgt_node_free(n);
 		return (1);
 	}
 
+	free(str);
 	tgt_node_free(n);
 	return (0);
 }
diff --git a/usr/src/lib/libiscsitgt/common/xml.c b/usr/src/lib/libiscsitgt/common/xml.c
index 7f59ae219e..9a4a540572 100644
--- a/usr/src/lib/libiscsitgt/common/xml.c
+++ b/usr/src/lib/libiscsitgt/common/xml.c
@@ -457,6 +457,7 @@ tgt_node_add(tgt_node_t *p, tgt_node_t *c)
 	if ((p == NULL) || (c == NULL))
 		return;
 
+	c->x_parent = p;
 	if (p->x_child == NULL)
 		p->x_child = c;
 	else {
@@ -683,10 +684,12 @@ tgt_node_dup(tgt_node_t *n)
 		return (NULL);
 	if (node_name(d, (xmlChar *)n->x_name) == False)
 		return (NULL);
-	if (node_value(d, (xmlChar *)n->x_value, True) == False)
+	if (n->x_value && (node_value(d, (xmlChar *)n->x_value, True) == False))
 		return (NULL);
 	for (c = n->x_child; c; c = c->x_sibling)
 		(void) tgt_node_add(d, tgt_node_dup(c));
+	for (c = n->x_attr; c; c = c->x_sibling)
+		(void) tgt_node_add_attr(d, tgt_node_dup(c));
 	return (d);
 }
 
diff --git a/usr/src/pkgdefs/SUNWiscsitgtu/preremove b/usr/src/pkgdefs/SUNWiscsitgtu/preremove
index 9ed9f3d8e4..3fbda1fdda 100644
--- a/usr/src/pkgdefs/SUNWiscsitgtu/preremove
+++ b/usr/src/pkgdefs/SUNWiscsitgtu/preremove
@@ -45,7 +45,7 @@ SERVICE="svc:/system/iscsitgt:default"
 # Check to see if the service is enabled and if so disable it.
 #
 
-SVCPROP=`svcprop -p general/enabled ${SERVICE}`
+SVCPROP=`svcprop -c -p general/enabled ${SERVICE}`
 
 if [ "${SVCPROP}" = "true" ]; then
     svcadm disable ${SERVICE}