xcopy testing with ddpt

DISCUSSION Thomas Glanzmann 3 participants 5 messages Oct 7, 2013 – Oct 8, 2013 10 messages not in the archive

Subsystems (2)

SCSI SCSI target

lore

Share & tools

lore

Thomas Glanzmann

author

2013-10-07 04:03 UTC #10 lore raw

In reply to a message not in the archive

Hello Doug,

Quoted 1 line · Douglas Gilbert

Great, another one working.

yes. :-)

Quoted 8 lines

So this saniq/HP/lefthand system does not support fetching the xcopy operating parameters, which will cause sg_xcopy and ddpt to give up. These could be defaulted to something sane and then use those default values to attempt the command that actually does the work: EXTENDED_COPY(LID1). Googled around and couldn't find any workflow for this (for the saniq product). Do you have any technical documentation for this product that might throw some light on this?

I don't have any technical documentation describing EXTENDED_COPY. However I know that it works with ESX server. So what I did is sniffing the SCSI commands. Find the pcap here:

https://thomas.glanzmann.de/tmp/xcopy.pcap.bz2 (920K)
https://thomas.glanzmann.de/tmp/onexcopy.pcap (4K)

Hopefully that helps you figure out what is going on. My first though was that we were doing the 100 MB in 4 chunks. That means approx 25 MB per chunk (not precisely). However maybe that is to much for the SAN/IQ. Maybe we should go easy on it and try 4 MB or 16 MB chunks. I have configured the ESX to 16 MB chunks (the maximum ESX supports) using the following command:

esxcfg-advcfg -s 16384 /DataMover/MaxHWTransferSize

If you want access to the system using ssh, let me know.

Quoted 3 lines

Good. Now sg_xcopy and ddpt (my versions) output debug lines like this:
/dev/sdh: LEFTHAND iSCSIDisk a500 [pdt=0, 3pc=1]

perfect.

Quoted 3 lines

Quoted 1 line
Unit serial number: ca7e1e04bb286ee443fe05e985a11d240000000000000019
Interesting serial number :-)

no idea how they calculate it.

Quoted 6 lines

BTW list_id=0 has a special meaning in some context (buried deep in T10 documents: spc4r36j.pdf). That is probably why Hannes Reinecke defaulted that list_id to
1. I could understand the target XCOPY implementation only accepting one xcopy sequence at a time, but why restrict it to list_id=0 ? A question for NaB ...

Nab, do you have any input for us?

Quick wrap up what we did so far: Doug asked me to test ddpt and sg_xcopy of sg3-utils beta on your target. After setting the list_id=0 both tools work out of the box. The test setup is:

- 2 100 MB LUNs
- Createing a filesystem on the first and copy some date on it
- Use

ddpt if=/dev/sg3 iflag=xcopy list_id=0 of=/dev/sg4 bs=512 sg_xcopy if=/dev/sdc of=/dev/sdd list_id=0

to copy the data from LUN 1 to LUN 2. And do a md5sum to verify that the user data are exactly the same.

Cheers,
Thomas

Nicholas A. Bellinger

2013-10-07 22:10 UTC #11 lore raw

Quoted 4 lines · Thomas Glanzmann

Hello Doug,
Quoted 1 line · Douglas Gilbert
Great, another one working.

