Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 8 Mar 2012 01:22:03 GMT
From:      Xin LI <delphij@FreeBSD.org>
To:        FreeBSD-gnats-submit@FreeBSD.org
Cc:        aoyama@peach.ne.jp
Subject:   ports/165844: [PATCH] reload support for net/istgt
Message-ID:  <201203080122.q281M3lh054796@freefall.freebsd.org>
Resent-Message-ID: <201203080130.q281UAVM054979@freefall.freebsd.org>

next in thread | raw e-mail | index | archive | help

>Number:         165844
>Category:       ports
>Synopsis:       [PATCH] reload support for net/istgt
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    freebsd-ports-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          change-request
>Submitter-Id:   current-users
>Arrival-Date:   Thu Mar 08 01:30:10 UTC 2012
>Closed-Date:
>Last-Modified:
>Originator:     Xin LI
>Release:        FreeBSD 8.2-STABLE i386
>Organization:
The FreeBSD Project
>Environment:
System: FreeBSD freefall.freebsd.org 8.2-STABLE FreeBSD 8.2-STABLE #5 r227907: Wed Nov 23 21:55:50 UTC 2011 simon@freefall.freebsd.org:/usr/obj/usr/src/sys/FREEFALL i386


>Description:
	Currently istgt does not support on-the-fly reloading of its configurations,
which means the administrator has to restart the daemon to make new settings in
effect.

	This is quite inconvenient when istgt is used in conjection with some "sensitive"
initiators, for instance, Xen.
>How-To-Repeat:
	Do active read-writes from a Xen client and restart istgt.
>Fix:

	The proposed patch adds a new 'reload' verb to the rc.d script.

	The rest of the patch that implements the underlying functionality
was written by Kun Huang, sponsored by Applied Operations, LLC.

--- istgt.diff begins here ---
Index: Makefile
===================================================================
RCS file: /home/ncvs/ports/net/istgt/Makefile,v
retrieving revision 1.28
diff -u -r1.28 Makefile
--- Makefile	9 Oct 2011 17:04:02 -0000	1.28
+++ Makefile	5 Mar 2012 21:36:01 -0000
@@ -7,6 +7,7 @@
 
 PORTNAME=	istgt
 PORTVERSION=	20111008
+PORTREVISION=	1
 CATEGORIES=	net
 MASTER_SITES=	http://www.peach.ne.jp/archives/istgt/
 
