Re: [PATCH] sbp-target: Delete an error message for a failed memory allocation in three functions

2017-12-10 Thread Chris Boot
On 10/12/2017 19:10, SF Markus Elfring wrote:
> From: Markus Elfring 
> Date: Sun, 10 Dec 2017 19:54:11 +0100
> 
> Omit an extra message for a memory allocation failure in these functions.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring 
[snip]

Looks good to me.

Acked-by: Chris Boot 

Thanks,
Chris

-- 
Chris Boot
bo...@boo.tc


qla2xxx firmware crashes in target mode

2015-10-19 Thread Chris Boot
detected (4 Gbps).
[484976.448002] qla2xxx [:05:00.0]-0121:9: Failed to enable
receiving of RSCN requests: 0x2.

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: [PATCH 7/8] target_core_alua: disallow READ_CAPACITY when in standby

2015-06-18 Thread Chris Boot
On 18/06/15 10:43, Hannes Reinecke wrote:
> Strictly speaking SPC doesn't require READ CAPACITY and friends
> to be supported while in the port is in standby.

Hi Hannes,

I'd really rather this didn't go away. Yes, strictly speaking SPC
doesn't require these commands but Linux in practice does, and ISTR this
was added at my request too.

We need it on our storage setups to prevent the Linux SCSI stack from
exploding. If this is removed here, they'll start exploding again until
the fix goes in and the initiators are updated.

Could this please be kept as an option or something?

Cheers,
Chris

