[PATCH V3 0/7] Drivers: scsi: storvsc: Bug fixes and improvements

[PATCH V3 0/7] Drivers: scsi: storvsc: Bug fixes and improvements

[K. Y. Srinivasan]

In this patch set I have fixed a few bugs and implemented some enhancements.

In this version of the patch I have addressed comments from
Christoph Hellwig <hch@infradead.org>

K. Y. Srinivasan (7):
  Drivers: scsi: storvsc: Change the limits to reflect the values on
    the host
  Drivers: scsi: storvsc: Set cmd_per_lun to reflect value supported by
    the Host
  Drivers: scsi: storvsc: Filter commands based on the storage protocol
    version
  Drivers: scsi: storvsc: Fix a bug in handling VMBUS protocol version
  Drivers: scsi: storvsc: Implement a timedout handler
  drivers: scsi: storvsc: Set srb_flags in all cases
  drivers: scsi: storvsc: Correctly handle TEST_UNIT_READY failure

 drivers/scsi/storvsc_drv.c |  111 ++++++++++++++++++++++++++++++-------------
 1 files changed, 77 insertions(+), 34 deletions(-)

-- 
1.7.4.1

[PATCH V3 1/7] Drivers: scsi: storvsc: Change the limits to reflect the values on the host

[K. Y. Srinivasan]

Hyper-V hosts can support multiple targets and multiple channels and larger number of
LUNs per target. Update the code to reflect this. With this patch we can correctly
enumerate all the paths in a multi-path storage environment.

In this version of the patch I have addressed comments from
Christoph Hellwig <hch@infradead.org>

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Cc: <stable@vger.kernel.org>
---
 drivers/scsi/storvsc_drv.c |   47 +++++++++++++++++++++++++++++---------------
 1 files changed, 31 insertions(+), 16 deletions(-)

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 9969fa1..8938b13 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -330,17 +330,17 @@ static int storvsc_timeout = 180;
 
 static void storvsc_on_channel_callback(void *context);
 
-/*
- * In Hyper-V, each port/path/target maps to 1 scsi host adapter.  In
- * reality, the path/target is not used (ie always set to 0) so our
- * scsi host adapter essentially has 1 bus with 1 target that contains
- * up to 256 luns.
- */
-#define STORVSC_MAX_LUNS_PER_TARGET			64
-#define STORVSC_MAX_TARGETS				1
-#define STORVSC_MAX_CHANNELS				1
+#define STORVSC_MAX_LUNS_PER_TARGET			255
+#define STORVSC_MAX_TARGETS				2
+#define STORVSC_MAX_CHANNELS				8
 
+#define STORVSC_FC_MAX_LUNS_PER_TARGET			255
+#define STORVSC_FC_MAX_TARGETS				128
+#define STORVSC_FC_MAX_CHANNELS				8
 
