Comments inline. On Fri, Aug 15, 2008 at 7:45 PM, FUJITA Tomonori <fujita.tomonori at lab.ntt.co.jp> wrote: > 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 ? */ NAK SMC-3r11 (dated 28 Feb, 2008) lists byte 6 as obsolete or reserved. It also does not make sense in a iSCSI environment as the DATA TRANSFER DEVICES will most likely be exported as a different target and hence a different bus/lun at the initiator side. > @@ -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; > +} > + > +/** ACK I would recommend submitting the addition of each SCSI OP code as a unique stand-alone patch. Another similar patch for INITIALIZE_ELEMENT_STATUS_WITH_RANGE would also be 'handy' and can also be implemented as a NO-OP. Although, both INITIALIZE ELEMENT STATUS and INITIALIZE_ELEMENT STATUS WITH RANGE are listed as optional OP codes (Reference: SMC-3r11), most backup software require/expect these commands to be supported. > * 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, ACK > @@ -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,}, > Cheers Mark Harvey -- 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 |