public inbox for linux-scsi@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] scsi: hyper-v storvsc changes by Ubuntu
@ 2014-05-16 15:39 Ian Abbott
  2014-05-16 15:39 ` [PATCH 1/2] scsi: hyper-v storvsc switch up to SPC-3 Ian Abbott
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Ian Abbott @ 2014-05-16 15:39 UTC (permalink / raw)
  To: linux-scsi, devel
  Cc: Tim Gardner, Haiyang Zhang, James E.J. Bottomley, Ian Abbott,
	Andy Whitcroft

These changes to the Microsoft Hyper-V storage driver in Ubuntu Saucy's
3.13 kernel look useful for the mainline kernel, especially as they
enable 'TRIM' support.

Andy Whitcroft (2):
  scsi: hyper-v storvsc switch up to SPC-3
  scsi: hyper-v storvsc -- host takes MAINTENANCE_IN commands badly --
    elide them

 drivers/scsi/storvsc_drv.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

-- 
1.9.2

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

* [PATCH 1/2] scsi: hyper-v storvsc switch up to SPC-3
  2014-05-16 15:39 [PATCH 0/2] scsi: hyper-v storvsc changes by Ubuntu Ian Abbott
@ 2014-05-16 15:39 ` Ian Abbott
  2014-05-16 17:11   ` Dan Carpenter
  2014-05-16 17:14   ` James Bottomley
  2014-05-16 15:39 ` [PATCH 2/2] scsi: hyper-v storvsc -- host takes MAINTENANCE_IN commands badly -- elide them Ian Abbott
  2014-05-21 14:24 ` [PATCH 0/2] scsi: hyper-v storvsc changes by Ubuntu Andy Whitcroft
  2 siblings, 2 replies; 16+ messages in thread
From: Ian Abbott @ 2014-05-16 15:39 UTC (permalink / raw)
  To: linux-scsi, devel
  Cc: Andy Whitcroft, Haiyang Zhang, Tim Gardner, James E.J. Bottomley

From: Andy Whitcroft <apw@canonical.com>

Suggested-by: James Bottomley <James.Bottomley@HansenPartnership.com>
Signed-off-by: Andy Whitcroft <apw@canonical.com>
---
Original patch title changed slightly by Ian Abbott <abbotti@mev.co.uk>
due to typo.
---
 drivers/scsi/storvsc_drv.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 9969fa1..3903c8a 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -1441,6 +1441,14 @@ static int storvsc_device_configure(struct scsi_device *sdevice)
 
 	sdevice->no_write_same = 1;
 
+	/*
+	 * hyper-v lies about its capabilities indicating it is only SPC-2
+	 * compliant, but actually implements the core SPC-3 features.
+	 * If we pretend to be SPC-3, we send RC16 which activates trim and
+	 * will query the appropriate VPD pages to enable trim.
+	 */
+	sdevice->scsi_level = SCSI_SPC_3;
+
 	return 0;
 }
 
-- 
1.9.2

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

* [PATCH 2/2] scsi: hyper-v storvsc -- host takes MAINTENANCE_IN commands badly -- elide them
  2014-05-16 15:39 [PATCH 0/2] scsi: hyper-v storvsc changes by Ubuntu Ian Abbott
  2014-05-16 15:39 ` [PATCH 1/2] scsi: hyper-v storvsc switch up to SPC-3 Ian Abbott
@ 2014-05-16 15:39 ` Ian Abbott
  2014-05-16 17:42   ` James Bottomley
  2014-05-21 14:24 ` [PATCH 0/2] scsi: hyper-v storvsc changes by Ubuntu Andy Whitcroft
  2 siblings, 1 reply; 16+ messages in thread
From: Ian Abbott @ 2014-05-16 15:39 UTC (permalink / raw)
  To: linux-scsi, devel
  Cc: Andy Whitcroft, K. Y. Srinivasan, Haiyang Zhang,
	James E.J. Bottomley, Tim Gardner

From: Andy Whitcroft <apw@canonical.com>

BugLink: http://bugs.launchpad.net/bugs/1234417
Signed-off-by: Andy Whitcroft <apw@canonical.com>
Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
---
Original patch title was "UBUNTU: SAUCE: storvsc -- host takes
MAINTENANCE_IN commands badly elide them".  Changed by Ian Abbott
<abbotti@mev.co.uk>
---
 drivers/scsi/storvsc_drv.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 3903c8a..c1eddd1 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -1539,6 +1539,8 @@ static bool storvsc_scsi_cmd_ok(struct scsi_cmnd *scmnd)
 	 * this. So, don't send it.
 	 */
 	case SET_WINDOW:
+	/* the host does not handle MAINTENANCE_IN preventing boot.*/
+	case MAINTENANCE_IN:
 		scmnd->result = ILLEGAL_REQUEST << 16;
 		allowed = false;
 		break;
-- 
1.9.2


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

