linux-kernel - RE: [PATCH RESEND 1/2] Drivers: scsi: storvsc: Set the scsi result correctly when SRB status is INVALID

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <426367E2313C2449837CD2DE46E7EAF902244A@BY2PRD0310MB378.namprd03.prod.outlook.com>
Date:	Thu, 29 Mar 2012 14:50:25 +0000
From:	KY Srinivasan <kys@...rosoft.com>
To:	James Bottomley <James.Bottomley@...senPartnership.com>
CC:	"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"devel@...uxdriverproject.org" <devel@...uxdriverproject.org>,
	"ohering@...e.com" <ohering@...e.com>,
	"hch@...radead.org" <hch@...radead.org>,
	"linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>
Subject: RE: [PATCH RESEND 1/2] Drivers: scsi: storvsc: Set the scsi result
 correctly when SRB status is INVALID



> -----Original Message-----
> From: James Bottomley [mailto:James.Bottomley@...senPartnership.com]
> Sent: Thursday, March 29, 2012 4:02 AM
> To: KY Srinivasan
> Cc: gregkh@...uxfoundation.org; linux-kernel@...r.kernel.org;
> devel@...uxdriverproject.org; ohering@...e.com; hch@...radead.org; linux-
> scsi@...r.kernel.org
> Subject: RE: [PATCH RESEND 1/2] Drivers: scsi: storvsc: Set the scsi result correctly
> when SRB status is INVALID
> 
> On Tue, 2012-03-27 at 15:32 +0000, KY Srinivasan wrote:
> > > > > James, unfortunately based on the current SRB codes I get back from the
> > > > > host, I don't know which commands that failed ought to be retried and
> which
> > > > > ones should not be; I simply get a single SRB error code for cases where
> the
> > > > > host filtered the unsupported commands as well as the case where the
> host
> > > > > supported the command and something failed in the command
> execution.
> > > > > If there is something I can try in this driver to fix this problem, I am more
> than
> > > > > happy to try it. If it involves getting changes into  the host (win8, win2k8
> etc.),
> > > > > I am willing to start a conversation with the relevant teams, but I cannot
> > > > > obviously determine when such changes will ship. However, I do need
> > > > > solution for the problem now.
> > > > >
> > > > > I appreciate your taking the time to help me gravitate towards the
> > > > > correct solution here. Given my constraints, let me know what is the
> > > > > best way forward here.
> > > >
> > > > Ping.
> > >
> > > On what? What don't you understand about the above?
> > >
> > > The failure path needs to look like the following metacode
> > >
> > > case SRB_whatever
> > >
> > > if (retryable command)
> > > 	return DID_TARGET_FAILURE
> > > else
> > > 	setup sense and error
> > > 	return DID_PASSTHROUGH
> > >
> >
> > Thanks James and I am sorry for taking so much of your time. I suspect the
> check for
> > retryable commands is to be based on the sense data that might be available.
> Assuming
> > this is what you had in mind, as I look the  scsi_error.c, scsi_check_sense()
> appears to
> > do what I would need here. Would you be willing to take patch that exports this
> function.
> > Or, is there a better way to identify commands that would be re-tried.
> 
> Erm, no, I was thinking of something much more simple.  Right at the
> moment if (retryable_command) is if (scmnd->cmnd[0] != ATA_16)
> 
> Although I'm sure there are other commands that are not retryable ... I
> don't really know since I'm not sure what else your hypervisor will
> choke on.  It might be simpler to have a list of known good retryable
> commands (READ_ WRITE_ GET_CAPACITY_ INQUIRY etc. i.e. the commands sd
> normally uses) and assume anything outside that range isn't retryable
> but someone who knows what's going on in the hypervisor needs to decide
> this.
> 
> The point is that you don't pre-filter.  You send the command and then
> try and work out from the generic error and the type of command you sent
> what happened.  That way, if the hypervisor ever works properly (or
> simply passes commands through to real devices) it will "just work"
> rather than failing because of the pre-filter.
> 
> We should only use the pre-filter if the effects of the command are
> fatal (like we have to a lot in USB because the firmware crashes and
> hangs).

Thanks for the clarification. I will have the patch out shortly.

K. Y