(CC'ing Hannes)

Quoted 9 lines

Quoted 6 lines
BTW list_id=0 has a special meaning in some context (buried deep in T10 documents: spc4r36j.pdf). That is probably why Hannes Reinecke defaulted that list_id to
1. I could understand the target XCOPY implementation only accepting one xcopy sequence at a time, but why restrict it to list_id=0 ? A question for NaB ...
Nab, do you have any input for us?

It was my original understanding that when OPERATING_PARAMETERS is reporting SNLID=1 (Supports No ListID), the initiator is expected to send EXTENDED_COPY parameter lists with ListID Usage 11b + ListID=0. Since we're ignoring the value of ListID for now anyways, I agree that it doesn't make much sense to fail for a non zero value here..

However, the main concern that made me add this check to begin with was the case with ListID Usage 00b + 10b, where the copy server is expected to keep a per I_T list of in-use ListIDs, and return CHECK_CONDITION + ILLEGAL REQUEST/OPERATION IN PROGRESS for a ListID for a copy sequence already in progress.

Given that don't have this per I_T list that tracks ListIDs yet, it seemed wrong at the time to allow non zero ListIDs to be processed.. ;)

Also, it's worth mentioning that the target XCOPY implementation does in fact support multiple copy sequences per device at a time, and there is currently no hard limit enforced for the number of copies, aside from the normal fabric dependent NodeACL queue_depth, et al. OPERATING PARAMETERS is currently reporting 1 for TOTAL CONCURRENT COPIES and MAXIMUM CONCURRENT COPIES, and I'll likely be adding a device attribute to control this depth, and enforce it's usage for v3.13 code.

--nab

Nicholas A. Bellinger

2013-10-07 22:29 UTC #12 lore raw

Quoted 24 lines · Nicholas A. Bellinger

Quoted 4 lines · Thomas Glanzmann
Hello Doug,
Quoted 1 line · Douglas Gilbert
Great, another one working.
(CC'ing Hannes)
Quoted 9 lines
Quoted 6 lines
BTW list_id=0 has a special meaning in some context (buried deep in T10 documents: spc4r36j.pdf). That is probably why Hannes Reinecke defaulted that list_id to
1. I could understand the target XCOPY implementation only accepting one xcopy sequence at a time, but why restrict it to list_id=0 ? A question for NaB ...
Nab, do you have any input for us?
It was my original understanding that when OPERATING_PARAMETERS is reporting SNLID=1 (Supports No ListID), the initiator is expected to send EXTENDED_COPY parameter lists with ListID Usage 11b + ListID=0. Since we're ignoring the value of ListID for now anyways, I agree that it doesn't make much sense to fail for a non zero value here..

Quoted 6 lines

However, the main concern that made me add this check to begin with was the case with ListID Usage 00b + 10b, where the copy server is expected to keep a per I_T list of in-use ListIDs, and return CHECK_CONDITION + ILLEGAL REQUEST/OPERATION IN PROGRESS for a ListID for a copy sequence already in progress.

How about the following patch to allow non zero ListIDs, but only when ListID Usage is set to 11b..?

drivers/target/target_core_xcopy.c +4 -3

index 6b9774c..3a3ea31 100644
@@ -911,11 +911,12 @@ 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);
+       list_id_usage = (p[1] & 0x18);
+       if (list_id != 0x00 && list_id_usage != 0x11) {
+               pr_err("XCOPY with non zero list_id: 0x%02x, and list_id_usage:"
+                      " 0x%02x\n", list_id, list_id_usage);
                goto out;
        }
-       list_id_usage = (p[1] & 0x18);
        /*
         * Determine TARGET DESCRIPTOR LIST LENGTH + SEGMENT DESCRIPTOR LIST LENGTH
         */

AFAICT this should make ddpt happy, as it's already be setting ListID Usage = 11b when it gets OPERATING PARAMETERS -> HELD_DATA = 0.

Doug, WDYT..?

--nab

Chris Boot

2013-10-07 23:07 UTC #13 lore raw

Quoted 56 lines · Nicholas A. Bellinger

Quoted 24 lines · Nicholas A. Bellinger
Quoted 4 lines · Thomas Glanzmann
Hello Doug,
Quoted 1 line · Douglas Gilbert
Great, another one working.
(CC'ing Hannes)
Quoted 9 lines
Quoted 6 lines
BTW list_id=0 has a special meaning in some context (buried deep in T10 documents: spc4r36j.pdf). That is probably why Hannes Reinecke defaulted that list_id to
1. I could understand the target XCOPY implementation only accepting one xcopy sequence at a time, but why restrict it to list_id=0 ? A question for NaB ...
Nab, do you have any input for us?
It was my original understanding that when OPERATING_PARAMETERS is reporting SNLID=1 (Supports No ListID), the initiator is expected to send EXTENDED_COPY parameter lists with ListID Usage 11b + ListID=0. Since we're ignoring the value of ListID for now anyways, I agree that it doesn't make much sense to fail for a non zero value here..
Quoted 6 lines
However, the main concern that made me add this check to begin with was the case with ListID Usage 00b + 10b, where the copy server is expected to keep a per I_T list of in-use ListIDs, and return CHECK_CONDITION + ILLEGAL REQUEST/OPERATION IN PROGRESS for a ListID for a copy sequence already in progress.
How about the following patch to allow non zero ListIDs, but only when ListID Usage is set to 11b..?
drivers/target/target_core_xcopy.c +4 -3
index 6b9774c..3a3ea31 100644
@@ -911,11 +911,12 @@ 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);
+       list_id_usage = (p[1] & 0x18);
+       if (list_id != 0x00 && list_id_usage != 0x11) {
+               pr_err("XCOPY with non zero list_id: 0x%02x, and list_id_usage:"
+                      " 0x%02x\n", list_id, list_id_usage);
                goto out;
        }
-       list_id_usage = (p[1] & 0x18);
        /*
         * Determine TARGET DESCRIPTOR LIST LENGTH + SEGMENT DESCRIPTOR LIST LENGTH
         */

Quoted 2 lines

AFAICT this should make ddpt happy, as it's already be setting ListID Usage = 11b when it gets OPERATING PARAMETERS -> HELD_DATA = 0.

0x11 != 11b (but == 11h)

If 0x18 is the correct mask I think you want to compare against 0x18, otherwise you probably want to shift down by 3 bits and compare against 0x03 or 0b11...

HTH,
Chris

Signature · 2 lines

Chris Boot
[email protected]

Nicholas A. Bellinger

2013-10-08 00:24 UTC #14 lore raw

Quoted 57 lines · Chris Boot

Quoted 56 lines · Nicholas A. Bellinger
Quoted 24 lines · Nicholas A. Bellinger
Quoted 4 lines · Thomas Glanzmann
Hello Doug,
Quoted 1 line · Douglas Gilbert
Great, another one working.
(CC'ing Hannes)
Quoted 9 lines
Quoted 6 lines
BTW list_id=0 has a special meaning in some context (buried deep in T10 documents: spc4r36j.pdf). That is probably why Hannes Reinecke defaulted that list_id to
1. I could understand the target XCOPY implementation only accepting one xcopy sequence at a time, but why restrict it to list_id=0 ? A question for NaB ...
Nab, do you have any input for us?
It was my original understanding that when OPERATING_PARAMETERS is reporting SNLID=1 (Supports No ListID), the initiator is expected to send EXTENDED_COPY parameter lists with ListID Usage 11b + ListID=0. Since we're ignoring the value of ListID for now anyways, I agree that it doesn't make much sense to fail for a non zero value here..
Quoted 6 lines
However, the main concern that made me add this check to begin with was the case with ListID Usage 00b + 10b, where the copy server is expected to keep a per I_T list of in-use ListIDs, and return CHECK_CONDITION + ILLEGAL REQUEST/OPERATION IN PROGRESS for a ListID for a copy sequence already in progress.
How about the following patch to allow non zero ListIDs, but only when ListID Usage is set to 11b..?
drivers/target/target_core_xcopy.c +4 -3
index 6b9774c..3a3ea31 100644
@@ -911,11 +911,12 @@ 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);
+       list_id_usage = (p[1] & 0x18);
+       if (list_id != 0x00 && list_id_usage != 0x11) {
+               pr_err("XCOPY with non zero list_id: 0x%02x, and list_id_usage:"
+                      " 0x%02x\n", list_id, list_id_usage);
                goto out;
        }
-       list_id_usage = (p[1] & 0x18);
        /*
         * Determine TARGET DESCRIPTOR LIST LENGTH + SEGMENT DESCRIPTOR LIST LENGTH
         */

Quoted 9 lines

Quoted 2 lines
AFAICT this should make ddpt happy, as it's already be setting ListID Usage = 11b when it gets OPERATING PARAMETERS -> HELD_DATA = 0.
0x11 != 11b (but == 11h)
If 0x18 is the correct mask I think you want to compare against 0x18, otherwise you probably want to shift down by 3 bits and compare against 0x03 or 0b11...

Er, duh, yes.. Looking at what sg_xcopy and ddpt are doing here again,
they are in fact using list_id_usage=10b (0x02) by default, so enforcing a check for 11b (0x03) is not going to work as originally expected..

How about the following to simply ignore the list_id..?

--nab

diff --git a/drivers/target/target_core_xcopy.c b/drivers/target/target_core_xcopy.c
index 6b9774c..fe98555 100644

drivers/target/target_core_xcopy.c +2 -5

@@ -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
         */