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@xxxxxxxxxxxxx> wrote:
> On Mon, 16 Jul 2012 21:52:44 +1000
> ronnie sahlberg <ronniesahlberg@xxxxxxxxx> wrote:
>
>> From d68d6a5f6451532f0257d2b25aaa233cbd200271 Mon Sep 17 00:00:00 2001
>> From: Ronnie Sahlberg <ronniesahlberg@xxxxxxxxx>
>> 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@xxxxxxxxx>
>> ---
>> 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
>>
Attachment:
0001-STARTSTOPUNIT-bugfixes.patch.gz
Description: GNU Zip compressed data
Attachment:
0001-STARTSTOPUNIT-bugfixes.patch
Description: Binary data
