lkml.org 
[lkml]   [2012]   [Mar]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH RESEND 1/2] Drivers: scsi: storvsc: Set the scsi result correctly when SRB status is INVALID
    From
    Date
    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@gmail.com> and
    > Douglas Gilbert <dgilbert@interlog.com> for suggesting the correct approach
    > here.
    >
    > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
    > Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
    > ---
    > 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




    \
     
     \ /
      Last update: 2012-03-19 17:15    [W:5.252 / U:0.004 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site