linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH RESEND V2 3/7] scsi: storvsc: Untangle the storage protocol negotiation from the vmbus protocol negotiation.
  2015-08-13 15:43   ` [PATCH RESEND V2 3/7] scsi: storvsc: Untangle the storage protocol negotiation from the vmbus protocol negotiation K. Y. Srinivasan
@ 2015-08-13 14:33     ` Johannes Thumshirn
  2015-08-13 15:18       ` KY Srinivasan
  2015-08-27  1:40     ` James Bottomley
  1 sibling, 1 reply; 13+ messages in thread
From: Johannes Thumshirn @ 2015-08-13 14:33 UTC (permalink / raw)
  To: K. Y. Srinivasan, Keith Mange
  Cc: gregkh, linux-kernel, devel, ohering, jbottomley, hch,
	linux-scsi, apw, vkuznets, jasowang

"K. Y. Srinivasan" <kys@microsoft.com> writes:

> From: Keith Mange <keith.mange@microsoft.com>
>
> Currently we are making decisions based on vmbus protocol versions
> that have been negotiated; use storage potocol versions instead.
>
> Tested-by: Alex Ng <alexng@microsoft.com>
> Signed-off-by: Keith Mange <keith.mange@microsoft.com>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> ---
>  drivers/scsi/storvsc_drv.c |  109 +++++++++++++++++++++++++++++++++++---------
>  1 files changed, 87 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> index 5f9d133..f29871e 100644
> --- a/drivers/scsi/storvsc_drv.c
> +++ b/drivers/scsi/storvsc_drv.c
> @@ -56,14 +56,18 @@
>   * V1 RC > 2008/1/31:  2.0
>   * Win7: 4.2
>   * Win8: 5.1
> + * Win8.1: 6.0
> + * Win10: 6.2
>   */
>  
>  #define VMSTOR_PROTO_VERSION(MAJOR_, MINOR_)	((((MAJOR_) & 0xff) << 8) | \
>  						(((MINOR_) & 0xff)))
>  
> +#define VMSTOR_PROTO_VERSION_WIN6	VMSTOR_PROTO_VERSION(2, 0)
>  #define VMSTOR_PROTO_VERSION_WIN7	VMSTOR_PROTO_VERSION(4, 2)
>  #define VMSTOR_PROTO_VERSION_WIN8	VMSTOR_PROTO_VERSION(5, 1)
> -
> +#define VMSTOR_PROTO_VERSION_WIN8_1	VMSTOR_PROTO_VERSION(6, 0)
> +#define VMSTOR_PROTO_VERSION_WIN10	VMSTOR_PROTO_VERSION(6, 2)
>  
>  /*  Packet structure describing virtual storage requests. */
>  enum vstor_packet_operation {
> @@ -205,6 +209,46 @@ struct vmscsi_request {
>  
>  
>  /*
> + * The list of storage protocols in order of preference.
> + */
> +struct vmstor_protocol {
> +	int protocol_version;
> +	int sense_buffer_size;
> +	int vmscsi_size_delta;
> +};
> +
> +#define VMSTOR_NUM_PROTOCOLS    5

can't you just use ARRAY_SIZE() here, so you don't have to touch the
constant every time a new protocol is appended to the list?

> +
> +const struct vmstor_protocol vmstor_protocols[VMSTOR_NUM_PROTOCOLS] = {
> +	{
> +		VMSTOR_PROTO_VERSION_WIN10,
> +		POST_WIN7_STORVSC_SENSE_BUFFER_SIZE,
> +		0
> +	},
> +	{
> +		VMSTOR_PROTO_VERSION_WIN8_1,
> +		POST_WIN7_STORVSC_SENSE_BUFFER_SIZE,
> +		0
> +	},
> +	{
> +		VMSTOR_PROTO_VERSION_WIN8,
> +		POST_WIN7_STORVSC_SENSE_BUFFER_SIZE,
> +		0
> +	},
> +	{
> +		VMSTOR_PROTO_VERSION_WIN7,
> +		PRE_WIN8_STORVSC_SENSE_BUFFER_SIZE,
> +		sizeof(struct vmscsi_win8_extension),
> +	},
> +	{
> +		VMSTOR_PROTO_VERSION_WIN6,
> +		PRE_WIN8_STORVSC_SENSE_BUFFER_SIZE,
> +		sizeof(struct vmscsi_win8_extension),
> +	}
> +};
> +

Thanks,
        Johannes
-- 
Johannes Thumshirn                                           Storage
jthumshirn@suse.de                                 +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600  D0D0 0393 969D 2D76 0850

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH RESEND V2 3/7] scsi: storvsc: Untangle the storage protocol negotiation from the vmbus protocol negotiation.
  2015-08-13 14:33     ` Johannes Thumshirn
@ 2015-08-13 15:18       ` KY Srinivasan
  2015-08-14  6:45         ` Johannes Thumshirn
  0 siblings, 1 reply; 13+ messages in thread
From: KY Srinivasan @ 2015-08-13 15:18 UTC (permalink / raw)
  To: Johannes Thumshirn, Keith Mange
  Cc: gregkh, linux-kernel, devel, ohering, jbottomley, hch,
	linux-scsi, apw, vkuznets, jasowang

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 3995 bytes --]



> -----Original Message-----
> From: Johannes Thumshirn [mailto:jthumshirn@suse.de]
> Sent: Thursday, August 13, 2015 7:34 AM
> To: KY Srinivasan <kys@microsoft.com>; Keith Mange
> <Keith.Mange@microsoft.com>
> Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; ohering@suse.com;
> jbottomley@parallels.com; hch@infradead.org; linux-scsi@vger.kernel.org;
> apw@canonical.com; vkuznets@redhat.com; jasowang@redhat.com
> Subject: Re: [PATCH RESEND V2 3/7] scsi: storvsc: Untangle the storage
> protocol negotiation from the vmbus protocol negotiation.
> 
> "K. Y. Srinivasan" <kys@microsoft.com> writes:
> 
> > From: Keith Mange <keith.mange@microsoft.com>
> >
> > Currently we are making decisions based on vmbus protocol versions
> > that have been negotiated; use storage potocol versions instead.
> >
> > Tested-by: Alex Ng <alexng@microsoft.com>
> > Signed-off-by: Keith Mange <keith.mange@microsoft.com>
> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > ---
> >  drivers/scsi/storvsc_drv.c |  109
> +++++++++++++++++++++++++++++++++++---------
> >  1 files changed, 87 insertions(+), 22 deletions(-)
> >
> > diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> > index 5f9d133..f29871e 100644
> > --- a/drivers/scsi/storvsc_drv.c
> > +++ b/drivers/scsi/storvsc_drv.c
> > @@ -56,14 +56,18 @@
> >   * V1 RC > 2008/1/31:  2.0
> >   * Win7: 4.2
> >   * Win8: 5.1
> > + * Win8.1: 6.0
> > + * Win10: 6.2
> >   */
> >
> >  #define VMSTOR_PROTO_VERSION(MAJOR_, MINOR_)	((((MAJOR_)
> & 0xff) << 8) | \
> >  						(((MINOR_) & 0xff)))
> >
> > +#define VMSTOR_PROTO_VERSION_WIN6
> 	VMSTOR_PROTO_VERSION(2, 0)
> >  #define VMSTOR_PROTO_VERSION_WIN7
> 	VMSTOR_PROTO_VERSION(4, 2)
> >  #define VMSTOR_PROTO_VERSION_WIN8
> 	VMSTOR_PROTO_VERSION(5, 1)
> > -
> > +#define VMSTOR_PROTO_VERSION_WIN8_1
> 	VMSTOR_PROTO_VERSION(6, 0)
> > +#define VMSTOR_PROTO_VERSION_WIN10
> 	VMSTOR_PROTO_VERSION(6, 2)
> >
> >  /*  Packet structure describing virtual storage requests. */
> >  enum vstor_packet_operation {
> > @@ -205,6 +209,46 @@ struct vmscsi_request {
> >
> >
> >  /*
> > + * The list of storage protocols in order of preference.
> > + */
> > +struct vmstor_protocol {
> > +	int protocol_version;
> > +	int sense_buffer_size;
> > +	int vmscsi_size_delta;
> > +};
> > +
> > +#define VMSTOR_NUM_PROTOCOLS    5
> 
> can't you just use ARRAY_SIZE() here, so you don't have to touch the
> constant every time a new protocol is appended to the list?

Certainly. These patches have been floating around for more than a month now and if it is ok
with you, I will submit a patch on top of this current series to address the concern you have raised.
James, please let me know.

Regards,

K. Y 
> 
> > +
> > +const struct vmstor_protocol
> vmstor_protocols[VMSTOR_NUM_PROTOCOLS] = {
> > +	{
> > +		VMSTOR_PROTO_VERSION_WIN10,
> > +		POST_WIN7_STORVSC_SENSE_BUFFER_SIZE,
> > +		0
> > +	},
> > +	{
> > +		VMSTOR_PROTO_VERSION_WIN8_1,
> > +		POST_WIN7_STORVSC_SENSE_BUFFER_SIZE,
> > +		0
> > +	},
> > +	{
> > +		VMSTOR_PROTO_VERSION_WIN8,
> > +		POST_WIN7_STORVSC_SENSE_BUFFER_SIZE,
> > +		0
> > +	},
> > +	{
> > +		VMSTOR_PROTO_VERSION_WIN7,
> > +		PRE_WIN8_STORVSC_SENSE_BUFFER_SIZE,
> > +		sizeof(struct vmscsi_win8_extension),
> > +	},
> > +	{
> > +		VMSTOR_PROTO_VERSION_WIN6,
> > +		PRE_WIN8_STORVSC_SENSE_BUFFER_SIZE,
> > +		sizeof(struct vmscsi_win8_extension),
> > +	}
> > +};
> > +
> 
> Thanks,
>         Johannes
> --
> Johannes Thumshirn                                           Storage
> jthumshirn@suse.de                                 +49 911 74053 689
> SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
> GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
> HRB 21284 (AG Nürnberg)
> Key fingerprint = EC38 9CAB C2C4 F25D 8600  D0D0 0393 969D 2D76 0850
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH RESEND V2 0/7] scsi: storvsc: Some miscellaneous cleanup
@ 2015-08-13 15:43 K. Y. Srinivasan
  2015-08-13 15:43 ` [PATCH RESEND V2 1/7] scsi: storvsc: Rather than look for sets of specific protocol versions, make decisions based on ranges K. Y. Srinivasan
  0 siblings, 1 reply; 13+ messages in thread
From: K. Y. Srinivasan @ 2015-08-13 15:43 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, ohering, jbottomley, hch,
	linux-scsi, apw, vkuznets, jasowang
  Cc: K. Y. Srinivasan

Cleanup version handling as well as base feature detection on storage
version as opposed to host version.

In this version, I have addressed comments from Dan Carpenter.

K. Y. Srinivasan (1):
  scsi: storvsc: Set the error code correctly in failure conditions

Keith Mange (6):
  scsi: storvsc: Rather than look for sets of specific protocol
    versions, make decisions based on ranges.
  scsi: storvsc: Use a single value to track protocol versions
  scsi: storvsc: Untangle the storage protocol negotiation from the
    vmbus protocol negotiation.
  scsi: storvsc: use correct defaults for values determined by protocol
    negotiation
  scsi: storvsc: use storage protocol version to determine storage
    capabilities
  scsi: storvsc: Allow write_same when host is windows 10

 drivers/scsi/storvsc_drv.c |  206 ++++++++++++++++++++++++++++----------------
 1 files changed, 132 insertions(+), 74 deletions(-)

-- 
1.7.4.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH RESEND V2 1/7] scsi: storvsc: Rather than look for sets of specific protocol versions, make decisions based on ranges.
  2015-08-13 15:43 [PATCH RESEND V2 0/7] scsi: storvsc: Some miscellaneous cleanup K. Y. Srinivasan
