[Stgt-devel] [Scst-devel] Integration of SCST in the mainstream Linux kernel

[FUJITA Tomonori]

On Mon, 10 Mar 2008 13:17:39 +0100
Tomasz Chmielewski <mangoo at wpkg.org> wrote:

> FUJITA Tomonori schrieb:
> > On Mon, 10 Mar 2008 11:48:09 +0100
> > Tomasz Chmielewski <mangoo at wpkg.org> wrote:
> > 
> >> FUJITA Tomonori schrieb:
> >>
> >> (...)
> >>
> >>>> But it looks that there is still this issue then - so it may fail sometimes:
> >>>>
> >>>>   there is a race
> >>>>   between the first and second operations (that is, you need to peform
> >>>>   the second right after the first, before the initiator tries to
> >>>>   connect). I'm fine with changing the default state to 'offline'.
> >>>>
> >>>> ?
> >>> Then, at least, try that to see it works for you. After that, you can
> >>> say something like, it works for me, but I want the default state
> >>> 'offline'.
> >> No, it doesn't work for me because of the race mentioned above, or, 
> >> described once again, below - this is what happens to the target and 
> >> initiator if we want to restart the target (i.e., target software 
> >> upgrade or target machine restart, because of kernel, hardware upgrade 
> >> etc.):
> > 
> > Can you try the following patch works with the latest git:
> > 
> > diff --git a/usr/target.c b/usr/target.c
> > index 50769f6..89f90cb 100644
> > --- a/usr/target.c
> > +++ b/usr/target.c
> > @@ -1636,7 +1636,7 @@ int tgt_target_create(int lld, int tid, char *args)
> >  
> >  	target->bst = bst;
> >  
> > -	target->target_state = SCSI_TARGET_RUNNING;
> > +	target->target_state = SCSI_TARGET_OFFLINE;
> >  	target->lid = lld;
> >  
> >  	list_for_each_entry(pos, &target_list, target_siblings)
> 
> No, it doesn't solve this problem.
> 
> The thing is, when tgtd is (re)started, it has no targets configured, 
> but it already listens and accepts incoming connections.
> 
> So if an initiator reconnects to tgtd, tgtd can't reply:
> 
>    target iqn.2001-04.com.example:storage.disk2.iris.sys1.xyz offlined
>    try again later
> 
> because there are no targets configured.
> 
> Instead, it still replies "no such target", and the initiator which 
> reconnects reports an IO error.

OK, can you try the following patch?

I've added 'system status':

rose:/home/fujita# ./tgt/usr/tgtadm --op show --mode sys
System:
    Status=offline
iSNS:
    iSNS=Off
    iSNSServerIP=
    iSNSServerPort=3205
    iSNSAccessControl=Off

If the system status is 'offline', tgtd accepts a new connection and
then close it.

rose:/home/fujita# ./tgt/usr/tgtadm --op update --mode sys -n state -v
running

rose:/home/fujita# ./tgt/usr/tgtadm --op show --mode sys
System:
    Status=running
iSNS:
    iSNS=Off
    iSNSServerIP=
    iSNSServerPort=3205
    iSNSAccessControl=Off

When you finish configuring targets, then change the state to
'running'.

The patch is hacky but if it works, I'll clean up and merge it.


diff --git a/usr/iscsi/iscsi_tcp.c b/usr/iscsi/iscsi_tcp.c
index 09ed0e5..c1f97b6 100644
--- a/usr/iscsi/iscsi_tcp.c
+++ b/usr/iscsi/iscsi_tcp.c
@@ -104,6 +104,9 @@ static void accept_connection(int afd, int events, void *data)
 		return;
 	}
 
+	if (!is_system_available())
+		goto out;
+
 	ret = set_keepalive(fd);
 	if (ret)
 		goto out;