* Re: [PATCH 1/2] scsi: hyper-v storvsc switch up to SPC-3
  2014-05-16 15:39 ` [PATCH 1/2] scsi: hyper-v storvsc switch up to SPC-3 Ian Abbott
@ 2014-05-16 17:11   ` Dan Carpenter
  2014-05-16 17:14   ` James Bottomley
  1 sibling, 0 replies; 16+ messages in thread
From: Dan Carpenter @ 2014-05-16 17:11 UTC (permalink / raw)
  To: Ian Abbott
  Cc: linux-scsi, Haiyang Zhang, James E.J. Bottomley, Andy Whitcroft,
	devel, Tim Gardner

On Fri, May 16, 2014 at 04:39:07PM +0100, Ian Abbott wrote:
> From: Andy Whitcroft <apw@canonical.com>
> 
> Suggested-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> Signed-off-by: Andy Whitcroft <apw@canonical.com>

You should be signing these as well.

regards,
dan carpenter

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

* Re: [PATCH 1/2] scsi: hyper-v storvsc switch up to SPC-3
  2014-05-16 15:39 ` [PATCH 1/2] scsi: hyper-v storvsc switch up to SPC-3 Ian Abbott
  2014-05-16 17:11   ` Dan Carpenter
@ 2014-05-16 17:14   ` James Bottomley
  2014-05-16 17:39     ` Ian Abbott
  1 sibling, 1 reply; 16+ messages in thread
From: James Bottomley @ 2014-05-16 17:14 UTC (permalink / raw)
  To: Ian Abbott; +Cc: Tim Gardner, linux-scsi, Haiyang Zhang, Andy Whitcroft, devel

On Fri, 2014-05-16 at 16:39 +0100, Ian Abbott wrote:
> From: Andy Whitcroft <apw@canonical.com>
> 
> Suggested-by: James Bottomley <James.Bottomley@HansenPartnership.com>

That is my patch, isn't it, just with a slightly modified comment:

http://marc.info/?l=linux-scsi&m=137908428211951

Andy promised to go off and test it and that's where the thread ended. I
take it the results of the testing was positive?  I was expecting him to
report back on that so KY could ack the patch.

James

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

* Re: [PATCH 1/2] scsi: hyper-v storvsc switch up to SPC-3
  2014-05-16 17:14   ` James Bottomley
@ 2014-05-16 17:39     ` Ian Abbott
  2014-05-16 17:58       ` James Bottomley
  0 siblings, 1 reply; 16+ messages in thread
From: Ian Abbott @ 2014-05-16 17:39 UTC (permalink / raw)
  To: James Bottomley
  Cc: Tim Gardner, linux-scsi, Haiyang Zhang, Andy Whitcroft, devel

On 2014-05-16 18:14, James Bottomley wrote:
> On Fri, 2014-05-16 at 16:39 +0100, Ian Abbott wrote:
>> From: Andy Whitcroft <apw@canonical.com>
>>
>> Suggested-by: James Bottomley <James.Bottomley@HansenPartnership.com>
>
> That is my patch, isn't it, just with a slightly modified comment:
>
> http://marc.info/?l=linux-scsi&m=137908428211951

I believe so, yes.  Looking at Ubuntu's kernel repository, Andy reverted 
his original 4 patches and applied your patch instead.  I'm not sure 
about the other patch (PATCH 2/2) that disables the MAINTENANCE_IN 
command.  Perhaps that was needed as a consequence of claiming to be 
SCSI level SPC-3?

> Andy promised to go off and test it and that's where the thread ended. I
> take it the results of the testing was positive?  I was expecting him to
> report back on that so KY could ack the patch.
>
> James

The patch seems to be in Ubuntu Saucy's 3.11 kernel version 3.11.0-12.18 
onwards - see 
http://kernel.ubuntu.com/git?p=ubuntu/ubuntu-saucy.git;a=log;h=refs/tags/Ubuntu-3.11.0-12.18 
for the logs.

Would you like me to resubmit the patch with you as the author?  There 
isn't a "Signed-off-by:" line for you on this patch at the moment.  Is 
it okay for me to add one?

-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@mev.co.uk>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-

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

* Re: [PATCH 2/2] scsi: hyper-v storvsc -- host takes MAINTENANCE_IN commands badly -- elide them
  2014-05-16 15:39 ` [PATCH 2/2] scsi: hyper-v storvsc -- host takes MAINTENANCE_IN commands badly -- elide them Ian Abbott
@ 2014-05-16 17:42   ` James Bottomley
  0 siblings, 0 replies; 16+ messages in thread
From: James Bottomley @ 2014-05-16 17:42 UTC (permalink / raw)
  To: Ian Abbott; +Cc: Tim Gardner, linux-scsi, Haiyang Zhang, Andy Whitcroft, devel

On Fri, 2014-05-16 at 16:39 +0100, Ian Abbott wrote:
> From: Andy Whitcroft <apw@canonical.com>
> 
> BugLink: http://bugs.launchpad.net/bugs/1234417

This is a pretty low quality bug report; where's the analysis that
should be in your patch?

The problem is, is it not, that when you turn on trim we now also probe
for write same using report opcodes (which is a SAI MAINTENANCE IN
commad) and that's causing the boot hang.

Firstly, that might be an indicator that the SPC3 route isn't a good one
because there might be more hidden nasties like this ... and that goes
back to how much testing have you done with the patch I suggested?

If it is only a WRITE SAME problem, then the preferred route would be
simply to set no_write_same in the template.

James

