On Fri, Nov 04, 2011 at 01:40:00PM -0700, Nicholas A. Bellinger wrote: > Well, I think the removal of target specific returns universally cleans > things up (more) from the perspective of fabric module code. It's for sure better than before, no argument there. > I don't > really see a reason for doing this anymore if cmd->scsi_sense_reason is > set directly by target core, and if fabric module code is just checking > for non zero or less than zero return values to determine exception > status. If it is a bool, make it a bool and don't confuse people with the errno values. But from looking at the code it seems like we don't even need that - if you add TCM_SUCCESS and TCM_QUEUE_FULL values to enum tcm_sense_reason_table we could express all possible return values from the I/O path that way. If you make sure TCM_SUCCESS is 0 you still have your if (!ret) check for error path, but we get a much clearer picture of what is going on. > > > + if (!task->task_error_status) > > > + cmd->scsi_sense_reason = > > > + TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE; > > > + > > > > task_error_status is never set after your patch. > > > > Whoops. I'm not sure if we really have a functional need for > task->task_error_status anymore, but it still might be useful for > debugging purposes. I'll look at dropping this as well. I don't think we do. Even before your patch is was used as a rather whacky boolean during I/O completion. -- To unsubscribe from this list: send the line "unsubscribe target-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
