Re: xcopy testing with ddpt

Re: xcopy testing with ddpt

[Thomas Glanzmann]

Hello Doug,

* Douglas Gilbert <dgilbert@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: ca7e1e04bb286ee443fe05e985a11d240000000000000019

> 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

Re: xcopy testing with ddpt

[Nicholas A. Bellinger]

On Mon, 2013-10-07 at 06:03 +0200, Thomas Glanzmann wrote:
> Hello Doug,
> 
> * Douglas Gilbert <dgilbert@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

Re: xcopy testing with ddpt

[Nicholas A. Bellinger]

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 <dgilbert@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.

Doug, WDYT..?

--nab

Re: xcopy testing with ddpt

[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 <dgilbert@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
bootc@bootc.net

Re: xcopy testing with ddpt

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