> Signed-off-by: Andy Whitcroft <apw@canonical.com>
> Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
> ---
> Original patch title was "UBUNTU: SAUCE: storvsc -- host takes
> MAINTENANCE_IN commands badly elide them".  Changed by Ian Abbott
> <abbotti@mev.co.uk>
> ---
>  drivers/scsi/storvsc_drv.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> index 3903c8a..c1eddd1 100644
> --- a/drivers/scsi/storvsc_drv.c
> +++ b/drivers/scsi/storvsc_drv.c
> @@ -1539,6 +1539,8 @@ static bool storvsc_scsi_cmd_ok(struct scsi_cmnd *scmnd)
>  	 * this. So, don't send it.
>  	 */
>  	case SET_WINDOW:
> +	/* the host does not handle MAINTENANCE_IN preventing boot.*/
> +	case MAINTENANCE_IN:
>  		scmnd->result = ILLEGAL_REQUEST << 16;
>  		allowed = false;
>  		break;

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

* Re: [PATCH 1/2] scsi: hyper-v storvsc switch up to SPC-3
  2014-05-16 17:39     ` Ian Abbott
@ 2014-05-16 17:58       ` James Bottomley
  2014-05-16 18:18         ` Ian Abbott
  0 siblings, 1 reply; 16+ messages in thread
From: James Bottomley @ 2014-05-16 17:58 UTC (permalink / raw)
  To: Ian Abbott
  Cc: linux-scsi, devel, Andy Whitcroft, K. Y. Srinivasan,
	Haiyang Zhang, Tim Gardner

On Fri, 2014-05-16 at 18:39 +0100, Ian Abbott wrote:
> On 2014-05-16 18:14, James Bottomley wrote:
> > On Fri, 2014-05-16 at 16:39 +0100, Ian Abbott wrote:
> >> From: Andy Whitcroft <apw@canonical.com>
> >>
> >> Suggested-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> >
> > That is my patch, isn't it, just with a slightly modified comment:
> >
> > http://marc.info/?l=linux-scsi&m=137908428211951
> 
> I believe so, yes.  Looking at Ubuntu's kernel repository, Andy reverted 
> his original 4 patches and applied your patch instead.  I'm not sure 
> about the other patch (PATCH 2/2) that disables the MAINTENANCE_IN 
> command.  Perhaps that was needed as a consequence of claiming to be 
> SCSI level SPC-3?

Yes, see other email.

> > Andy promised to go off and test it and that's where the thread ended. I
> > take it the results of the testing was positive?  I was expecting him to
> > report back on that so KY could ack the patch.
> >
> > James
> 
> The patch seems to be in Ubuntu Saucy's 3.11 kernel version 3.11.0-12.18 
> onwards - see 
> http://kernel.ubuntu.com/git?p=ubuntu/ubuntu-saucy.git;a=log;h=refs/tags/Ubuntu-3.11.0-12.18 
> for the logs.
> 
> Would you like me to resubmit the patch with you as the author?  There 
> isn't a "Signed-off-by:" line for you on this patch at the moment.  Is 
> it okay for me to add one?

I'm not really comfortable with the way these patches are being
submitted.  I really need Andy to justify what's been done and why, then
find an upstream acceptable format then for the Microsoft Hyper-V guys
to ack them.  We need more information than you can infer simply from
the patches being in Ubuntu.  If Andy's off somewhere, we can wait
because this is just simply feature enablement; the bug doesn't show
unless you enable trim on hv storvsc.

James



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

* Re: [PATCH 1/2] scsi: hyper-v storvsc switch up to SPC-3
  2014-05-16 17:58       ` James Bottomley
@ 2014-05-16 18:18         ` Ian Abbott
  2014-05-16 18:25           ` James Bottomley
  0 siblings, 1 reply; 16+ messages in thread
From: Ian Abbott @ 2014-05-16 18:18 UTC (permalink / raw)
  To: James Bottomley
  Cc: linux-scsi, devel, Andy Whitcroft, K. Y. Srinivasan,
	Haiyang Zhang, Tim Gardner

On 2014-05-16 18:58, James Bottomley wrote:
> On Fri, 2014-05-16 at 18:39 +0100, Ian Abbott wrote:
>> On 2014-05-16 18:14, James Bottomley wrote:
>>> On Fri, 2014-05-16 at 16:39 +0100, Ian Abbott wrote:
>>>> From: Andy Whitcroft <apw@canonical.com>
>>>>
>>>> Suggested-by: James Bottomley <James.Bottomley@HansenPartnership.com>
>>>
>>> That is my patch, isn't it, just with a slightly modified comment:
>>>
>>> http://marc.info/?l=linux-scsi&m=137908428211951
>>
>> I believe so, yes.  Looking at Ubuntu's kernel repository, Andy reverted
>> his original 4 patches and applied your patch instead.  I'm not sure
>> about the other patch (PATCH 2/2) that disables the MAINTENANCE_IN
>> command.  Perhaps that was needed as a consequence of claiming to be
>> SCSI level SPC-3?
>
> Yes, see other email.
>
>>> Andy promised to go off and test it and that's where the thread ended. I
>>> take it the results of the testing was positive?  I was expecting him to
>>> report back on that so KY could ack the patch.
>>>
>>> James
>>
>> The patch seems to be in Ubuntu Saucy's 3.11 kernel version 3.11.0-12.18
>> onwards - see
>> http://kernel.ubuntu.com/git?p=ubuntu/ubuntu-saucy.git;a=log;h=refs/tags/Ubuntu-3.11.0-12.18
>> for the logs.
>>
>> Would you like me to resubmit the patch with you as the author?  There
>> isn't a "Signed-off-by:" line for you on this patch at the moment.  Is
>> it okay for me to add one?
>
> I'm not really comfortable with the way these patches are being
> submitted.  I really need Andy to justify what's been done and why, then
> find an upstream acceptable format then for the Microsoft Hyper-V guys
> to ack them.  We need more information than you can infer simply from
> the patches being in Ubuntu.  If Andy's off somewhere, we can wait
> because this is just simply feature enablement; the bug doesn't show
> unless you enable trim on hv storvsc.

