On 05/18/2010 08:13 AM, Nicholas A. Bellinger wrote: > From: Nicholas Bellinger <nab at linux-iscsi.org> > > This patch updates usr/bs_sg.c:bs_sg_cmd_submit() to process incoming scsi-generic IO > of type SG_DXFER_TO_DEV and SG_DXFER_FROM_DEV using a single new struct scsi_cmnd->sg_iovec > set using existing scsi_cmd.h scsi_get_[out,in]_[buffer,length]() macros. > > It also makes set_cmd_failed() properly return SAM_STAT_CHECK_CONDITION, which is expected > by the bs_sg_cmd_submit() caller in usr/scsi.c:scsi_cmd_perform() > > Finally, this patch adds struct backingstore_template->bs_passthrough=1 that is used by > bs_sg.c here and the next patch to hand off struct scsi_cmd for CDB passthrough. > > Signed-off-by: Nicholas A. Bellinger <nab at linux-iscsi.org> > --- > usr/bs_sg.c | 31 ++++++++++++++++++++++++------- Why not add a new bs_bsg.c backing store that goes through BSG and not SG and do it properly for all type of targets and commands? Just a thought. > usr/scsi_cmnd.h | 4 ++++ > usr/tgtd.h | 1 + > 3 files changed, 29 insertions(+), 7 deletions(-) > > diff --git a/usr/bs_sg.c b/usr/bs_sg.c > index 8baa480..9696392 100644 > --- a/usr/bs_sg.c > +++ b/usr/bs_sg.c > @@ -82,7 +82,7 @@ static int graceful_write(int fd, void *p_write, int to_write) > return 0; > } > > -static void set_cmd_failed(struct scsi_cmd *cmd) > +static int set_cmd_failed(struct scsi_cmd *cmd) > { > int result = SAM_STAT_CHECK_CONDITION; > uint16_t asc = ASC_READ_ERROR; > @@ -90,11 +90,14 @@ static void set_cmd_failed(struct scsi_cmd *cmd) > > scsi_set_result(cmd, result); > sense_data_build(cmd, key, asc); > + > + return result; > } > > static int bs_sg_cmd_submit(struct scsi_cmd *cmd) > { > struct scsi_lu *dev = cmd->dev; > + struct sg_iovec *iov = &cmd->sg_iovec; > int fd = dev->fd; > struct sg_io_hdr io_hdr; > int err = 0; > @@ -104,14 +107,27 @@ static int bs_sg_cmd_submit(struct scsi_cmd *cmd) > io_hdr.cmd_len = cmd->scb_len; > io_hdr.cmdp = cmd->scb; > > + if (scsi_get_data_dir(cmd) == DATA_BIDIRECTIONAL) { Using bsg just really cleans this all function up. IN_CMD => IN_HDR OUT_CMD => OUT_HDR no ifs and buts. The tgt's cmd looks like struct sg_io_v4 as if they are twins ;-) > + eprintf("Unable to process DATA_BIDIRECTIONAL to" > + " SG_IO backstore\n"); > + return set_cmd_failed(cmd); > + } > + > if (scsi_get_data_dir(cmd) == DATA_WRITE) { > io_hdr.dxfer_direction = SG_DXFER_TO_DEV; > - io_hdr.dxfer_len = scsi_get_out_length(cmd); > - io_hdr.dxferp = (void *)scsi_get_out_buffer(cmd); dxferp is already void* why do you need the cast? > - } else { > + io_hdr.dxfer_len = iov->iov_len = scsi_get_out_length(cmd); > + iov->iov_base = (void *)scsi_get_out_buffer(cmd); > + io_hdr.dxferp = &cmd->sg_iovec; > + io_hdr.iovec_count = 1; > + } else if (scsi_get_data_dir(cmd) == DATA_READ) { > io_hdr.dxfer_direction = SG_DXFER_FROM_DEV; > - io_hdr.dxfer_len = scsi_get_in_length(cmd); > - io_hdr.dxferp = (void *)scsi_get_in_buffer(cmd); > + io_hdr.dxfer_len = iov->iov_len = scsi_get_in_length(cmd); > + iov->iov_base = (void *)scsi_get_in_buffer(cmd); > + io_hdr.dxferp = &cmd->sg_iovec; > + io_hdr.iovec_count = 1; > + } else { > + io_hdr.dxfer_direction = SG_DXFER_NONE; > + io_hdr.dxfer_len = 0; > } > io_hdr.mx_sb_len = sizeof(cmd->sense_buffer); > io_hdr.sbp = cmd->sense_buffer; You might want to look into the new SG_FLAG_Q_AT_TAIL but this is available only in new Kernels so it'll be a little hack > @@ -125,7 +141,7 @@ static int bs_sg_cmd_submit(struct scsi_cmd *cmd) > set_cmd_async(cmd); > else { > eprintf("failed to start cmd 0x%p\n", cmd); > - set_cmd_failed(cmd); > + return set_cmd_failed(cmd); > } > return 0; > } > @@ -238,6 +254,7 @@ static int bs_sg_cmd_done(struct scsi_cmd *cmd) > static struct backingstore_template sg_bst = { > .bs_name = "sg", > .bs_datasize = 0, > + .bs_passthrough = 1, > .bs_open = bs_sg_open, > .bs_close = bs_sg_close, > .bs_cmd_submit = bs_sg_cmd_submit, And again the Second patch should just fold into this one. And the final disposition will just set the .bs_cmd_submit to the proper one. [BTW if you want to keep struct backingstore_template sg_bst const (Which it is not, but should be) then just have two of them and switch between them] Boaz > diff --git a/usr/scsi_cmnd.h b/usr/scsi_cmnd.h > index 011f3e6..776ffb3 100644 > --- a/usr/scsi_cmnd.h > +++ b/usr/scsi_cmnd.h > @@ -17,6 +17,8 @@ struct scsi_data_buffer { > uint64_t buffer; > }; > > +#include <scsi/sg.h> > + > struct scsi_cmd { > struct target *c_target; > /* linked it_nexus->cmd_hash_list */ > @@ -31,6 +33,8 @@ struct scsi_cmd { > enum data_direction data_dir; > struct scsi_data_buffer in_sdb; > struct scsi_data_buffer out_sdb; > + /* used for bs_sg.c:bs_sg_cmd_submit() */ > + struct sg_iovec sg_iovec; > > uint64_t cmd_itn_id; > uint64_t offset; > diff --git a/usr/tgtd.h b/usr/tgtd.h > index 3323a9b..4f26a29 100644 > --- a/usr/tgtd.h > +++ b/usr/tgtd.h > @@ -117,6 +117,7 @@ struct device_type_template { > struct backingstore_template { > const char *bs_name; > int bs_datasize; > + int bs_passthrough; > int (*bs_open)(struct scsi_lu *dev, char *path, int *fd, uint64_t *size); > void (*bs_close)(struct scsi_lu *dev); > int (*bs_init)(struct scsi_lu *dev); -- 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 |