[stgt] [PATCH] fix three bugs in startstopunit

[ronnie sahlberg]

Tomo,

Thanks. Good suggestions.

I have attached a new patch with your style improvements.


regards
ronnie sahlberg

On Tue, Jul 17, 2012 at 8:39 AM, FUJITA Tomonori
<fujita.tomonori at lab.ntt.co.jp> wrote:
> On Mon, 16 Jul 2012 21:52:44 +1000
> ronnie sahlberg <ronniesahlberg at gmail.com> wrote:
>
>> From d68d6a5f6451532f0257d2b25aaa233cbd200271 Mon Sep 17 00:00:00 2001
>> From: Ronnie Sahlberg <ronniesahlberg at gmail.com>
>> Date: Mon, 16 Jul 2012 21:45:09 +1000
>> Subject: [PATCH] STARTSTOPUNIT bugfixes
>>
>> If power condition != 0  then we should ignore LOEJ and START bits
>> and not do any online/offline operations.
>>
>> Fix the check for LOEJ and make sure this flag is set to 0 or 1, not 0 or 2.
>>
>> Check that the removable attribute is set. We should not call
>> online/offline for non-removable luns
>>
>> Signed-off-by: Ronnie Sahlberg <ronniesahlberg at gmail.com>
>> ---
>>  usr/spc.c |   14 +++++++++-----
>>  1 files changed, 9 insertions(+), 5 deletions(-)
>
> Thanks, some style comments.
>
>> diff --git a/usr/spc.c b/usr/spc.c
>> index 117c9f3..7dce5a6 100644
>> --- a/usr/spc.c
>> +++ b/usr/spc.c
>> @@ -345,17 +345,21 @@ sense:
>>  int spc_start_stop(int host_no, struct scsi_cmd *cmd)
>>  {
>>       uint8_t *scb = cmd->scb;
>> -     int start, loej;
>> +     int start, loej, pwrcnd;
>>
>>       scsi_set_in_resid_by_actual(cmd, 0);
>>
>>       if (device_reserved(cmd))
>>               return SAM_STAT_RESERVATION_CONFLICT;
>>
>> -     loej  = scb[4] & 0x02;
>> -     start = scb[4] & 0x01;
>> +     pwrcnd = scb[4] & 0xf0;
>> +     if (pwrcnd != 0)
>
> if (pwrcnd)
>
> ?
>
>> +             return SAM_STAT_GOOD;
>> +
>> +     loej   = (scb[4] & 0x02) ? 1 : 0;
>
> What's the point?
>
>> +     start  = scb[4] & 0x01;
>>
>> -     if (loej == 1 && start == 0) {
>> +     if (loej == 1 && start == 0 && cmd->dev->attrs.removable) {
>
> if (loej && !start && cmd->dev->attrs.removable)
>
> ?
>
>>               if (lu_prevent_removal(cmd->dev)) {
>>                       if (cmd->dev->attrs.online) {
>>                               /*  online == media is present */
>> @@ -370,7 +374,7 @@ int spc_start_stop(int host_no, struct scsi_cmd *cmd)
>>               }
>>               spc_lu_offline(cmd->dev);
>>       }
>> -     if (loej == 1 && start == 1)
>> +     if (loej == 1 && start == 1 && cmd->dev->attrs.removable)
>
> if (loej && start && cmd->dev->attrs.removable)
>
> ?
>
>>               spc_lu_online(cmd->dev);
>>
>>       return SAM_STAT_GOOD;
>> --
>> 1.7.3.1
>>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-STARTSTOPUNIT-bugfixes.patch.gz
Type: application/x-gzip
Size: 852 bytes
Desc: not available
URL: <http://lists.wpkg.org/pipermail/stgt/attachments/20120717/105a747d/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-STARTSTOPUNIT-bugfixes.patch
Type: application/octet-stream
Size: 1634 bytes
Desc: not available
URL: <http://lists.wpkg.org/pipermail/stgt/attachments/20120717/105a747d/attachment-0002.obj>