Re: xcopy testing with ddpt

2013-10-07 Thread Nicholas A. Bellinger 
On Mon, 2013-10-07 at 06:03 +0200, Thomas Glanzmann wrote:
 Hello Doug,
 
 * Douglas Gilbert dgilb...@interlog.com [2013-10-07 00:58]:
  Great, another one working.

(CC'ing Hannes)

  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

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: xcopy testing with ddpt

2013-10-07 Thread Chris Boot 
On 07/10/2013 23:38, Nicholas A. Bellinger wrote:
 On Mon, 2013-10-07 at 15:18 -0700, Nicholas A. Bellinger wrote:
 On Mon, 2013-10-07 at 06:03 +0200, Thomas Glanzmann wrote:
 Hello Doug,

 * Douglas Gilbert dgilb...@interlog.com [2013-10-07 00:58]:
 Great, another one working.

 (CC'ing Hannes)

 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.

 
 How about the following patch to allow non zero ListIDs, but only when
 ListID Usage is set to 11b..?
 
 diff --git a/drivers/target/target_core_xcopy.c 
 b/drivers/target/target_core_xcopy.c
 index 6b9774c..3a3ea31 100644
 --- a/drivers/target/target_core_xcopy.c
 +++ b/drivers/target/target_core_xcopy.c
 @@ -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.

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

-- 
Chris Boot
bo...@bootc.net
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: xcopy testing with ddpt

2013-10-07 Thread Nicholas A. Bellinger 
On Tue, 2013-10-08 at 00:07 +0100, Chris Boot wrote:
 On 07/10/2013 23:38, Nicholas A. Bellinger wrote:
  On Mon, 2013-10-07 at 15:18 -0700, Nicholas A. Bellinger wrote:
  On Mon, 2013-10-07 at 06:03 +0200, Thomas Glanzmann wrote:
  Hello Doug,
 
  * Douglas Gilbert dgilb...@interlog.com [2013-10-07 00:58]:
  Great, another one working.
 
  (CC'ing Hannes)
 
  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.
 
  
  How about the following patch to allow non zero ListIDs, but only when
  ListID Usage is set to 11b..?
  
  diff --git a/drivers/target/target_core_xcopy.c 
  b/drivers/target/target_core_xcopy.c
  index 6b9774c..3a3ea31 100644
  --- a/drivers/target/target_core_xcopy.c
  +++ b/drivers/target/target_core_xcopy.c
  @@ -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.
 
 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
--- 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
 */


--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: xcopy testing with ddpt

2013-10-06 Thread Thomas Glanzmann 
Hello Doug,

* Douglas Gilbert dgilb...@interlog.com [2013-10-07 00:58]:
 Great, another one working.

yes. :-)

 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.

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

perfect.

   Unit serial number: ca7e1e04bb286ee443fe05e985a11d240019

 Interesting serial number :-)

no idea how they calculate it.

 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
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html