diff --git a/usr/iscsi/isns.c b/usr/iscsi/isns.c
index 81b2874..7bbf875 100644
--- a/usr/iscsi/isns.c
+++ b/usr/iscsi/isns.c
@@ -982,7 +982,7 @@ overflow:
 }
 
 enum {
-	Opt_isns, Opt_ip, Opt_port, Opt_ac, Opt_err,
+	Opt_isns, Opt_ip, Opt_port, Opt_ac, Opt_state, Opt_err,
 };
 
 static match_table_t tokens = {
@@ -990,6 +990,7 @@ static match_table_t tokens = {
 	{Opt_ip, "iSNSServerIP=%s"},
 	{Opt_port, "iSNSServerPort=%d"},
 	{Opt_ac, "iSNSAccessControl=%s"},
+	{Opt_state, "state=%s"},
 	{Opt_err, NULL},
 };
 
@@ -1001,7 +1002,7 @@ int isns_update(char *params)
 	while ((p = strsep(&params, ",")) != NULL) {
 		substring_t args[MAX_OPT_ARGS];
 		int token;
-		char tmp[8];
+		char tmp[16];
 
 		if (!*p)
 			continue;
@@ -1030,6 +1031,10 @@ int isns_update(char *params)
 			match_strncpy(tmp, &args[0], sizeof(tmp));
 			use_isns_ac = !strcmp(tmp, "On");
 			break;
+		case Opt_state:
+			match_strncpy(tmp, &args[0], sizeof(tmp));
+			system_set_state(tmp);
+			break;
 		default:
 			ret = TGTADM_INVALID_REQUEST;
 		}
diff --git a/usr/mgmt.c b/usr/mgmt.c
index 041f0ff..711c7f4 100644
--- a/usr/mgmt.c
+++ b/usr/mgmt.c
@@ -262,13 +262,14 @@ static int sys_mgmt(int lld_no, struct mgmt_task *mtask)
 		rsp->len = sizeof(*rsp);
 		break;
 	case OP_SHOW:
-		if (tgt_drivers[lld_no]->show) {
-			err = tgt_drivers[lld_no]->show(req->mode,
-							req->tid, req->sid,
-							req->cid, req->lun,
-							mtask->buf, len);
-			set_show_results(rsp, &err);
+		err = system_show(req->mode, mtask->buf, len);
+		if (err >= 0 && tgt_drivers[lld_no]->show) {
+			err += tgt_drivers[lld_no]->show(req->mode,
+							 req->tid, req->sid,
+							 req->cid, req->lun,
+							 mtask->buf + err, len - err);
 		}
+		set_show_results(rsp, &err);
 		break;
 	default:
 		break;
diff --git a/usr/target.c b/usr/target.c
index 50769f6..c7583ee 100644
--- a/usr/target.c
+++ b/usr/target.c
@@ -1716,3 +1716,63 @@ int account_show(char *buf, int rest)
 overflow:
 	return max;
 }
+
+static struct {
+	enum tgt_system_state value;
+	char *name;
+} system_state[] = {
+	{TGT_SYSTEM_OFFLINE, "offline"},
+	{TGT_SYSTEM_RUNNING, "running"},
+};
+
+static char *system_state_name(enum scsi_target_state state)
+{
+	int i;
+	char *name = NULL;
+
+	for (i = 0; i < ARRAY_SIZE(system_state); i++) {
+		if (system_state[i].value == state) {
+			name = system_state[i].name;
+			break;
+		}
+	}
+	return name;
+}
+
+static enum tgt_system_state sys_state = TGT_SYSTEM_OFFLINE;
+
+int system_set_state(char *str)
+{
+	int i, err = TGTADM_INVALID_REQUEST;
+
+	for (i = 0; i < ARRAY_SIZE(target_state); i++) {
+		if (!strcmp(system_state[i].name, str)) {
+			sys_state = system_state[i].value;
+			err = 0;
+			break;
+		}
+	}
+	return err;
+}
+
+int system_show(int mode, char *buf, int rest)
+{
+	int total = 0, max = rest;
+
+	/* FIXME: too hacky */
+	if (mode != MODE_SYSTEM)
+		return 0;
+
+	shprintf(total, buf, rest, "System:\n");
+	shprintf(total, buf, rest, _TAB1 "Status=%s\n",
+		 system_state_name(sys_state));
+
+	return total;
+overflow:
+	return max;
+}
+
+int is_system_available(void)
+{
+	return (sys_state == TGT_SYSTEM_RUNNING);
+}
diff --git a/usr/tgtd.h b/usr/tgtd.h
index c083fbe..c0b8a0c 100644
--- a/usr/tgtd.h
+++ b/usr/tgtd.h
@@ -24,6 +24,11 @@
 #define _TAB3 _TAB1 _TAB1 _TAB1
 #define _TAB4 _TAB2 _TAB2
 
+enum tgt_system_state {
+	TGT_SYSTEM_OFFLINE = 1,
+	TGT_SYSTEM_RUNNING,
+};
+
 enum scsi_target_state {
 	SCSI_TARGET_OFFLINE = 1,
 	SCSI_TARGET_RUNNING,
@@ -183,6 +188,9 @@ extern int tgt_target_create(int lld, int tid, char *args);
 extern int tgt_target_destroy(int lld, int tid);
 extern char *tgt_targetname(int tid);
 extern int tgt_target_show_all(char *buf, int rest);
+int system_set_state(char *str);
+int system_show(int mode, char *buf, int rest);
+int is_system_available(void);
 
 extern int tgt_bind_host_to_target(int tid, int host_no);
 extern int tgt_unbind_host_to_target(int tid, int host_no);