[PATCH 0/3] target: EXTENDED_COPY bugfixes for v3.12-rc

[PATCH 0/3] target: EXTENDED_COPY bugfixes for v3.12-rc

[Nicholas A. Bellinger]

From: Nicholas Bellinger <nab@linux-iscsi.org>

Hi folks,

Here are a few extra EXTENDED_COPY bugfixes that have come up recently.

This includes:

  - Reporting the correct ASC/ASCQ during target_do_xcopy() failures
  - Allow non zero list IDs to be processed as SNLID=1
  - Reject copy offload requests on source + destination devices when
    device attribute emulate_3pc has been explicitly disabled.

Thanks again to Thomas + Doug for their testing + feedback!

--nab

Nicholas Bellinger (3):
  target: Make target_do_xcopy failures return INVALID_PARAMETER_LIST
  target: Allow non zero ListID in EXTENDED_COPY parameter list
  target: Reject EXTENDED_COPY when emulate_3pc is disabled

 drivers/target/target_core_xcopy.c |   33 ++++++++++++++++++++-------------
 1 file changed, 20 insertions(+), 13 deletions(-)

-- 
1.7.10.4

[PATCH 1/3] target: Make target_do_xcopy failures return INVALID_PARAMETER_LIST

[Nicholas A. Bellinger]

From: Nicholas Bellinger <nab@linux-iscsi.org>

This patch changes target_do_xcopy() to properly return
TCM_INVALID_PARAMETER_LIST instead of TCM_INVALID_CDB_FIELD
for failures related to the EXTENDED_COPY parameter list parsing.

Also, move struct xcopy_op allocation ahead of kmapping to
handle the special TCM_OUT_OF_RESOURCES case.

Reported-by: Thomas Glanzmann <thomas@glanzmann.de>
Reported-by: Douglas Gilbert <dgilbert@interlog.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
 drivers/target/target_core_xcopy.c |   17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/target/target_core_xcopy.c b/drivers/target/target_core_xcopy.c
index 3da4fd1..6b9774c 100644
--- a/drivers/target/target_core_xcopy.c
+++ b/drivers/target/target_core_xcopy.c
@@ -896,9 +896,17 @@ sense_reason_t target_do_xcopy(struct se_cmd *se_cmd)
 		return TCM_UNSUPPORTED_SCSI_OPCODE;
 	}
 
+	xop = kzalloc(sizeof(struct xcopy_op), GFP_KERNEL);
+	if (!xop) {
+		pr_err("Unable to allocate xcopy_op\n");
+		return TCM_OUT_OF_RESOURCES;
+	}
+	xop->xop_se_cmd = se_cmd;
+
 	p = transport_kmap_data_sg(se_cmd);
 	if (!p) {
 		pr_err("transport_kmap_data_sg() failed in target_do_xcopy\n");
+		kfree(xop);
 		return TCM_OUT_OF_RESOURCES;
 	}
 
@@ -920,13 +928,6 @@ sense_reason_t target_do_xcopy(struct se_cmd *se_cmd)
 		goto out;
 	}
 
-	xop = kzalloc(sizeof(struct xcopy_op), GFP_KERNEL);
-	if (!xop) {
-		pr_err("Unable to allocate xcopy_op\n");
-		goto out;
-	}
-	xop->xop_se_cmd = se_cmd;
-
 	pr_debug("Processing XCOPY with list_id: 0x%02x list_id_usage: 0x%02x"
 		" tdll: %hu sdll: %u inline_dl: %u\n", list_id, list_id_usage,
 		tdll, sdll, inline_dl);
@@ -957,7 +958,7 @@ out:
 	if (p)
 		transport_kunmap_data_sg(se_cmd);
 	kfree(xop);
-	return TCM_INVALID_CDB_FIELD;
+	return TCM_INVALID_PARAMETER_LIST;
 }
 
 static sense_reason_t target_rcr_operating_parameters(struct se_cmd *se_cmd)
-- 
1.7.10.4

[PATCH 2/3] target: Allow non zero ListID in EXTENDED_COPY parameter list

[Nicholas A. Bellinger]

From: Nicholas Bellinger <nab@linux-iscsi.org>

This patch changes target_do_xcopy() to allow processing of non-zero
ListIDs in EXTENDED_COPY parameter list data, instead of returning
CHECK_CONDITION status.

