* [PATCH 0/1] scsi: target: core: Fix sense key for invalid XCOPY request
@ 2021-06-24 11:19 Sergey Samoylenko
2021-06-24 11:19 ` [PATCH 1/1] " Sergey Samoylenko
2021-07-21 21:45 ` [PATCH 0/1] " David Disseldorp
0 siblings, 2 replies; 8+ messages in thread
From: Sergey Samoylenko @ 2021-06-24 11:19 UTC (permalink / raw)
To: martin.petersen, michael.christie, target-devel
Cc: linux-scsi, linux, Sergey Samoylenko
EXTENDED COPY tests in libiscsi [1] show that TCM doesn't
follow SPC4 when detects invalid parameters in a XCOPY
command or IO errors. The replies from TCM contain wrong sense
key or ASCQ for incorrect request.
The series fixes the following tests from libiscsi:
SCSI.ExtendedCopy.DescrType
SCSI.ExtendedCopy.DescrLimits
SCSI.ExtendedCopy.ParamHdr
SCSI.ExtendedCopy.ValidSegDescr
SCSI.ExtendedCopy.ValidTgtDescr
1. https://github.com/sahlberg/libiscsi
Sergey Samoylenko (1):
scsi: target: core: Fix sense key for invalid XCOPY request
drivers/target/target_core_xcopy.c | 26 +++++++++++++++-----------
1 file changed, 15 insertions(+), 11 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/1] scsi: target: core: Fix sense key for invalid XCOPY request
2021-06-24 11:19 [PATCH 0/1] scsi: target: core: Fix sense key for invalid XCOPY request Sergey Samoylenko
@ 2021-06-24 11:19 ` Sergey Samoylenko
2021-06-24 15:49 ` kernel test robot
2021-06-24 15:57 ` Bart Van Assche
2021-07-21 21:45 ` [PATCH 0/1] " David Disseldorp
1 sibling, 2 replies; 8+ messages in thread
From: Sergey Samoylenko @ 2021-06-24 11:19 UTC (permalink / raw)
To: martin.petersen, michael.christie, target-devel
Cc: linux-scsi, linux, Sergey Samoylenko, Roman Bolshakov,
Konstantin Shelekhin
TCM fails to pass the following tests in libiscsi:
SCSI.ExtendedCopy.DescrType
SCSI.ExtendedCopy.DescrLimits
SCSI.ExtendedCopy.ParamHdr
SCSI.ExtendedCopy.ValidSegDescr
SCSI.ExtendedCopy.ValidTgtDescr
XCOPY always returns the same NOT READY sense key for all
detected errors. It changes a sense key for invalid requests
to ILLEGAL REQUEST sense key, for aborted transferring data
(IO error and etc.) to COPY ABORTED.
Reviewed-by: Roman Bolshakov <r.bolshakov@yadro.com>
Reviewed-by: Konstantin Shelekhin <k.shelekhin@yadro.com>
Signed-off-by: Sergey Samoylenko <s.samoylenko@yadro.com>
---
drivers/target/target_core_xcopy.c | 26 +++++++++++++++-----------
1 file changed, 15 insertions(+), 11 deletions(-)
diff --git a/drivers/target/target_core_xcopy.c b/drivers/target/target_core_xcopy.c
index 0f1319336f3e..64baf3e8c079 100644
--- a/drivers/target/target_core_xcopy.c
+++ b/drivers/target/target_core_xcopy.c
@@ -674,12 +674,16 @@ static void target_xcopy_do_work(struct work_struct *work)
unsigned int max_sectors;
int rc = 0;
unsigned short nolb, max_nolb, copied_nolb = 0;
+ sense_reason_t sense_rc;
- if (target_parse_xcopy_cmd(xop) != TCM_NO_SENSE)
+ sense_rc = target_parse_xcopy_cmd(xop);
+ if (sense_rc != TCM_NO_SENSE)
goto err_free;
- if (WARN_ON_ONCE(!xop->src_dev) || WARN_ON_ONCE(!xop->dst_dev))
+ if (WARN_ON_ONCE(!xop->src_dev) || WARN_ON_ONCE(!xop->dst_dev)) {
+ sense_rc = TCM_INVALID_PARAMETER_LIST;
goto err_free;
+ }
src_dev = xop->src_dev;
dst_dev = xop->dst_dev;
@@ -762,20 +766,20 @@ static void target_xcopy_do_work(struct work_struct *work)
return;
out:
+ /*
+ * The XCOPY command was aborted after some data was transferred.
+ * Terminate command with CHECK CONDITION status, with the sense key
+ * set to COPY ABORTED.
+ */
+ sense_rc = TCM_COPY_TARGET_DEVICE_NOT_REACHABLE;
xcopy_pt_undepend_remotedev(xop);
target_free_sgl(xop->xop_data_sg, xop->xop_data_nents);
err_free:
kfree(xop);
- /*
- * Don't override an error scsi status if it has already been set
- */
- if (ec_cmd->scsi_status == SAM_STAT_GOOD) {
- pr_warn_ratelimited("target_xcopy_do_work: rc: %d, Setting X-COPY"
- " CHECK_CONDITION -> sending response\n", rc);
- ec_cmd->scsi_status = SAM_STAT_CHECK_CONDITION;
- }
- target_complete_cmd(ec_cmd, ec_cmd->scsi_status);
+ pr_warn_ratelimited("target_xcopy_do_work: rc: %d, sense: %u,"
+ " XCOPY operation failed\n", rc, sense_rc);
+ target_complete_cmd_with_sense(ec_cmd, sense_rc);
}
/*
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] scsi: target: core: Fix sense key for invalid XCOPY request
2021-06-24 11:19 ` [PATCH 1/1] " Sergey Samoylenko
@ 2021-06-24 15:49 ` kernel test robot
2021-06-24 15:57 ` Bart Van Assche
1 sibling, 0 replies; 8+ messages in thread
From: kernel test robot @ 2021-06-24 15:49 UTC (permalink / raw)
To: Sergey Samoylenko, martin.petersen, michael.christie, target-devel
Cc: kbuild-all, clang-built-linux, linux-scsi, linux,
Sergey Samoylenko, Roman Bolshakov, Konstantin Shelekhin
[-- Attachment #1: Type: text/plain, Size: 6969 bytes --]
Hi Sergey,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on mkp-scsi/for-next]
[also build test ERROR on v5.13-rc7 next-20210624]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Sergey-Samoylenko/scsi-target-core-Fix-sense-key-for-invalid-XCOPY-request/20210624-192229
base: https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
config: x86_64-randconfig-a001-20210622 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 7c8a507272587f181ec29401453949ebcd8fec65)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
# https://github.com/0day-ci/linux/commit/2af81ab452a5bda2c33f25a230cda9f97ebb0431
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Sergey-Samoylenko/scsi-target-core-Fix-sense-key-for-invalid-XCOPY-request/20210624-192229
git checkout 2af81ab452a5bda2c33f25a230cda9f97ebb0431
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> drivers/target/target_core_xcopy.c:782:2: error: implicit declaration of function 'target_complete_cmd_with_sense' [-Werror,-Wimplicit-function-declaration]
target_complete_cmd_with_sense(ec_cmd, sense_rc);
^
drivers/target/target_core_xcopy.c:782:2: note: did you mean 'target_complete_cmd_with_length'?
include/target/target_core_backend.h:78:6: note: 'target_complete_cmd_with_length' declared here
void target_complete_cmd_with_length(struct se_cmd *, u8, int);
^
1 error generated.
vim +/target_complete_cmd_with_sense +782 drivers/target/target_core_xcopy.c
667
668 static void target_xcopy_do_work(struct work_struct *work)
669 {
670 struct xcopy_op *xop = container_of(work, struct xcopy_op, xop_work);
671 struct se_cmd *ec_cmd = xop->xop_se_cmd;
672 struct se_device *src_dev, *dst_dev;
673 sector_t src_lba, dst_lba, end_lba;
674 unsigned int max_sectors;
675 int rc = 0;
676 unsigned short nolb, max_nolb, copied_nolb = 0;
677 sense_reason_t sense_rc;
678
679 sense_rc = target_parse_xcopy_cmd(xop);
680 if (sense_rc != TCM_NO_SENSE)
681 goto err_free;
682
683 if (WARN_ON_ONCE(!xop->src_dev) || WARN_ON_ONCE(!xop->dst_dev)) {
684 sense_rc = TCM_INVALID_PARAMETER_LIST;
685 goto err_free;
686 }
687
688 src_dev = xop->src_dev;
689 dst_dev = xop->dst_dev;
690 src_lba = xop->src_lba;
691 dst_lba = xop->dst_lba;
692 nolb = xop->nolb;
693 end_lba = src_lba + nolb;
694 /*
695 * Break up XCOPY I/O into hw_max_sectors sized I/O based on the
696 * smallest max_sectors between src_dev + dev_dev, or
697 */
698 max_sectors = min(src_dev->dev_attrib.hw_max_sectors,
699 dst_dev->dev_attrib.hw_max_sectors);
700 max_sectors = min_t(u32, max_sectors, XCOPY_MAX_SECTORS);
701
702 max_nolb = min_t(u16, max_sectors, ((u16)(~0U)));
703
704 pr_debug("target_xcopy_do_work: nolb: %hu, max_nolb: %hu end_lba: %llu\n",
705 nolb, max_nolb, (unsigned long long)end_lba);
706 pr_debug("target_xcopy_do_work: Starting src_lba: %llu, dst_lba: %llu\n",
707 (unsigned long long)src_lba, (unsigned long long)dst_lba);
708
709 while (src_lba < end_lba) {
710 unsigned short cur_nolb = min(nolb, max_nolb);
711 u32 cur_bytes = cur_nolb * src_dev->dev_attrib.block_size;
712
713 if (cur_bytes != xop->xop_data_bytes) {
714 /*
715 * (Re)allocate a buffer large enough to hold the XCOPY
716 * I/O size, which can be reused each read / write loop.
717 */
718 target_free_sgl(xop->xop_data_sg, xop->xop_data_nents);
719 rc = target_alloc_sgl(&xop->xop_data_sg,
720 &xop->xop_data_nents,
721 cur_bytes,
722 false, false);
723 if (rc < 0)
724 goto out;
725 xop->xop_data_bytes = cur_bytes;
726 }
727
728 pr_debug("target_xcopy_do_work: Calling read src_dev: %p src_lba: %llu,"
729 " cur_nolb: %hu\n", src_dev, (unsigned long long)src_lba, cur_nolb);
730
731 rc = target_xcopy_read_source(ec_cmd, xop, src_dev, src_lba, cur_nolb);
732 if (rc < 0)
733 goto out;
734
735 src_lba += cur_nolb;
736 pr_debug("target_xcopy_do_work: Incremented READ src_lba to %llu\n",
737 (unsigned long long)src_lba);
738
739 pr_debug("target_xcopy_do_work: Calling write dst_dev: %p dst_lba: %llu,"
740 " cur_nolb: %hu\n", dst_dev, (unsigned long long)dst_lba, cur_nolb);
741
742 rc = target_xcopy_write_destination(ec_cmd, xop, dst_dev,
743 dst_lba, cur_nolb);
744 if (rc < 0)
745 goto out;
746
747 dst_lba += cur_nolb;
748 pr_debug("target_xcopy_do_work: Incremented WRITE dst_lba to %llu\n",
749 (unsigned long long)dst_lba);
750
751 copied_nolb += cur_nolb;
752 nolb -= cur_nolb;
753 }
754
755 xcopy_pt_undepend_remotedev(xop);
756 target_free_sgl(xop->xop_data_sg, xop->xop_data_nents);
757 kfree(xop);
758
759 pr_debug("target_xcopy_do_work: Final src_lba: %llu, dst_lba: %llu\n",
760 (unsigned long long)src_lba, (unsigned long long)dst_lba);
761 pr_debug("target_xcopy_do_work: Blocks copied: %hu, Bytes Copied: %u\n",
762 copied_nolb, copied_nolb * dst_dev->dev_attrib.block_size);
763
764 pr_debug("target_xcopy_do_work: Setting X-COPY GOOD status -> sending response\n");
765 target_complete_cmd(ec_cmd, SAM_STAT_GOOD);
766 return;
767
768 out:
769 /*
770 * The XCOPY command was aborted after some data was transferred.
771 * Terminate command with CHECK CONDITION status, with the sense key
772 * set to COPY ABORTED.
773 */
774 sense_rc = TCM_COPY_TARGET_DEVICE_NOT_REACHABLE;
775 xcopy_pt_undepend_remotedev(xop);
776 target_free_sgl(xop->xop_data_sg, xop->xop_data_nents);
777
778 err_free:
779 kfree(xop);
780 pr_warn_ratelimited("target_xcopy_do_work: rc: %d, sense: %u,"
781 " XCOPY operation failed\n", rc, sense_rc);
> 782 target_complete_cmd_with_sense(ec_cmd, sense_rc);
783 }
784
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 45142 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] scsi: target: core: Fix sense key for invalid XCOPY request
2021-06-24 11:19 ` [PATCH 1/1] " Sergey Samoylenko
2021-06-24 15:49 ` kernel test robot
@ 2021-06-24 15:57 ` Bart Van Assche
2021-06-24 17:27 ` Sergey Samoylenko
1 sibling, 1 reply; 8+ messages in thread
From: Bart Van Assche @ 2021-06-24 15:57 UTC (permalink / raw)
To: Sergey Samoylenko, martin.petersen, michael.christie, target-devel
Cc: linux-scsi, linux, Roman Bolshakov, Konstantin Shelekhin
On 6/24/21 4:19 AM, Sergey Samoylenko wrote:
> + pr_warn_ratelimited("target_xcopy_do_work: rc: %d, sense: %u,"
> + " XCOPY operation failed\n", rc, sense_rc);
Please do not split format strings across multiple lines. Checkpatch
should have complained about this.
Thanks,
Bart.
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH 1/1] scsi: target: core: Fix sense key for invalid XCOPY request
2021-06-24 15:57 ` Bart Van Assche
@ 2021-06-24 17:27 ` Sergey Samoylenko
0 siblings, 0 replies; 8+ messages in thread
From: Sergey Samoylenko @ 2021-06-24 17:27 UTC (permalink / raw)
To: Bart Van Assche, martin.petersen, michael.christie, target-devel
Cc: linux-scsi, linux, Roman Bolshakov, Konstantin Shelekhin
Thank you Bart,
I used API (target_complete_cmd_with_sense) which is not in
mainline kernel. I need to change the patch.
Sorry, this is my mistake.
Sergey.
>On 6/24/21 4:19 AM, Sergey Samoylenko wrote:
>> + pr_warn_ratelimited("target_xcopy_do_work: rc: %d, sense: %u,"
>> + " XCOPY operation failed\n", rc, sense_rc);
>
>Please do not split format strings across multiple lines. Checkpatch
>should have complained about this.
>
>Thanks,
>
>Bart.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/1] scsi: target: core: Fix sense key for invalid XCOPY request
2021-06-24 11:19 [PATCH 0/1] scsi: target: core: Fix sense key for invalid XCOPY request Sergey Samoylenko
2021-06-24 11:19 ` [PATCH 1/1] " Sergey Samoylenko
@ 2021-07-21 21:45 ` David Disseldorp
2021-07-22 11:03 ` Sergey Samoylenko
1 sibling, 1 reply; 8+ messages in thread
From: David Disseldorp @ 2021-07-21 21:45 UTC (permalink / raw)
To: Sergey Samoylenko
Cc: martin.petersen, michael.christie, target-devel, linux-scsi, linux
Hi Sergey,
On Thu, 24 Jun 2021 14:19:25 +0300, Sergey Samoylenko wrote:
> EXTENDED COPY tests in libiscsi [1] show that TCM doesn't
> follow SPC4 when detects invalid parameters in a XCOPY
> command or IO errors. The replies from TCM contain wrong sense
> key or ASCQ for incorrect request.
>
> The series fixes the following tests from libiscsi:
We've hit this too. The incorrect sense reporting appears to also affect
VMware XCOPY fallback to initiator driven READ/WRITE. I'm pretty sure
this is a regression from d877d7275be34ad70ce92bcbb4bb36cec77ed004, so
should probably be marked as such via a Fixes tag.
Cheers, David
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH 0/1] scsi: target: core: Fix sense key for invalid XCOPY request
2021-07-21 21:45 ` [PATCH 0/1] " David Disseldorp
@ 2021-07-22 11:03 ` Sergey Samoylenko
2021-07-22 13:27 ` David Disseldorp
0 siblings, 1 reply; 8+ messages in thread
From: Sergey Samoylenko @ 2021-07-22 11:03 UTC (permalink / raw)
To: David Disseldorp, Bart Van Assche
Cc: martin.petersen, michael.christie, target-devel, linux-scsi, linux
Hi David,
> Hi Sergey,
>
> On Thu, 24 Jun 2021 14:19:25 +0300, Sergey Samoylenko wrote:
>
>> EXTENDED COPY tests in libiscsi [1] show that TCM doesn't follow SPC4
>> when detects invalid parameters in a XCOPY command or IO errors. The
>> replies from TCM contain wrong sense key or ASCQ for incorrect
>> request.
>>
>> The series fixes the following tests from libiscsi:
>
> We've hit this too. The incorrect sense reporting appears to also affect VMware XCOPY fallback to initiator driven READ/WRITE. I'm pretty sure this is a regression from
> d877d7275be34ad70ce92bcbb4bb36cec77ed004, so should probably be marked as such via a Fixes tag.
>
> Cheers, David
The d877d7275be34ad70ce92bcbb4bb36cec77ed004 was added for v4.10.x kernel and it was necessary
for to avoid LUN removal race conditions. Later you excluded using configfs in the XCOPY workqueue.
It was the 2896c93811e39d63a4d9b63ccf12a8fbc226e5e4.
If we remove the d877d7275be34ad70ce92bcbb4bb36cec77ed004, will it break anything?
We have accumulated many changes between v4.10 and v5.14.
David, maybe can we move the helper 'target_complete_cmd_with_sense' from your path to mainline kernel?
I think it will be useful in the future.
Best regards,
Sergey
From 2e96d8ac2695a13edf71976bd099003dda52056d Mon Sep 17 00:00:00 2001
From: Mike Christie <michaelc@cs.wisc.edu>
Date: Wed, 29 Jul 2015 04:23:49 -0500
Subject: [PATCH] target: compare and write backend driver sense handling
References: fate#318836
Patch-mainline: Not yet, SES2 clustered LIO/RBD
Currently, backend drivers seem to only fail IO with
SAM_STAT_CHECK_CONDITION which gets us
TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE.
For compare and write support we will want to be able to fail with
TCM_MISCOMPARE_VERIFY. This patch adds a new helper that allows backend
drivers to fail with specific sense codes.
It also allows the backend driver to set the miscompare offset.
Signed-off-by: Mike Christie <michaelc@cs.wisc.edu>
Acked-by: David Disseldorp <ddiss@suse.de>
[ddiss@suse.de rebase against ab78fef4d5 and 9ec1e1ce3a]
---
drivers/target/target_core_transport.c | 34 ++++++++++++++++++++++++++++++---
include/target/target_core_backend.h | 1
include/target/target_core_base.h | 5 +++-
3 files changed, 36 insertions(+), 4 deletions(-)
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -718,8 +718,7 @@ static void target_complete_failure_work
{
struct se_cmd *cmd = container_of(work, struct se_cmd, work);
- transport_generic_request_failure(cmd,
- TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE);
+ transport_generic_request_failure(cmd, cmd->sense_reason);
}
/*
@@ -837,7 +836,8 @@ static bool target_cmd_interrupted(struc
}
/* May be called from interrupt context so must not sleep. */
-void target_complete_cmd(struct se_cmd *cmd, u8 scsi_status)
+static void __target_complete_cmd(struct se_cmd *cmd, u8 scsi_status,
+ sense_reason_t sense_reason)
{
int success;
unsigned long flags;
@@ -846,6 +846,7 @@ void target_complete_cmd(struct se_cmd *
return;
cmd->scsi_status = scsi_status;
+ cmd->sense_reason = sense_reason;
spin_lock_irqsave(&cmd->t_state_lock, flags);
switch (cmd->scsi_status) {
@@ -871,8 +872,22 @@ void target_complete_cmd(struct se_cmd *
else
queue_work(target_completion_wq, &cmd->work);
}
+
+void target_complete_cmd(struct se_cmd *cmd, u8 scsi_status)
+{
+ __target_complete_cmd(cmd, scsi_status, scsi_status ?
+ TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE :
+ TCM_NO_SENSE);
+}
EXPORT_SYMBOL(target_complete_cmd);
+void target_complete_cmd_with_sense(struct se_cmd *cmd,
+ sense_reason_t sense_reason)
+{
+ __target_complete_cmd(cmd, SAM_STAT_CHECK_CONDITION, sense_reason);
+}
+EXPORT_SYMBOL(target_complete_cmd_with_sense);
+
void target_set_cmd_data_length(struct se_cmd *cmd, int length)
{
if (length < cmd->data_length) {
@@ -1917,6 +1932,7 @@ void transport_generic_request_failure(s
case TCM_UNSUPPORTED_TARGET_DESC_TYPE_CODE:
case TCM_TOO_MANY_SEGMENT_DESCS:
case TCM_UNSUPPORTED_SEGMENT_DESC_TYPE_CODE:
+ case TCM_MISCOMPARE_VERIFY:
break;
case TCM_OUT_OF_RESOURCES:
cmd->scsi_status = SAM_STAT_TASK_SET_FULL;
@@ -3101,11 +3117,13 @@ bool transport_wait_for_tasks(struct se_
}
EXPORT_SYMBOL(transport_wait_for_tasks);
+
struct sense_info {
u8 key;
u8 asc;
u8 ascq;
bool add_sector_info;
+ bool add_sense_info;
};
static const struct sense_info sense_info_table[] = {
@@ -3203,6 +3221,7 @@ static const struct sense_info sense_inf
.key = MISCOMPARE,
.asc = 0x1d, /* MISCOMPARE DURING VERIFY OPERATION */
.ascq = 0x00,
+ .add_sense_info = true,
},
[TCM_LOGICAL_BLOCK_GUARD_CHECK_FAILED] = {
.key = ABORTED_COMMAND,
@@ -3255,6 +3274,13 @@ static const struct sense_info sense_inf
},
};
+static void transport_err_sense_info(unsigned char *buffer, u32 info)
+{
+ buffer[SPC_INFO_VALIDITY_OFFSET] |= 0x80;
+ /* Sense Information */
+ put_unaligned_be32(info, &buffer[3]);
+}
+
/**
* translate_sense_reason - translate a sense reason into T10 key, asc and ascq
* @cmd: SCSI command in which the resulting sense buffer or SCSI status will
@@ -3304,6 +3330,8 @@ static void translate_sense_reason(struc
WARN_ON_ONCE(scsi_set_sense_information(buffer,
cmd->scsi_sense_length,
cmd->bad_sector) < 0);
+ if (si->add_sense_info)
+ transport_err_sense_info(buffer, cmd->sense_info);
}
int
--- a/include/target/target_core_backend.h
+++ b/include/target/target_core_backend.h
@@ -74,6 +74,7 @@ void target_backend_unregister(const str
void target_complete_cmd(struct se_cmd *, u8);
void target_set_cmd_data_length(struct se_cmd *, int);
+void target_complete_cmd_with_sense(struct se_cmd *, sense_reason_t);
void target_complete_cmd_with_length(struct se_cmd *, u8, int);
void transport_copy_sense_to_cmd(struct se_cmd *, unsigned char *);
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -22,11 +22,12 @@
*/
#define TRANSPORT_SENSE_BUFFER 96
/* Used by transport_send_check_condition_and_sense() */
+#define SPC_INFO_VALIDITY_OFFSET 0
#define SPC_SENSE_KEY_OFFSET 2
#define SPC_ADD_SENSE_LEN_OFFSET 7
#define SPC_DESC_TYPE_OFFSET 8
#define SPC_ADDITIONAL_DESC_LEN_OFFSET 9
-#define SPC_VALIDITY_OFFSET 10
+#define SPC_CMD_INFO_VALIDITY_OFFSET 10
#define SPC_ASC_KEY_OFFSET 12
#define SPC_ASCQ_KEY_OFFSET 13
#define TRANSPORT_IQN_LEN 224
@@ -452,6 +453,8 @@ enum target_core_dif_check {
#define TCM_ACA_TAG 0x24
struct se_cmd {
+ sense_reason_t sense_reason;
+ u32 sense_info;
/* SAM response code being sent to initiator */
u8 scsi_status;
u8 scsi_asc;
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/1] scsi: target: core: Fix sense key for invalid XCOPY request
2021-07-22 11:03 ` Sergey Samoylenko
@ 2021-07-22 13:27 ` David Disseldorp
0 siblings, 0 replies; 8+ messages in thread
From: David Disseldorp @ 2021-07-22 13:27 UTC (permalink / raw)
To: Sergey Samoylenko
Cc: Bart Van Assche, martin.petersen, michael.christie, target-devel,
linux-scsi, linux
On Thu, 22 Jul 2021 11:03:02 +0000, Sergey Samoylenko wrote:
> Hi David,
>
> > Hi Sergey,
> >
> > On Thu, 24 Jun 2021 14:19:25 +0300, Sergey Samoylenko wrote:
> >
> >> EXTENDED COPY tests in libiscsi [1] show that TCM doesn't follow SPC4
> >> when detects invalid parameters in a XCOPY command or IO errors. The
> >> replies from TCM contain wrong sense key or ASCQ for incorrect
> >> request.
> >>
> >> The series fixes the following tests from libiscsi:
> >
> > We've hit this too. The incorrect sense reporting appears to also affect VMware XCOPY fallback to initiator driven READ/WRITE. I'm pretty sure this is a regression from
> > d877d7275be34ad70ce92bcbb4bb36cec77ed004, so should probably be marked as such via a Fixes tag.
> >
> > Cheers, David
>
> The d877d7275be34ad70ce92bcbb4bb36cec77ed004 was added for v4.10.x kernel and it was necessary
> for to avoid LUN removal race conditions. Later you excluded using configfs in the XCOPY workqueue.
> It was the 2896c93811e39d63a4d9b63ccf12a8fbc226e5e4.
>
> If we remove the d877d7275be34ad70ce92bcbb4bb36cec77ed004, will it break anything?
> We have accumulated many changes between v4.10 and v5.14.
>
> David, maybe can we move the helper 'target_complete_cmd_with_sense' from your path to mainline kernel?
> I think it will be useful in the future.
I don't think it makes sense to revert d877d7275be34. I agree that
Mike's target_complete_cmd_with_sense() patch should be helpful for
proper sense propagation here.
Cheers, David
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-07-22 13:28 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-24 11:19 [PATCH 0/1] scsi: target: core: Fix sense key for invalid XCOPY request Sergey Samoylenko
2021-06-24 11:19 ` [PATCH 1/1] " Sergey Samoylenko
2021-06-24 15:49 ` kernel test robot
2021-06-24 15:57 ` Bart Van Assche
2021-06-24 17:27 ` Sergey Samoylenko
2021-07-21 21:45 ` [PATCH 0/1] " David Disseldorp
2021-07-22 11:03 ` Sergey Samoylenko
2021-07-22 13:27 ` David Disseldorp
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).