Date:	Thu, 29 Mar 2012 09:02:14 +0100
From:	James Bottomley <James.Bottomley@...senPartnership.com>
To:	KY Srinivasan <kys@...rosoft.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

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).

James


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists