Seems to be a bug or two in smc.c

[FUJITA Tomonori]

Added CC to Mark Harvey,

On Thu, 14 Aug 2008 16:44:24 -0700
"Richard Sharpe" <realrichardsharpe at gmail.com> wrote:

> In the process of getting BakBone to see my VTL and work with it
> properly, I think I found a couple of buglets.
> 
> BakBone issues an INITIALIZE_ELEMENT_STATUS and seems to give up if
> told that the command was not implemented.
> 
> Also, we need to properly fill in the LUN for data transfer devices
> and protect against being asked for too little data, because we can
> corrupt things.
> 
> Attached is a possible patch.

=
--- /home/rsharpe/open-source-devel/linux-scsi-tgt/usr/smc.c	2008-07-30 10:26:58.000000000 -0700
+++ smc.c	2008-08-14 16:21:30.000000000 -0700
@@ -142,7 +142,14 @@
 	data[3] = 0;	/* Reserved */
 	data[4] = (s->asc >> 8) & 0xff;	/* Additional Sense Code */
 	data[5] = s->asc & 0xff;	/* Additional Sense Code Qualifier */
-	/* [6], [7] & [8] reserved */
+	/* [6], [7] & [8] reserved except for data transfer element SMC3 */
+	if (element_type == ELEMENT_DATA_TRANSFER) {
+		/*
+		 * Only works if on same bus as the media changer and
+		 * lun is <= 7 ...
+		 */
+		data[6] = 0x10 | (s->drive_lun & 0x07); /* Add in LUN */
+	}
 	data[9] = (s->cart_type & 0xf);
 
 	if (s->last_addr) {	/* Source address is valid ? */
@@ -225,6 +232,23 @@
 }
 
 /**
+ * smc_initialize_element_status - INITIALIZE ELEMENT STATUS op code
+ *
+ * Some backup libraries seem to require this.
+ *
+ * Support the SCSI op code INITIALIZE_ELEMENT_STATUS
+ * Ref: smc3r10a, 6.2
+ */
+static int smc_initialize_element_status(int host_no, struct scsi_cmd *cmd)
+{
+	/*
+	 * Should do some error checking here ... the spec says some stuff
+	 * about having a reservation
+	 */
+	return SAM_STAT_GOOD;
+}
+
+/**
  * smc_read_element_status  -  READ ELEMENT STATUS op code
  *
  * Support the SCSI op code READ ELEMENT STATUS
@@ -241,12 +265,14 @@
 	uint8_t dvcid;
 	int alloc_len;
 	uint16_t count = 0;
+	uint32_t slot_count = 0;
 	int first = 0;		/* First valid slot location */
 	int len = 8;
 	int elementSize;
 	int ret;
 	unsigned char key = ILLEGAL_REQUEST;
 	uint16_t asc = ASC_INVALID_FIELD_IN_CDB;
+	struct slot *s;
 
 	scb = cmd->scb;
 	element_type = scb[1] & 0x0f;
@@ -258,6 +284,14 @@
 	if (scsi_get_in_length(cmd) < alloc_len)
 		goto sense;
 
+	/*
+	 * To protect against given alloc_len less than we need, we allocate
+	 * enough memory to build element descriptors for all slots
+	 * and only copy what we were asked.
+	 */
+	list_for_each_entry(s, &smc->slots, slot_siblings)
+		slot_count++;
+
 	elementSize = determine_element_sz(dvcid, voltag);
 
 	scsi_set_in_resid_by_actual(cmd, 0);
@@ -269,7 +303,7 @@
 		}
 	}
 
-	data = zalloc(alloc_len);
+	data = zalloc(slot_count * elementSize + len);
 	if (!data) {
 		dprintf("Can't allocate enough memory for cmd\n");
 		key = HARDWARE_ERROR;
@@ -287,7 +321,7 @@
 						  ELEMENT_MEDIUM_TRANSPORT,
 						  &first, req_start_elem,
 						  dvcid, voltag);
-		len = count * elementSize;
+		len += count * elementSize;
 		count += build_element_descriptors(&data[len], &smc->slots,
 						   ELEMENT_STORAGE,
 						   &first, req_start_elem,
@@ -748,7 +782,7 @@
 		{spc_illegal_op,},
 		{spc_illegal_op,},
 		{spc_illegal_op,},
-		{spc_illegal_op,},
+		{smc_initialize_element_status,},
 
 		{spc_illegal_op,},
 		{spc_illegal_op,},
--
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