[PATCH] cciss: fix incorrect scsi status reporting

[PATCH] cciss: fix incorrect scsi status reporting

[Stephen M. Cameron]

From: Stephen M. Cameron <scameron@beardog.cce.hp.com>

Delete code which sets SCSI status incorrectly as it's already
been set correctly above this incorrect code.  Bug was introduced
by b0e15f6db1110319cb2e747e59e1200450a5ba3e
"cciss: fix typo that causes scsi status to be lost." in 2009.

Signed-off-by: Stephen M. Cameron <scameron@beardog.cce.hp.com>
Reported-and-tested-by: Roel van Meer <roel.vanmeer@bokxing.nl>
Cc: stable@vger.kernel.org
---
 drivers/block/cciss_scsi.c |   11 +----------
 1 files changed, 1 insertions(+), 10 deletions(-)

diff --git a/drivers/block/cciss_scsi.c b/drivers/block/cciss_scsi.c
index acda773..38aa6dd 100644
--- a/drivers/block/cciss_scsi.c
+++ b/drivers/block/cciss_scsi.c
@@ -763,16 +763,7 @@ static void complete_scsi_command(CommandList_struct *c, int timeout,
 		{
 			case CMD_TARGET_STATUS:
 				/* Pass it up to the upper layers... */
-				if( ei->ScsiStatus)
-                		{
-#if 0
-                    			printk(KERN_WARNING "cciss: cmd %p "
-						"has SCSI Status = %x\n",
-						c, ei->ScsiStatus);
-#endif
-					cmd->result |= (ei->ScsiStatus << 1);
-                		}
-				else {  /* scsi status is zero??? How??? */
+				if (!ei->ScsiStatus) {
 					
 	/* Ordinarily, this case should never happen, but there is a bug
 	   in some released firmware revisions that allows it to happen

Re: [PATCH] cciss: fix incorrect scsi status reporting

[Andrew Morton]

On Fri, 04 May 2012 13:21:14 -0500
"Stephen M. Cameron" <scameron@beardog.cce.hp.com> wrote:

> From: Stephen M. Cameron <scameron@beardog.cce.hp.com>
> 
> Delete code which sets SCSI status incorrectly as it's already
> been set correctly above this incorrect code.  Bug was introduced
> by b0e15f6db1110319cb2e747e59e1200450a5ba3e
> "cciss: fix typo that causes scsi status to be lost." in 2009.
> 
> Signed-off-by: Stephen M. Cameron <scameron@beardog.cce.hp.com>
> Reported-and-tested-by: Roel van Meer <roel.vanmeer@bokxing.nl>
> Cc: stable@vger.kernel.org

If the bug is three years old, it presumably isn't a terribly serious
one.  Why was the -stable backport recommended?

Re: [PATCH] cciss: fix incorrect scsi status reporting

[Stephen Cameron]

On Wed, May 9, 2012 at 4:59 PM, Andrew Morton <akpm@linux-foundation.org> wrote:
> On Fri, 04 May 2012 13:21:14 -0500
> "Stephen M. Cameron" <scameron@beardog.cce.hp.com> wrote:
>
>> From: Stephen M. Cameron <scameron@beardog.cce.hp.com>
>>
>> Delete code which sets SCSI status incorrectly as it's already
>> been set correctly above this incorrect code.  Bug was introduced
>> by b0e15f6db1110319cb2e747e59e1200450a5ba3e
>> "cciss: fix typo that causes scsi status to be lost." in 2009.
>>
>> Signed-off-by: Stephen M. Cameron <scameron@beardog.cce.hp.com>
>> Reported-and-tested-by: Roel van Meer <roel.vanmeer@bokxing.nl>
>> Cc: stable@vger.kernel.org
>
> If the bug is three years old, it presumably isn't a terribly serious
> one.  Why was the -stable backport recommended?

(added roel.vanmeer@bokxing.nl to cc list)

Due to my subjective interpretation of stable_kernel_rules.txt,
specifically this part:

 - It must fix a problem that causes a build error (but not for things
   marked CONFIG_BROKEN), an oops, a hang, data corruption, a real
   security issue, or some "oh, that's not good" issue.  In short, something
   critical.

I thought it fell under the "oh, that's not good", in that, according
to the tester, iirc, it made his tape drive not show up at all, which
seemed bad (though not a symptom I observed directly), and it seemed
like a trivial fix to something obviously incorrect, so thought stable
might like to know about it.  But, maybe not.

-- steve