Re: [PATCH 4/5] target: user: Fix sense data handling

["Nicholas A. Bellinger" <nab@linux-iscsi.org>]

On Thu, 2017-07-06 at 23:05 -0700, Nicholas A. Bellinger wrote:
> On Fri, 2017-07-07 at 14:14 +0900, Damien Le Moal wrote:
> > Nicholas,
> > 
> > On 7/7/17 13:50, Nicholas A. Bellinger wrote:
> > > Hey MNC & Co,
> > > 
> > > On Wed, 2017-06-28 at 12:44 -0500, Mike Christie wrote:
> > >> On 06/28/2017 12:58 AM, Damien Le Moal wrote:
> > >>> If the user request handler completed the request with a CHECK CONDITION
> > >>> status, tcmu_handle_completion() copies the command entry sense data
> > >>> into the session request structure sense data. However, the sense data
> > >>> length indicated by the field scsi_sense_length is not set and equal to
> > >>> 0, resulting in the copy being a no-op and failure to propagate the
> > >>> sense data back to the initiator. To fix this, properly set the session
> > >>> command sense data length and also set the session command
> > >>> SCF_TRANSPORT_TASK_SENSE flag to indicate that the command has valid
> > >>> sense data.
> > >>>
> > >>> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
> > >>> ---
> > >>>  drivers/target/target_core_user.c | 4 +++-
> > >>>  1 file changed, 3 insertions(+), 1 deletion(-)
> > >>>
> > >>> diff --git a/drivers/target/target_core_user.c b/drivers/target/target_core_user.c
> > >>> index beb5f09..7426b4c 100644
> > >>> --- a/drivers/target/target_core_user.c
> > >>> +++ b/drivers/target/target_core_user.c
> > >>> @@ -831,7 +831,9 @@ static void tcmu_handle_completion(struct tcmu_cmd *cmd, struct tcmu_cmd_entry *
> > >>>  		entry->rsp.scsi_status = SAM_STAT_CHECK_CONDITION;
> > >>>  	} else if (entry->rsp.scsi_status == SAM_STAT_CHECK_CONDITION) {
> > >>>  		memcpy(se_cmd->sense_buffer, entry->rsp.sense_buffer,
> > >>> -			       se_cmd->scsi_sense_length);
> > >>> +		       TRANSPORT_SENSE_BUFFER);
> > >>> +		se_cmd->scsi_sense_length = TRANSPORT_SENSE_BUFFER;
> > >>> +		se_cmd->se_cmd_flags |= SCF_TRANSPORT_TASK_SENSE;
> > >>>  	} else if (se_cmd->se_cmd_flags & SCF_BIDI) {
> > >>>  		/* Get Data-In buffer before clean up */
> > >>>  		gather_data_area(udev, cmd, true);
> > >>>
> > >>
> > >> I have a patch similar to this and 5/5 in my set:
> > >>
> > >> https://www.spinics.net/lists/target-devel/msg15430.html
> > >>
> > >> If yours gets merged first then I will build my set over them, so patch
> > >> looks ok to me.
> > >>
> > >> Reviewed-by: Mike Christie <mchristi@redhat.com>
> > > 
> > > The RFC patches above from May 31st weren't merged because I thought you
> > > where going to send out a second series..
> > 
> > My apologies. I have been busy with other things and could not get to that.
> > 

Oh btw, I was talking to MNC wrt to his original patch series, and not
yours.  :)