TBH, I'm out of my depth on this, but hopefully I've kicked up the dust 
a bit, since the patches (good or bad) have been languishing in Ubuntu's 
repositories since October!

-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@mev.co.uk>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-

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

* Re: [PATCH 1/2] scsi: hyper-v storvsc switch up to SPC-3
  2014-05-16 18:18         ` Ian Abbott
@ 2014-05-16 18:25           ` James Bottomley
  2014-05-16 19:28             ` KY Srinivasan
  0 siblings, 1 reply; 16+ messages in thread
From: James Bottomley @ 2014-05-16 18:25 UTC (permalink / raw)
  To: Ian Abbott
  Cc: linux-scsi, devel, Andy Whitcroft, K. Y. Srinivasan,
	Haiyang Zhang, Tim Gardner

On Fri, 2014-05-16 at 19:18 +0100, Ian Abbott wrote:
> On 2014-05-16 18:58, James Bottomley wrote:
> > On Fri, 2014-05-16 at 18:39 +0100, Ian Abbott wrote:
> >> On 2014-05-16 18:14, James Bottomley wrote:
> >>> On Fri, 2014-05-16 at 16:39 +0100, Ian Abbott wrote:
> >>>> From: Andy Whitcroft <apw@canonical.com>
> >>>>
> >>>> Suggested-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> >>>
> >>> That is my patch, isn't it, just with a slightly modified comment:
> >>>
> >>> http://marc.info/?l=linux-scsi&m=137908428211951
> >>
> >> I believe so, yes.  Looking at Ubuntu's kernel repository, Andy reverted
> >> his original 4 patches and applied your patch instead.  I'm not sure
> >> about the other patch (PATCH 2/2) that disables the MAINTENANCE_IN
> >> command.  Perhaps that was needed as a consequence of claiming to be
> >> SCSI level SPC-3?
> >
> > Yes, see other email.
> >
> >>> Andy promised to go off and test it and that's where the thread ended. I
> >>> take it the results of the testing was positive?  I was expecting him to
> >>> report back on that so KY could ack the patch.
> >>>
> >>> James
> >>
> >> The patch seems to be in Ubuntu Saucy's 3.11 kernel version 3.11.0-12.18
> >> onwards - see
> >> http://kernel.ubuntu.com/git?p=ubuntu/ubuntu-saucy.git;a=log;h=refs/tags/Ubuntu-3.11.0-12.18
> >> for the logs.
> >>
> >> Would you like me to resubmit the patch with you as the author?  There
> >> isn't a "Signed-off-by:" line for you on this patch at the moment.  Is
> >> it okay for me to add one?
> >
> > I'm not really comfortable with the way these patches are being
> > submitted.  I really need Andy to justify what's been done and why, then
> > find an upstream acceptable format then for the Microsoft Hyper-V guys
> > to ack them.  We need more information than you can infer simply from
> > the patches being in Ubuntu.  If Andy's off somewhere, we can wait
> > because this is just simply feature enablement; the bug doesn't show
> > unless you enable trim on hv storvsc.
> 
> TBH, I'm out of my depth on this, but hopefully I've kicked up the dust 
> a bit, since the patches (good or bad) have been languishing in Ubuntu's 
> repositories since October!

That's OK, thanks for doing this; I'd forgotten about the patch.

James



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

* RE: [PATCH 1/2] scsi: hyper-v storvsc switch up to SPC-3
  2014-05-16 18:25           ` James Bottomley
@ 2014-05-16 19:28             ` KY Srinivasan
  2014-05-16 19:50               ` KY Srinivasan
  0 siblings, 1 reply; 16+ messages in thread
From: KY Srinivasan @ 2014-05-16 19:28 UTC (permalink / raw)
  To: James Bottomley, Ian Abbott
  Cc: linux-scsi@vger.kernel.org, devel@linuxdriverproject.org,
	Andy Whitcroft, Haiyang Zhang, Tim Gardner



