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.bin> -------------- next part -------------- A non-text attachment was scrubbed... Name: 0001-STARTSTOPUNIT-bugfixes.patch Type: application/octet-stream Size: 1633 bytes Desc: not available URL: <http://lists.wpkg.org/pipermail/stgt/attachments/20120717/105a747d/attachment.obj> |