@ 2015-08-13 15:43 ` K. Y. Srinivasan
  2015-08-13 15:43   ` [PATCH RESEND V2 2/7] scsi: storvsc: Use a single value to track protocol versions K. Y. Srinivasan
                     ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: K. Y. Srinivasan @ 2015-08-13 15:43 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, ohering, jbottomley, hch,
	linux-scsi, apw, vkuznets, jasowang
  Cc: Keith Mange, K. Y. Srinivasan

From: Keith Mange <keith.mange@microsoft.com>

Rather than look for sets of specific protocol versions,
make decisions based on ranges. This will be safer and require fewer changes
going forward as we add more storage protocol versions.

Tested-by: Alex Ng <alexng@microsoft.com>
Signed-off-by: Keith Mange <keith.mange@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/scsi/storvsc_drv.c |   11 +++--------
 1 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 3c6584f..582f3b5 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -988,8 +988,7 @@ static int storvsc_channel_init(struct hv_device *device)
 	 * support multi-channel.
 	 */
 	max_chns = vstor_packet->storage_channel_properties.max_channel_cnt;
-	if ((vmbus_proto_version != VERSION_WIN7) &&
-	   (vmbus_proto_version != VERSION_WS2008))  {
+	if (vmbus_proto_version >= VERSION_WIN8) {
 		if (vstor_packet->storage_channel_properties.flags &
 		    STORAGE_CHANNEL_SUPPORTS_MULTI_CHANNEL)
 			process_sub_channels = true;
@@ -1758,9 +1757,7 @@ static int storvsc_probe(struct hv_device *device,
 	 * set state to properly communicate with the host.
 	 */
 
-	switch (vmbus_proto_version) {
-	case VERSION_WS2008:
-	case VERSION_WIN7:
+	if (vmbus_proto_version < VERSION_WIN8) {
 		sense_buffer_size = PRE_WIN8_STORVSC_SENSE_BUFFER_SIZE;
 		vmscsi_size_delta = sizeof(struct vmscsi_win8_extension);
 		vmstor_current_major = VMSTOR_WIN7_MAJOR;
@@ -1768,8 +1765,7 @@ static int storvsc_probe(struct hv_device *device,
 		max_luns_per_target = STORVSC_IDE_MAX_LUNS_PER_TARGET;
 		max_targets = STORVSC_IDE_MAX_TARGETS;
 		max_channels = STORVSC_IDE_MAX_CHANNELS;
-		break;
-	default:
+	} else {
 		sense_buffer_size = POST_WIN7_STORVSC_SENSE_BUFFER_SIZE;
 		vmscsi_size_delta = 0;
 		vmstor_current_major = VMSTOR_WIN8_MAJOR;
@@ -1783,7 +1779,6 @@ static int storvsc_probe(struct hv_device *device,
 		 * VCPUs in the guest.
 		 */
 		max_sub_channels = (num_cpus / storvsc_vcpus_per_sub_channel);
-		break;
 	}
 
 	scsi_driver.can_queue = (max_outstanding_req_per_channel *
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH RESEND V2 2/7] scsi: storvsc: Use a single value to track protocol versions
  2015-08-13 15:43 ` [PATCH RESEND V2 1/7] scsi: storvsc: Rather than look for sets of specific protocol versions, make decisions based on ranges K. Y. Srinivasan
@ 2015-08-13 15:43   ` K. Y. Srinivasan
  2015-08-13 15:43   ` [PATCH RESEND V2 3/7] scsi: storvsc: Untangle the storage protocol negotiation from the vmbus protocol negotiation K. Y. Srinivasan
                     ` (4 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: K. Y. Srinivasan @ 2015-08-13 15:43 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, ohering, jbottomley, hch,
	linux-scsi, apw, vkuznets, jasowang
  Cc: Keith Mange, K. Y. Srinivasan

From: Keith Mange <keith.mange@microsoft.com>

Use a single value to track protocol versions to simplify
comparisons and to be consistent with vmbus version tracking.

Tested-by: Alex Ng <alexng@microsoft.com>
Signed-off-by: Keith Mange <keith.mange@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/scsi/storvsc_drv.c |   35 +++++++++--------------------------
 1 files changed, 9 insertions(+), 26 deletions(-)

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 582f3b5..5f9d133 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -58,12 +58,11 @@
  * Win8: 5.1
  */
 
+#define VMSTOR_PROTO_VERSION(MAJOR_, MINOR_)	((((MAJOR_) & 0xff) << 8) | \
+						(((MINOR_) & 0xff)))
 
-#define VMSTOR_WIN7_MAJOR 4
-#define VMSTOR_WIN7_MINOR 2
-
-#define VMSTOR_WIN8_MAJOR 5
-#define VMSTOR_WIN8_MINOR 1
+#define VMSTOR_PROTO_VERSION_WIN7	VMSTOR_PROTO_VERSION(4, 2)
+#define VMSTOR_PROTO_VERSION_WIN8	VMSTOR_PROTO_VERSION(5, 1)
 
 
 /*  Packet structure describing virtual storage requests. */
@@ -161,8 +160,7 @@ static int sense_buffer_size;
  */
 
 static int vmscsi_size_delta;
-static int vmstor_current_major;
-static int vmstor_current_minor;
+static int vmstor_proto_version;
 
 struct vmscsi_win8_extension {
 	/*
@@ -481,18 +479,6 @@ done:
 	kfree(wrk);
 }
 
-/*
- * Major/minor macros.  Minor version is in LSB, meaning that earlier flat
- * version numbers will be interpreted as "0.x" (i.e., 1 becomes 0.1).
- */
-
-static inline u16 storvsc_get_version(u8 major, u8 minor)
-{
-	u16 version;
-
-	version = ((major << 8) | minor);
-	return version;
-}
 
 /*
  * We can get incoming messages from the host that are not in response to
@@ -930,8 +916,7 @@ static int storvsc_channel_init(struct hv_device *device)
 	vstor_packet->operation = VSTOR_OPERATION_QUERY_PROTOCOL_VERSION;
 	vstor_packet->flags = REQUEST_COMPLETION_FLAG;
 
-	vstor_packet->version.major_minor =
-		storvsc_get_version(vmstor_current_major, vmstor_current_minor);
+	vstor_packet->version.major_minor = vmstor_proto_version;
 
 	/*
 	 * The revision number is only used in Windows; set it to 0.
@@ -1562,7 +1547,7 @@ static int storvsc_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *scmnd)
 	u32 payload_sz;
 	u32 length;
 
-	if (vmstor_current_major <= VMSTOR_WIN8_MAJOR) {
+	if (vmstor_proto_version <= VMSTOR_PROTO_VERSION_WIN8) {
 		/*
 		 * On legacy hosts filter unimplemented commands.
 		 * Future hosts are expected to correctly handle
@@ -1760,16 +1745,14 @@ static int storvsc_probe(struct hv_device *device,
 	if (vmbus_proto_version < VERSION_WIN8) {
 		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;
+		vmstor_proto_version = VMSTOR_PROTO_VERSION_WIN7;
 		max_luns_per_target = STORVSC_IDE_MAX_LUNS_PER_TARGET;
 		max_targets = STORVSC_IDE_MAX_TARGETS;
 		max_channels = STORVSC_IDE_MAX_CHANNELS;
 	} else {
 		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;
+		vmstor_proto_version = VMSTOR_PROTO_VERSION_WIN8;
 		max_luns_per_target = STORVSC_MAX_LUNS_PER_TARGET;
 		max_targets = STORVSC_MAX_TARGETS;
 		max_channels = STORVSC_MAX_CHANNELS;
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH RESEND V2 3/7] scsi: storvsc: Untangle the storage protocol negotiation from the vmbus protocol negotiation.
  2015-08-13 15:43 ` [PATCH RESEND V2 1/7] scsi: storvsc: Rather than look for sets of specific protocol versions, make decisions based on ranges K. Y. Srinivasan
  2015-08-13 15:43   ` [PATCH RESEND V2 2/7] scsi: storvsc: Use a single value to track protocol versions K. Y. Srinivasan
@ 2015-08-13 15:43   ` K. Y. Srinivasan
  2015-08-13 14:33     ` Johannes Thumshirn
  2015-08-27  1:40     ` James Bottomley
  2015-08-13 15:43   ` [PATCH RESEND V2 4/7] scsi: storvsc: use correct defaults for values determined by " K. Y. Srinivasan
                     ` (3 subsequent siblings)
  5 siblings, 2 replies; 13+ messages in thread
From: K. Y. Srinivasan @ 2015-08-13 15:43 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, ohering, jbottomley, hch,
	linux-scsi, apw, vkuznets, jasowang
  Cc: Keith Mange, K. Y. Srinivasan

From: Keith Mange <keith.mange@microsoft.com>

Currently we are making decisions based on vmbus protocol versions
that have been negotiated; use storage potocol versions instead.

Tested-by: Alex Ng <alexng@microsoft.com>
Signed-off-by: Keith Mange <keith.mange@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/scsi/storvsc_drv.c |  109 +++++++++++++++++++++++++++++++++++---------
 1 files changed, 87 insertions(+), 22 deletions(-)

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 5f9d133..f29871e 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -56,14 +56,18 @@
  * V1 RC > 2008/1/31:  2.0
  * Win7: 4.2
  * Win8: 5.1
+ * Win8.1: 6.0
+ * Win10: 6.2
  */
 
 #define VMSTOR_PROTO_VERSION(MAJOR_, MINOR_)	((((MAJOR_) & 0xff) << 8) | \
 						(((MINOR_) & 0xff)))
 
+#define VMSTOR_PROTO_VERSION_WIN6	VMSTOR_PROTO_VERSION(2, 0)
 #define VMSTOR_PROTO_VERSION_WIN7	VMSTOR_PROTO_VERSION(4, 2)
 #define VMSTOR_PROTO_VERSION_WIN8	VMSTOR_PROTO_VERSION(5, 1)
-
+#define VMSTOR_PROTO_VERSION_WIN8_1	VMSTOR_PROTO_VERSION(6, 0)
+#define VMSTOR_PROTO_VERSION_WIN10	VMSTOR_PROTO_VERSION(6, 2)
 
 /*  Packet structure describing virtual storage requests. */
 enum vstor_packet_operation {
@@ -205,6 +209,46 @@ struct vmscsi_request {
 
 
 /*
+ * The list of storage protocols in order of preference.
+ */
+struct vmstor_protocol {
+	int protocol_version;
+	int sense_buffer_size;
+	int vmscsi_size_delta;
+};
+
+#define VMSTOR_NUM_PROTOCOLS    5
+
+const struct vmstor_protocol vmstor_protocols[VMSTOR_NUM_PROTOCOLS] = {
+	{
+		VMSTOR_PROTO_VERSION_WIN10,
+		POST_WIN7_STORVSC_SENSE_BUFFER_SIZE,
+		0
+	},
+	{
+		VMSTOR_PROTO_VERSION_WIN8_1,
+		POST_WIN7_STORVSC_SENSE_BUFFER_SIZE,
+		0
+	},
+	{
+		VMSTOR_PROTO_VERSION_WIN8,
+		POST_WIN7_STORVSC_SENSE_BUFFER_SIZE,
+		0
+	},
+	{
+		VMSTOR_PROTO_VERSION_WIN7,
+		PRE_WIN8_STORVSC_SENSE_BUFFER_SIZE,
+		sizeof(struct vmscsi_win8_extension),
+	},
+	{
+		VMSTOR_PROTO_VERSION_WIN6,
+		PRE_WIN8_STORVSC_SENSE_BUFFER_SIZE,
+		sizeof(struct vmscsi_win8_extension),
+	}
+};
+
+
+/*
  * This structure is sent during the intialization phase to get the different
  * properties of the channel.
  */
@@ -871,7 +915,7 @@ static int storvsc_channel_init(struct hv_device *device)
 	struct storvsc_device *stor_device;
 	struct storvsc_cmd_request *request;
 	struct vstor_packet *vstor_packet;
-	int ret, t;
+	int ret, t, i;
 	int max_chns;
 	bool process_sub_channels = false;
 
@@ -911,36 +955,59 @@ static int storvsc_channel_init(struct hv_device *device)
 		goto cleanup;
 
 
-	/* reuse the packet for version range supported */
-	memset(vstor_packet, 0, sizeof(struct vstor_packet));
-	vstor_packet->operation = VSTOR_OPERATION_QUERY_PROTOCOL_VERSION;
-	vstor_packet->flags = REQUEST_COMPLETION_FLAG;
+	for (i = 0; i < VMSTOR_NUM_PROTOCOLS; i++) {
+		/* reuse the packet for version range supported */
+		memset(vstor_packet, 0, sizeof(struct vstor_packet));
+		vstor_packet->operation =
+			VSTOR_OPERATION_QUERY_PROTOCOL_VERSION;
+		vstor_packet->flags = REQUEST_COMPLETION_FLAG;
 
-	vstor_packet->version.major_minor = vmstor_proto_version;
+		vstor_packet->version.major_minor =
+			vmstor_protocols[i].protocol_version;
 
-	/*
-	 * The revision number is only used in Windows; set it to 0.
-	 */
-	vstor_packet->version.revision = 0;
+		/*
+		 * The revision number is only used in Windows; set it to 0.
+		 */
+		vstor_packet->version.revision = 0;
 
-	ret = vmbus_sendpacket(device->channel, vstor_packet,
+		ret = vmbus_sendpacket(device->channel, vstor_packet,
 			       (sizeof(struct vstor_packet) -
 				vmscsi_size_delta),
 			       (unsigned long)request,
 			       VM_PKT_DATA_INBAND,
 			       VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
-	if (ret != 0)
-		goto cleanup;
+		if (ret != 0)
+			goto cleanup;
 
-	t = wait_for_completion_timeout(&request->wait_event, 5*HZ);
-	if (t == 0) {
-		ret = -ETIMEDOUT;
-		goto cleanup;
+		t = wait_for_completion_timeout(&request->wait_event, 5*HZ);
+		if (t == 0) {
+			ret = -ETIMEDOUT;
+			goto cleanup;
+		}
+
+		if (vstor_packet->operation != VSTOR_OPERATION_COMPLETE_IO) {
+			ret = -EINVAL;
+			goto cleanup;
+		}
+
+		if (vstor_packet->status == 0) {
+			vmstor_proto_version =
+				vmstor_protocols[i].protocol_version;
+
+			sense_buffer_size =
+				vmstor_protocols[i].sense_buffer_size;
+
+			vmscsi_size_delta =
+				vmstor_protocols[i].vmscsi_size_delta;
+
+			break;
+		}
 	}
 
-	if (vstor_packet->operation != VSTOR_OPERATION_COMPLETE_IO ||
-	    vstor_packet->status != 0)
+	if (vstor_packet->status != 0) {
+		ret = -EINVAL;
 		goto cleanup;
+	}
 
 
 	memset(vstor_packet, 0, sizeof(struct vstor_packet));
@@ -1745,14 +1812,12 @@ static int storvsc_probe(struct hv_device *device,
 	if (vmbus_proto_version < VERSION_WIN8) {
 		sense_buffer_size = PRE_WIN8_STORVSC_SENSE_BUFFER_SIZE;
 		vmscsi_size_delta = sizeof(struct vmscsi_win8_extension);
-		vmstor_proto_version = VMSTOR_PROTO_VERSION_WIN7;
 		max_luns_per_target = STORVSC_IDE_MAX_LUNS_PER_TARGET;
 		max_targets = STORVSC_IDE_MAX_TARGETS;
 		max_channels = STORVSC_IDE_MAX_CHANNELS;
 	} else {
 		sense_buffer_size = POST_WIN7_STORVSC_SENSE_BUFFER_SIZE;
 		vmscsi_size_delta = 0;
-		vmstor_proto_version = VMSTOR_PROTO_VERSION_WIN8;
 		max_luns_per_target = STORVSC_MAX_LUNS_PER_TARGET;
 		max_targets = STORVSC_MAX_TARGETS;
 		max_channels = STORVSC_MAX_CHANNELS;
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH RESEND V2 4/7] scsi: storvsc: use correct defaults for values determined by protocol negotiation
  2015-08-13 15:43 ` [PATCH RESEND V2 1/7] scsi: storvsc: Rather than look for sets of specific protocol versions, make decisions based on ranges K. Y. Srinivasan
  2015-08-13 15:43   ` [PATCH RESEND V2 2/7] scsi: storvsc: Use a single value to track protocol versions K. Y. Srinivasan
  2015-08-13 15:43   ` [PATCH RESEND V2 3/7] scsi: storvsc: Untangle the storage protocol negotiation from the vmbus protocol negotiation K. Y. Srinivasan
@ 2015-08-13 15:43   ` K. Y. Srinivasan
  2015-08-13 15:43   ` [PATCH RESEND V2 5/7] scsi: storvsc: use storage protocol version to determine storage capabilities K. Y. Srinivasan
                     ` (2 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: K. Y. Srinivasan @ 2015-08-13 15:43 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, ohering, jbottomley, hch,
	linux-scsi, apw, vkuznets, jasowang
  Cc: Keith Mange, K. Y. Srinivasan

From: Keith Mange <keith.mange@microsoft.com>

Use correct defaults for values determined by protocol negotiation,
instead of resetting them with every scsi controller.

Tested-by: Alex Ng <alexng@microsoft.com>
Signed-off-by: Keith Mange <keith.mange@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/scsi/storvsc_drv.c |   33 +++++++++++++++++++--------------
 1 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index f29871e..6f38cdf 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -151,19 +151,17 @@ struct hv_fc_wwn_packet {
 
 /*
  * Sense buffer size changed in win8; have a run-time
- * variable to track the size we should use.
+ * variable to track the size we should use.  This value will
+ * likely change during protocol negotiation but it is valid
+ * to start by assuming pre-Win8.
  */
-static int sense_buffer_size;
+static int sense_buffer_size = PRE_WIN8_STORVSC_SENSE_BUFFER_SIZE;
 
 /*
- * The size of the vmscsi_request has changed in win8. The
- * additional size is because of new elements added to the
- * structure. These elements are valid only when we are talking
- * to a win8 host.
- * Track the correction to size we need to apply.
- */
-
-static int vmscsi_size_delta;
+ * The storage protocol version is determined during the
+ * initial exchange with the host.  It will indicate which
+ * storage functionality is available in the host.
+*/
 static int vmstor_proto_version;
 
 struct vmscsi_win8_extension {
@@ -209,6 +207,17 @@ struct vmscsi_request {
 
 
 /*
+ * The size of the vmscsi_request has changed in win8. The
+ * additional size is because of new elements added to the
+ * structure. These elements are valid only when we are talking
+ * to a win8 host.
+ * Track the correction to size we need to apply. This value
+ * will likely change during protocol negotiation but it is
+ * valid to start by assuming pre-Win8.
+ */
+static int vmscsi_size_delta = sizeof(struct vmscsi_win8_extension);
+
+/*
  * The list of storage protocols in order of preference.
  */
 struct vmstor_protocol {
@@ -1810,14 +1819,10 @@ static int storvsc_probe(struct hv_device *device,
 	 */
 
 	if (vmbus_proto_version < VERSION_WIN8) {
-		sense_buffer_size = PRE_WIN8_STORVSC_SENSE_BUFFER_SIZE;
-		vmscsi_size_delta = sizeof(struct vmscsi_win8_extension);
 		max_luns_per_target = STORVSC_IDE_MAX_LUNS_PER_TARGET;
 		max_targets = STORVSC_IDE_MAX_TARGETS;
 		max_channels = STORVSC_IDE_MAX_CHANNELS;
 	} else {
-		sense_buffer_size = POST_WIN7_STORVSC_SENSE_BUFFER_SIZE;
-		vmscsi_size_delta = 0;
 		max_luns_per_target = STORVSC_MAX_LUNS_PER_TARGET;
 		max_targets = STORVSC_MAX_TARGETS;
 		max_channels = STORVSC_MAX_CHANNELS;
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH RESEND V2 5/7] scsi: storvsc: use storage protocol version to determine storage capabilities
  2015-08-13 15:43 ` [PATCH RESEND V2 1/7] scsi: storvsc: Rather than look for sets of specific protocol versions, make decisions based on ranges K. Y. Srinivasan
                     ` (2 preceding siblings ...)
  2015-08-13 15:43   ` [PATCH RESEND V2 4/7] scsi: storvsc: use correct defaults for values determined by " K. Y. Srinivasan
@ 2015-08-13 15:43   ` K. Y. Srinivasan
  2015-08-13 15:43   ` [PATCH RESEND V2 6/7] scsi: storvsc: Allow write_same when host is windows 10 K. Y. Srinivasan
  2015-08-13 15:43   ` [PATCH RESEND V2 7/7] scsi: storvsc: Set the error code correctly in failure conditions K. Y. Srinivasan
  5 siblings, 0 replies; 13+ messages in thread
From: K. Y. Srinivasan @ 2015-08-13 15:43 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, ohering, jbottomley, hch,
	linux-scsi, apw, vkuznets, jasowang
  Cc: Keith Mange, K. Y. Srinivasan

From: Keith Mange <keith.mange@microsoft.com>

Use storage protocol version instead of vmbus protocol
version when determining storage capabilities.

Tested-by: Alex Ng <alexng@microsoft.com>
Signed-off-by: Keith Mange <keith.mange@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/scsi/storvsc_drv.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 6f38cdf..58fa47a 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -1049,7 +1049,7 @@ static int storvsc_channel_init(struct hv_device *device)
 	 * support multi-channel.
 	 */
 	max_chns = vstor_packet->storage_channel_properties.max_channel_cnt;
-	if (vmbus_proto_version >= VERSION_WIN8) {
+	if (vmstor_proto_version >= VMSTOR_PROTO_VERSION_WIN8) {
 		if (vstor_packet->storage_channel_properties.flags &
 		    STORAGE_CHANNEL_SUPPORTS_MULTI_CHANNEL)
 			process_sub_channels = true;
@@ -1491,9 +1491,9 @@ static int storvsc_device_configure(struct scsi_device *sdevice)
 	 * if the device is a MSFT virtual device.
 	 */
 	if (!strncmp(sdevice->vendor, "Msft", 4)) {
-		switch (vmbus_proto_version) {
-		case VERSION_WIN8:
-		case VERSION_WIN8_1:
+		switch (vmstor_proto_version) {
+		case VMSTOR_PROTO_VERSION_WIN8:
+		case VMSTOR_PROTO_VERSION_WIN8_1:
 			sdevice->scsi_level = SCSI_SPC_3;
 			break;
 		}
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH RESEND V2 6/7] scsi: storvsc: Allow write_same when host is windows 10
  2015-08-13 15:43 ` [PATCH RESEND V2 1/7] scsi: storvsc: Rather than look for sets of specific protocol versions, make decisions based on ranges K. Y. Srinivasan
                     ` (3 preceding siblings ...)
  2015-08-13 15:43   ` [PATCH RESEND V2 5/7] scsi: storvsc: use storage protocol version to determine storage capabilities K. Y. Srinivasan
@ 2015-08-13 15:43   ` K. Y. Srinivasan
  2015-08-13 15:43   ` [PATCH RESEND V2 7/7] scsi: storvsc: Set the error code correctly in failure conditions K. Y. Srinivasan
  5 siblings, 0 replies; 13+ messages in thread
From: K. Y. Srinivasan @ 2015-08-13 15:43 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, ohering, jbottomley, hch,
	linux-scsi, apw, vkuznets, jasowang
  Cc: Keith Mange, K. Y. Srinivasan

From: Keith Mange <keith.mange@microsoft.com>

Allow WRITE_SAME for Windows10 and above hosts.

Tested-by: Alex Ng <alexng@microsoft.com>
Signed-off-by: Keith Mange <keith.mange@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/scsi/storvsc_drv.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 58fa47a..021cbdf 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -1488,7 +1488,8 @@ static int storvsc_device_configure(struct scsi_device *sdevice)
 
 	/*
 	 * If the host is WIN8 or WIN8 R2, claim conformance to SPC-3
-	 * if the device is a MSFT virtual device.
+	 * if the device is a MSFT virtual device.  If the host is
+	 * WIN10 or newer, allow write_same.
 	 */
 	if (!strncmp(sdevice->vendor, "Msft", 4)) {
 		switch (vmstor_proto_version) {
@@ -1497,6 +1498,9 @@ static int storvsc_device_configure(struct scsi_device *sdevice)
 			sdevice->scsi_level = SCSI_SPC_3;
 			break;
 		}
+
+		if (vmstor_proto_version >= VMSTOR_PROTO_VERSION_WIN10)
+			sdevice->no_write_same = 0;
 	}
 
 	return 0;
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH RESEND V2 7/7] scsi: storvsc: Set the error code correctly in failure conditions
  2015-08-13 15:43 ` [PATCH RESEND V2 1/7] scsi: storvsc: Rather than look for sets of specific protocol versions, make decisions based on ranges K. Y. Srinivasan
                     ` (4 preceding siblings ...)
  2015-08-13 15:43   ` [PATCH RESEND V2 6/7] scsi: storvsc: Allow write_same when host is windows 10 K. Y. Srinivasan
@ 2015-08-13 15:43   ` K. Y. Srinivasan
  5 siblings, 0 replies; 13+ messages in thread
From: K. Y. Srinivasan @ 2015-08-13 15:43 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, ohering, jbottomley, hch,
	linux-scsi, apw, vkuznets, jasowang
  Cc: K. Y. Srinivasan

In the function storvsc_channel_init(), error code was not getting
set correctly in some of the failure cases. Fix this issue.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
---
 drivers/scsi/storvsc_drv.c |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 021cbdf..aac8297 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -960,8 +960,10 @@ static int storvsc_channel_init(struct hv_device *device)
 	}
 
 	if (vstor_packet->operation != VSTOR_OPERATION_COMPLETE_IO ||
-	    vstor_packet->status != 0)
+	    vstor_packet->status != 0) {
+		ret = -EINVAL;
 		goto cleanup;
+	}
 
 
 	for (i = 0; i < VMSTOR_NUM_PROTOCOLS; i++) {
@@ -1040,8 +1042,10 @@ static int storvsc_channel_init(struct hv_device *device)
 	}
 
 	if (vstor_packet->operation != VSTOR_OPERATION_COMPLETE_IO ||
-	    vstor_packet->status != 0)
+	    vstor_packet->status != 0) {
+		ret = -EINVAL;
 		goto cleanup;
+	}
 
 	/*
 	 * Check to see if multi-channel support is there.
@@ -1078,8 +1082,10 @@ static int storvsc_channel_init(struct hv_device *device)
 	}
 
 	if (vstor_packet->operation != VSTOR_OPERATION_COMPLETE_IO ||
-	    vstor_packet->status != 0)
+	    vstor_packet->status != 0) {
+		ret = -EINVAL;
 		goto cleanup;
+	}
 
 	if (process_sub_channels)
 		handle_multichannel_storage(device, max_chns);
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH RESEND V2 3/7] scsi: storvsc: Untangle the storage protocol negotiation from the vmbus protocol negotiation.
  2015-08-13 15:18       ` KY Srinivasan
@ 2015-08-14  6:45         ` Johannes Thumshirn
  2015-08-14 19:26           ` KY Srinivasan
  0 siblings, 1 reply; 13+ messages in thread
From: Johannes Thumshirn @ 2015-08-14  6:45 UTC (permalink / raw)
  To: KY Srinivasan
  Cc: Keith Mange, gregkh, linux-kernel, devel, ohering, jbottomley,
	hch, linux-scsi, apw, vkuznets, jasowang

KY Srinivasan <kys@microsoft.com> writes:

>> -----Original Message-----
>> From: Johannes Thumshirn [mailto:jthumshirn@suse.de]
>> Sent: Thursday, August 13, 2015 7:34 AM
>> To: KY Srinivasan <kys@microsoft.com>; Keith Mange
>> <Keith.Mange@microsoft.com>
>> Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
>> devel@linuxdriverproject.org; ohering@suse.com;
>> jbottomley@parallels.com; hch@infradead.org; linux-scsi@vger.kernel.org;
>> apw@canonical.com; vkuznets@redhat.com; jasowang@redhat.com
>> Subject: Re: [PATCH RESEND V2 3/7] scsi: storvsc: Untangle the storage
>> protocol negotiation from the vmbus protocol negotiation.
>> 
>> "K. Y. Srinivasan" <kys@microsoft.com> writes:
>> 
>> > From: Keith Mange <keith.mange@microsoft.com>
>> >
>> > Currently we are making decisions based on vmbus protocol versions
>> > that have been negotiated; use storage potocol versions instead.
>> >
>> > Tested-by: Alex Ng <alexng@microsoft.com>
>> > Signed-off-by: Keith Mange <keith.mange@microsoft.com>
>> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
>> > ---
>> >  drivers/scsi/storvsc_drv.c |  109
>> +++++++++++++++++++++++++++++++++++---------
>> >  1 files changed, 87 insertions(+), 22 deletions(-)
>> >
>> > diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
>> > index 5f9d133..f29871e 100644
>> > --- a/drivers/scsi/storvsc_drv.c
>> > +++ b/drivers/scsi/storvsc_drv.c
>> > @@ -56,14 +56,18 @@
>> >   * V1 RC > 2008/1/31:  2.0
>> >   * Win7: 4.2
>> >   * Win8: 5.1
>> > + * Win8.1: 6.0
>> > + * Win10: 6.2
>> >   */
>> >
>> >  #define VMSTOR_PROTO_VERSION(MAJOR_, MINOR_)	((((MAJOR_)
>> & 0xff) << 8) | \
>> >  						(((MINOR_) & 0xff)))
>> >
>> > +#define VMSTOR_PROTO_VERSION_WIN6
>> 	VMSTOR_PROTO_VERSION(2, 0)
>> >  #define VMSTOR_PROTO_VERSION_WIN7
>> 	VMSTOR_PROTO_VERSION(4, 2)
>> >  #define VMSTOR_PROTO_VERSION_WIN8
>> 	VMSTOR_PROTO_VERSION(5, 1)
>> > -
>> > +#define VMSTOR_PROTO_VERSION_WIN8_1
>> 	VMSTOR_PROTO_VERSION(6, 0)
>> > +#define VMSTOR_PROTO_VERSION_WIN10
>> 	VMSTOR_PROTO_VERSION(6, 2)
>> >
>> >  /*  Packet structure describing virtual storage requests. */
>> >  enum vstor_packet_operation {
>> > @@ -205,6 +209,46 @@ struct vmscsi_request {
>> >
>> >
>> >  /*
>> > + * The list of storage protocols in order of preference.
>> > + */
>> > +struct vmstor_protocol {
>> > +	int protocol_version;
>> > +	int sense_buffer_size;
>> > +	int vmscsi_size_delta;
>> > +};
>> > +
>> > +#define VMSTOR_NUM_PROTOCOLS    5
>> 
>> can't you just use ARRAY_SIZE() here, so you don't have to touch the
>> constant every time a new protocol is appended to the list?
>
> Certainly. These patches have been floating around for more than a month now and if it is ok
> with you, I will submit a patch on top of this current series to address the concern you have raised.
> James, please let me know.
>

No objections from my side, but it's up to James to decide what and when
he picks up patches.

Thanks,
        Johannes

-- 
Johannes Thumshirn                                           Storage
jthumshirn@suse.de                                 +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600  D0D0 0393 969D 2D76 0850

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH RESEND V2 3/7] scsi: storvsc: Untangle the storage protocol negotiation from the vmbus protocol negotiation.
  2015-08-14  6:45         ` Johannes Thumshirn
@ 2015-08-14 19:26           ` KY Srinivasan
  0 siblings, 0 replies; 13+ messages in thread
From: KY Srinivasan @ 2015-08-14 19:26 UTC (permalink / raw)
  To: Johannes Thumshirn
  Cc: Keith Mange, gregkh, linux-kernel, devel, ohering, jbottomley,
	hch, linux-scsi, apw, vkuznets, jasowang

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 4000 bytes --]



> -----Original Message-----
> From: Johannes Thumshirn [mailto:jthumshirn@suse.de]
> Sent: Thursday, August 13, 2015 11:46 PM
> To: KY Srinivasan <kys@microsoft.com>
> Cc: Keith Mange <Keith.Mange@microsoft.com>;
> gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; ohering@suse.com;
> jbottomley@parallels.com; hch@infradead.org; linux-scsi@vger.kernel.org;
> apw@canonical.com; vkuznets@redhat.com; jasowang@redhat.com
> Subject: Re: [PATCH RESEND V2 3/7] scsi: storvsc: Untangle the storage
> protocol negotiation from the vmbus protocol negotiation.
> 
> KY Srinivasan <kys@microsoft.com> writes:
> 
> >> -----Original Message-----
> >> From: Johannes Thumshirn [mailto:jthumshirn@suse.de]
> >> Sent: Thursday, August 13, 2015 7:34 AM
> >> To: KY Srinivasan <kys@microsoft.com>; Keith Mange
> >> <Keith.Mange@microsoft.com>
> >> Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> >> devel@linuxdriverproject.org; ohering@suse.com;
> >> jbottomley@parallels.com; hch@infradead.org; linux-
> scsi@vger.kernel.org;
> >> apw@canonical.com; vkuznets@redhat.com; jasowang@redhat.com
> >> Subject: Re: [PATCH RESEND V2 3/7] scsi: storvsc: Untangle the storage
> >> protocol negotiation from the vmbus protocol negotiation.
> >>
> >> "K. Y. Srinivasan" <kys@microsoft.com> writes:
> >>
> >> > From: Keith Mange <keith.mange@microsoft.com>
> >> >
> >> > Currently we are making decisions based on vmbus protocol versions
> >> > that have been negotiated; use storage potocol versions instead.
> >> >
> >> > Tested-by: Alex Ng <alexng@microsoft.com>
> >> > Signed-off-by: Keith Mange <keith.mange@microsoft.com>
> >> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> >> > ---
> >> >  drivers/scsi/storvsc_drv.c |  109
> >> +++++++++++++++++++++++++++++++++++---------
> >> >  1 files changed, 87 insertions(+), 22 deletions(-)
> >> >
> >> > diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> >> > index 5f9d133..f29871e 100644
> >> > --- a/drivers/scsi/storvsc_drv.c
> >> > +++ b/drivers/scsi/storvsc_drv.c
> >> > @@ -56,14 +56,18 @@
> >> >   * V1 RC > 2008/1/31:  2.0
> >> >   * Win7: 4.2
> >> >   * Win8: 5.1
> >> > + * Win8.1: 6.0
> >> > + * Win10: 6.2
> >> >   */
> >> >
> >> >  #define VMSTOR_PROTO_VERSION(MAJOR_, MINOR_)	((((MAJOR_)
> >> & 0xff) << 8) | \
> >> >  						(((MINOR_) & 0xff)))
> >> >
> >> > +#define VMSTOR_PROTO_VERSION_WIN6
> >> 	VMSTOR_PROTO_VERSION(2, 0)
> >> >  #define VMSTOR_PROTO_VERSION_WIN7
> >> 	VMSTOR_PROTO_VERSION(4, 2)
> >> >  #define VMSTOR_PROTO_VERSION_WIN8
> >> 	VMSTOR_PROTO_VERSION(5, 1)
> >> > -
> >> > +#define VMSTOR_PROTO_VERSION_WIN8_1
> >> 	VMSTOR_PROTO_VERSION(6, 0)
> >> > +#define VMSTOR_PROTO_VERSION_WIN10
> >> 	VMSTOR_PROTO_VERSION(6, 2)
> >> >
> >> >  /*  Packet structure describing virtual storage requests. */
> >> >  enum vstor_packet_operation {
> >> > @@ -205,6 +209,46 @@ struct vmscsi_request {
> >> >
> >> >
> >> >  /*
> >> > + * The list of storage protocols in order of preference.
> >> > + */
> >> > +struct vmstor_protocol {
> >> > +	int protocol_version;
> >> > +	int sense_buffer_size;
> >> > +	int vmscsi_size_delta;
> >> > +};
> >> > +
> >> > +#define VMSTOR_NUM_PROTOCOLS    5
> >>
> >> can't you just use ARRAY_SIZE() here, so you don't have to touch the
> >> constant every time a new protocol is appended to the list?
> >
> > Certainly. These patches have been floating around for more than a month
> now and if it is ok
> > with you, I will submit a patch on top of this current series to address the
> concern you have raised.
> > James, please let me know.
> >
> 
> No objections from my side, but it's up to James to decide what and when
> he picks up patches.

James,

I have sent a separate patch on top of the set I sent yesterday that addresses Johannes' comments.

Regards,

K. Y
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH RESEND V2 3/7] scsi: storvsc: Untangle the storage protocol negotiation from the vmbus protocol negotiation.
  2015-08-13 15:43   ` [PATCH RESEND V2 3/7] scsi: storvsc: Untangle the storage protocol negotiation from the vmbus protocol negotiation K. Y. Srinivasan
  2015-08-13 14:33     ` Johannes Thumshirn
@ 2015-08-27  1:40     ` James Bottomley
  1 sibling, 0 replies; 13+ messages in thread
From: James Bottomley @ 2015-08-27  1:40 UTC (permalink / raw)
  To: K. Y. Srinivasan
  Cc: gregkh, linux-kernel, devel, ohering, jbottomley, hch,
	linux-scsi, apw, vkuznets, jasowang, Keith Mange

On Thu, 2015-08-13 at 08:43 -0700, K. Y. Srinivasan wrote:
> From: Keith Mange <keith.mange@microsoft.com>
> 
> Currently we are making decisions based on vmbus protocol versions
> that have been negotiated; use storage potocol versions instead.
> 
> Tested-by: Alex Ng <alexng@microsoft.com>
> Signed-off-by: Keith Mange <keith.mange@microsoft.com>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> ---
>  drivers/scsi/storvsc_drv.c |  109 +++++++++++++++++++++++++++++++++++---------
>  1 files changed, 87 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> index 5f9d133..f29871e 100644
> --- a/drivers/scsi/storvsc_drv.c
> +++ b/drivers/scsi/storvsc_drv.c
> @@ -56,14 +56,18 @@
>   * V1 RC > 2008/1/31:  2.0
>   * Win7: 4.2
>   * Win8: 5.1
> + * Win8.1: 6.0
> + * Win10: 6.2
>   */
>  
>  #define VMSTOR_PROTO_VERSION(MAJOR_, MINOR_)	((((MAJOR_) & 0xff) << 8) | \
>  						(((MINOR_) & 0xff)))
>  
> +#define VMSTOR_PROTO_VERSION_WIN6	VMSTOR_PROTO_VERSION(2, 0)
>  #define VMSTOR_PROTO_VERSION_WIN7	VMSTOR_PROTO_VERSION(4, 2)
>  #define VMSTOR_PROTO_VERSION_WIN8	VMSTOR_PROTO_VERSION(5, 1)
> -
> +#define VMSTOR_PROTO_VERSION_WIN8_1	VMSTOR_PROTO_VERSION(6, 0)
> +#define VMSTOR_PROTO_VERSION_WIN10	VMSTOR_PROTO_VERSION(6, 2)
>  
>  /*  Packet structure describing virtual storage requests. */
>  enum vstor_packet_operation {
> @@ -205,6 +209,46 @@ struct vmscsi_request {
>  
> 
>  /*
> + * The list of storage protocols in order of preference.
> + */
> +struct vmstor_protocol {
> +	int protocol_version;
> +	int sense_buffer_size;
> +	int vmscsi_size_delta;
> +};
> +
> +#define VMSTOR_NUM_PROTOCOLS    5
> +
> +const struct vmstor_protocol vmstor_protocols[VMSTOR_NUM_PROTOCOLS] = {

Sparse doesn't like this not being static:

  CHECK   drivers/scsi/storvsc_drv.c
drivers/scsi/storvsc_drv.c:221:30: warning: symbol 'vmstor_protocols'
was not declared. Should it be static?
 
I fixed it up.

James



^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2015-08-27  1:40 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-08-13 15:43 [PATCH RESEND V2 0/7] scsi: storvsc: Some miscellaneous cleanup K. Y. Srinivasan
2015-08-13 15:43 ` [PATCH RESEND V2 1/7] scsi: storvsc: Rather than look for sets of specific protocol versions, make decisions based on ranges K. Y. Srinivasan
2015-08-13 15:43   ` [PATCH RESEND V2 2/7] scsi: storvsc: Use a single value to track protocol versions K. Y. Srinivasan
2015-08-13 15:43   ` [PATCH RESEND V2 3/7] scsi: storvsc: Untangle the storage protocol negotiation from the vmbus protocol negotiation K. Y. Srinivasan
2015-08-13 14:33     ` Johannes Thumshirn
2015-08-13 15:18       ` KY Srinivasan
2015-08-14  6:45         ` Johannes Thumshirn
2015-08-14 19:26           ` KY Srinivasan
2015-08-27  1:40     ` James Bottomley
2015-08-13 15:43   ` [PATCH RESEND V2 4/7] scsi: storvsc: use correct defaults for values determined by " K. Y. Srinivasan
2015-08-13 15:43   ` [PATCH RESEND V2 5/7] scsi: storvsc: use storage protocol version to determine storage capabilities K. Y. Srinivasan
2015-08-13 15:43   ` [PATCH RESEND V2 6/7] scsi: storvsc: Allow write_same when host is windows 10 K. Y. Srinivasan
2015-08-13 15:43   ` [PATCH RESEND V2 7/7] scsi: storvsc: Set the error code correctly in failure conditions K. Y. Srinivasan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).