> -----Original Message-----
> From: James Bottomley [mailto:James.Bottomley@HansenPartnership.com]
> Sent: Friday, May 16, 2014 11:25 AM
> To: Ian Abbott
> Cc: linux-scsi@vger.kernel.org; devel@linuxdriverproject.org; Andy
> Whitcroft; KY Srinivasan; Haiyang Zhang; Tim Gardner
> Subject: Re: [PATCH 1/2] scsi: hyper-v storvsc switch up to SPC-3
> 
> On Fri, 2014-05-16 at 19:18 +0100, Ian Abbott wrote:
> > On 2014-05-16 18:58, James Bottomley wrote:
> > > On Fri, 2014-05-16 at 18:39 +0100, Ian Abbott wrote:
> > >> On 2014-05-16 18:14, James Bottomley wrote:
> > >>> On Fri, 2014-05-16 at 16:39 +0100, Ian Abbott wrote:
> > >>>> From: Andy Whitcroft <apw@canonical.com>
> > >>>>
> > >>>> Suggested-by: James Bottomley
> > >>>> <James.Bottomley@HansenPartnership.com>
> > >>>
> > >>> That is my patch, isn't it, just with a slightly modified comment:
> > >>>
> > >>> http://marc.info/?l=linux-scsi&m=137908428211951
> > >>
> > >> I believe so, yes.  Looking at Ubuntu's kernel repository, Andy
> > >> reverted his original 4 patches and applied your patch instead.
> > >> I'm not sure about the other patch (PATCH 2/2) that disables the
> > >> MAINTENANCE_IN command.  Perhaps that was needed as a
> consequence
> > >> of claiming to be SCSI level SPC-3?
> > >
> > > Yes, see other email.
> > >
> > >>> Andy promised to go off and test it and that's where the thread
> > >>> ended. I take it the results of the testing was positive?  I was
> > >>> expecting him to report back on that so KY could ack the patch.
> > >>>
> > >>> James
> > >>
> > >> The patch seems to be in Ubuntu Saucy's 3.11 kernel version
> > >> 3.11.0-12.18 onwards - see
> > >> http://kernel.ubuntu.com/git?p=ubuntu/ubuntu-saucy.git;a=log;h=refs
> > >> /tags/Ubuntu-3.11.0-12.18
> > >> for the logs.
> > >>
> > >> Would you like me to resubmit the patch with you as the author?
> > >> There isn't a "Signed-off-by:" line for you on this patch at the
> > >> moment.  Is it okay for me to add one?
> > >
> > > I'm not really comfortable with the way these patches are being
> > > submitted.  I really need Andy to justify what's been done and why,
> > > then find an upstream acceptable format then for the Microsoft
> > > Hyper-V guys to ack them.  We need more information than you can
> > > infer simply from the patches being in Ubuntu.  If Andy's off
> > > somewhere, we can wait because this is just simply feature
> > > enablement; the bug doesn't show unless you enable trim on hv storvsc.
> >
> > TBH, I'm out of my depth on this, but hopefully I've kicked up the
> > dust a bit, since the patches (good or bad) have been languishing in
> > Ubuntu's repositories since October!
> 
> That's OK, thanks for doing this; I'd forgotten about the patch.

Sorry to be jumping in late on this discussion. If I remember correctly, the patch
Ubuntu  has been carrying I think is the original patch from Andy that James did not like.
Checking with the Windows guys, they are not comfortable declaring compliance
with SPC_3 when no testing has been done to verify this compliance. Hopefully, soon our host
will be SPC_3 compliant and we will not need Andy's (original patch).

Regards,

K. Y


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

* RE: [PATCH 1/2] scsi: hyper-v storvsc switch up to SPC-3
  2014-05-16 19:28             ` KY Srinivasan
@ 2014-05-16 19:50               ` KY Srinivasan
  0 siblings, 0 replies; 16+ messages in thread
From: KY Srinivasan @ 2014-05-16 19:50 UTC (permalink / raw)
  To: KY Srinivasan, James Bottomley, Ian Abbott
  Cc: Andy Whitcroft, devel@linuxdriverproject.org, Haiyang Zhang,
	Tim Gardner, linux-scsi@vger.kernel.org