> Signed-off-by: Hannes Reinecke 
> ---
>  drivers/target/target_core_alua.c | 9 -
>  1 file changed, 9 deletions(-)
> 
> diff --git a/drivers/target/target_core_alua.c 
> b/drivers/target/target_core_alua.c
> index edaf1b9..a62e58b 100644
> --- a/drivers/target/target_core_alua.c
> +++ b/drivers/target/target_core_alua.c
> @@ -561,16 +561,7 @@ static inline int core_alua_state_standby(
>   case REPORT_LUNS:
>   case RECEIVE_DIAGNOSTIC:
>   case SEND_DIAGNOSTIC:
> - case READ_CAPACITY:
>   return 0;
> - case SERVICE_ACTION_IN_16:
> - switch (cdb[1] & 0x1f) {
> - case SAI_READ_CAPACITY_16:
> - return 0;
> - default:
> - set_ascq(cmd, ASCQ_04H_ALUA_TG_PT_STANDBY);
> - return 1;
> - }
>   case MAINTENANCE_IN:
>   switch (cdb[1] & 0x1f) {
>   case MI_REPORT_TARGET_PGS:
> 


-- 
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 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  [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: qla2xxx: issues when creating a new target port

2013-03-30 Thread Chris Boot

On 08/03/2013 21:25, Chris Boot wrote:

On 05/03/13 22:15, Chris Boot wrote:

I'll see if I can get enough hardware and time together to test this
over the weekend to confirm everything I said above and gather the logs
with the error logging enabled. I'll keep you posted.


I have run some tests against 3.8.2, and attached the logs.

I can only replicate the problems I ran into with the following 
sequence of events:


1. Link on the qla246x target must be down to start with (tested with 
QLA2460 and QLA2464).

2. /qla2xxx create 21:00:00:1b:32:11:a8:24
3. disable
4. luns/ create /backstores/iblock/test
5. acls/ create 10:00:00:06:2b:11:3f:a1
6. enable
7. Plug in the link to an initiator.

Even with 'set global auto_enable_tpgt=false', at step 3 the port 
comes up enabled. It seems I remembered wrongly that disabling the 
target port does not successfully disable it:


jones-hdd bootc # cat 
/sys/kernel/config/target/qla2xxx/21\:00\:00\:1b\:32\:11\:a8\:24/tpgt_1/enable 


1
jones-hdd bootc # echo 0 > 
/sys/kernel/config/target/qla2xxx/21\:00\:00\:1b\:32\:11\:a8\:24/tpgt_1/enable
jones-hdd bootc # cat 
/sys/kernel/config/target/qla2xxx/21\:00\:00\:1b\:32\:11\:a8\:24/tpgt_1/enable 


0

Once the target is configured with a LUN and ACL, enabled, and the 
link brought up by an initiator, the initiator fails to see any LUNs 
and eventually times out. The debug logging shows interesting lines 
such as:


Mar  8 21:06:59 jones-hdd kernel: [  183.441093] qla2xxx 
[:01:00.0]-f821:4: New command while device 8801a1b51400 is 
shutting down
Mar  8 21:06:59 jones-hdd kernel: [  183.441096] qla2xxx 
[:01:00.0]-e859:4: qla_target: Unable to send command to target 
for req, ignoring.


I definitely think there is a bug in there, and it feels like it's 
triggered by disabling the target port with the link down - but I'm 
only guessing at this point.


Hi folks,

Has anyone had a chance to look at the above?

Cheers,
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: qla2xxx: issues when creating a new target port

2013-03-05 Thread Chris Boot
On 05/03/2013 21:52, Nicholas A. Bellinger wrote:
> Hi Chris,
>
> On Sun, 2013-03-03 at 13:49 +0000, Chris Boot wrote:
>> Hi all,
>>
>> When creating a new target port in targetcli (/qla2xxx create
>> 21:0x:...), and I have nothing plugged-in to the port, the creation
>> process takes quite some time. After a few seconds, the kernel prints
>> a message saying "Cable is unplugged...". So far so good.
>>
> AFAIK the delay is expected here when no physical link is detected.

Yes, I expected the delay at this point.

>> It seems that when the target is created, it comes up in the 'enabled'
>> state, regardless of me having set 'auto_enable_tpgt=false'. Running
>> 'disable' from targetcli seems to try to bring the link up again
>> ("Performing ISP error recovery" then "Cable is unplugged...") and the
>> target remains in the enabled state.
>>
> Running 'disable' from targetcli will perform:
>
>   echo 0 > /sys/kernel/config/target/qla2xxx/$FC_WWPN/$TPGT/enable
>
> that will effectively disable target mode on the $FC_WWPN port in
> question, regardless of what else (LUNs, ACLs) are configured.
>
> Target mode is only re-enabled at the HW level for the port with:
>
>   echo 1 > /sys/kernel/config/target/qla2xxx/$FC_WWPN/$TPGT/enable

I can't easily perform this test again now, maybe I can do it later this
weekend. From memory, though, running 'disable' in targetcli _or_
running 'echo 0 >
/sys/kernel/config/target/qla2xxx/$FC_WWPN/$TPGT/enable' both indeed
have the same effect.

>> If I then add LUNs and ACLs to the target, everything appears OK, but
>> plugging the target port into an initiator gives odd behaviour. The
>> initiator spends a very long time scanning the target and eventually
>> times out and finds no LUNs. The only way to get the port working
>> again is to save the configuration and reboot the target server, which
>> is very frustrating.
>>
> Can you double check the value of ../qla2xxx/$FC_WWPN/$TPGT/enable when
> this happens..?  From the description above it sounds like your
> explicitly disabling target mode on the port.

Again, from memory, after doing a 'disable' or 'echo 0 > .../enable'
results in the 'enable' file still returning value 1. I also seem to
remember, when trying to unload the qla2xxx module, getting odd
behaviour and OOPSes - though I would want to double check this. I had
the same behaviour with two qla2xxx HBAs (one 2460 and one 2464).

>> I'm using targetcli as found in Debian wheezy with kernel 3.8.0,
>> though I have had this problem for a little while now...
>>
> For reference, can you provide the HBAs that your reproducing with..?
>
> Also, a target side log with
>
>modprobe qla2xxx ql2xextended_error_logging=0x7fff
>
> would be helpful as well.

I'll see if I can get enough hardware and time together to test this
over the weekend to confirm everything I said above and gather the logs
with the error logging enabled. I'll keep you posted.

Cheers,
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


qla2xxx: issues when creating a new target port

2013-03-03 Thread Chris Boot
Hi all,

When creating a new target port in targetcli (/qla2xxx create 21:0x:...), and I 
have nothing plugged-in to the port, the creation process takes quite some 
time. After a few seconds, the kernel prints a message saying "Cable is 
unplugged...". So far so good.

It seems that when the target is created, it comes up in the 'enabled' state, 
regardless of me having set 'auto_enable_tpgt=false'. Running 'disable' from 
targetcli seems to try to bring the link up again ("Performing ISP error 
recovery" then "Cable is unplugged...") and the target remains in the enabled 
state.

If I then add LUNs and ACLs to the target, everything appears OK, but plugging 
the target port into an initiator gives odd behaviour. The initiator spends a 
very long time scanning the target and eventually times out and finds no LUNs. 
The only way to get the port working again is to save the configuration and 
reboot the target server, which is very frustrating.

I'm using targetcli as found in Debian wheezy with kernel 3.8.0, though I have 
had this problem for a little while now...

Cheers,
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: [PATCH 2/3] target: Fix error checking for UNMAP commands

2013-02-09 Thread Chris Boot

On 08/02/2013 23:18, Roland Dreier wrote:

diff --git a/drivers/target/target_core_iblock.c 
b/drivers/target/target_core_iblock.c
index b526d23..b72ca5b 100644
--- a/drivers/target/target_core_iblock.c
+++ b/drivers/target/target_core_iblock.c
@@ -2674,6 +2675,15 @@ transport_send_check_condition_and_sense(struct se_cmd 
*cmd,
/* INVALID FIELD IN PARAMETER LIST */
buffer[SPC_ASC_KEY_OFFSET] = 0x26;
break;
+   case TCM_PARAMETER_LIST_LENGTH_ERROR:
+   /* CURRENT ERROR */
+   buffer[0] = 0x70;
+   buffer[SPC_ADD_SENSE_LEN_OFFSET] = 10;
+   /* ILLEGAL REQUEST */
+   buffer[SPC_SENSE_KEY_OFFSET] = ILLEGAL_REQUEST;
+   /* INVALID FIELD IN PARAMETER LIST */
+   buffer[SPC_ASC_KEY_OFFSET] = 0x1a;
+   break;
case TCM_UNEXPECTED_UNSOLICITED_DATA:
/* CURRENT ERROR */
buffer[0] = 0x70;


Nitpick: I suspect a simple copy & paste error; "INVALID FIELD IN 
PARAMETER LIST" in your comment should probably read "PARAMETER LIST 
LENGTH ERROR" instead.


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: [Suggestion] drivers/target/sbp/ : set tport->tpg to NULL when clean up for failure.

2012-12-10 Thread Chris Boot
On 10/12/12 02:39, Chen Gang wrote:
> Hello Chris Boot:
>
>   need I send the relative patch ?

Hi Chen,

Sorry, life got in the way this weekend. I'll try to get the patch sent
out today.

Cheers,
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: [Suggestion] drivers/target/sbp/ : set tport->tpg to NULL when clean up for failure.

2012-12-06 Thread Chris Boot
On 06/12/12 04:24, Chen Gang wrote:
> Hello Maintainers: 
>
> in drivers/target/sbp/sbp_target.c:
>
>tport->tpg must be NULL before process it in function sbp_make_tpg. (line 
> 2185..2188)
>tport->tpg assigned a ptr (line 2198)
>if processing failed, not set tport->tpg = NULL (line 2208..2212, 
> 2217..2221)
>
>we have done: when free tport->tpg, set it to NULL (line 2233..2234)
>
>is it valuable to let tport->tpg = NULL, when clean up for failure ?
>


Yes, that's a good catch. The way it is now, if we fail to set up the
TPG, the port will be left in an inconsistent state. I'll prepare a
patch ASAP unless you'd rather do it yourself.

Thanks,
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