On Sun, 6 Jun 2010 20:50:25 -0700 "Nicholas A. Bellinger" <nab@xxxxxxxxxxxxxxx> wrote: > From: Nicholas Bellinger <nab@xxxxxxxxxxxxxxx> > > This patch adds two new queueing and completion function pointers to struct scsi_lu called ->cmd_perform() > and ->cmd_done() for handling existing internal STGT port emulation and the struct scsi_cmd > passthrough with bs_sg.c. It retains the struct device_type_template->cmd_passthrough() > from the original patches, which still appears to be necessary for a device type to perform passthrough. > Also as before we modify the struct device_type_template sbc_template->cmd_passthrough() for sbc.c / > TYPE_DISK that we want to use passthrough for bs_sg LUNs. > > For the setup path, we update tgt_device_create() to check if lu->cmd_perform() and lu->cmd_done() > have been set by struct backingstore_template->bs_init(). We expect bs_sg to setup these > pointers for us using the new target_cmd_perform_passthrough() and __cmd_done_passthrough() (see below). > Otherwise we setup the pointers following existing logic with target_cmd_perform() (also below) and > __cmd_done() for the non bs_sg case. > > For the queue path and struct scsi_lu->cmd_perform() it made sense to split up target_cmd_queue() > into two functions, the original code at the tail of target_cmd_queue() now becomes > target_cmd_perform() and calls existing STGT port emulation code via cmd_enabled() -> scsi_cmd_perform(). > A new function for passthrough has been added called target_cmd_perform_passthrough() that will do > struct scsi_lu->dev_type_template.cmd_passthrough() into the device type for bs_sg LUNs. > > For the completion path and struct scsi_lu->cmd_done(), a new __cmd_done_passthrough() > has been added minus the original cmd_hlist_remove() and SCSI TASK attr checking in > __cmd_done(). __cmd_done() is used for the existing port emulation case, and modify the > two original users target_cmd_lookup() and abort_cmd() to call cmd->dev->cmd_done() instead. > > Finally, it adds the passthrough case to tgt_device_create() in order to locate the > struct device_type_template * using device_type_passthrough(). > > This patch has been tested with STGT/iSCSI and TCM_Loop SPC-4 iSCSI Target Port emulation. > > Signed-off-by: Nicholas A. Bellinger <nab@xxxxxxxxxxxxxxx> > --- > usr/target.c | 113 ++++++++++++++++++++++++++++++++++++++++++++++++++++------ > usr/tgtd.h | 16 ++++++++ > 2 files changed, 118 insertions(+), 11 deletions(-) > > diff --git a/usr/target.c b/usr/target.c > index c848757..3e2aa2b 100644 > --- a/usr/target.c > +++ b/usr/target.c > @@ -48,11 +48,27 @@ int device_type_register(struct device_type_template *t) > return 0; > } > > -static struct device_type_template *device_type_lookup(int type) > +static struct device_type_template *device_type_passthrough(void) > { > struct device_type_template *t; > > list_for_each_entry(t, &device_type_list, device_type_siblings) { > + if (t->cmd_passthrough != NULL) > + return t; > + } > + return NULL; > +} > + > +static struct device_type_template *device_type_lookup(int type, int passthrough) > +{ > + struct device_type_template *t; > + > + if (passthrough) > + return device_type_passthrough(); > + > + list_for_each_entry(t, &device_type_list, device_type_siblings) { > + if (t->cmd_passthrough != NULL) > + continue; > if (t->type == type) > return t; > } > @@ -425,11 +441,13 @@ static match_table_t device_tokens = { > {Opt_err, NULL}, > }; I think that we can avoid the above complexity if we create a new device type. > +static void __cmd_done(struct target *, struct scsi_cmd *); > + > int tgt_device_create(int tid, int dev_type, uint64_t lun, char *params, > int backing) > { > char *p, *path = NULL, *bstype = NULL; > - int ret = 0; > + int ret = 0, passthrough = 0; > struct target *target; > struct scsi_lu *lu, *pos; > struct device_type_template *t; > @@ -479,10 +497,11 @@ int tgt_device_create(int tid, int dev_type, uint64_t lun, char *params, > ret = TGTADM_INVALID_REQUEST; > goto out; > } > + passthrough = bst->bs_passthrough; > } > } > > - t = device_type_lookup(dev_type); > + t = device_type_lookup(dev_type, passthrough); > if (!t) { > eprintf("Unknown device type %d\n", dev_type); > ret = TGTADM_INVALID_REQUEST; > @@ -515,6 +534,26 @@ int tgt_device_create(int tid, int dev_type, uint64_t lun, char *params, > if (ret) > goto fail_lu_init; > } > + /* > + * Check if struct scsi_lu->cmd_perform() has been filled in > + * by the SG_IO backstore for passthrough (SG_IO) in ->bs_init() call. > + * If the function pointer does not exist, use the default internal > + * target_cmd_perform() and __cmd_done() calls. > + */ > + if (!(lu->cmd_perform)) { > + lu->cmd_perform = &target_cmd_perform; > + lu->cmd_done = &__cmd_done; > + } else if (!(lu->cmd_done)) { > + eprintf("Unable to locate struct scsi_lu->cmd_done() with" > + " valid ->cmd_perform()\n"); > + ret = TGTADM_INVALID_REQUEST; > + goto fail_bs_init; > + } else if (!(lu->dev_type_template.cmd_passthrough)) { > + eprintf("Unable to locate ->cmd_passthrough() handler" > + " for device type template\n"); > + ret = TGTADM_INVALID_REQUEST; > + goto fail_bs_init; > + } I think that it's more simple to set up the function pointers first then let bs_init overwrite them. -- To unsubscribe from this list: send the line "unsubscribe stgt" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html