> -----Original Message-----
> From: driverdev-devel-bounces@linuxdriverproject.org [mailto:driverdev-
> devel-bounces@linuxdriverproject.org] On Behalf Of KY Srinivasan
> Sent: Friday, May 16, 2014 12:29 PM
> To: James Bottomley; Ian Abbott
> Cc: Andy Whitcroft; devel@linuxdriverproject.org; Haiyang Zhang; Tim
> Gardner; linux-scsi@vger.kernel.org
> Subject: RE: [PATCH 1/2] scsi: hyper-v storvsc switch up to SPC-3
> 
> 
> 
> > -----Original Message-----
> > From: James Bottomley
> [mailto:James.Bottomley@HansenPartnership.com]
> > Sent: Friday, May 16, 2014 11:25 AM
> > To: Ian Abbott
> > Cc: linux-scsi@vger.kernel.org; devel@linuxdriverproject.org; Andy
> > Whitcroft; KY Srinivasan; Haiyang Zhang; Tim Gardner
> > Subject: Re: [PATCH 1/2] scsi: hyper-v storvsc switch up to SPC-3
> >
> > On Fri, 2014-05-16 at 19:18 +0100, Ian Abbott wrote:
> > > On 2014-05-16 18:58, James Bottomley wrote:
> > > > On Fri, 2014-05-16 at 18:39 +0100, Ian Abbott wrote:
> > > >> On 2014-05-16 18:14, James Bottomley wrote:
> > > >>> On Fri, 2014-05-16 at 16:39 +0100, Ian Abbott wrote:
> > > >>>> From: Andy Whitcroft <apw@canonical.com>
> > > >>>>
> > > >>>> Suggested-by: James Bottomley
> > > >>>> <James.Bottomley@HansenPartnership.com>
> > > >>>
> > > >>> That is my patch, isn't it, just with a slightly modified comment:
> > > >>>
> > > >>> http://marc.info/?l=linux-scsi&m=137908428211951
> > > >>
> > > >> I believe so, yes.  Looking at Ubuntu's kernel repository, Andy
> > > >> reverted his original 4 patches and applied your patch instead.
> > > >> I'm not sure about the other patch (PATCH 2/2) that disables the
> > > >> MAINTENANCE_IN command.  Perhaps that was needed as a
> > consequence
> > > >> of claiming to be SCSI level SPC-3?
> > > >
> > > > Yes, see other email.
> > > >
> > > >>> Andy promised to go off and test it and that's where the thread
> > > >>> ended. I take it the results of the testing was positive?  I was
> > > >>> expecting him to report back on that so KY could ack the patch.
> > > >>>
> > > >>> James
> > > >>
> > > >> The patch seems to be in Ubuntu Saucy's 3.11 kernel version
> > > >> 3.11.0-12.18 onwards - see
> > > >> http://kernel.ubuntu.com/git?p=ubuntu/ubuntu-saucy.git;a=log;h=re
> > > >> fs
> > > >> /tags/Ubuntu-3.11.0-12.18
> > > >> for the logs.
> > > >>
> > > >> Would you like me to resubmit the patch with you as the author?
> > > >> There isn't a "Signed-off-by:" line for you on this patch at the
> > > >> moment.  Is it okay for me to add one?
> > > >
> > > > I'm not really comfortable with the way these patches are being
> > > > submitted.  I really need Andy to justify what's been done and
> > > > why, then find an upstream acceptable format then for the
> > > > Microsoft Hyper-V guys to ack them.  We need more information than
> > > > you can infer simply from the patches being in Ubuntu.  If Andy's
> > > > off somewhere, we can wait because this is just simply feature
> > > > enablement; the bug doesn't show unless you enable trim on hv
> storvsc.
> > >
> > > TBH, I'm out of my depth on this, but hopefully I've kicked up the
> > > dust a bit, since the patches (good or bad) have been languishing in
> > > Ubuntu's repositories since October!
> >
> > That's OK, thanks for doing this; I'd forgotten about the patch.
> 
> Sorry to be jumping in late on this discussion. If I remember correctly, the
> patch Ubuntu  has been carrying I think is the original patch from Andy that
> James did not like.
> Checking with the Windows guys, they are not comfortable declaring
> compliance with SPC_3 when no testing has been done to verify this
> compliance. Hopefully, soon our host will be SPC_3 compliant and we will not
> need Andy's (original patch).

I spoke too soon. James, Ubuntu is indeed carrying your patch (with a more PC comment).
The comment I had about Windows folks not wanting to claim support for SPC_3 yet still stands.

Regards,

K. Y
> _______________________________________________
> devel mailing list
> devel@linuxdriverproject.org
> http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* Re: [PATCH 0/2] scsi: hyper-v storvsc changes by Ubuntu
  2014-05-16 15:39 [PATCH 0/2] scsi: hyper-v storvsc changes by Ubuntu Ian Abbott
  2014-05-16 15:39 ` [PATCH 1/2] scsi: hyper-v storvsc switch up to SPC-3 Ian Abbott
  2014-05-16 15:39 ` [PATCH 2/2] scsi: hyper-v storvsc -- host takes MAINTENANCE_IN commands badly -- elide them Ian Abbott
@ 2014-05-21 14:24 ` Andy Whitcroft
  2014-05-22 10:49   ` KY Srinivasan
  2 siblings, 1 reply; 16+ messages in thread
From: Andy Whitcroft @ 2014-05-21 14:24 UTC (permalink / raw)
  To: Ian Abbott
  Cc: linux-scsi, devel, K. Y. Srinivasan, Haiyang Zhang,
	James E.J. Bottomley, Tim Gardner

On 16 May 2014 16:39, Ian Abbott <abbotti@mev.co.uk> wrote:
> These changes to the Microsoft Hyper-V storage driver in Ubuntu Saucy's
> 3.13 kernel look useful for the mainline kernel, especially as they
> enable 'TRIM' support.
>
> Andy Whitcroft (2):
>   scsi: hyper-v storvsc switch up to SPC-3
>   scsi: hyper-v storvsc -- host takes MAINTENANCE_IN commands badly --
>     elide them
>
>  drivers/scsi/storvsc_drv.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)


The back story here is a little complex.  The main issue is that the
Hyper-V drives claim to be SPC-2, and yet implement the SPC-3
extensions for TRIM.  We did attempt to upstream quirks to allow these
features to be negotiated specifically for the Hyper-V virtual drives
(minimum regression potential) but these were NAKd, and it was
suggested that just overriding the Hyper-V drives to SPC-3
unconditionally was more appropriate.  The first of the patches here
does does this.  This has been sitting in our tree for some time as it
was not clear that this would be entirely safe, though the SPC-3 bits
are in theory at least mostly detected.  That said this change has
been in Ubuntu for a full cycle now and does not seem to have caused
any issues.  If KY is happy we should likely submit it formally.  The
second fix I believed was already being submitted to mainline.

KY?

-apw

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