+#define STORVSC_IDE_MAX_LUNS_PER_TARGET			64
+#define STORVSC_IDE_MAX_TARGETS				1
+#define STORVSC_IDE_MAX_CHANNELS			1
 
 struct storvsc_cmd_request {
 	struct list_head entry;
@@ -1691,7 +1691,6 @@ static struct scsi_host_template scsi_driver = {
 	.slave_destroy =	storvsc_device_destroy,
 	.slave_configure =	storvsc_device_configure,
 	.cmd_per_lun =		1,
-	/* 64 max_queue * 1 target */
 	.can_queue =		STORVSC_MAX_IO_REQUESTS*STORVSC_MAX_TARGETS,
 	.this_id =		-1,
 	/* no use setting to 0 since ll_blk_rw reset it to 1 */
@@ -1756,6 +1755,9 @@ static int storvsc_probe(struct hv_device *device,
 	}
 
 
+	if (dev_id->driver_data == SFC_GUID)
+		scsi_driver.can_queue = (STORVSC_MAX_IO_REQUESTS *
+					 STORVSC_FC_MAX_TARGETS);
 	host = scsi_host_alloc(&scsi_driver,
 			       sizeof(struct hv_host_device));
 	if (!host)
@@ -1789,12 +1791,25 @@ static int storvsc_probe(struct hv_device *device,
 	host_dev->path = stor_device->path_id;
 	host_dev->target = stor_device->target_id;
 
-	/* max # of devices per target */
-	host->max_lun = STORVSC_MAX_LUNS_PER_TARGET;
-	/* max # of targets per channel */
-	host->max_id = STORVSC_MAX_TARGETS;
-	/* max # of channels */
-	host->max_channel = STORVSC_MAX_CHANNELS - 1;
+	switch (dev_id->driver_data) {
+	case SFC_GUID:
+		host->max_lun = STORVSC_FC_MAX_LUNS_PER_TARGET;
+		host->max_id = STORVSC_FC_MAX_TARGETS;
+		host->max_channel = STORVSC_FC_MAX_CHANNELS - 1;
+		break;
+
+	case SCSI_GUID:
+		host->max_lun = STORVSC_MAX_LUNS_PER_TARGET;
+		host->max_id = STORVSC_MAX_TARGETS;
+		host->max_channel = STORVSC_MAX_CHANNELS - 1;
+		break;
+
+	default:
+		host->max_lun = STORVSC_IDE_MAX_LUNS_PER_TARGET;
+		host->max_id = STORVSC_IDE_MAX_TARGETS;
+		host->max_channel = STORVSC_IDE_MAX_CHANNELS - 1;
+		break;
+	}
 	/* max cmd length */
 	host->max_cmd_len = STORVSC_MAX_CMD_LEN;
 
-- 
1.7.4.1

[PATCH V3 2/7] Drivers: scsi: storvsc: Set cmd_per_lun to reflect value supported by the Host

[K. Y. Srinivasan]

Set cmd_per_lun to reflect value supported by the Host.
In this version of the patch I have addressed comments from
Christoph Hellwig <hch@infradead.org>

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Cc: <stable@vger.kernel.org>
---
 drivers/scsi/storvsc_drv.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 8938b13..cebcef7 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -1690,7 +1690,7 @@ static struct scsi_host_template scsi_driver = {
 	.slave_alloc =		storvsc_device_alloc,
 	.slave_destroy =	storvsc_device_destroy,
 	.slave_configure =	storvsc_device_configure,
-	.cmd_per_lun =		1,
+	.cmd_per_lun =		255,
 	.can_queue =		STORVSC_MAX_IO_REQUESTS*STORVSC_MAX_TARGETS,
 	.this_id =		-1,
 	/* no use setting to 0 since ll_blk_rw reset it to 1 */
-- 
1.7.4.1

[PATCH V3 3/7] Drivers: scsi: storvsc: Filter commands based on the storage protocol version

[K. Y. Srinivasan]

Going forward it is possible that some of the commands that are not currently
implemented will be implemented on future Windows hosts. Even if they are not
implemented, we are told the host will corrrectly handle unsupported
commands (by returning appropriate return code and sense information).
Make command filtering depend on the host version.

In this version of the patch I have addressed comments from
Christoph Hellwig <hch@infradead.org>

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Cc: <stable@vger.kernel.org>
---
 drivers/scsi/storvsc_drv.c |   16 +++++++++++++---
 1 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index cebcef7..8f8847e 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -1553,9 +1553,19 @@ static int storvsc_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *scmnd)
 	struct vmscsi_request *vm_srb;
 	struct stor_mem_pools *memp = scmnd->device->hostdata;
 
-	if (!storvsc_scsi_cmd_ok(scmnd)) {
-		scmnd->scsi_done(scmnd);
-		return 0;
+	if (vmstor_current_major <= VMSTOR_WIN8_MAJOR) {
+		/*
+		 * On legacy hosts filter unimplemented commands.
+		 * Future hosts are expected to correctly handle
+		 * unsupported commands. Furthermore, it is
+		 * possible that some of the currently
+		 * unsupported commands maybe supported in
+		 * future versions of the host.
+		 */
+		if (!storvsc_scsi_cmd_ok(scmnd)) {
+			scmnd->scsi_done(scmnd);
+			return 0;
+		}
 	}
 
 	request_size = sizeof(struct storvsc_cmd_request);
-- 
1.7.4.1

[PATCH V3 4/7] Drivers: scsi: storvsc: Fix a bug in handling VMBUS protocol version

[K. Y. Srinivasan]

Based on the negotiated VMBUS protocol version, we adjust the size of the storage
protocol messages. The two sizes we currently handle are pre-win8 and post-win8.
In WS2012 R2, we are negotiating higher VMBUS protocol version than the win8
version. Make adjustments to correctly handle this.

In this version of the patch I have addressed comments from
Christoph Hellwig <hch@infradead.org>

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Cc: <stable@vger.kernel.org>
---
 drivers/scsi/storvsc_drv.c |   17 ++++++++++-------
 1 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 8f8847e..7e8a642 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -1752,19 +1752,22 @@ static int storvsc_probe(struct hv_device *device,
 	 * set state to properly communicate with the host.
 	 */
 
-	if (vmbus_proto_version == VERSION_WIN8) {
-		sense_buffer_size = POST_WIN7_STORVSC_SENSE_BUFFER_SIZE;
-		vmscsi_size_delta = 0;
-		vmstor_current_major = VMSTOR_WIN8_MAJOR;
-		vmstor_current_minor = VMSTOR_WIN8_MINOR;
-	} else {
+	switch (vmbus_proto_version) {
+	case VERSION_WS2008:
+	case VERSION_WIN7:
 		sense_buffer_size = PRE_WIN8_STORVSC_SENSE_BUFFER_SIZE;
 		vmscsi_size_delta = sizeof(struct vmscsi_win8_extension);
 		vmstor_current_major = VMSTOR_WIN7_MAJOR;
 		vmstor_current_minor = VMSTOR_WIN7_MINOR;
+		break;
+	default:
+		sense_buffer_size = POST_WIN7_STORVSC_SENSE_BUFFER_SIZE;
+		vmscsi_size_delta = 0;
+		vmstor_current_major = VMSTOR_WIN8_MAJOR;
+		vmstor_current_minor = VMSTOR_WIN8_MINOR;
+		break;
 	}
 
-
 	if (dev_id->driver_data == SFC_GUID)
 		scsi_driver.can_queue = (STORVSC_MAX_IO_REQUESTS *
 					 STORVSC_FC_MAX_TARGETS);
-- 
1.7.4.1

[PATCH 5/7] Drivers: scsi: storvsc: Implement a timedout handler

[K. Y. Srinivasan]

On Azure, we have seen instances of unbounded I/O latencies. To deal with
this issue, implement handler that can reset the timeout. Note that the
host gaurantees that it will respond to each command that has been issued.


Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Cc: <stable@vger.kernel.org>
---
 drivers/scsi/storvsc_drv.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 7e8a642..3516761 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -33,6 +33,7 @@
 #include <linux/device.h>
 #include <linux/hyperv.h>
 #include <linux/mempool.h>
+#include <linux/blkdev.h>
 #include <scsi/scsi.h>
 #include <scsi/scsi_cmnd.h>
 #include <scsi/scsi_host.h>
@@ -1518,6 +1519,14 @@ static int storvsc_host_reset_handler(struct scsi_cmnd *scmnd)
 	return SUCCESS;
 }
 
+static enum blk_eh_timer_return storvsc_timeout_handler(struct scsi_cmnd *scmnd)
+{
+	/*
+	 * The host will respond; ask for more time.
+	 */
+	return BLK_EH_RESET_TIMER;
+}
+
 static bool storvsc_scsi_cmd_ok(struct scsi_cmnd *scmnd)
 {
 	bool allowed = true;
@@ -1697,6 +1706,7 @@ static struct scsi_host_template scsi_driver = {
 	.bios_param =		storvsc_get_chs,
 	.queuecommand =		storvsc_queuecommand,
 	.eh_host_reset_handler =	storvsc_host_reset_handler,
+	.eh_timed_out =		storvsc_timeout_handler,
 	.slave_alloc =		storvsc_device_alloc,
 	.slave_destroy =	storvsc_device_destroy,
 	.slave_configure =	storvsc_device_configure,
-- 
1.7.4.1

[PATCH 6/7] drivers: scsi: storvsc: Set srb_flags in all cases

[K. Y. Srinivasan]

Correctly set SRB flags for all valid I/O directions. Some IHV drivers on the
Windows host require this. The host validates the command and SRB flags
prior to passing the command down to native driver stack.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Cc: <stable@vger.kernel.org>
---
 drivers/scsi/storvsc_drv.c |   12 +++++-------
 1 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 3516761..9342ba4 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -1599,26 +1599,24 @@ static int storvsc_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *scmnd)
 	vm_srb = &cmd_request->vstor_packet.vm_srb;
 	vm_srb->win8_extension.time_out_value = 60;
 
+	vm_srb->win8_extension.srb_flags |=
+		(SRB_FLAGS_QUEUE_ACTION_ENABLE |
+		SRB_FLAGS_DISABLE_SYNCH_TRANSFER);
 
 	/* Build the SRB */
 	switch (scmnd->sc_data_direction) {
 	case DMA_TO_DEVICE:
 		vm_srb->data_in = WRITE_TYPE;
 		vm_srb->win8_extension.srb_flags |= SRB_FLAGS_DATA_OUT;
-		vm_srb->win8_extension.srb_flags |=
-			(SRB_FLAGS_QUEUE_ACTION_ENABLE |
-			SRB_FLAGS_DISABLE_SYNCH_TRANSFER);
 		break;
 	case DMA_FROM_DEVICE:
 		vm_srb->data_in = READ_TYPE;
 		vm_srb->win8_extension.srb_flags |= SRB_FLAGS_DATA_IN;
-		vm_srb->win8_extension.srb_flags |=
-			(SRB_FLAGS_QUEUE_ACTION_ENABLE |
-			SRB_FLAGS_DISABLE_SYNCH_TRANSFER);
 		break;
 	default:
 		vm_srb->data_in = UNKNOWN_TYPE;
-		vm_srb->win8_extension.srb_flags = 0;
+		vm_srb->win8_extension.srb_flags |= (SRB_FLAGS_DATA_IN |
+						     SRB_FLAGS_DATA_OUT);
 		break;
 	}
 
-- 
1.7.4.1

[PATCH 7/7] drivers: scsi: storvsc: Correctly handle TEST_UNIT_READY failure

[K. Y. Srinivasan]

On some Windows hosts on FC SANs, TEST_UNIT_READY can return SRB_STATUS_ERROR.
Correctly handle this. Note that there is sufficient sense information to
support scsi error handling even in this case.

In this version of the patch I have addressed comments from
Christoph Hellwig <hch@infradead.org>

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Cc: <stable@vger.kernel.org>
---
 drivers/scsi/storvsc_drv.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 9342ba4..29d0329 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -1018,6 +1018,13 @@ static void storvsc_handle_error(struct vmscsi_request *vm_srb,
 		case ATA_12:
 			set_host_byte(scmnd, DID_PASSTHROUGH);
 			break;
+		/*
+		 * On Some Windows hosts TEST_UNIT_READY command can return
+		 * SRB_STATUS_ERROR, let the upper level code deal with it
+		 * based on the sense information.
+		 */
+		case TEST_UNIT_READY:
+			break;
 		default:
 			set_host_byte(scmnd, DID_TARGET_FAILURE);
 		}
-- 
1.7.4.1

Re: [PATCH V3 0/7] Drivers: scsi: storvsc: Bug fixes and improvements

[Christoph Hellwig]

The series looks good to me, although I'll probably put the commit
message for the timer patch into the comment for the eh_timed_out
handler.

Can I get another set of reviews?  I've seen Olaf has put them into the
SLES tree, so I'd expect he's done a review already?

RE: [PATCH V3 0/7] Drivers: scsi: storvsc: Bug fixes and improvements

[KY Srinivasan]

> -----Original Message-----
> From: Christoph Hellwig [mailto:hch@infradead.org]
> Sent: Sunday, July 13, 2014 2:51 AM
> To: KY Srinivasan
> Cc: jasowang@redhat.com; apw@canonical.com; linux-
> kernel@vger.kernel.org; devel@linuxdriverproject.org; ohering@suse.com;
> jbottomley@parallels.com; hch@infradead.org; linux-scsi@vger.kernel.org
> Subject: Re: [PATCH V3 0/7] Drivers: scsi: storvsc: Bug fixes and
> improvements
> 
> The series looks good to me, although I'll probably put the commit message
> for the timer patch into the comment for the eh_timed_out handler.

Do you want me to send the patch out with the additional comments. Would
you be checking these in.

K. Y
> 
> Can I get another set of reviews?  I've seen Olaf has put them into the SLES
> tree, so I'd expect he's done a review already?

Re: [PATCH V3 1/7] Drivers: scsi: storvsc: Change the limits to reflect the values on the host

[Hannes Reinecke]

On 07/12/2014 06:48 PM, K. Y. Srinivasan wrote:
> Hyper-V hosts can support multiple targets and multiple channels and larger number of
> LUNs per target. Update the code to reflect this. With this patch we can correctly
> enumerate all the paths in a multi-path storage environment.
>
> In this version of the patch I have addressed comments from
> Christoph Hellwig <hch@infradead.org>
>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> Cc: <stable@vger.kernel.org>
> ---
>   drivers/scsi/storvsc_drv.c |   47 +++++++++++++++++++++++++++++---------------
>   1 files changed, 31 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> index 9969fa1..8938b13 100644
> --- a/drivers/scsi/storvsc_drv.c
> +++ b/drivers/scsi/storvsc_drv.c
> @@ -330,17 +330,17 @@ static int storvsc_timeout = 180;
>
>   static void storvsc_on_channel_callback(void *context);
>
> -/*
> - * In Hyper-V, each port/path/target maps to 1 scsi host adapter.  In
> - * reality, the path/target is not used (ie always set to 0) so our
> - * scsi host adapter essentially has 1 bus with 1 target that contains
> - * up to 256 luns.
> - */
> -#define STORVSC_MAX_LUNS_PER_TARGET			64
> -#define STORVSC_MAX_TARGETS				1
> -#define STORVSC_MAX_CHANNELS				1
> +#define STORVSC_MAX_LUNS_PER_TARGET			255
> +#define STORVSC_MAX_TARGETS				2
> +#define STORVSC_MAX_CHANNELS				8
>
> +#define STORVSC_FC_MAX_LUNS_PER_TARGET			255
> +#define STORVSC_FC_MAX_TARGETS				128
> +#define STORVSC_FC_MAX_CHANNELS				8
>
> +#define STORVSC_IDE_MAX_LUNS_PER_TARGET			64
> +#define STORVSC_IDE_MAX_TARGETS				1
> +#define STORVSC_IDE_MAX_CHANNELS			1
>
>   struct storvsc_cmd_request {
>   	struct list_head entry;
Limiting max_lun to 255 will make the driver to _not_ respond to 
LUNs higher than that; ie Well-known LUN won't work here.
Also the SCSI stack will be using REPORT LUNS anyway since you're 
advertising SPC-2 compliance. So your driver runs into issues if 
Hyper-V would ever return more than 256 LUNs with the REPORT LUN 
command or if any of the LUNs has an addressing scheme other than
'0x00'.
I would suggest to raise this to the technical limit (ie the largest 
LUN which the _protocol_ supports) and let REPORT LUNS deal with the 
actual LUNs.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)

Re: [PATCH V3 2/7] Drivers: scsi: storvsc: Set cmd_per_lun to reflect value supported by the Host

[Hannes Reinecke]

On 07/12/2014 06:48 PM, K. Y. Srinivasan wrote:
> Set cmd_per_lun to reflect value supported by the Host.
> In this version of the patch I have addressed comments from
> Christoph Hellwig <hch@infradead.org>
>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> Cc: <stable@vger.kernel.org>
> ---
>   drivers/scsi/storvsc_drv.c |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> index 8938b13..cebcef7 100644
> --- a/drivers/scsi/storvsc_drv.c
> +++ b/drivers/scsi/storvsc_drv.c
> @@ -1690,7 +1690,7 @@ static struct scsi_host_template scsi_driver = {
>   	.slave_alloc =		storvsc_device_alloc,
>   	.slave_destroy =	storvsc_device_destroy,
>   	.slave_configure =	storvsc_device_configure,
> -	.cmd_per_lun =		1,
> +	.cmd_per_lun =		255,
>   	.can_queue =		STORVSC_MAX_IO_REQUESTS*STORVSC_MAX_TARGETS,
>   	.this_id =		-1,
>   	/* no use setting to 0 since ll_blk_rw reset it to 1 */
>
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)

Re: [PATCH V3 3/7] Drivers: scsi: storvsc: Filter commands based on the storage protocol version

[Hannes Reinecke]

On 07/12/2014 06:48 PM, K. Y. Srinivasan wrote:
> Going forward it is possible that some of the commands that are not currently
> implemented will be implemented on future Windows hosts. Even if they are not
> implemented, we are told the host will corrrectly handle unsupported
> commands (by returning appropriate return code and sense information).
> Make command filtering depend on the host version.
>
> In this version of the patch I have addressed comments from
> Christoph Hellwig <hch@infradead.org>
>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> Cc: <stable@vger.kernel.org>
> ---
>   drivers/scsi/storvsc_drv.c |   16 +++++++++++++---
>   1 files changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> index cebcef7..8f8847e 100644
> --- a/drivers/scsi/storvsc_drv.c
> +++ b/drivers/scsi/storvsc_drv.c
> @@ -1553,9 +1553,19 @@ static int storvsc_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *scmnd)
>   	struct vmscsi_request *vm_srb;
>   	struct stor_mem_pools *memp = scmnd->device->hostdata;
>
> -	if (!storvsc_scsi_cmd_ok(scmnd)) {
> -		scmnd->scsi_done(scmnd);
> -		return 0;
> +	if (vmstor_current_major <= VMSTOR_WIN8_MAJOR) {
> +		/*
> +		 * On legacy hosts filter unimplemented commands.
> +		 * Future hosts are expected to correctly handle
> +		 * unsupported commands. Furthermore, it is
> +		 * possible that some of the currently
> +		 * unsupported commands maybe supported in
> +		 * future versions of the host.
> +		 */
> +		if (!storvsc_scsi_cmd_ok(scmnd)) {
> +			scmnd->scsi_done(scmnd);
> +			return 0;
> +		}
>   	}
>
>   	request_size = sizeof(struct storvsc_cmd_request);
>
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)

Re: [PATCH V3 4/7] Drivers: scsi: storvsc: Fix a bug in handling VMBUS protocol version

[Hannes Reinecke]

On 07/12/2014 06:48 PM, K. Y. Srinivasan wrote:
> Based on the negotiated VMBUS protocol version, we adjust the size of the storage
> protocol messages. The two sizes we currently handle are pre-win8 and post-win8.
> In WS2012 R2, we are negotiating higher VMBUS protocol version than the win8
> version. Make adjustments to correctly handle this.
>
> In this version of the patch I have addressed comments from
> Christoph Hellwig <hch@infradead.org>
>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> Cc: <stable@vger.kernel.org>
> ---
>   drivers/scsi/storvsc_drv.c |   17 ++++++++++-------
>   1 files changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> index 8f8847e..7e8a642 100644
> --- a/drivers/scsi/storvsc_drv.c
> +++ b/drivers/scsi/storvsc_drv.c
> @@ -1752,19 +1752,22 @@ static int storvsc_probe(struct hv_device *device,
>   	 * set state to properly communicate with the host.
>   	 */
>
> -	if (vmbus_proto_version == VERSION_WIN8) {
> -		sense_buffer_size = POST_WIN7_STORVSC_SENSE_BUFFER_SIZE;
> -		vmscsi_size_delta = 0;
> -		vmstor_current_major = VMSTOR_WIN8_MAJOR;
> -		vmstor_current_minor = VMSTOR_WIN8_MINOR;
> -	} else {
> +	switch (vmbus_proto_version) {
> +	case VERSION_WS2008:
> +	case VERSION_WIN7:
>   		sense_buffer_size = PRE_WIN8_STORVSC_SENSE_BUFFER_SIZE;
>   		vmscsi_size_delta = sizeof(struct vmscsi_win8_extension);
>   		vmstor_current_major = VMSTOR_WIN7_MAJOR;
>   		vmstor_current_minor = VMSTOR_WIN7_MINOR;
> +		break;
> +	default:
> +		sense_buffer_size = POST_WIN7_STORVSC_SENSE_BUFFER_SIZE;
> +		vmscsi_size_delta = 0;
> +		vmstor_current_major = VMSTOR_WIN8_MAJOR;
> +		vmstor_current_minor = VMSTOR_WIN8_MINOR;
> +		break;
>   	}
>
> -
>   	if (dev_id->driver_data == SFC_GUID)
>   		scsi_driver.can_queue = (STORVSC_MAX_IO_REQUESTS *
>   					 STORVSC_FC_MAX_TARGETS);
>
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)

Re: [PATCH 5/7] Drivers: scsi: storvsc: Implement a timedout handler

[Hannes Reinecke]

On 07/12/2014 06:48 PM, K. Y. Srinivasan wrote:
> On Azure, we have seen instances of unbounded I/O latencies. To deal with
> this issue, implement handler that can reset the timeout. Note that the
> host gaurantees that it will respond to each command that has been issued.
>
>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> Cc: <stable@vger.kernel.org>
> ---
>   drivers/scsi/storvsc_drv.c |   10 ++++++++++
>   1 files changed, 10 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> index 7e8a642..3516761 100644
> --- a/drivers/scsi/storvsc_drv.c
> +++ b/drivers/scsi/storvsc_drv.c
> @@ -33,6 +33,7 @@
>   #include <linux/device.h>
>   #include <linux/hyperv.h>
>   #include <linux/mempool.h>
> +#include <linux/blkdev.h>
>   #include <scsi/scsi.h>
>   #include <scsi/scsi_cmnd.h>
>   #include <scsi/scsi_host.h>
> @@ -1518,6 +1519,14 @@ static int storvsc_host_reset_handler(struct scsi_cmnd *scmnd)
>   	return SUCCESS;
>   }
>
> +static enum blk_eh_timer_return storvsc_timeout_handler(struct scsi_cmnd *scmnd)
> +{
> +	/*
> +	 * The host will respond; ask for more time.
> +	 */
> +	return BLK_EH_RESET_TIMER;
> +}
> +
>   static bool storvsc_scsi_cmd_ok(struct scsi_cmnd *scmnd)
>   {
>   	bool allowed = true;
> @@ -1697,6 +1706,7 @@ static struct scsi_host_template scsi_driver = {
>   	.bios_param =		storvsc_get_chs,
>   	.queuecommand =		storvsc_queuecommand,
>   	.eh_host_reset_handler =	storvsc_host_reset_handler,
> +	.eh_timed_out =		storvsc_timeout_handler,
>   	.slave_alloc =		storvsc_device_alloc,
>   	.slave_destroy =	storvsc_device_destroy,
>   	.slave_configure =	storvsc_device_configure,
>
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)

Re: [PATCH 6/7] drivers: scsi: storvsc: Set srb_flags in all cases

[Hannes Reinecke]

On 07/12/2014 06:48 PM, K. Y. Srinivasan wrote:
> Correctly set SRB flags for all valid I/O directions. Some IHV drivers on the
> Windows host require this. The host validates the command and SRB flags
> prior to passing the command down to native driver stack.
>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> Cc: <stable@vger.kernel.org>
> ---
>   drivers/scsi/storvsc_drv.c |   12 +++++-------
>   1 files changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> index 3516761..9342ba4 100644
> --- a/drivers/scsi/storvsc_drv.c
> +++ b/drivers/scsi/storvsc_drv.c
> @@ -1599,26 +1599,24 @@ static int storvsc_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *scmnd)
>   	vm_srb = &cmd_request->vstor_packet.vm_srb;
>   	vm_srb->win8_extension.time_out_value = 60;
>
> +	vm_srb->win8_extension.srb_flags |=
> +		(SRB_FLAGS_QUEUE_ACTION_ENABLE |
> +		SRB_FLAGS_DISABLE_SYNCH_TRANSFER);
>
>   	/* Build the SRB */
>   	switch (scmnd->sc_data_direction) {
>   	case DMA_TO_DEVICE:
>   		vm_srb->data_in = WRITE_TYPE;
>   		vm_srb->win8_extension.srb_flags |= SRB_FLAGS_DATA_OUT;
> -		vm_srb->win8_extension.srb_flags |=
> -			(SRB_FLAGS_QUEUE_ACTION_ENABLE |
> -			SRB_FLAGS_DISABLE_SYNCH_TRANSFER);
>   		break;
>   	case DMA_FROM_DEVICE:
>   		vm_srb->data_in = READ_TYPE;
>   		vm_srb->win8_extension.srb_flags |= SRB_FLAGS_DATA_IN;
> -		vm_srb->win8_extension.srb_flags |=
> -			(SRB_FLAGS_QUEUE_ACTION_ENABLE |
> -			SRB_FLAGS_DISABLE_SYNCH_TRANSFER);
>   		break;
>   	default:
>   		vm_srb->data_in = UNKNOWN_TYPE;
> -		vm_srb->win8_extension.srb_flags = 0;
> +		vm_srb->win8_extension.srb_flags |= (SRB_FLAGS_DATA_IN |
> +						     SRB_FLAGS_DATA_OUT);
>   		break;
>   	}
>
>
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)

Re: [PATCH 7/7] drivers: scsi: storvsc: Correctly handle TEST_UNIT_READY failure

[Hannes Reinecke]

On 07/12/2014 06:48 PM, K. Y. Srinivasan wrote:
> On some Windows hosts on FC SANs, TEST_UNIT_READY can return SRB_STATUS_ERROR.
> Correctly handle this. Note that there is sufficient sense information to
> support scsi error handling even in this case.
>
> In this version of the patch I have addressed comments from
> Christoph Hellwig <hch@infradead.org>
>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> Cc: <stable@vger.kernel.org>
> ---
>   drivers/scsi/storvsc_drv.c |    7 +++++++
>   1 files changed, 7 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> index 9342ba4..29d0329 100644
> --- a/drivers/scsi/storvsc_drv.c
> +++ b/drivers/scsi/storvsc_drv.c
> @@ -1018,6 +1018,13 @@ static void storvsc_handle_error(struct vmscsi_request *vm_srb,
>   		case ATA_12:
>   			set_host_byte(scmnd, DID_PASSTHROUGH);
>   			break;
> +		/*
> +		 * On Some Windows hosts TEST_UNIT_READY command can return
> +		 * SRB_STATUS_ERROR, let the upper level code deal with it
> +		 * based on the sense information.
> +		 */
> +		case TEST_UNIT_READY:
> +			break;
>   		default:
>   			set_host_byte(scmnd, DID_TARGET_FAILURE);
>   		}
>
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)

Re: [PATCH V3 1/7] Drivers: scsi: storvsc: Change the limits to reflect the values on the host

[Christoph Hellwig]

On Mon, Jul 14, 2014 at 08:15:17AM +0200, Hannes Reinecke wrote:
> Limiting max_lun to 255 will make the driver to _not_ respond to LUNs higher
> than that; ie Well-known LUN won't work here.
> Also the SCSI stack will be using REPORT LUNS anyway since you're
> advertising SPC-2 compliance. So your driver runs into issues if Hyper-V
> would ever return more than 256 LUNs with the REPORT LUN command or if any
> of the LUNs has an addressing scheme other than
> '0x00'.
> I would suggest to raise this to the technical limit (ie the largest LUN
> which the _protocol_ supports) and let REPORT LUNS deal with the actual
> LUNs.

I suspect hypverv doesn't support anything more.  For now I'd be
inclined to just put it in ASAP and if your suggestion works out fix it
up later, although I'll wait a bit for more review feedback.

Re: [PATCH V3 1/7] Drivers: scsi: storvsc: Change the limits to reflect the values on the host

[Hannes Reinecke]

On 07/14/2014 10:30 AM, Christoph Hellwig wrote:
> On Mon, Jul 14, 2014 at 08:15:17AM +0200, Hannes Reinecke wrote:
>> Limiting max_lun to 255 will make the driver to _not_ respond to LUNs higher
>> than that; ie Well-known LUN won't work here.
>> Also the SCSI stack will be using REPORT LUNS anyway since you're
>> advertising SPC-2 compliance. So your driver runs into issues if Hyper-V
>> would ever return more than 256 LUNs with the REPORT LUN command or if any
>> of the LUNs has an addressing scheme other than
>> '0x00'.
>> I would suggest to raise this to the technical limit (ie the largest LUN
>> which the _protocol_ supports) and let REPORT LUNS deal with the actual
>> LUNs.
>
> I suspect hypverv doesn't support anything more.  For now I'd be
> inclined to just put it in ASAP and if your suggestion works out fix it
> up later, although I'll wait a bit for more review feedback.
>
Okay, that's fine by me.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)

Re: [PATCH V3 1/7] Drivers: scsi: storvsc: Change the limits to reflect the values on the host

[Christoph Hellwig]

On Mon, Jul 14, 2014 at 10:57:53AM +0200, Hannes Reinecke wrote:
> Okay, that's fine by me.

Should I take this as a reviewed-by tag?

Re: [PATCH V3 1/7] Drivers: scsi: storvsc: Change the limits to reflect the values on the host

[Hannes Reinecke]

On 07/14/2014 11:00 AM, Christoph Hellwig wrote:
> On Mon, Jul 14, 2014 at 10:57:53AM +0200, Hannes Reinecke wrote:
>> Okay, that's fine by me.
>
> Should I take this as a reviewed-by tag?
>
Yes, please do.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)

RE: [PATCH V3 1/7] Drivers: scsi: storvsc: Change the limits to reflect the values on the host

[KY Srinivasan]

> -----Original Message-----
> From: Hannes Reinecke [mailto:hare@suse.de]
> Sent: Monday, July 14, 2014 1:58 AM
> To: Christoph Hellwig
> Cc: KY Srinivasan; jasowang@redhat.com; apw@canonical.com; linux-
> kernel@vger.kernel.org; devel@linuxdriverproject.org; ohering@suse.com;
> jbottomley@parallels.com; linux-scsi@vger.kernel.org;
> stable@vger.kernel.org
> Subject: Re: [PATCH V3 1/7] Drivers: scsi: storvsc: Change the limits to reflect
> the values on the host
> 
> On 07/14/2014 10:30 AM, Christoph Hellwig wrote:
> > On Mon, Jul 14, 2014 at 08:15:17AM +0200, Hannes Reinecke wrote:
> >> Limiting max_lun to 255 will make the driver to _not_ respond to LUNs
> >> higher than that; ie Well-known LUN won't work here.
> >> Also the SCSI stack will be using REPORT LUNS anyway since you're
> >> advertising SPC-2 compliance. So your driver runs into issues if
> >> Hyper-V would ever return more than 256 LUNs with the REPORT LUN
> >> command or if any of the LUNs has an addressing scheme other than
> >> '0x00'.
> >> I would suggest to raise this to the technical limit (ie the largest
> >> LUN which the _protocol_ supports) and let REPORT LUNS deal with the
> >> actual LUNs.
> >
> > I suspect hypverv doesn't support anything more.  For now I'd be
> > inclined to just put it in ASAP and if your suggestion works out fix
> > it up later, although I'll wait a bit for more review feedback.
> >
> Okay, that's fine by me.

Christoph,

Is this patch-set ready to be checked in. Let me know if you want me to make any
further corrections.

Regards,

K. Y
> 
> Cheers,
> 
> Hannes
> --
> Dr. Hannes Reinecke		      zSeries & Storage
> hare@suse.de			      +49 911 74053 688
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
> GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)

Re: [PATCH V3 1/7] Drivers: scsi: storvsc: Change the limits to reflect the values on the host

[Christoph Hellwig]

On Wed, Jul 16, 2014 at 05:26:48PM +0000, KY Srinivasan wrote:
> Christoph,
> 
> Is this patch-set ready to be checked in. Let me know if you want me to make any
> further corrections.

Hi Ky,

I've applied it locally, but I'm still waiting on reviews for two
important core fixes before pushing out the updated tree.

RE: [PATCH V3 1/7] Drivers: scsi: storvsc: Change the limits to reflect the values on the host

[KY Srinivasan]

> -----Original Message-----
> From: Christoph Hellwig [mailto:hch@infradead.org]
> Sent: Wednesday, July 16, 2014 10:29 AM
> To: KY Srinivasan
> Cc: Hannes Reinecke; jasowang@redhat.com; apw@canonical.com; linux-
> kernel@vger.kernel.org; devel@linuxdriverproject.org; ohering@suse.com;
> jbottomley@parallels.com; linux-scsi@vger.kernel.org;
> stable@vger.kernel.org
> Subject: Re: [PATCH V3 1/7] Drivers: scsi: storvsc: Change the limits to reflect
> the values on the host
> 
> On Wed, Jul 16, 2014 at 05:26:48PM +0000, KY Srinivasan wrote:
> > Christoph,
> >
> > Is this patch-set ready to be checked in. Let me know if you want me
> > to make any further corrections.
> 
> Hi Ky,
> 
> I've applied it locally, but I'm still waiting on reviews for two important core
> fixes before pushing out the updated tree.

Thank you.

K. Y