[PATCH] sd: do not set changed flag on all unit attention conditions

[PATCH] sd: do not set changed flag on all unit attention conditions

[Paolo Bonzini]

Right now, I/O will fail as soon as a unit attention condition is
detected on a unit with removable media.  However, this is not
always necessary.  There are some cases (such as "Capacity data
has changed") where no particular action is needed.  On the
other hand, all problematic cases have to report at least one
of "No medium" and/or a "Medium may have changed", so restrict
our attention to those.

This patch fixes resizing a removable medium with virtio-scsi.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 drivers/scsi/scsi_lib.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index b583277..6d8ca08 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -843,8 +843,11 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
 	} else if (sense_valid && !sense_deferred) {
 		switch (sshdr.sense_key) {
 		case UNIT_ATTENTION:
-			if (cmd->device->removable) {
-				/* Detected disc change.  Set a bit
+			if (cmd->device->removable &&
+			    (sshdr.asc == 0x3a ||
+			     (sshdr.asc == 0x28 && sshdr.ascq == 0x00))) {
+				/* "No medium" or "Medium may have changed."
+				 * This means a disc change.  Set a bit
 				 * and quietly refuse further access.
 				 */
 				cmd->device->changed = 1;
-- 
1.7.1

Re: [PATCH] sd: do not set changed flag on all unit attention conditions

[James Bottomley]

On Mon, 2012-07-16 at 18:06 +0200, Paolo Bonzini wrote:
> Right now, I/O will fail as soon as a unit attention condition is
> detected on a unit with removable media.  However, this is not
> always necessary.  There are some cases (such as "Capacity data
> has changed") where no particular action is needed.  On the
> other hand, all problematic cases have to report at least one
> of "No medium" and/or a "Medium may have changed", so restrict
> our attention to those.
> 
> This patch fixes resizing a removable medium with virtio-scsi.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  drivers/scsi/scsi_lib.c |    7 +++++--
>  1 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index b583277..6d8ca08 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -843,8 +843,11 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
>  	} else if (sense_valid && !sense_deferred) {
>  		switch (sshdr.sense_key) {
>  		case UNIT_ATTENTION:
> -			if (cmd->device->removable) {
> -				/* Detected disc change.  Set a bit
> +			if (cmd->device->removable &&
> +			    (sshdr.asc == 0x3a ||
> +			     (sshdr.asc == 0x28 && sshdr.ascq == 0x00))) {
> +				/* "No medium" or "Medium may have changed."
> +				 * This means a disc change.  Set a bit

This type of change would likely cause a huge cascade of errors in real
removable media devices.  Under the MMC standards, which a lot of the
older removable discs seem to follow, UNIT ATTENTION indicates either
medium change or device reset (which we check for and eat lower down);
we can't rely on them giving proper SBC-2 sense codes.  If you want to
pretend to be removable media, you have to conform to its standards.

James

Re: [PATCH] sd: do not set changed flag on all unit attention conditions

[Paolo Bonzini]

Il 16/07/2012 18:18, James Bottomley ha scritto:
>> > diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
>> > index b583277..6d8ca08 100644
>> > --- a/drivers/scsi/scsi_lib.c
>> > +++ b/drivers/scsi/scsi_lib.c
>> > @@ -843,8 +843,11 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
>> >  	} else if (sense_valid && !sense_deferred) {
>> >  		switch (sshdr.sense_key) {
>> >  		case UNIT_ATTENTION:
>> > -			if (cmd->device->removable) {
>> > -				/* Detected disc change.  Set a bit
>> > +			if (cmd->device->removable &&
>> > +			    (sshdr.asc == 0x3a ||
>> > +			     (sshdr.asc == 0x28 && sshdr.ascq == 0x00))) {
>> > +				/* "No medium" or "Medium may have changed."
>> > +				 * This means a disc change.  Set a bit
> This type of change would likely cause a huge cascade of errors in real
> removable media devices.  Under the MMC standards, which a lot of the
> older removable discs seem to follow, UNIT ATTENTION indicates either
> medium change or device reset (which we check for and eat lower down);
> we can't rely on them giving proper SBC-2 sense codes.  If you want to
> pretend to be removable media, you have to conform to its standards.

Would you accept a patch doing the opposite, i.e. passing some sense
codes such as PARAMETERS CHANGED and TARGET OPERATING CONDITIONS HAVE
CHANGED?

Paolo

Re: [PATCH] sd: do not set changed flag on all unit attention conditions

[James Bottomley]

On Mon, 2012-07-16 at 19:20 +0200, Paolo Bonzini wrote:
> Il 16/07/2012 18:18, James Bottomley ha scritto:
> >> > diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> >> > index b583277..6d8ca08 100644
> >> > --- a/drivers/scsi/scsi_lib.c
> >> > +++ b/drivers/scsi/scsi_lib.c
> >> > @@ -843,8 +843,11 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
> >> >  	} else if (sense_valid && !sense_deferred) {
> >> >  		switch (sshdr.sense_key) {
> >> >  		case UNIT_ATTENTION:
> >> > -			if (cmd->device->removable) {
> >> > -				/* Detected disc change.  Set a bit
> >> > +			if (cmd->device->removable &&
> >> > +			    (sshdr.asc == 0x3a ||
> >> > +			     (sshdr.asc == 0x28 && sshdr.ascq == 0x00))) {
> >> > +				/* "No medium" or "Medium may have changed."
> >> > +				 * This means a disc change.  Set a bit
> > This type of change would likely cause a huge cascade of errors in real
> > removable media devices.  Under the MMC standards, which a lot of the
> > older removable discs seem to follow, UNIT ATTENTION indicates either
> > medium change or device reset (which we check for and eat lower down);
> > we can't rely on them giving proper SBC-2 sense codes.  If you want to
> > pretend to be removable media, you have to conform to its standards.
> 
> Would you accept a patch doing the opposite, i.e. passing some sense
> codes such as PARAMETERS CHANGED and TARGET OPERATING CONDITIONS HAVE
> CHANGED?

Could you explain what the problem actually is?  It looks like you had a
reason to mark virtio-scsi as removable, even though it isn't, and now
you want to add further hacks because being removable doesn't quite
work.

Lets go back and see if there's a more correct way to do whatever it is
you want to do.

James

Re: [PATCH] sd: do not set changed flag on all unit attention conditions

[Paolo Bonzini]

Il 17/07/2012 09:45, James Bottomley ha scritto:
> On Mon, 2012-07-16 at 19:20 +0200, Paolo Bonzini wrote:
>> Il 16/07/2012 18:18, James Bottomley ha scritto:
>>>>> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
>>>>> index b583277..6d8ca08 100644
>>>>> --- a/drivers/scsi/scsi_lib.c
>>>>> +++ b/drivers/scsi/scsi_lib.c
>>>>> @@ -843,8 +843,11 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
>>>>>  	} else if (sense_valid && !sense_deferred) {
>>>>>  		switch (sshdr.sense_key) {
>>>>>  		case UNIT_ATTENTION:
>>>>> -			if (cmd->device->removable) {
>>>>> -				/* Detected disc change.  Set a bit
>>>>> +			if (cmd->device->removable &&
>>>>> +			    (sshdr.asc == 0x3a ||
>>>>> +			     (sshdr.asc == 0x28 && sshdr.ascq == 0x00))) {
>>>>> +				/* "No medium" or "Medium may have changed."
>>>>> +				 * This means a disc change.  Set a bit
>>> This type of change would likely cause a huge cascade of errors in real
>>> removable media devices.  Under the MMC standards, which a lot of the
>>> older removable discs seem to follow, UNIT ATTENTION indicates either
>>> medium change or device reset (which we check for and eat lower down);
>>> we can't rely on them giving proper SBC-2 sense codes.  If you want to
>>> pretend to be removable media, you have to conform to its standards.
>>
>> Would you accept a patch doing the opposite, i.e. passing some sense
>> codes such as PARAMETERS CHANGED and TARGET OPERATING CONDITIONS HAVE
>> CHANGED?
> 
> Could you explain what the problem actually is?  It looks like you had a
> reason to mark virtio-scsi as removable, even though it isn't, and now
> you want to add further hacks because being removable doesn't quite
> work.

It's not specific to virtio-scsi, in fact I expect that virtio-scsi will
be almost always used with non-removable disks.

However, QEMU's SCSI target is not used just for virtio-scsi (for
example it can be used for USB storage), and it lets you mark a disk as
removable---why? because there exists real hardware that presents itself
as an SBC removable disk.  The only thing that is specific to
virtualization, is support for online resizing (which generates a unit
attention condition CAPACITY DATA HAS CHANGED).

Paolo

Re: [PATCH] sd: do not set changed flag on all unit attention conditions

[James Bottomley]

On Tue, 2012-07-17 at 10:34 +0200, Paolo Bonzini wrote:
> Il 17/07/2012 09:45, James Bottomley ha scritto:
> > On Mon, 2012-07-16 at 19:20 +0200, Paolo Bonzini wrote:
> >> Il 16/07/2012 18:18, James Bottomley ha scritto:
> >>>>> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> >>>>> index b583277..6d8ca08 100644
> >>>>> --- a/drivers/scsi/scsi_lib.c
> >>>>> +++ b/drivers/scsi/scsi_lib.c
> >>>>> @@ -843,8 +843,11 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
> >>>>>  	} else if (sense_valid && !sense_deferred) {
> >>>>>  		switch (sshdr.sense_key) {
> >>>>>  		case UNIT_ATTENTION:
> >>>>> -			if (cmd->device->removable) {
> >>>>> -				/* Detected disc change.  Set a bit
> >>>>> +			if (cmd->device->removable &&
> >>>>> +			    (sshdr.asc == 0x3a ||
> >>>>> +			     (sshdr.asc == 0x28 && sshdr.ascq == 0x00))) {
> >>>>> +				/* "No medium" or "Medium may have changed."
> >>>>> +				 * This means a disc change.  Set a bit
> >>> This type of change would likely cause a huge cascade of errors in real
> >>> removable media devices.  Under the MMC standards, which a lot of the
> >>> older removable discs seem to follow, UNIT ATTENTION indicates either
> >>> medium change or device reset (which we check for and eat lower down);
> >>> we can't rely on them giving proper SBC-2 sense codes.  If you want to
> >>> pretend to be removable media, you have to conform to its standards.
> >>
> >> Would you accept a patch doing the opposite, i.e. passing some sense
> >> codes such as PARAMETERS CHANGED and TARGET OPERATING CONDITIONS HAVE
> >> CHANGED?
> > 
> > Could you explain what the problem actually is?  It looks like you had a
> > reason to mark virtio-scsi as removable, even though it isn't, and now
> > you want to add further hacks because being removable doesn't quite
> > work.
> 
> It's not specific to virtio-scsi, in fact I expect that virtio-scsi will
> be almost always used with non-removable disks.
> 
> However, QEMU's SCSI target is not used just for virtio-scsi (for
> example it can be used for USB storage), and it lets you mark a disk as
> removable---why? because there exists real hardware that presents itself
> as an SBC removable disk.  The only thing that is specific to
> virtualization, is support for online resizing (which generates a unit
> attention condition CAPACITY DATA HAS CHANGED).

So what's the problem?  If you're doing pass through of a physical disk,
we pick up removable from its inquiry string ... a physical removable
device doesn't get resized.  If you have a virtual disk you want to
resize, you don't set the removable flag in the inquiry data.

James

Re: [PATCH] sd: do not set changed flag on all unit attention conditions

[Paolo Bonzini]

Il 17/07/2012 10:40, James Bottomley ha scritto:
>> > 
>> > It's not specific to virtio-scsi, in fact I expect that virtio-scsi will
>> > be almost always used with non-removable disks.
>> > 
>> > However, QEMU's SCSI target is not used just for virtio-scsi (for
>> > example it can be used for USB storage), and it lets you mark a disk as
>> > removable---why? because there exists real hardware that presents itself
>> > as an SBC removable disk.  The only thing that is specific to
>> > virtualization, is support for online resizing (which generates a unit
>> > attention condition CAPACITY DATA HAS CHANGED).
> So what's the problem?  If you're doing pass through of a physical disk,
> we pick up removable from its inquiry string ... a physical removable
> device doesn't get resized.  If you have a virtual disk you want to
> resize, you don't set the removable flag in the inquiry data.

In practice people will do what you said, and it's not a problem.

However, there's nothing that prevents you from running qemu with a
removable SCSI disk, and then resizing it.  I would like this to work,
because SBC allows it and there's no reason why it shouldn't.

Paolo

Re: [PATCH] sd: do not set changed flag on all unit attention conditions

[James Bottomley]

On Tue, 2012-07-17 at 10:54 +0200, Paolo Bonzini wrote:
> Il 17/07/2012 10:40, James Bottomley ha scritto:
> >> > 
> >> > It's not specific to virtio-scsi, in fact I expect that virtio-scsi will
> >> > be almost always used with non-removable disks.
> >> > 
> >> > However, QEMU's SCSI target is not used just for virtio-scsi (for
> >> > example it can be used for USB storage), and it lets you mark a disk as
> >> > removable---why? because there exists real hardware that presents itself
> >> > as an SBC removable disk.  The only thing that is specific to
> >> > virtualization, is support for online resizing (which generates a unit
> >> > attention condition CAPACITY DATA HAS CHANGED).
> > So what's the problem?  If you're doing pass through of a physical disk,
> > we pick up removable from its inquiry string ... a physical removable
> > device doesn't get resized.  If you have a virtual disk you want to
> > resize, you don't set the removable flag in the inquiry data.
> 
> In practice people will do what you said, and it's not a problem.
> 
> However, there's nothing that prevents you from running qemu with a
> removable SCSI disk, and then resizing it.  I would like this to work,
> because SBC allows it and there's no reason why it shouldn't.

There's no such thing in the market today as a removable disk that's
resizeable.  Removable disks are for things like backup cartridges and
ageing jazz drives.  Worse: most removeable devices today are USB card
readers whose standards compliance varies from iffy to non existent.
Resizeable disks are currently the province of storage arrays.

We don't do stuff just because the standards allows it; just the
opposite: we try to use the smallest implementations from the standards
we can get away with just because the more things we do, the more
exceptions and broken devices we come across.

James


James

Re: [PATCH] sd: do not set changed flag on all unit attention conditions

[Paolo Bonzini]

Il 17/07/2012 11:11, James Bottomley ha scritto:
> We don't do stuff just because the standards allows it; just the
> opposite: we try to use the smallest implementations from the standards
> we can get away with just because the more things we do, the more
> exceptions and broken devices we come across.

Yes, I realize failing only on specific sense codes as I did it in the
patch is not going to work.  However, the other way round is not
problematic (explicitly allow some sense codes, fail on all others).

Another example is "target operating conditions have changed".  QEMU
cannot report such changes because scsi_error prints a warning (fine)
and then passes the unit attention upwards.  With removable drives, this
has the same problem as resizing.

Paolo

Re: [PATCH] sd: do not set changed flag on all unit attention conditions

[James Bottomley]

On Tue, 2012-07-17 at 11:28 +0200, Paolo Bonzini wrote:
> Il 17/07/2012 11:11, James Bottomley ha scritto:
> > We don't do stuff just because the standards allows it; just the
> > opposite: we try to use the smallest implementations from the standards
> > we can get away with just because the more things we do, the more
> > exceptions and broken devices we come across.
> 
> Yes, I realize failing only on specific sense codes as I did it in the
> patch is not going to work.  However, the other way round is not
> problematic (explicitly allow some sense codes, fail on all others).

Heh, I once thought that, but there's no end to the strange ideas USB
manufacturers get.

> Another example is "target operating conditions have changed".  QEMU
> cannot report such changes because scsi_error prints a warning (fine)
> and then passes the unit attention upwards.  With removable drives, this
> has the same problem as resizing.

Why would a removable device ever use any of the codes under this ASC
when the medium hasn't changed?  They're all for arrays (well except
0x10 and 0x11 ... and they're only supposed to apply to tape changers
with autoload support declared in the control mode page).

The unanswered point is still that there's no use case for a device
that's both removable and requires array like sense code support.

James

Re: [PATCH] sd: do not set changed flag on all unit attention conditions

[Paolo Bonzini]

Il 17/07/2012 14:21, James Bottomley ha scritto:
>> Yes, I realize failing only on specific sense codes as I did it in the
>> patch is not going to work.  However, the other way round is not
>> problematic (explicitly allow some sense codes, fail on all others).
> 
> Heh, I once thought that, but there's no end to the strange ideas USB
> manufacturers get.

:)

>> > Another example is "target operating conditions have changed".  QEMU
>> > cannot report such changes because scsi_error prints a warning (fine)
>> > and then passes the unit attention upwards.  With removable drives, this
>> > has the same problem as resizing.
> Why would a removable device ever use any of the codes under this ASC
> when the medium hasn't changed?  They're all for arrays (well except
> 0x10 and 0x11 ... and they're only supposed to apply to tape changers
> with autoload support declared in the control mode page).

There are also a couple of generic ones such as "reported luns have
changed".  I also considered briefly using "inquiry data has changed" to
reload the unmap parameters after live snapshot or storage migration.  I
dropped the idea, please don't put me in the same bin as the USB
manufacturers.

Paolo

Re: [PATCH] sd: do not set changed flag on all unit attention conditions

[James Bottomley]

On Tue, 2012-07-17 at 14:31 +0200, Paolo Bonzini wrote:
> Il 17/07/2012 14:21, James Bottomley ha scritto:
> >> Yes, I realize failing only on specific sense codes as I did it in the
> >> patch is not going to work.  However, the other way round is not
> >> problematic (explicitly allow some sense codes, fail on all others).
> > 
> > Heh, I once thought that, but there's no end to the strange ideas USB
> > manufacturers get.
> 
> :)
> 
> >> > Another example is "target operating conditions have changed".  QEMU
> >> > cannot report such changes because scsi_error prints a warning (fine)
> >> > and then passes the unit attention upwards.  With removable drives, this
> >> > has the same problem as resizing.
> > Why would a removable device ever use any of the codes under this ASC
> > when the medium hasn't changed?  They're all for arrays (well except
> > 0x10 and 0x11 ... and they're only supposed to apply to tape changers
> > with autoload support declared in the control mode page).
> 
> There are also a couple of generic ones such as "reported luns have
> changed".  I also considered briefly using "inquiry data has changed" to
> reload the unmap parameters after live snapshot or storage migration.  I
> dropped the idea, please don't put me in the same bin as the USB
> manufacturers.

I'm not ... but at the same time removable functionality in sd.c is
special cased in quite a few areas which makes it fragile but exercised.
Its fragility is somewhat mitigated by the fact that a lot of the
special casing separates the removable from the non-removable paths.

I don't want to increase that fragility by entangling the removable and
non-removable paths without a good reason for doing so.

James

Re: [PATCH] sd: do not set changed flag on all unit attention conditions

[Christoph Hellwig]

On Tue, Jul 17, 2012 at 10:11:57AM +0100, James Bottomley wrote:
> There's no such thing in the market today as a removable disk that's
> resizeable.  Removable disks are for things like backup cartridges and
> ageing jazz drives.  Worse: most removeable devices today are USB card
> readers whose standards compliance varies from iffy to non existent.
> Resizeable disks are currently the province of storage arrays.

The virtual disks exported by aacraid are both marked removable and
can be resized.

Re: [PATCH] sd: do not set changed flag on all unit attention conditions

[Paolo Bonzini]

Il 17/07/2012 18:36, Christoph Hellwig ha scritto:
>> > There's no such thing in the market today as a removable disk that's
>> > resizeable.  Removable disks are for things like backup cartridges and
>> > ageing jazz drives.  Worse: most removeable devices today are USB card
>> > readers whose standards compliance varies from iffy to non existent.
>> > Resizeable disks are currently the province of storage arrays.
> The virtual disks exported by aacraid are both marked removable and
> can be resized.

Do they report resizing via unit attention?  I can skip this part on
removable virtio-scsi disks if that's what real hardware does, it would
also work.

Paolo

Re: [PATCH] sd: do not set changed flag on all unit attention conditions

[Christoph Hellwig]

On Tue, Jul 17, 2012 at 06:47:15PM +0200, Paolo Bonzini wrote:
> Il 17/07/2012 18:36, Christoph Hellwig ha scritto:
> >> > There's no such thing in the market today as a removable disk that's
> >> > resizeable.  Removable disks are for things like backup cartridges and
> >> > ageing jazz drives.  Worse: most removeable devices today are USB card
> >> > readers whose standards compliance varies from iffy to non existent.
> >> > Resizeable disks are currently the province of storage arrays.
> > The virtual disks exported by aacraid are both marked removable and
> > can be resized.
> 
> Do they report resizing via unit attention?  I can skip this part on
> removable virtio-scsi disks if that's what real hardware does, it would
> also work.

I don't have access to aacraid hardware currently.

Re: [PATCH] sd: do not set changed flag on all unit attention conditions

[Mike Christie]

On 07/17/2012 10:47 AM, Paolo Bonzini wrote:
> Il 17/07/2012 18:36, Christoph Hellwig ha scritto:
>>>> There's no such thing in the market today as a removable disk that's
>>>> resizeable.  Removable disks are for things like backup cartridges and
>>>> ageing jazz drives.  Worse: most removeable devices today are USB card
>>>> readers whose standards compliance varies from iffy to non existent.
>>>> Resizeable disks are currently the province of storage arrays.
>> The virtual disks exported by aacraid are both marked removable and
>> can be resized.
> 
> Do they report resizing via unit attention?  I can skip this part on
> removable virtio-scsi disks if that's what real hardware does, it would
> also work.
> 

Not sure if we are talking about the same thing.

So can virtio-scsi send a UA with asc/ascq that indicates the lun
changed size? Other drivers do this. I updated Hannes's patches the
other day to support UAs like those in userspace.

I just saw the code in the patch where virtio-scsi gets that event.

Re: [PATCH] sd: do not set changed flag on all unit attention conditions

[Mike Christie]

On 07/17/2012 12:45 PM, Mike Christie wrote:
> On 07/17/2012 10:47 AM, Paolo Bonzini wrote:
>> Il 17/07/2012 18:36, Christoph Hellwig ha scritto:
>>>>> There's no such thing in the market today as a removable disk that's
>>>>> resizeable.  Removable disks are for things like backup cartridges and
>>>>> ageing jazz drives.  Worse: most removeable devices today are USB card
>>>>> readers whose standards compliance varies from iffy to non existent.
>>>>> Resizeable disks are currently the province of storage arrays.
>>> The virtual disks exported by aacraid are both marked removable and
>>> can be resized.
>>
>> Do they report resizing via unit attention?  I can skip this part on
>> removable virtio-scsi disks if that's what real hardware does, it would
>> also work.
>>
> 
> Not sure if we are talking about the same thing.
> 
> So can virtio-scsi send a UA with asc/ascq that indicates the lun
> changed size? Other drivers do this. I updated Hannes's patches the
> other day to support UAs like those in userspace.
> 
> I just saw the code in the patch where virtio-scsi gets that event.

Was not done. I meant I saw that patch where virtio-scsi gets that
virtio_scsi_event and kicks of a rescan based off of that.

Re: [PATCH] sd: do not set changed flag on all unit attention conditions

[Paolo Bonzini]

Il 17/07/2012 20:49, Mike Christie ha scritto:
> > Not sure if we are talking about the same thing.
> > 
> > So can virtio-scsi send a UA with asc/ascq that indicates the lun
> > changed size? Other drivers do this. I updated Hannes's patches the
> > other day to support UAs like those in userspace.
> > 
> > I just saw the code in the patch where virtio-scsi gets that event.
> Was not done. I meant I saw that patch where virtio-scsi gets that
> virtio_scsi_event and kicks of a rescan based off of that.

Yes, it sends both (event + UA).  Right now Linux ignores the UA, and I
wanted virtio-scsi to match real hardware as much as possible so I
copied what aacraid does.  The event also has the advantage over UA that
it the revalidate is done immediately, not the next time the unit is
accessed.

Paolo

Re: [PATCH] sd: do not set changed flag on all unit attention conditions

[James Bottomley]

On Tue, 2012-07-17 at 12:36 -0400, Christoph Hellwig wrote:
> On Tue, Jul 17, 2012 at 10:11:57AM +0100, James Bottomley wrote:
> > There's no such thing in the market today as a removable disk that's
> > resizeable.  Removable disks are for things like backup cartridges and
> > ageing jazz drives.  Worse: most removeable devices today are USB card
> > readers whose standards compliance varies from iffy to non existent.
> > Resizeable disks are currently the province of storage arrays.
> 
> The virtual disks exported by aacraid are both marked removable and
> can be resized.

So what are properties of these things? ... or is this just an instance
of a RAID manufacturer hacking around a problem by adding a removable
flag?

James

Re: [PATCH] sd: do not set changed flag on all unit attention conditions

[Hannes Reinecke]

On 07/17/2012 11:11 AM, James Bottomley wrote:
> On Tue, 2012-07-17 at 10:54 +0200, Paolo Bonzini wrote:
>> Il 17/07/2012 10:40, James Bottomley ha scritto:
>>>>>
>>>>> It's not specific to virtio-scsi, in fact I expect that virtio-scsi will
>>>>> be almost always used with non-removable disks.
>>>>>
>>>>> However, QEMU's SCSI target is not used just for virtio-scsi (for
>>>>> example it can be used for USB storage), and it lets you mark a disk as
>>>>> removable---why? because there exists real hardware that presents itself
>>>>> as an SBC removable disk.  The only thing that is specific to
>>>>> virtualization, is support for online resizing (which generates a unit
>>>>> attention condition CAPACITY DATA HAS CHANGED).
>>> So what's the problem?  If you're doing pass through of a physical disk,
>>> we pick up removable from its inquiry string ... a physical removable
>>> device doesn't get resized.  If you have a virtual disk you want to
>>> resize, you don't set the removable flag in the inquiry data.
>>
>> In practice people will do what you said, and it's not a problem.
>>
>> However, there's nothing that prevents you from running qemu with a
>> removable SCSI disk, and then resizing it.  I would like this to work,
>> because SBC allows it and there's no reason why it shouldn't.
> 
> There's no such thing in the market today as a removable disk that's
> resizeable.  Removable disks are for things like backup cartridges and
> ageing jazz drives.  Worse: most removeable devices today are USB card
> readers whose standards compliance varies from iffy to non existent.
> Resizeable disks are currently the province of storage arrays.
> 
Ho-hum. I beg to disagree.

drivers/scsi/aacraid/aachba.c:2266

		/* Do not cache partition table for arrays */
		scsicmd->device->removable = 1;

To the extend of my knowledge aacraid does this _precisely_ to allow
for resizing; in effect every open() will trigger a device revalidation.

So I guess by just setting the 'removable' flag you should be okay.
You might need to remount it, but that's another story.

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] sd: do not set changed flag on all unit attention conditions

[Hannes Reinecke]

On 07/17/2012 11:59 PM, James Bottomley wrote:
> On Tue, 2012-07-17 at 12:36 -0400, Christoph Hellwig wrote:
>> On Tue, Jul 17, 2012 at 10:11:57AM +0100, James Bottomley wrote:
>>> There's no such thing in the market today as a removable disk that's
>>> resizeable.  Removable disks are for things like backup cartridges and
>>> ageing jazz drives.  Worse: most removeable devices today are USB card
>>> readers whose standards compliance varies from iffy to non existent.
>>> Resizeable disks are currently the province of storage arrays.
>>
>> The virtual disks exported by aacraid are both marked removable and
>> can be resized.
> 
> So what are properties of these things? ... or is this just an instance
> of a RAID manufacturer hacking around a problem by adding a removable
> flag?
> 
Presumably.

The general intention is to automatically catch any disk resizing.
As the SCSI stack (used to) ignore these things that was their way
of working around it.

Curiously, though; the aacraid driver is the only one doing this,
plus the process is quite involved (using a proprietary application
for doing so etc).

None of the FC driver do this, despite the fact that resizing a disk
is even easier here.

I even tried to remove that line once, but then got told off by then
Adaptec that I would break their apps.
Since then there's a patch in the SLES kernel for adding a module
option switching off this behaviour.

We should ask Adaptec/PMC-Sierra here.

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] sd: do not set changed flag on all unit attention conditions

[Hannes Reinecke]

On 07/17/2012 11:59 PM, James Bottomley wrote:
> On Tue, 2012-07-17 at 12:36 -0400, Christoph Hellwig wrote:
>> On Tue, Jul 17, 2012 at 10:11:57AM +0100, James Bottomley wrote:
>>> There's no such thing in the market today as a removable disk that's
>>> resizeable.  Removable disks are for things like backup cartridges and
>>> ageing jazz drives.  Worse: most removeable devices today are USB card
>>> readers whose standards compliance varies from iffy to non existent.
>>> Resizeable disks are currently the province of storage arrays.
>>
>> The virtual disks exported by aacraid are both marked removable and
>> can be resized.
> 
> So what are properties of these things? ... or is this just an instance
> of a RAID manufacturer hacking around a problem by adding a removable
> flag?
> 
Presumably.

The general intention is to automatically catch any disk resizing.
As the SCSI stack (used to) ignore these things that was their way
of working around it.

Curiously, though; the aacraid driver is the only one doing this,
plus the process is quite involved (using a proprietary application
for doing so etc).

None of the FC driver do this, despite the fact that resizing a disk
is even easier here.

I even tried to remove that line once, but then got told off by then
Adaptec that I would break their apps.
Since then there's a patch in the SLES kernel for adding a module
option switching off this behaviour.

We should ask Adaptec/PMC-Sierra here.

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)


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html