Index: files/patch-istgt-rc
===================================================================
RCS file: files/patch-istgt-rc
diff -N files/patch-istgt-rc
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ files/patch-istgt-rc	8 Mar 2012 01:13:29 -0000
@@ -0,0 +1,25 @@
+--- etc/istgt.sh.in.orig	2011-08-31 11:53:08.000000000 -0700
++++ etc/istgt.sh.in	2012-03-07 17:12:49.615160257 -0800
+@@ -9,6 +9,7 @@
+ 
+ name="istgt"
+ rcvar=`set_rcvar`
++reload_cmd="istgt_reload"
+ extra_commands="reload"
+ 
+ load_rc_config $name
+@@ -18,6 +19,14 @@
+ : ${istgt_pidfile="/var/run/istgt.pid"}
+ : ${istgt_flags=""}
+ 
++#
++# Refresh configuration
++#
++istgt_reload()
++{
++	%%BINDIR%%/istgtcontrol refresh
++}
++
+ required_files="${istgt_config}"
+ pidfile="${istgt_pidfile}"
+ command="%%BINDIR%%/istgt"
Index: files/patch-istgt-reload
===================================================================
RCS file: files/patch-istgt-reload
diff -N files/patch-istgt-reload
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ files/patch-istgt-reload	5 Mar 2012 21:36:01 -0000
@@ -0,0 +1,438 @@
+diff --git src/istgt.c src/istgt.c
+index e62de97..8fd509e 100644
+--- src/istgt.c
++++ src/istgt.c
+@@ -71,6 +71,7 @@
+ #define PORTNUMLEN 32
+ 
+ ISTGT g_istgt;
++const char  *g_config_file;
+ 
+ #if 0
+ void
+@@ -1598,6 +1599,7 @@ main(int argc, char **argv)
+ 
+ 	/* read config files */
+ 	config = istgt_allocate_config();
++	g_config_file = config_file;
+ 	rc = istgt_read_config(config, config_file);
+ 	if (rc < 0) {
+ 		fprintf(stderr, "config error\n");
+@@ -1662,7 +1664,7 @@ main(int argc, char **argv)
+ 		istgt_free_config(config);
+ 		exit(EXIT_FAILURE);
+ 	}
+-	rc = istgt_lu_init(istgt);
++	rc = istgt_lu_init(istgt, 0);
+ 	if (rc < 0) {
+ 		ISTGT_ERRLOG("istgt_lu_init() failed\n");
+ 		goto initialize_error;
+@@ -1772,7 +1774,7 @@ main(int argc, char **argv)
+ #endif
+ 
+ 	/* create LUN threads for command queuing */
+-	rc = istgt_lu_create_threads(istgt);
++	rc = istgt_lu_create_threads(istgt, 0);
+ 	if (rc < 0) {
+ 		ISTGT_ERRLOG("lu_create_threads() failed\n");
+ 		goto initialize_error;
+@@ -1809,7 +1811,7 @@ main(int argc, char **argv)
+ 		istgt_close_portal(istgt);
+ 		istgt_close_uctl_portal(istgt);
+ 		istgt_iscsi_shutdown(istgt);
+-		istgt_lu_shutdown(istgt);
++		istgt_lu_shutdown(istgt, 0);
+ 		istgt_destory_initiator_group_array(istgt);
+ 		istgt_destroy_portal_array(istgt);
+ 		istgt_destroy_uctl_portal(istgt);
+@@ -1837,7 +1839,7 @@ main(int argc, char **argv)
+ 	istgt_close_portal(istgt);
+ 	istgt_close_uctl_portal(istgt);
+ 	istgt_iscsi_shutdown(istgt);
+-	istgt_lu_shutdown(istgt);
++	istgt_lu_shutdown(istgt, 0);
+ 	istgt_destory_initiator_group_array(istgt);
+ 	istgt_destroy_portal_array(istgt);
+ 	istgt_destroy_uctl_portal(istgt);
+diff --git src/istgt_lu.c src/istgt_lu.c
+index 783f809..c0c4f31 100644
+--- src/istgt_lu.c
++++ src/istgt_lu.c
+@@ -1202,7 +1202,7 @@ istgt_lu_set_local_settings(ISTGT_Ptr istgt, CF_SECTION *sp, ISTGT_LU_Ptr lu)
+ }
+ 
+ static int
+-istgt_lu_add_unit(ISTGT_Ptr istgt, CF_SECTION *sp)
++istgt_lu_add_unit(ISTGT_Ptr istgt, CF_SECTION *sp, int ignore_dup)
+ {
+ 	char buf[MAX_TMPBUF], buf2[MAX_TMPBUF];
+ 	ISTGT_LU_Ptr lu;
+@@ -1222,21 +1222,40 @@ istgt_lu_add_unit(ISTGT_Ptr istgt, CF_SECTION *sp)
+ 	int i, j, k;
+ 	int rc;
+ 
+-	ISTGT_TRACELOG(ISTGT_TRACE_DEBUG, "add unit %d\n", sp->num);
++	ISTGT_TRACELOG(ISTGT_TRACE_DEBUG, "add unit %d (ignore dup = %d)\n", sp->num, ignore_dup);
+ 
+ 	if (sp->num >= MAX_LOGICAL_UNIT) {
+ 		ISTGT_ERRLOG("LU%d: over maximum unit number\n", sp->num);
+ 		return -1;
+ 	}
+-	if (istgt->logical_unit[sp->num] != NULL) {
++	if (istgt->logical_unit[sp->num] != NULL && !ignore_dup) {
+ 		ISTGT_ERRLOG("LU%d: duplicate unit\n", sp->num);
+ 		return -1;
+ 	}
++	/* existing LUNs */
++	if (istgt->logical_unit[sp->num] != NULL && ignore_dup) {
++		istgt->logical_unit[sp->num]->to_add = 0;
++		istgt->logical_unit[sp->num]->to_remove = 0;
++		ISTGT_TRACELOG(ISTGT_TRACE_DEBUG, "skip existing unit %d\n", sp->num);
++		return 0;
++	}
++
++	/* new LUNs */
++	if (istgt->logical_unit[sp->num] == NULL && ignore_dup) {
++		ISTGT_TRACELOG(ISTGT_TRACE_DEBUG, "add NEW unit %d\n", sp->num);
++	}
+ 
+ 	lu = xmalloc(sizeof *lu);
+ 	memset(lu, 0, sizeof *lu);
+ 	lu->num = sp->num;
+ 	lu->istgt = istgt;
++
++	/* tag new LUNs */
++	if (ignore_dup) {
++		lu->to_add = 1;
++		lu->to_remove = 0;
++	}
++
+ 	istgt_lu_set_state(lu, ISTGT_STATE_INVALID);
+ 	nbs64 = istgt_lu_get_nbserial(istgt->nodebase);
+ #if 0
+@@ -1944,12 +1963,15 @@ istgt_lu_del_unit(ISTGT_Ptr istgt, ISTGT_LU_Ptr lu)
+ static void *luworker(void *arg);
+ 
+ int
+-istgt_lu_init(ISTGT_Ptr istgt)
++istgt_lu_init(ISTGT_Ptr istgt, int ignore_dup)
+ {
+ 	ISTGT_LU_Ptr lu;
+ 	CF_SECTION *sp;
+ 	int rc;
+ 	int i;
++	int lun_map[MAX_LOGICAL_UNIT];
++
++	memset(lun_map, 0, sizeof(int) * MAX_LOGICAL_UNIT);
+ 
+ 	ISTGT_TRACELOG(ISTGT_TRACE_DEBUG, "istgt_lu_init\n");
+ 	sp = istgt_find_cf_section(istgt->config, "Global");
+@@ -1969,11 +1991,12 @@ istgt_lu_init(ISTGT_Ptr istgt)
+ 				ISTGT_ERRLOG("tag %d is invalid\n", sp->num);
+ 				return -1;
+ 			}
+-			rc = istgt_lu_add_unit(istgt, sp);
++			rc = istgt_lu_add_unit(istgt, sp, ignore_dup);
+ 			if (rc < 0) {
+ 				ISTGT_ERRLOG("lu_add_unit() failed\n");
+ 				return -1;
+ 			}
++			lun_map[sp->num] = 1; /* in new LUN list */
+ 		}
+ 		sp = sp->next;
+ 	}
+@@ -1982,6 +2005,22 @@ istgt_lu_init(ISTGT_Ptr istgt)
+ 		lu = istgt->logical_unit[i];
+ 		if (lu == NULL)
+ 			continue;
++		/* tag to be removed LUNs */
++		if (lun_map[i] == 0) {
++			istgt->logical_unit[i]->to_add = 0;
++			istgt->logical_unit[i]->to_remove = 1;
++			continue;
++		}
++		/* only process newly added LUNs here beyond in loop */
++		if (ignore_dup &&
++		    (istgt->logical_unit[i]->to_add == 1 &&
++		     istgt->logical_unit[i]->to_remove == 0)) {
++			ISTGT_TRACELOG(ISTGT_TRACE_DEBUG, "adding unit %d\n", i);
++		} else if (ignore_dup &&
++		    !(istgt->logical_unit[i]->to_add == 1 &&
++		      istgt->logical_unit[i]->to_remove == 0)) {
++			continue;
++		}
+ 
+ 		rc = pthread_mutex_init(&lu->mutex, NULL);
+ 		if (rc != 0) {
+@@ -2069,7 +2108,7 @@ istgt_lu_set_all_state(ISTGT_Ptr istgt, ISTGT_STATE state)
+ }
+ 
+ int
+-istgt_lu_create_threads(ISTGT_Ptr istgt)
++istgt_lu_create_threads(ISTGT_Ptr istgt, int new_only)
+ {
+ #ifdef HAVE_PTHREAD_SET_NAME_NP
+ 	char buf[MAX_TMPBUF];
+@@ -2084,6 +2123,10 @@ istgt_lu_create_threads(ISTGT_Ptr istgt)
+ 		lu = istgt->logical_unit[i];
+ 		if (lu == NULL)
+ 			continue;
++		if (new_only && !lu->to_add) {
++			ISTGT_TRACELOG(ISTGT_TRACE_DEBUG, "skip existing LUN %d\n", i);
++			continue;
++		}
+ 
+ 		if (lu->queue_depth != 0) {
+ 			/* create LU thread */
+@@ -2114,18 +2157,22 @@ istgt_lu_create_threads(ISTGT_Ptr istgt)
+ }
+ 
+ int
+-istgt_lu_shutdown(ISTGT_Ptr istgt)
++istgt_lu_shutdown(ISTGT_Ptr istgt, int removed_only)
+ {
+ 	ISTGT_LU_Ptr lu;
+ 	int rc;
+ 	int i;
+ 
+-	ISTGT_TRACELOG(ISTGT_TRACE_DEBUG, "istgt_lu_shutdown\n");
++	ISTGT_TRACELOG(ISTGT_TRACE_DEBUG, "istgt_lu_shutdown (removed_only = %d\n", removed_only);
+ 
+ 	for (i = 0; i < MAX_LOGICAL_UNIT; i++) {
+ 		lu = istgt->logical_unit[i];
+ 		if (lu == NULL)
+ 			continue;
++		if (removed_only && !lu->to_remove) {
++			ISTGT_TRACELOG(ISTGT_TRACE_DEBUG, "skip existing LUNs %d\n", i);
++			continue;
++		}
+ 		istgt_lu_set_state(lu, ISTGT_STATE_SHUTDOWN);
+ 
+ 		switch (lu->type) {
+diff --git src/istgt_lu.h src/istgt_lu.h
+index 72c5a6c..9484266 100644
+--- src/istgt_lu.h
++++ src/istgt_lu.h
+@@ -236,6 +236,9 @@ typedef struct istgt_lu_t {
+ 	ISTGT_LU_TSIH tsih[MAX_LU_TSIH];
+ 	int maxmap;
+ 	ISTGT_LU_MAP map[MAX_LU_MAP];
++
++	int to_add;
++	int to_remove;
+ } ISTGT_LU;
+ typedef ISTGT_LU *ISTGT_LU_Ptr;
+ 
+diff --git src/istgt_lu_ctl.c src/istgt_lu_ctl.c
+index 70c93e1..47e2fd3 100644
+--- src/istgt_lu_ctl.c
++++ src/istgt_lu_ctl.c
+@@ -965,6 +965,113 @@ istgt_uctl_cmd_change(UCTL_Ptr uctl)
+ 	return UCTL_CMD_OK;
+ }
+ 
++extern const char *g_config_file;
++extern ISTGT g_istgt;
++
++static int
++istgt_uctl_cmd_refresh(UCTL_Ptr uctl)
++{
++	char *arg;
++	int rc, i, j;
++	CONFIG *config;
++	ISTGT_Ptr istgt;
++
++	arg = uctl->arg;
++	istgt = &g_istgt;
++
++	/* reload config, might add/remove LUNs */
++	/* step 1: re-read config files */
++	config = istgt_allocate_config();
++	rc = istgt_read_config(config, g_config_file);
++	if (rc < 0) {
++		fprintf(stderr, "refresh config error\n");
++		return UCTL_CMD_ERR;
++	}
++	if (config->section == NULL) {
++		fprintf(stderr, "empty config\n");
++		istgt_free_config(config);
++		return UCTL_CMD_ERR;
++	}
++	istgt_free_config(istgt->config);
++	istgt->config = config;
++
++	/* step 2: add new units, init new LUNs, tag to-add/remove LUNs */
++	rc = istgt_lu_init(istgt, 1);
++	if (rc < 0) {
++		ISTGT_ERRLOG("istgt_lu_init() failed\n");
++		istgt_free_config(config);
++		return UCTL_CMD_ERR;
++	}
++
++	for (i = 0; i < MAX_LOGICAL_UNIT; i++) {
++		ISTGT_LU_Ptr lu = istgt->logical_unit[i];
++		if (lu == NULL)
++			continue;
++		if (lu->to_add)
++			ISTGT_TRACELOG(ISTGT_TRACE_DEBUG, "newly added LUN %s\n", lu->name);
++		if (lu->to_remove)
++			ISTGT_TRACELOG(ISTGT_TRACE_DEBUG, "newly removed LUN %s\n", lu->name);
++	}
++
++	/* creat new threads for new LUNs, do work in luworker() */
++	/* create LUN threads for command queuing */
++	rc = istgt_lu_create_threads(istgt, 1);
++	if (rc < 0) {
++		ISTGT_ERRLOG("lu_create_threads() failed\n");
++		istgt_free_config(config);
++		return UCTL_CMD_ERR;
++	}
++	/* start taking IOs */
++	for (i = 0; i < MAX_LOGICAL_UNIT; i++) {
++		ISTGT_LU_Ptr lu = istgt->logical_unit[i];
++		if (lu == NULL)
++			continue;
++		if (lu->to_add) {
++			istgt_lu_set_state(lu, ISTGT_STATE_RUNNING);
++			ISTGT_TRACELOG(ISTGT_TRACE_DEBUG, "set newly added LUN as running %s\n", lu->name);
++		}
++	}
++
++	/* step 3: shutdown removed LUNs */
++	/* clear outstanding IOs */
++	for (i = 0; i < MAX_LOGICAL_UNIT; i++) {
++		ISTGT_LU_Ptr lu = istgt->logical_unit[i];
++		if (lu == NULL)
++			continue;
++		if (lu->to_remove) {
++			for (j = 0; j<lu->maxlun; j++)
++				istgt_lu_clear_all_task(lu, j);
++			ISTGT_TRACELOG(ISTGT_TRACE_DEBUG, "clear newly removed LUN queue %s\n", lu
++->name);
++		}
++	}
++
++	/* sync and close LUNs */
++	istgt_lu_shutdown(istgt, 1);
++
++	/* clear tag of to_add/to_remove, get ready for next refresh */
++	for (i = 0; i < MAX_LOGICAL_UNIT; i++) {
++		ISTGT_LU_Ptr lu = istgt->logical_unit[i];
++		if (lu == NULL)
++			continue;
++		if (lu->to_add)
++			lu->to_add = 0;
++		if (lu->to_remove)
++			lu->to_remove = 0;
++	}
++
++	/* logging event */
++	ISTGT_NOTICELOG("Configuration refresh requested from %s\n",
++	    uctl->caddr);
++
++	/* refresh succeeded */
++	istgt_uctl_snprintf(uctl, "OK %s\n", uctl->cmd);
++	rc = istgt_uctl_writeline(uctl);
++	if (rc != UCTL_CMD_OK)
++		return rc;
++	return UCTL_CMD_OK;
++}
++
+ static int
+ istgt_uctl_cmd_reset(UCTL_Ptr uctl)
+ {
+@@ -1183,6 +1290,7 @@ static ISTGT_UCTL_CMD_TABLE istgt_uctl_cmd_table[] =
+ 	{ "UNLOAD",  istgt_uctl_cmd_unload },
+ 	{ "LOAD",    istgt_uctl_cmd_load },
+ 	{ "CHANGE",  istgt_uctl_cmd_change },
++	{ "REFRESH", istgt_uctl_cmd_refresh },
+ 	{ "RESET",   istgt_uctl_cmd_reset },
+ 	{ "INFO",    istgt_uctl_cmd_info },
+ 	{ NULL,      NULL },
+diff --git src/istgt_lu_disk.c src/istgt_lu_disk.c
+index 84eff07..40c9402 100644
+--- src/istgt_lu_disk.c
++++ src/istgt_lu_disk.c
+@@ -794,6 +794,7 @@ istgt_lu_disk_shutdown(ISTGT_Ptr istgt, ISTGT_LU_Ptr lu)
+ 		xfree(spec);
+ 		lu->lun[i].spec = NULL;
+ 	}
++	lu->maxlun = 0;
+ 
+ 	return 0;
+ }
+diff --git src/istgt_proto.h src/istgt_proto.h
+index 8318f64..9bc842a 100644
+--- src/istgt_proto.h
++++ src/istgt_proto.h
+@@ -85,10 +85,10 @@ int istgt_lu_parse_media_flags(const char *flags);
+ uint64_t istgt_lu_parse_media_size(const char *file, const char *size, int *flags);
+ PORTAL *istgt_lu_find_portalgroup(ISTGT_Ptr istgt, int tag);
+ INITIATOR_GROUP *istgt_lu_find_initiatorgroup(ISTGT_Ptr istgt, int tag);
+-int istgt_lu_init(ISTGT_Ptr istgt);
++int istgt_lu_init(ISTGT_Ptr istgt, int ignore_dup);
+ int istgt_lu_set_all_state(ISTGT_Ptr istgt, ISTGT_STATE state);
+-int istgt_lu_create_threads(ISTGT_Ptr istgt);
+-int istgt_lu_shutdown(ISTGT_Ptr istgt);
++int istgt_lu_create_threads(ISTGT_Ptr istgt, int new_only);
++int istgt_lu_shutdown(ISTGT_Ptr istgt, int removed_only);
+ int istgt_lu_islun2lun(uint64_t islun);
+ uint64_t istgt_lu_lun2islun(int lun, int maxlun);
+ int istgt_lu_reset(ISTGT_LU_Ptr lu, uint64_t lun);
+diff --git src/istgtcontrol.c src/istgtcontrol.c
+index 46d6e98..489691d 100644
+--- src/istgtcontrol.c
++++ src/istgtcontrol.c
+@@ -494,6 +494,36 @@ exec_change(UCTL_Ptr uctl)
+ }
+ 
+ static int
++exec_refresh(UCTL_Ptr uctl)
++{
++	const char *delim = ARGS_DELIM;
++	char *arg;
++	char *result;
++	int rc;
++
++	/* send command */
++	uctl_snprintf(uctl, "REFRESH \n");
++	rc = uctl_writeline(uctl);
++	if (rc != UCTL_CMD_OK)
++		return rc;
++
++	/* receive result */
++	rc = uctl_readline(uctl);
++	if (rc != UCTL_CMD_OK)
++		return rc;
++	arg = trim_string(uctl->recvbuf);
++	result = strsepq(&arg, delim);
++	strupr(result);
++	if (strcmp(result, "OK") != 0) {
++		if (is_err_req_auth(uctl, arg))
++			return UCTL_CMD_REQAUTH;
++		fprintf(stderr, "ERROR %s\n", arg);
++		return UCTL_CMD_ERR;
++	}
++	return UCTL_CMD_OK;
++}
++
++static int
+ exec_reset(UCTL_Ptr uctl)
+ {
+ 	const char *delim = ARGS_DELIM;
+@@ -591,6 +621,7 @@ static EXEC_TABLE exec_table[] =
+ 	{ "UNLOAD",  exec_unload,   0, 1 },
+ 	{ "LOAD",    exec_load,     0, 1 },
+ 	{ "CHANGE",  exec_change,   1, 1 },
++	{ "REFRESH", exec_refresh,  0, 0 },
+ 	{ "RESET",   exec_reset,    0, 1 },
+ 	{ "INFO",    exec_info,     0, 0 },
+ 	{ NULL,      NULL,          0, 0 },
+@@ -1054,6 +1085,7 @@ usage(void)
+ 	printf(" load       load media to specified unit\n");
+ 	printf(" unload     unload media from specified unit\n");
+ 	printf(" change     change media with <file> at specified unit\n");
++	printf(" refresh    refresh to reload target and lun configuration\n");
+ 	printf(" reset      reset specified lun of target\n");
+ 	printf(" info       show connections of target\n");
+ }
--- istgt.diff ends here ---


>Release-Note:
>Audit-Trail:
>Unformatted:



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201203080122.q281M3lh054796>