On Fri, Nov 04, 2011 at 02:14:27PM +0000, Nicholas A. Bellinger wrote:
> From: Nicholas Bellinger <nab@xxxxxxxxxxxxxxx>
>
> This patch removes legacy usage of PYX_TRANSPORT_* return codes in a number
> of locations and addresses cases where transport_generic_request_failure()
> was returning the incorrect sense upon CHECK_CONDITION status after the
> v3.1 converson to use errno return codes.
>
> This includes the conversion of transport_generic_request_failure() to
> process cmd->scsi_sense_reason and handle extra TCM_RESERVATION_CONFLICT
> before calling transport_send_check_condition_and_sense() to queue up
> response status. It also drops PYX_TRANSPORT_OUT_OF_MEMORY_RESOURCES legacy
> usgae, and returns TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE w/ a response
> for these cases.
>
> transport_generic_allocate_tasks(), transport_generic_new_cmd(), backend
> SCF_SCSI_DATA_SG_IO_CDB ->do_task(), and emulated ->execute_task() have
> all been updated to set se_cmd->scsi_sense_reason and return errno codes
> universally upon failure. This includes cmd->scsi_sense_reason assignment
> in target_core_alua.c, target_core_pr.c and target_core_cdb.c emulation code.
>
> Finally it updates fabric modules to remove the legacy usage, and for
> TFO->new_cmd_map() callers forwards return values outside of fabric code.
> iscsi-target has also been updated to remove a handful of special cases
> related to the cleanup, along w/ signaling QUEUE_FULL handling for
> ft_write_pending() and ibmvscsis_write_pending() fabric callers.
I really like the simplication by removing various layers of error
codes, but I wonder if it's still one too much. What prevents
us from returning the sense codes in-line from the submission functions
instead of returning some negative error value and then looking at
scsi_sense_reason?
A few additional comments below:
> + if (ret < 0 || cmd->se_cmd.se_cmd_flags & SCF_SCSI_CDB_EXCEPTION) {
It's generally safer to keep the braces around & expressions.
> if (cmd->t_tasks_failed) {
> - if (!task->task_error_status) {
> - task->task_error_status =
> - PYX_TRANSPORT_UNKNOWN_SAM_OPCODE;
> - cmd->transport_error_status =
> - PYX_TRANSPORT_UNKNOWN_SAM_OPCODE;
> - }
> + if (!task->task_error_status)
> + cmd->scsi_sense_reason =
> + TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
> +
task_error_status is never set after your patch.
--
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
