> -----Original Message----- > From: James Bottomley [mailto:James.Bottomley@xxxxxxxxxxxxxxxxxxxxx] > Sent: Monday, March 19, 2012 12:13 PM > To: KY Srinivasan > Cc: gregkh@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; > devel@xxxxxxxxxxxxxxxxxxxxxx; ohering@xxxxxxxx; hch@xxxxxxxxxxxxx; linux- > scsi@xxxxxxxxxxxxxxx > Subject: Re: [PATCH RESEND 1/2] Drivers: scsi: storvsc: Set the scsi result correctly > when SRB status is INVALID > > On Sun, 2012-03-18 at 17:12 -0700, K. Y. Srinivasan wrote: > > Currently Windows hosts only support a subset of scsi commands and for > commands > > that are not supported, the host returns a generic SRB failure status. > > However, they have agreed to change the return value to indicate that > > the command is not supported. In preparation for that, handle the > > SRB_STATUS_INVALID_REQUEST return value correctly. > > > > I would like to thank Jeff Garzik <jgpobox@xxxxxxxxx> and > > Douglas Gilbert <dgilbert@xxxxxxxxxxxx> for suggesting the correct approach > > here. > > > > Signed-off-by: K. Y. Srinivasan <kys@xxxxxxxxxxxxx> > > Reviewed-by: Haiyang Zhang <haiyangz@xxxxxxxxxxxxx> > > --- > > drivers/scsi/storvsc_drv.c | 12 ++++++++++++ > > 1 files changed, 12 insertions(+), 0 deletions(-) > > > > diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c > > index 44c7a48..018c363 100644 > > --- a/drivers/scsi/storvsc_drv.c > > +++ b/drivers/scsi/storvsc_drv.c > > @@ -202,6 +202,7 @@ enum storvsc_request_type { > > #define SRB_STATUS_INVALID_LUN 0x20 > > #define SRB_STATUS_SUCCESS 0x01 > > #define SRB_STATUS_ERROR 0x04 > > +#define SRB_STATUS_INVALID_REQUEST 0x06 > > I don't really think this is the correct approach. We already have a > SCSI error return for this, which you're now translating in the driver > and hypervisor. Rather than have a special byte return of > SRB_STATUS_INVALID_REQUEST, why not have the hypervisor do the right > thing and fill in the ILLEGAL_REQUEST sense return. That way you don't > need a special error code and you don't need to construct the sense > buffer in the driver. Now HyperV will be correctly set up for both pass > through and emulated responses. It's surely not much work and you > already process sense data correctly in storvsc_command_completion(), so > you wouldn't need any patches to the driver for this approach. James, the issue here is that currently shipping Windows hosts don't even do what I am handling here. Based on the input I got from you and Christoph, I convinced the windows team to at least return the SRB status that indicates an illegal request. I will suggest to them that perhaps they should also set the correct sense code and so I would not need this patch. However, keep in mind that there is no current ETA on when Windows will ship with these changes - Windows 8 may ship with code where they would return an invalid SRB status, but they are not setting the sense code, hence this patch. When the Window host does the "right thing" I will clean this up, but I don't know when that will be. More importantly, the second patch in this series where I filter out the ATA_16 command on the guest is really important for us. Without that patch on a range on windows hosts including the current beta version of windows8 where the host is returning a generic error in response to ATA_16 command, we cannot boot many Linux distros. If you prefer, I can drop the first patch and re-submit the second patch for consideration now. Regards, K. Y _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
