[stgt] strange problem vSphere vCLI and stgt

Arne Redlich arne.redlich at googlemail.com
Sun Mar 20 20:42:51 CET 2011


Am Sonntag, den 20.03.2011, 20:26 +0100 schrieb Arne Redlich:
> Am Donnerstag, den 17.03.2011, 21:00 +0900 schrieb FUJITA Tomonori:
> > On Thu, 17 Mar 2011 11:27:38 +0100
> > frederik.vos at linvirt.nl wrote:
> [...]
>  
> > > Tried the same with ietd, same results as with the netapp.
> > > Please notice, i am not blaming stgt, just trying to find an answer  
> > > why, and how to fix it.
> > > If you need more information, tests etc. Just let me know
> > 
> > I suspect that vSphere can't handle a ssc device (lun 0). Does someone
> > have the updated patch to remove ssc lun 0?
> 
> Below is basically the same patch I sent quite a while ago to remove the
> dummy LUN 0 (only a cosmetic modification to make it applicable to git
> head).
> Only mildly tested (compile, expose a file as lun 0 and checked
> visibility of it from linux and windows initiators), so YMMV.

Some more testing shows that LUN 0 cannot be removed - the updated
version below should fix this.

Arne

-
>From 0d204bda91efaeda3f73c9e7ce932869e63a8545 Mon Sep 17 00:00:00 2001
From: Arne Redlich <arne.redlich at googlemail.com>
Date: Sun, 20 Mar 2011 20:37:08 +0100
Subject: [PATCH] Remove dummy RAID controller from LUN 0

The dummy RAID controller serves 2 purposes:
(1) commands that are addressed to an inexistent LUN are redirected to it
(2) it provides a LUN 0 by default which is required by the SCSI spec
.

(1) is obviously wrong because instead of "wrong lun" "invalid cdb" is returned
to the initiator. A "shadow LUN" of type NO_LUN is now used for this purpose.
This LU uses bs_null as backingstore, so there are no idle threads spawned for
it (in contrast to the previous dummy raid controller at LUN 0).

(2) confuses some OSes / users (Windows prompts for drivers,
Solaris repeatedly tries to online the LU, but does not succeed).

It's now the user's responsibility to attach a LU to LUN 0 to adhere to the
SCSI spec (Solaris / WinXP don't insist, Linux does!).

Signed-off-by: Arne Redlich <arne.redlich at googlemail.com>
---
 usr/Makefile |    2 +-
 usr/list.h   |    3 ++
 usr/nolun.c  |   95 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 usr/spc.c    |   10 ++++++
 usr/spc.h    |    1 +
 usr/target.c |   28 +++++++++--------
 usr/target.h |    2 +
 7 files changed, 127 insertions(+), 14 deletions(-)
 create mode 100644 usr/nolun.c

diff --git a/usr/Makefile b/usr/Makefile
index ea175cd..9ac79c0 100644
--- a/usr/Makefile
+++ b/usr/Makefile
@@ -53,7 +53,7 @@ LIBS += -lpthread
 
 PROGRAMS += tgtd tgtadm tgtimg
 TGTD_OBJS += tgtd.o mgmt.o target.o scsi.o log.o driver.o util.o work.o \
-		parser.o spc.o sbc.o mmc.o osd.o scc.o smc.o \
+		parser.o spc.o sbc.o mmc.o nolun.o osd.o scc.o smc.o \
 		ssc.o bs_ssc.o libssc.o \
 		bs_null.o bs_sg.o bs.o libcrc32c.o
 
diff --git a/usr/list.h b/usr/list.h
index 2f80a56..698a684 100644
--- a/usr/list.h
+++ b/usr/list.h
@@ -32,6 +32,9 @@ static inline void INIT_LIST_HEAD(struct list_head *list)
 #define list_first_entry(ptr, type, member) \
 	list_entry((ptr)->next, type, member)
 
+#define list_last_entry(ptr, type, member) \
+	list_entry((ptr)->prev, type, member)
+
 static inline int list_empty(const struct list_head *head)
 {
 	return head->next == head;
diff --git a/usr/nolun.c b/usr/nolun.c
new file mode 100644
index 0000000..0f516a0
--- /dev/null
+++ b/usr/nolun.c
@@ -0,0 +1,95 @@
+/*
+ * Processing of SCSI commands addressed to inexistent LUNs.
+ *
+ * Based on scc.c:
+ *
+ * Copyright (C) 2007 FUJITA Tomonori <tomof at acm.org>
+ * Copyright (C) 2007 Mike Christie <michaelc at cs.wisc.edu>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation, version 2 of the
+ * License.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ */
+#include <errno.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <stdint.h>
+#include <unistd.h>
+#include <linux/fs.h>
+
+#include "list.h"
+#include "util.h"
+#include "tgtd.h"
+#include "target.h"
+#include "driver.h"
+#include "scsi.h"
+#include "tgtadm_error.h"
+#include "spc.h"
+
+static int nolun_lu_init(struct scsi_lu *lu)
+{
+	memset(&lu->attrs, 0x0, sizeof(lu->attrs));
+	/*
+	 * TODO: use PQ 11b (per. dev. supp but not connected) and dev type 0x0
+	 * (SBC-2) instead?
+	 */
+	lu->attrs.device_type = 0x1f; /* unknown / no device type */
+	lu->attrs.qualifier = 0x3; /* peripheral dev. not supp. on this LU */
+
+	return 0;
+}
+
+static int nolun_lu_config(struct scsi_lu *lu __attribute__((unused)),
+			   char *params __attribute__((unused)))
+{
+	return TGTADM_NO_LUN;
+}
+
+static int nolun_lu_online(struct scsi_lu *lu __attribute__((unused)))
+{
+	/* TODO: correct return value? most consumers don't check at all ... */
+	return -ENOENT;
+}
+
+static int nolun_lu_offline(struct scsi_lu *lu __attribute__((unused)))
+{
+	return 0;
+}
+
+static void nolun_lu_exit(struct scsi_lu *lu __attribute__((unused)))
+{
+	return;
+}
+
+static struct device_type_template nolun_template = {
+	.type		= TYPE_NO_LUN,
+	.lu_init	= nolun_lu_init,
+	.lu_config	= nolun_lu_config,
+	.lu_online	= nolun_lu_online,
+	.lu_offline	= nolun_lu_offline,
+	.lu_exit	= nolun_lu_exit,
+	.ops		= {
+		[0x0 ... 0x11] =  {spc_invalid_lun,},
+		{spc_inquiry,}, /* 0x12 */
+		[ 0x13 ... 0x9f] =  {spc_invalid_lun},
+		{spc_report_luns,}, /* 0xA0 */
+		[0xa1 ... 0xff] = {spc_invalid_lun},
+	}
+};
+
+__attribute__((constructor)) static void nolun_init(void)
+{
+	device_type_register(&nolun_template);
+}
diff --git a/usr/spc.c b/usr/spc.c
index ea901e2..94fa1f9 100644
--- a/usr/spc.c
+++ b/usr/spc.c
@@ -270,6 +270,8 @@ int spc_report_luns(int host_no, struct scsi_cmd *cmd)
 	nr_luns = 0;
 
 	list_for_each_entry(lu, dev_list, device_siblings) {
+		if (lu->lun == TGT_SHADOW_LUN)
+			continue;
 		nr_luns++;
 
 		if (!alen)
@@ -1590,6 +1592,14 @@ int spc_illegal_op(int host_no, struct scsi_cmd *cmd)
 	return SAM_STAT_CHECK_CONDITION;
 }
 
+int spc_invalid_lun(int host_no, struct scsi_cmd *cmd)
+{
+	dump_cdb(cmd);
+	scsi_set_in_resid_by_actual(cmd, 0);
+	sense_data_build(cmd, ILLEGAL_REQUEST, ASC_LUN_NOT_SUPPORTED);
+	return SAM_STAT_CHECK_CONDITION;
+}
+
 enum {
 	Opt_scsi_id, Opt_scsi_sn,
 	Opt_vendor_id, Opt_product_id,
diff --git a/usr/spc.h b/usr/spc.h
index 430a882..23fab97 100644
--- a/usr/spc.h
+++ b/usr/spc.h
@@ -12,6 +12,7 @@ extern int spc_test_unit(int host_no, struct scsi_cmd *cmd);
 extern int spc_request_sense(int host_no, struct scsi_cmd *cmd);
 extern int spc_prevent_allow_media_removal(int host_no, struct scsi_cmd *cmd);
 extern int spc_illegal_op(int host_no, struct scsi_cmd *cmd);
+extern int spc_invalid_lun(int host_no, struct scsi_cmd *cmd);
 extern int spc_lu_init(struct scsi_lu *lu);
 
 typedef int (match_fn_t)(struct scsi_lu *lu, char *params);
diff --git a/usr/target.c b/usr/target.c
index b8b8715..79319df 100644
--- a/usr/target.c
+++ b/usr/target.c
@@ -510,7 +510,10 @@ int tgt_device_create(int tid, int dev_type, uint64_t lun, char *params,
 		goto out;
 	}
 
-	bst = target->bst;
+	bst = (lun == TGT_SHADOW_LUN) ?
+		get_backingstore_template("null") :
+		target->bst;
+
 	if (backing) {
 		if (bstype) {
 			bst = get_backingstore_template(bstype);
@@ -675,10 +678,6 @@ int tgt_device_destroy(int tid, uint64_t lun, int force)
 
 	dprintf("%u %" PRIu64 "\n", tid, lun);
 
-	/* lun0 is special */
-	if (!lun && !force)
-		return TGTADM_INVALID_REQUEST;
-
 	lu = __device_lookup(tid, lun, &target);
 	if (!lu) {
 		eprintf("device %" PRIu64 " not found\n", lun);
@@ -934,11 +933,11 @@ int target_cmd_queue(int tid, struct scsi_cmd *cmd)
 	cmd->dev_id = dev_id;
 	dprintf("%p %x %" PRIx64 "\n", cmd, cmd->scb[0], dev_id);
 	cmd->dev = device_lookup(target, dev_id);
-	/* use LUN0 */
+	/* use TGT_SHADOW_LUN */
 	if (!cmd->dev)
-		cmd->dev = list_first_entry(&target->device_list,
-					    struct scsi_lu,
-					    device_siblings);
+		cmd->dev = list_last_entry(&target->device_list,
+					   struct scsi_lu,
+					   device_siblings);
 
 	cmd->itn_lu_info = it_nexus_lu_info_lookup(itn, cmd->dev->lun);
 
@@ -1767,7 +1766,9 @@ int tgt_target_show_all(char *buf, int rest)
 		}
 
 		shprintf(total, buf, rest, _TAB1 "LUN information:\n");
-		list_for_each_entry(lu, &target->device_list, device_siblings)
+		list_for_each_entry(lu, &target->device_list, device_siblings) {
+			if (lu->lun == TGT_SHADOW_LUN)
+				continue;
 			shprintf(total, buf, rest,
 				 _TAB2 "LUN: %" PRIu64 "\n"
   				 _TAB3 "Type: %s\n"
@@ -1794,6 +1795,7 @@ int tgt_target_show_all(char *buf, int rest)
 					"None",
 				 lu->path ? : "None",
 				 open_flags_to_str(strflags, lu->bsoflags));
+		}
 
 		if (!strcmp(tgt_drivers[target->lid]->name, "iscsi") ||
 		    !strcmp(tgt_drivers[target->lid]->name, "iser")) {
@@ -1911,7 +1913,7 @@ int tgt_target_create(int lld, int tid, char *args)
 	INIT_LIST_HEAD(&target->acl_list);
 	INIT_LIST_HEAD(&target->it_nexus_list);
 
-	tgt_device_create(tid, TYPE_RAID, 0, NULL, 0);
+	tgt_device_create(tid, TYPE_NO_LUN, TGT_SHADOW_LUN, NULL, 0);
 
 	if (tgt_drivers[lld]->target_create)
 		tgt_drivers[lld]->target_create(target);
@@ -1938,8 +1940,8 @@ int tgt_target_destroy(int lld_no, int tid)
 	}
 
 	while (!list_empty(&target->device_list)) {
-		/* we remove lun0 last */
-		lu = list_entry(target->device_list.prev, struct scsi_lu,
+		/* we remove TGT_SHADOW_LUN last */
+		lu = list_entry(target->device_list.next, struct scsi_lu,
 				device_siblings);
 		ret = tgt_device_destroy(tid, lu->lun, 1);
 		if (ret)
diff --git a/usr/target.h b/usr/target.h
index 9283431..5cb102f 100644
--- a/usr/target.h
+++ b/usr/target.h
@@ -8,6 +8,8 @@
 #define	HASH_ORDER	4
 #define	hashfn(val)	hash_long((unsigned long) (val), HASH_ORDER)
 
+#define TGT_SHADOW_LUN 0xffffffffffffffffULL
+
 struct acl_entry {
 	char *address;
 	struct list_head aclent_list;
-- 
1.7.1



--
To unsubscribe from this list: send the line "unsubscribe stgt" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html



More information about the stgt mailing list