diff mbox

[5/5] target: core: Fix failed command sense data handling

Message ID 20170628055900.22889-6-damien.lemoal@wdc.com (mailing list archive)
State New, archived
Headers show

Commit Message

Damien Le Moal June 28, 2017, 5:59 a.m. UTC
For a target device without a transport->transport_complete method
defined (e.g. target_core_user), target_complete_cmd() will always
result in a failed command completion being processed through target
failure completion work even when the command failure comes from the
target processing and has valid sense data (and hence does not require
sense data emulation as done in the failure work processing). To ensure
that the failed command sense data is propagated as indicated by the
target, make sure that the normal "ok" work completion path is used by
moving the command SCF_TRANSPORT_TASK_SENSE flag test out of the
transport_complete defined conditional.

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 drivers/target/target_core_transport.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Mike Christie June 28, 2017, 5:45 p.m. UTC | #1
On 06/28/2017 12:59 AM, Damien Le Moal wrote:
> For a target device without a transport->transport_complete method
> defined (e.g. target_core_user), target_complete_cmd() will always
> result in a failed command completion being processed through target
> failure completion work even when the command failure comes from the
> target processing and has valid sense data (and hence does not require
> sense data emulation as done in the failure work processing). To ensure
> that the failed command sense data is propagated as indicated by the
> target, make sure that the normal "ok" work completion path is used by
> moving the command SCF_TRANSPORT_TASK_SENSE flag test out of the
> transport_complete defined conditional.
> 
> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
> ---
>  drivers/target/target_core_transport.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
> index f1b3a46..a18e4db 100644
> --- a/drivers/target/target_core_transport.c
> +++ b/drivers/target/target_core_transport.c
> @@ -719,10 +719,11 @@ void target_complete_cmd(struct se_cmd *cmd, u8 scsi_status)
>  		dev->transport->transport_complete(cmd,
>  				cmd->t_data_sg,
>  				transport_get_sense_buffer(cmd));
> -		if (cmd->se_cmd_flags & SCF_TRANSPORT_TASK_SENSE)
> -			success = 1;
>  	}
>  
> +	if (cmd->se_cmd_flags & SCF_TRANSPORT_TASK_SENSE)
> +		success = 1;
> +
>  	/*
>  	 * Check for case where an explicit ABORT_TASK has been received
>  	 * and transport_wait_for_tasks() will be waiting for completion..
> 

Reviewed-by: Mike Christie <mchristi@redhat.com>
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Nicholas A. Bellinger July 7, 2017, 5:06 a.m. UTC | #2
(Adding MNC CC')

On Wed, 2017-06-28 at 14:59 +0900, Damien Le Moal wrote:
> For a target device without a transport->transport_complete method
> defined (e.g. target_core_user), target_complete_cmd() will always
> result in a failed command completion being processed through target
> failure completion work even when the command failure comes from the
> target processing and has valid sense data (and hence does not require
> sense data emulation as done in the failure work processing). To ensure
> that the failed command sense data is propagated as indicated by the
> target, make sure that the normal "ok" work completion path is used by
> moving the command SCF_TRANSPORT_TASK_SENSE flag test out of the
> transport_complete defined conditional.
> 
> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
> ---
>  drivers/target/target_core_transport.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)

Per MNC, skipping this patch in favor of target_complete_cmd() checking
se_cmd->scsi_status:

https://www.spinics.net/lists/target-devel/msg15436.html

--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
index f1b3a46..a18e4db 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -719,10 +719,11 @@  void target_complete_cmd(struct se_cmd *cmd, u8 scsi_status)
 		dev->transport->transport_complete(cmd,
 				cmd->t_data_sg,
 				transport_get_sense_buffer(cmd));
-		if (cmd->se_cmd_flags & SCF_TRANSPORT_TASK_SENSE)
-			success = 1;
 	}
 
+	if (cmd->se_cmd_flags & SCF_TRANSPORT_TASK_SENSE)
+		success = 1;
+
 	/*
 	 * Check for case where an explicit ABORT_TASK has been received
 	 * and transport_wait_for_tasks() will be waiting for completion..