As the copy offload implementation reports SNLID=1 (Supports No ListID)
in OPERATING PARAMETERS, any ListID value presented by the client is
currently ignored.

Also, properly extract list_id_usage for informational purposes.

Reported-by: Thomas Glanzmann <thomas@glanzmann.de>
Reported-by: Douglas Gilbert <dgilbert@interlog.com>
Cc: Hannes Reinecke <hare@suse.de>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
 drivers/target/target_core_xcopy.c |    7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/target/target_core_xcopy.c b/drivers/target/target_core_xcopy.c
index 6b9774c..fe98555 100644
--- a/drivers/target/target_core_xcopy.c
+++ b/drivers/target/target_core_xcopy.c
@@ -911,11 +911,8 @@ sense_reason_t target_do_xcopy(struct se_cmd *se_cmd)
 	}
 
 	list_id = p[0];
-	if (list_id != 0x00) {
-		pr_err("XCOPY with non zero list_id: 0x%02x\n", list_id);
-		goto out;
-	}
-	list_id_usage = (p[1] & 0x18);
+	list_id_usage = (p[1] & 0x18) >> 3;
+
 	/*
 	 * Determine TARGET DESCRIPTOR LIST LENGTH + SEGMENT DESCRIPTOR LIST LENGTH
 	 */
-- 
1.7.10.4

[PATCH 3/3] target: Reject EXTENDED_COPY when emulate_3pc is disabled

[Nicholas A. Bellinger]

From: Nicholas Bellinger <nab@linux-iscsi.org>

This patch rejects EXTENDED_COPY when the emulate_3pc attribute has
been explicitly disabled for the receiving device.

It also adds a similar check in target_xcopy_locate_se_dev_e4() to
ignore these devices when doing a search based upon the identifier
WWN provided by EXTENDED_COPY parameter list target descriptors.

Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
 drivers/target/target_core_xcopy.c |    9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/target/target_core_xcopy.c b/drivers/target/target_core_xcopy.c
index fe98555..eeeaf99 100644
--- a/drivers/target/target_core_xcopy.c
+++ b/drivers/target/target_core_xcopy.c
@@ -82,6 +82,9 @@ static int target_xcopy_locate_se_dev_e4(struct se_cmd *se_cmd, struct xcopy_op
 	mutex_lock(&g_device_mutex);
 	list_for_each_entry(se_dev, &g_device_list, g_dev_node) {
 
+		if (!se_dev->dev_attrib.emulate_3pc)
+			continue;
+
 		memset(&tmp_dev_wwn[0], 0, XCOPY_NAA_IEEE_REGEX_LEN);
 		target_xcopy_gen_naa_ieee(se_dev, &tmp_dev_wwn[0]);
 
@@ -884,12 +887,18 @@ out:
 
 sense_reason_t target_do_xcopy(struct se_cmd *se_cmd)
 {
+	struct se_device *dev = se_cmd->se_dev;
 	struct xcopy_op *xop = NULL;
 	unsigned char *p = NULL, *seg_desc;
 	unsigned int list_id, list_id_usage, sdll, inline_dl, sa;
 	int rc;
 	unsigned short tdll;
 
+	if (!dev->dev_attrib.emulate_3pc) {
+		pr_err("EXTENDED_COPY operation explicitly disabled\n");
+		return TCM_UNSUPPORTED_SCSI_OPCODE;
+	}
+
 	sa = se_cmd->t_task_cdb[1] & 0x1f;
 	if (sa != 0x00) {
 		pr_err("EXTENDED_COPY(LID4) not supported\n");
-- 
1.7.10.4

Re: [PATCH 0/3] target: EXTENDED_COPY bugfixes for v3.12-rc

[Thomas Glanzmann]

Hello Nab,

>   target: Make target_do_xcopy failures return INVALID_PARAMETER_LIST
>   target: Allow non zero ListID in EXTENDED_COPY parameter list
>   target: Reject EXTENDED_COPY when emulate_3pc is disabled

I pull them right now, rebuild and test them tomorrow in my currently
ongoing class. I'll report back by the end of the week if there have
been any problems, but I assume there will not be. I also wondered why
you allowed xcopy when the feature was not enabled, but never bothered
to ask. :-) However I'm still fighting with the demo mode discovery, I
should know the code by now.

Cheers,
        Thomas