* RE: [PATCH 0/2] scsi: hyper-v storvsc changes by Ubuntu
  2014-05-21 14:24 ` [PATCH 0/2] scsi: hyper-v storvsc changes by Ubuntu Andy Whitcroft
@ 2014-05-22 10:49   ` KY Srinivasan
  2014-05-22 14:38     ` James Bottomley
  0 siblings, 1 reply; 16+ messages in thread
From: KY Srinivasan @ 2014-05-22 10:49 UTC (permalink / raw)
  To: Andy Whitcroft, Ian Abbott
  Cc: linux-scsi@vger.kernel.org, devel@linuxdriverproject.org,
	Haiyang Zhang, James E.J. Bottomley, Tim Gardner



> -----Original Message-----
> From: Andy Whitcroft [mailto:apw@canonical.com]
> Sent: Wednesday, May 21, 2014 7:25 AM
> To: Ian Abbott
> Cc: linux-scsi@vger.kernel.org; devel@linuxdriverproject.org; KY Srinivasan;
> Haiyang Zhang; James E.J. Bottomley; Tim Gardner
> Subject: Re: [PATCH 0/2] scsi: hyper-v storvsc changes by Ubuntu
> 
> On 16 May 2014 16:39, Ian Abbott <abbotti@mev.co.uk> wrote:
> > These changes to the Microsoft Hyper-V storage driver in Ubuntu
> > Saucy's
> > 3.13 kernel look useful for the mainline kernel, especially as they
> > enable 'TRIM' support.
> >
> > Andy Whitcroft (2):
> >   scsi: hyper-v storvsc switch up to SPC-3
> >   scsi: hyper-v storvsc -- host takes MAINTENANCE_IN commands badly --
> >     elide them
> >
> >  drivers/scsi/storvsc_drv.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> 
> 
> The back story here is a little complex.  The main issue is that the Hyper-V
> drives claim to be SPC-2, and yet implement the SPC-3 extensions for TRIM.
> We did attempt to upstream quirks to allow these features to be negotiated
> specifically for the Hyper-V virtual drives (minimum regression potential) but
> these were NAKd, and it was suggested that just overriding the Hyper-V
> drives to SPC-3 unconditionally was more appropriate.  The first of the
> patches here does does this.  This has been sitting in our tree for some time
> as it was not clear that this would be entirely safe, though the SPC-3 bits are
> in theory at least mostly detected.  That said this change has been in Ubuntu
> for a full cycle now and does not seem to have caused any issues.  If KY is
> happy we should likely submit it formally.  The second fix I believed was
> already being submitted to mainline.
> 
> KY?

The Windows guys are not currently comfortable claiming conformance to SPC-3, as they have not done
the necessary testing. This will change hopefully soon.

K. Y
> 
> -apw

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

* Re: [PATCH 0/2] scsi: hyper-v storvsc changes by Ubuntu
  2014-05-22 10:49   ` KY Srinivasan
@ 2014-05-22 14:38     ` James Bottomley
  2014-05-22 22:41       ` KY Srinivasan
  0 siblings, 1 reply; 16+ messages in thread
From: James Bottomley @ 2014-05-22 14:38 UTC (permalink / raw)
  To: KY Srinivasan
  Cc: Andy Whitcroft, Ian Abbott, linux-scsi@vger.kernel.org,
	devel@linuxdriverproject.org, Haiyang Zhang, Tim Gardner

On Thu, 2014-05-22 at 10:49 +0000, KY Srinivasan wrote:
> 
> 
> > -----Original Message-----
> > From: Andy Whitcroft [mailto:apw@canonical.com]
> > Sent: Wednesday, May 21, 2014 7:25 AM
> > To: Ian Abbott
> > Cc: linux-scsi@vger.kernel.org; devel@linuxdriverproject.org; KY Srinivasan;
> > Haiyang Zhang; James E.J. Bottomley; Tim Gardner
> > Subject: Re: [PATCH 0/2] scsi: hyper-v storvsc changes by Ubuntu
> > 
> > On 16 May 2014 16:39, Ian Abbott <abbotti@mev.co.uk> wrote:
> > > These changes to the Microsoft Hyper-V storage driver in Ubuntu
> > > Saucy's
> > > 3.13 kernel look useful for the mainline kernel, especially as they
> > > enable 'TRIM' support.
> > >
> > > Andy Whitcroft (2):
> > >   scsi: hyper-v storvsc switch up to SPC-3
> > >   scsi: hyper-v storvsc -- host takes MAINTENANCE_IN commands badly --
> > >     elide them
> > >
> > >  drivers/scsi/storvsc_drv.c | 10 ++++++++++
> > >  1 file changed, 10 insertions(+)
> > 
> > 
> > The back story here is a little complex.  The main issue is that the Hyper-V
> > drives claim to be SPC-2, and yet implement the SPC-3 extensions for TRIM.
> > We did attempt to upstream quirks to allow these features to be negotiated
> > specifically for the Hyper-V virtual drives (minimum regression potential) but
> > these were NAKd, and it was suggested that just overriding the Hyper-V
> > drives to SPC-3 unconditionally was more appropriate.  The first of the
> > patches here does does this.  This has been sitting in our tree for some time
> > as it was not clear that this would be entirely safe, though the SPC-3 bits are
> > in theory at least mostly detected.  That said this change has been in Ubuntu
> > for a full cycle now and does not seem to have caused any issues.  If KY is
> > happy we should likely submit it formally.  The second fix I believed was
> > already being submitted to mainline.
> > 
> > KY?
> 
> The Windows guys are not currently comfortable claiming conformance to
> SPC-3, as they have not done
> the necessary testing. This will change hopefully soon.

Any bounds on the value of "soon" are we talking weeks or months?  I
think trim is a feature, which means no huge rush to get this in, but it
is nice to respond in a timely fashion to a request from a distribution
to enable a useful feature.

James



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

* RE: [PATCH 0/2] scsi: hyper-v storvsc changes by Ubuntu
  2014-05-22 14:38     ` James Bottomley
@ 2014-05-22 22:41       ` KY Srinivasan
  0 siblings, 0 replies; 16+ messages in thread
From: KY Srinivasan @ 2014-05-22 22:41 UTC (permalink / raw)
  To: James Bottomley
  Cc: Andy Whitcroft, Ian Abbott, linux-scsi@vger.kernel.org,
	devel@linuxdriverproject.org, Haiyang Zhang, Tim Gardner



> -----Original Message-----
> From: James Bottomley [mailto:James.Bottomley@HansenPartnership.com]
> Sent: Thursday, May 22, 2014 7:38 AM
> To: KY Srinivasan
> Cc: Andy Whitcroft; Ian Abbott; linux-scsi@vger.kernel.org;
> devel@linuxdriverproject.org; Haiyang Zhang; Tim Gardner
> Subject: Re: [PATCH 0/2] scsi: hyper-v storvsc changes by Ubuntu
> 
> On Thu, 2014-05-22 at 10:49 +0000, KY Srinivasan wrote:
> >
> >
> > > -----Original Message-----
> > > From: Andy Whitcroft [mailto:apw@canonical.com]
> > > Sent: Wednesday, May 21, 2014 7:25 AM
> > > To: Ian Abbott
> > > Cc: linux-scsi@vger.kernel.org; devel@linuxdriverproject.org; KY
> > > Srinivasan; Haiyang Zhang; James E.J. Bottomley; Tim Gardner
> > > Subject: Re: [PATCH 0/2] scsi: hyper-v storvsc changes by Ubuntu
> > >
> > > On 16 May 2014 16:39, Ian Abbott <abbotti@mev.co.uk> wrote:
> > > > These changes to the Microsoft Hyper-V storage driver in Ubuntu
> > > > Saucy's
> > > > 3.13 kernel look useful for the mainline kernel, especially as
> > > > they enable 'TRIM' support.
> > > >
> > > > Andy Whitcroft (2):
> > > >   scsi: hyper-v storvsc switch up to SPC-3
> > > >   scsi: hyper-v storvsc -- host takes MAINTENANCE_IN commands badly
> --
> > > >     elide them
> > > >
> > > >  drivers/scsi/storvsc_drv.c | 10 ++++++++++
> > > >  1 file changed, 10 insertions(+)
> > >
> > >
> > > The back story here is a little complex.  The main issue is that the
> > > Hyper-V drives claim to be SPC-2, and yet implement the SPC-3
> extensions for TRIM.
> > > We did attempt to upstream quirks to allow these features to be
> > > negotiated specifically for the Hyper-V virtual drives (minimum
> > > regression potential) but these were NAKd, and it was suggested that
> > > just overriding the Hyper-V drives to SPC-3 unconditionally was more
> > > appropriate.  The first of the patches here does does this.  This
> > > has been sitting in our tree for some time as it was not clear that
> > > this would be entirely safe, though the SPC-3 bits are in theory at
> > > least mostly detected.  That said this change has been in Ubuntu for
> > > a full cycle now and does not seem to have caused any issues.  If KY
> > > is happy we should likely submit it formally.  The second fix I believed was
> already being submitted to mainline.
> > >
> > > KY?
> >
> > The Windows guys are not currently comfortable claiming conformance to
> > SPC-3, as they have not done the necessary testing. This will change
> > hopefully soon.
> 
> Any bounds on the value of "soon" are we talking weeks or months?  I think
> trim is a feature, which means no huge rush to get this in, but it is nice to
> respond in a timely fashion to a request from a distribution to enable a useful
> feature.

James,

I know they have been testing for SPC-3 compliance. I have pinged them to see when they may give me
the go ahead. I will keep you posted.

Thanks,

K. Y

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

end of thread, other threads:[~2014-05-22 22:41 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-05-16 15:39 [PATCH 0/2] scsi: hyper-v storvsc changes by Ubuntu Ian Abbott
2014-05-16 15:39 ` [PATCH 1/2] scsi: hyper-v storvsc switch up to SPC-3 Ian Abbott
2014-05-16 17:11   ` Dan Carpenter
2014-05-16 17:14   ` James Bottomley
2014-05-16 17:39     ` Ian Abbott
2014-05-16 17:58       ` James Bottomley
2014-05-16 18:18         ` Ian Abbott
2014-05-16 18:25           ` James Bottomley
2014-05-16 19:28             ` KY Srinivasan
2014-05-16 19:50               ` KY Srinivasan
2014-05-16 15:39 ` [PATCH 2/2] scsi: hyper-v storvsc -- host takes MAINTENANCE_IN commands badly -- elide them Ian Abbott
2014-05-16 17:42   ` James Bottomley
2014-05-21 14:24 ` [PATCH 0/2] scsi: hyper-v storvsc changes by Ubuntu Andy Whitcroft
2014-05-22 10:49   ` KY Srinivasan
2014-05-22 14:38     ` James Bottomley
2014-05-22 22:41       ` KY Srinivasan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox