[PATCH 0/2] Drivers: scsi: storvsc: Fix issues with hot-add/remove of LUNs

[PATCH 0/2] Drivers: scsi: storvsc: Fix issues with hot-add/remove of LUNs

[K. Y. Srinivasan]

This patch-set addresses issues with LUN hot-add and remove. When the host
notifies the guest that a scan is needed, scan the host. Also, prior to
discovering new devices that may have been added, ensure we handle the
LUN remove case first.

K. Y. Srinivasan (2):
  Drivers: scsi: storvsc: In responce to a scan event, scan the host
  Drivers: scsi: storvsc: Force discovery of LUNs that may have been
    removed.

 drivers/scsi/storvsc_drv.c |   43 ++++++++++++++++++++++++++++++++-----------
 1 files changed, 32 insertions(+), 11 deletions(-)

-- 
1.7.4.1

[PATCH 0/2] Drivers: scsi: storvsc: Fix issues with hot-add/remove of LUNs

[K. Y. Srinivasan]

This patch-set addresses issues with LUN hot-add and remove. When the host
notifies the guest that a scan is needed, scan the host. Also, prior to
discovering new devices that may have been added, ensure we handle the
LUN remove case first.

K. Y. Srinivasan (2):
  Drivers: scsi: storvsc: In responce to a scan event, scan the host
  Drivers: scsi: storvsc: Force discovery of LUNs that may have been
    removed.

 drivers/scsi/storvsc_drv.c |   43 ++++++++++++++++++++++++++++++++-----------
 1 files changed, 32 insertions(+), 11 deletions(-)

-- 
1.7.4.1

[PATCH 1/2] Drivers: scsi: storvsc: In responce to a scan event, scan the host

[K. Y. Srinivasan]

The virtual HBA that storvsc implements can support multiple channels and
targets. So, scan the host when the host notifies that a scan is needed.

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

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 15ba695..02d1db6 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -426,21 +426,16 @@ done:
 	kfree(wrk);
 }
 
-static void storvsc_bus_scan(struct work_struct *work)
+static void storvsc_host_scan(struct work_struct *work)
 {
 	struct storvsc_scan_work *wrk;
-	int id, order_id;
+	struct Scsi_Host *host;
 
 	wrk = container_of(work, struct storvsc_scan_work, work);
-	for (id = 0; id < wrk->host->max_id; ++id) {
-		if (wrk->host->reverse_ordering)
-			order_id = wrk->host->max_id - id - 1;
-		else
-			order_id = id;
-
-		scsi_scan_target(&wrk->host->shost_gendev, 0,
-				order_id, SCAN_WILD_CARD, 1);
-	}
+	host = wrk->host;
+
+	scsi_scan_host(host);
+
 	kfree(wrk);
 }
 
@@ -1209,7 +1204,7 @@ static void storvsc_on_receive(struct hv_device *device,
 		if (!work)
 			return;
 
-		INIT_WORK(&work->work, storvsc_bus_scan);
+		INIT_WORK(&work->work, storvsc_host_scan);
 		work->host = stor_device->host;
 		schedule_work(&work->work);
 		break;
-- 
1.7.4.1

[PATCH 1/2] Drivers: scsi: storvsc: In responce to a scan event, scan the host

[K. Y. Srinivasan]

The virtual HBA that storvsc implements can support multiple channels and
targets. So, scan the host when the host notifies that a scan is needed.

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

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 15ba695..02d1db6 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -426,21 +426,16 @@ done:
 	kfree(wrk);
 }
 
-static void storvsc_bus_scan(struct work_struct *work)
+static void storvsc_host_scan(struct work_struct *work)
 {
 	struct storvsc_scan_work *wrk;
-	int id, order_id;
+	struct Scsi_Host *host;
 
 	wrk = container_of(work, struct storvsc_scan_work, work);
-	for (id = 0; id < wrk->host->max_id; ++id) {
-		if (wrk->host->reverse_ordering)
-			order_id = wrk->host->max_id - id - 1;
-		else
-			order_id = id;
-
-		scsi_scan_target(&wrk->host->shost_gendev, 0,
-				order_id, SCAN_WILD_CARD, 1);
-	}
+	host = wrk->host;
+
+	scsi_scan_host(host);
+
 	kfree(wrk);
 }
 
@@ -1209,7 +1204,7 @@ static void storvsc_on_receive(struct hv_device *device,
 		if (!work)
 			return;
 
-		INIT_WORK(&work->work, storvsc_bus_scan);
+		INIT_WORK(&work->work, storvsc_host_scan);
 		work->host = stor_device->host;
 		schedule_work(&work->work);
 		break;
-- 
1.7.4.1

[PATCH 2/2] Drivers: scsi: storvsc: Force discovery of LUNs that may have been removed.

[K. Y. Srinivasan]

The host asks the guest to scan when a LUN is removed or added.
The only way a guest can identify the removed LUN is when an I/O is
attempted on a removed LUN - the SRB status code indicates that the LUN
is invalid. We currently handle this SRB status and remove the device.

Rather than waiting for an I/O to remove the device, force the discovery of
LUNs that may have been removed prior to discovering LUNs that may have
been added.

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

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 02d1db6..fb38ca9 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -430,10 +430,36 @@ static void storvsc_host_scan(struct work_struct *work)
 {
 	struct storvsc_scan_work *wrk;
 	struct Scsi_Host *host;
+	struct scsi_device *sdev;
+	unsigned long flags;
 
 	wrk = container_of(work, struct storvsc_scan_work, work);
 	host = wrk->host;
 
+	/*
+	 * Before scanning the host, first check to see if any of the
+	 * currrently known devices have been hot removed. We issue a
+	 * "unit ready" command against all currently known devices.
+	 * This I/O will result in an error for devices that have been
+	 * removed. As part of handling the I/O error, we remove the device.
+	 *
+	 * When a LUN is added or removed, the host sends us a signal to
+	 * scan the host. Thus we are forced to discover the LUNs that
+	 * may have been removed this way.
+	 */
+	mutex_lock(&host->scan_mutex);
+	spin_lock_irqsave(host->host_lock, flags);
+	list_for_each_entry(sdev, &host->__devices, siblings) {
+		spin_unlock_irqrestore(host->host_lock, flags);
+		scsi_test_unit_ready(sdev, 1, 1, NULL);
+		spin_lock_irqsave(host->host_lock, flags);
+		continue;
+	}
+	spin_unlock_irqrestore(host->host_lock, flags);
+	mutex_unlock(&host->scan_mutex);
+	/*
+	 * Now scan the host to discover LUNs that may have been added.
+	 */
 	scsi_scan_host(host);
 
 	kfree(wrk);
-- 
1.7.4.1

[PATCH 2/2] Drivers: scsi: storvsc: Force discovery of LUNs that may have been removed.

[K. Y. Srinivasan]

The host asks the guest to scan when a LUN is removed or added.
The only way a guest can identify the removed LUN is when an I/O is
attempted on a removed LUN - the SRB status code indicates that the LUN
is invalid. We currently handle this SRB status and remove the device.

Rather than waiting for an I/O to remove the device, force the discovery of
LUNs that may have been removed prior to discovering LUNs that may have
been added.

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

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 02d1db6..fb38ca9 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -430,10 +430,36 @@ static void storvsc_host_scan(struct work_struct *work)
 {
 	struct storvsc_scan_work *wrk;
 	struct Scsi_Host *host;
+	struct scsi_device *sdev;
+	unsigned long flags;
 
 	wrk = container_of(work, struct storvsc_scan_work, work);
 	host = wrk->host;
 
+	/*
+	 * Before scanning the host, first check to see if any of the
+	 * currrently known devices have been hot removed. We issue a
+	 * "unit ready" command against all currently known devices.
+	 * This I/O will result in an error for devices that have been
+	 * removed. As part of handling the I/O error, we remove the device.
+	 *
+	 * When a LUN is added or removed, the host sends us a signal to
+	 * scan the host. Thus we are forced to discover the LUNs that
+	 * may have been removed this way.
+	 */
+	mutex_lock(&host->scan_mutex);
+	spin_lock_irqsave(host->host_lock, flags);
+	list_for_each_entry(sdev, &host->__devices, siblings) {
+		spin_unlock_irqrestore(host->host_lock, flags);
+		scsi_test_unit_ready(sdev, 1, 1, NULL);
+		spin_lock_irqsave(host->host_lock, flags);
+		continue;
+	}
+	spin_unlock_irqrestore(host->host_lock, flags);
+	mutex_unlock(&host->scan_mutex);
+	/*
+	 * Now scan the host to discover LUNs that may have been added.
+	 */
 	scsi_scan_host(host);
 
 	kfree(wrk);
-- 
1.7.4.1

Re: [PATCH 2/2] Drivers: scsi: storvsc: Force discovery of LUNs that may have been removed.

[Christoph Hellwig]

On Sat, Aug 16, 2014 at 08:09:48PM -0700, K. Y. Srinivasan wrote:
> The host asks the guest to scan when a LUN is removed or added.
> The only way a guest can identify the removed LUN is when an I/O is
> attempted on a removed LUN - the SRB status code indicates that the LUN
> is invalid. We currently handle this SRB status and remove the device.
> 
> Rather than waiting for an I/O to remove the device, force the discovery of
> LUNs that may have been removed prior to discovering LUNs that may have
> been added.

This looks pretty reasonable to me, but I wonder if we should move this
up to common code so that it happens for any host rescan triggered by
sysfs or other drivers as well.

Re: [PATCH 2/2] Drivers: scsi: storvsc: Force discovery of LUNs that may have been removed.

[Christoph Hellwig]

On Sat, Aug 16, 2014 at 08:09:48PM -0700, K. Y. Srinivasan wrote:
> The host asks the guest to scan when a LUN is removed or added.
> The only way a guest can identify the removed LUN is when an I/O is
> attempted on a removed LUN - the SRB status code indicates that the LUN
> is invalid. We currently handle this SRB status and remove the device.
> 
> Rather than waiting for an I/O to remove the device, force the discovery of
> LUNs that may have been removed prior to discovering LUNs that may have
> been added.

This looks pretty reasonable to me, but I wonder if we should move this
up to common code so that it happens for any host rescan triggered by
sysfs or other drivers as well.

RE: [PATCH 2/2] Drivers: scsi: storvsc: Force discovery of LUNs that may have been removed.

[KY Srinivasan]

> -----Original Message-----
> From: Christoph Hellwig [mailto:hch@infradead.org]
> Sent: Tuesday, August 19, 2014 10:55 AM
> To: KY Srinivasan
> Cc: hare@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; ohering@suse.com;
> jbottomley@parallels.com; linux-scsi@vger.kernel.org; Martin K. Petersen
> Subject: Re: [PATCH 2/2] Drivers: scsi: storvsc: Force discovery of LUNs that
> may have been removed.
> 
> On Sat, Aug 16, 2014 at 08:09:48PM -0700, K. Y. Srinivasan wrote:
> > The host asks the guest to scan when a LUN is removed or added.
> > The only way a guest can identify the removed LUN is when an I/O is
> > attempted on a removed LUN - the SRB status code indicates that the
> > LUN is invalid. We currently handle this SRB status and remove the device.
> >
> > Rather than waiting for an I/O to remove the device, force the
> > discovery of LUNs that may have been removed prior to discovering LUNs
> > that may have been added.
> 
> This looks pretty reasonable to me, but I wonder if we should move this up
> to common code so that it happens for any host rescan triggered by sysfs or
> other drivers as well.
Ping. Any decision on if/when this patch may be checked in.

Regards,

K. Y

RE: [PATCH 2/2] Drivers: scsi: storvsc: Force discovery of LUNs that may have been removed.

[KY Srinivasan]

> -----Original Message-----
> From: Christoph Hellwig [mailto:hch@infradead.org]
> Sent: Tuesday, August 19, 2014 10:55 AM
> To: KY Srinivasan
> Cc: hare@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; ohering@suse.com;
> jbottomley@parallels.com; linux-scsi@vger.kernel.org; Martin K. Petersen
> Subject: Re: [PATCH 2/2] Drivers: scsi: storvsc: Force discovery of LUNs that
> may have been removed.
> 
> On Sat, Aug 16, 2014 at 08:09:48PM -0700, K. Y. Srinivasan wrote:
> > The host asks the guest to scan when a LUN is removed or added.
> > The only way a guest can identify the removed LUN is when an I/O is
> > attempted on a removed LUN - the SRB status code indicates that the
> > LUN is invalid. We currently handle this SRB status and remove the device.
> >
> > Rather than waiting for an I/O to remove the device, force the
> > discovery of LUNs that may have been removed prior to discovering LUNs
> > that may have been added.
> 
> This looks pretty reasonable to me, but I wonder if we should move this up
> to common code so that it happens for any host rescan triggered by sysfs or
> other drivers as well.
Ping. Any decision on if/when this patch may be checked in.

Regards,

K. Y

Re: [PATCH 2/2] Drivers: scsi: storvsc: Force discovery of LUNs that may have been removed.

[Hannes Reinecke]

On 08/19/2014 07:54 PM, Christoph Hellwig wrote:
> On Sat, Aug 16, 2014 at 08:09:48PM -0700, K. Y. Srinivasan wrote:
>> The host asks the guest to scan when a LUN is removed or added.
>> The only way a guest can identify the removed LUN is when an I/O is
>> attempted on a removed LUN - the SRB status code indicates that the LUN
>> is invalid. We currently handle this SRB status and remove the device.
>>
>> Rather than waiting for an I/O to remove the device, force the discovery of
>> LUNs that may have been removed prior to discovering LUNs that may have
>> been added.
>
> This looks pretty reasonable to me, but I wonder if we should move this
> up to common code so that it happens for any host rescan triggered by
> sysfs or other drivers as well.
>
Not without proper testing.
Currently we cannot rescan existing devices; the inquiry string is 
nailed to the sdev structure. The only way to really refresh the 
information is to delete it and rescan it again.

And I really do _not_ want to do this automatically as the device 
might be busy due to various reasons (think of multipathing).
It tooks us ages to get this working with FC, and we finally settled 
to have a soft-remove implemented in the transport class.
And we still have issues with SAS HBAs, where at least the standard 
defines a mechanism. Trying this in the SCSI midlayer itself
is the road to disaster.

If we were to attempt this we would need to lift the dev_loss_tmo 
mechanism from the FC transport layer and make this a generic
facility for every HBA. But this is quite some work.

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 2/2] Drivers: scsi: storvsc: Force discovery of LUNs that may have been removed.

[Hannes Reinecke]

On 08/19/2014 07:54 PM, Christoph Hellwig wrote:
> On Sat, Aug 16, 2014 at 08:09:48PM -0700, K. Y. Srinivasan wrote:
>> The host asks the guest to scan when a LUN is removed or added.
>> The only way a guest can identify the removed LUN is when an I/O is
>> attempted on a removed LUN - the SRB status code indicates that the LUN
>> is invalid. We currently handle this SRB status and remove the device.
>>
>> Rather than waiting for an I/O to remove the device, force the discovery of
>> LUNs that may have been removed prior to discovering LUNs that may have
>> been added.
>
> This looks pretty reasonable to me, but I wonder if we should move this
> up to common code so that it happens for any host rescan triggered by
> sysfs or other drivers as well.
>
Not without proper testing.
Currently we cannot rescan existing devices; the inquiry string is 
nailed to the sdev structure. The only way to really refresh the 
information is to delete it and rescan it again.

And I really do _not_ want to do this automatically as the device 
might be busy due to various reasons (think of multipathing).
It tooks us ages to get this working with FC, and we finally settled 
to have a soft-remove implemented in the transport class.
And we still have issues with SAS HBAs, where at least the standard 
defines a mechanism. Trying this in the SCSI midlayer itself
is the road to disaster.

If we were to attempt this we would need to lift the dev_loss_tmo 
mechanism from the FC transport layer and make this a generic
facility for every HBA. But this is quite some work.

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 2/2] Drivers: scsi: storvsc: Force discovery of LUNs that may have been removed.

[Christoph Hellwig]

On Tue, Aug 26, 2014 at 10:54:51PM +0000, KY Srinivasan wrote:
> > This looks pretty reasonable to me, but I wonder if we should move this up
> > to common code so that it happens for any host rescan triggered by sysfs or
> > other drivers as well.
> Ping. Any decision on if/when this patch may be checked in.

I'll need a review or two for it still..

Re: [PATCH 2/2] Drivers: scsi: storvsc: Force discovery of LUNs that may have been removed.

[Christoph Hellwig]

On Tue, Aug 26, 2014 at 10:54:51PM +0000, KY Srinivasan wrote:
> > This looks pretty reasonable to me, but I wonder if we should move this up
> > to common code so that it happens for any host rescan triggered by sysfs or
> > other drivers as well.
> Ping. Any decision on if/when this patch may be checked in.

I'll need a review or two for it still..

Re: [PATCH 2/2] Drivers: scsi: storvsc: Force discovery of LUNs that may have been removed.

[Mike Christie]

On 08/27/2014 09:31 AM, Hannes Reinecke wrote:
> On 08/19/2014 07:54 PM, Christoph Hellwig wrote:
>> On Sat, Aug 16, 2014 at 08:09:48PM -0700, K. Y. Srinivasan wrote:
>>> The host asks the guest to scan when a LUN is removed or added.
>>> The only way a guest can identify the removed LUN is when an I/O is
>>> attempted on a removed LUN - the SRB status code indicates that the LUN
>>> is invalid. We currently handle this SRB status and remove the device.
>>>
>>> Rather than waiting for an I/O to remove the device, force the
>>> discovery of
>>> LUNs that may have been removed prior to discovering LUNs that may have
>>> been added.
>>
>> This looks pretty reasonable to me, but I wonder if we should move this
>> up to common code so that it happens for any host rescan triggered by
>> sysfs or other drivers as well.
>>
> Not without proper testing.
> Currently we cannot rescan existing devices; the inquiry string is
> nailed to the sdev structure. The only way to really refresh the
> information is to delete it and rescan it again.

How are distros handling 0x6/0x3f/0x0e (report luns changed) when it
gets passed to userspace? Is everyone kicking off a new full (add and
delete) scan to handle this or logging it? Is the driver returning this
when the LUNs change?

Also is the driver getting a 0x5/0x25/0 (invalid LUN) when the LUN does
not exist, or is it just getting that SRB_STATUS_INVALID_LUN error code?

Re: [PATCH 2/2] Drivers: scsi: storvsc: Force discovery of LUNs that may have been removed.

[Hannes Reinecke]

On 08/29/2014 04:42 AM, Mike Christie wrote:
> On 08/27/2014 09:31 AM, Hannes Reinecke wrote:
>> On 08/19/2014 07:54 PM, Christoph Hellwig wrote:
>>> On Sat, Aug 16, 2014 at 08:09:48PM -0700, K. Y. Srinivasan wrote:
>>>> The host asks the guest to scan when a LUN is removed or added.
>>>> The only way a guest can identify the removed LUN is when an I/O is
>>>> attempted on a removed LUN - the SRB status code indicates that the LUN
>>>> is invalid. We currently handle this SRB status and remove the device.
>>>>
>>>> Rather than waiting for an I/O to remove the device, force the
>>>> discovery of
>>>> LUNs that may have been removed prior to discovering LUNs that may have
>>>> been added.
>>>
>>> This looks pretty reasonable to me, but I wonder if we should move this
>>> up to common code so that it happens for any host rescan triggered by
>>> sysfs or other drivers as well.
>>>
>> Not without proper testing.
>> Currently we cannot rescan existing devices; the inquiry string is
>> nailed to the sdev structure. The only way to really refresh the
>> information is to delete it and rescan it again.
>
> How are distros handling 0x6/0x3f/0x0e (report luns changed) when it
> gets passed to userspace? Is everyone kicking off a new full (add and
> delete) scan to handle this or logging it? Is the driver returning this
> when the LUNs change?
>
Currently it's logged to userspace and ignored.
Doing an automated rescan has proven to be dangerous, as it
might disconnect any LUNs which are still in use by applications.
Especially HA or database setups tends to become very annoyed
when you do an automated rescan.

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 2/2] Drivers: scsi: storvsc: Force discovery of LUNs that may have been removed.

[Hannes Reinecke]

On 08/29/2014 04:42 AM, Mike Christie wrote:
> On 08/27/2014 09:31 AM, Hannes Reinecke wrote:
>> On 08/19/2014 07:54 PM, Christoph Hellwig wrote:
>>> On Sat, Aug 16, 2014 at 08:09:48PM -0700, K. Y. Srinivasan wrote:
>>>> The host asks the guest to scan when a LUN is removed or added.
>>>> The only way a guest can identify the removed LUN is when an I/O is
>>>> attempted on a removed LUN - the SRB status code indicates that the LUN
>>>> is invalid. We currently handle this SRB status and remove the device.
>>>>
>>>> Rather than waiting for an I/O to remove the device, force the
>>>> discovery of
>>>> LUNs that may have been removed prior to discovering LUNs that may have
>>>> been added.
>>>
>>> This looks pretty reasonable to me, but I wonder if we should move this
>>> up to common code so that it happens for any host rescan triggered by
>>> sysfs or other drivers as well.
>>>
>> Not without proper testing.
>> Currently we cannot rescan existing devices; the inquiry string is
>> nailed to the sdev structure. The only way to really refresh the
>> information is to delete it and rescan it again.
>
> How are distros handling 0x6/0x3f/0x0e (report luns changed) when it
> gets passed to userspace? Is everyone kicking off a new full (add and
> delete) scan to handle this or logging it? Is the driver returning this
> when the LUNs change?
>
Currently it's logged to userspace and ignored.
Doing an automated rescan has proven to be dangerous, as it
might disconnect any LUNs which are still in use by applications.
Especially HA or database setups tends to become very annoyed
when you do an automated rescan.

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 2/2] Drivers: scsi: storvsc: Force discovery of LUNs that may have been removed.

[Bart Van Assche]

On 08/29/14 08:19, Hannes Reinecke wrote:
> On 08/29/2014 04:42 AM, Mike Christie wrote:
>> How are distros handling 0x6/0x3f/0x0e (report luns changed) when it
>> gets passed to userspace? Is everyone kicking off a new full (add and
>> delete) scan to handle this or logging it? Is the driver returning this
>> when the LUNs change?
>>
> Currently it's logged to userspace and ignored.
> Doing an automated rescan has proven to be dangerous, as it
> might disconnect any LUNs which are still in use by applications.
> Especially HA or database setups tends to become very annoyed
> when you do an automated rescan.

Has it already been considered to add newly discovered LUNs
automatically and to leave it to the user to remove stale LUNs manually
? That would be similar to what the rescan-scsi-bus.sh script does
without option -r/--remove.

Bart.

Re: [PATCH 2/2] Drivers: scsi: storvsc: Force discovery of LUNs that may have been removed.

[Hannes Reinecke]

On 08/29/2014 09:39 AM, Bart Van Assche wrote:
> On 08/29/14 08:19, Hannes Reinecke wrote:
>> On 08/29/2014 04:42 AM, Mike Christie wrote:
>>> How are distros handling 0x6/0x3f/0x0e (report luns changed) when it
>>> gets passed to userspace? Is everyone kicking off a new full (add and
>>> delete) scan to handle this or logging it? Is the driver returning this
>>> when the LUNs change?
>>>
>> Currently it's logged to userspace and ignored.
>> Doing an automated rescan has proven to be dangerous, as it
>> might disconnect any LUNs which are still in use by applications.
>> Especially HA or database setups tends to become very annoyed
>> when you do an automated rescan.
>
> Has it already been considered to add newly discovered LUNs
> automatically and to leave it to the user to remove stale LUNs manually
> ? That would be similar to what the rescan-scsi-bus.sh script does
> without option -r/--remove.
>

As of now we're still missing an in-kernel infrastructure which 
would allow us to react on any sense codes; currently we're relying 
on the administrator to setup a udev rule 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 2/2] Drivers: scsi: storvsc: Force discovery of LUNs that may have been removed.

[Hannes Reinecke]

On 08/29/2014 09:39 AM, Bart Van Assche wrote:
> On 08/29/14 08:19, Hannes Reinecke wrote:
>> On 08/29/2014 04:42 AM, Mike Christie wrote:
>>> How are distros handling 0x6/0x3f/0x0e (report luns changed) when it
>>> gets passed to userspace? Is everyone kicking off a new full (add and
>>> delete) scan to handle this or logging it? Is the driver returning this
>>> when the LUNs change?
>>>
>> Currently it's logged to userspace and ignored.
>> Doing an automated rescan has proven to be dangerous, as it
>> might disconnect any LUNs which are still in use by applications.
>> Especially HA or database setups tends to become very annoyed
>> when you do an automated rescan.
>
> Has it already been considered to add newly discovered LUNs
> automatically and to leave it to the user to remove stale LUNs manually
> ? That would be similar to what the rescan-scsi-bus.sh script does
> without option -r/--remove.
>

As of now we're still missing an in-kernel infrastructure which 
would allow us to react on any sense codes; currently we're relying 
on the administrator to setup a udev rule 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 2/2] Drivers: scsi: storvsc: Force discovery of LUNs that may have been removed.

[James Bottomley]

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

On Fri, 2014-08-29 at 10:13 +0200, Hannes Reinecke wrote:
> On 08/29/2014 09:39 AM, Bart Van Assche wrote:
> > On 08/29/14 08:19, Hannes Reinecke wrote:
> >> On 08/29/2014 04:42 AM, Mike Christie wrote:
> >>> How are distros handling 0x6/0x3f/0x0e (report luns changed) when it
> >>> gets passed to userspace? Is everyone kicking off a new full (add and
> >>> delete) scan to handle this or logging it? Is the driver returning this
> >>> when the LUNs change?
> >>>
> >> Currently it's logged to userspace and ignored.
> >> Doing an automated rescan has proven to be dangerous, as it
> >> might disconnect any LUNs which are still in use by applications.
> >> Especially HA or database setups tends to become very annoyed
> >> when you do an automated rescan.
> >
> > Has it already been considered to add newly discovered LUNs
> > automatically and to leave it to the user to remove stale LUNs manually
> > ? That would be similar to what the rescan-scsi-bus.sh script does
> > without option -r/--remove.
> >
> 
> As of now we're still missing an in-kernel infrastructure which 
> would allow us to react on any sense codes; currently we're relying 
> on the administrator to setup a udev rule here.

Um, I thought this was supposed to solve that problem:

commit 279afdfe78a020b4b1a68bffd0009b961b12982e
Author: Ewan D. Milne <emilne@redhat.com>
Date:   Thu Aug 8 15:07:48 2013 -0400

    [SCSI] Generate uevents on certain unit attention codes

The idea was supposed to be that, as you say, log scrubbers are hard to
configure and break every time someone fixes a spelling error, so we
could now listen for a report luns data change uevent instead.

James

ÿôèº{.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¥

Re: [PATCH 2/2] Drivers: scsi: storvsc: Force discovery of LUNs that may have been removed.

[James Bottomley]

On Fri, 2014-08-29 at 10:13 +0200, Hannes Reinecke wrote:
> On 08/29/2014 09:39 AM, Bart Van Assche wrote:
> > On 08/29/14 08:19, Hannes Reinecke wrote:
> >> On 08/29/2014 04:42 AM, Mike Christie wrote:
> >>> How are distros handling 0x6/0x3f/0x0e (report luns changed) when it
> >>> gets passed to userspace? Is everyone kicking off a new full (add and
> >>> delete) scan to handle this or logging it? Is the driver returning this
> >>> when the LUNs change?
> >>>
> >> Currently it's logged to userspace and ignored.
> >> Doing an automated rescan has proven to be dangerous, as it
> >> might disconnect any LUNs which are still in use by applications.
> >> Especially HA or database setups tends to become very annoyed
> >> when you do an automated rescan.
> >
> > Has it already been considered to add newly discovered LUNs
> > automatically and to leave it to the user to remove stale LUNs manually
> > ? That would be similar to what the rescan-scsi-bus.sh script does
> > without option -r/--remove.
> >
> 
> As of now we're still missing an in-kernel infrastructure which 
> would allow us to react on any sense codes; currently we're relying 
> on the administrator to setup a udev rule here.

Um, I thought this was supposed to solve that problem:

commit 279afdfe78a020b4b1a68bffd0009b961b12982e
Author: Ewan D. Milne <emilne@redhat.com>
Date:   Thu Aug 8 15:07:48 2013 -0400

    [SCSI] Generate uevents on certain unit attention codes

The idea was supposed to be that, as you say, log scrubbers are hard to
configure and break every time someone fixes a spelling error, so we
could now listen for a report luns data change uevent instead.

James

Re: [PATCH 2/2] Drivers: scsi: storvsc: Force discovery of LUNs that may have been removed.

[Ewan Milne]

On Thu, 2014-08-28 at 21:42 -0500, Mike Christie wrote:
> On 08/27/2014 09:31 AM, Hannes Reinecke wrote:
> > On 08/19/2014 07:54 PM, Christoph Hellwig wrote:
> >> On Sat, Aug 16, 2014 at 08:09:48PM -0700, K. Y. Srinivasan wrote:
> >>> The host asks the guest to scan when a LUN is removed or added.
> >>> The only way a guest can identify the removed LUN is when an I/O is
> >>> attempted on a removed LUN - the SRB status code indicates that the LUN
> >>> is invalid. We currently handle this SRB status and remove the device.
> >>>
> >>> Rather than waiting for an I/O to remove the device, force the
> >>> discovery of
> >>> LUNs that may have been removed prior to discovering LUNs that may have
> >>> been added.
> >>
> >> This looks pretty reasonable to me, but I wonder if we should move this
> >> up to common code so that it happens for any host rescan triggered by
> >> sysfs or other drivers as well.
> >>
> > Not without proper testing.
> > Currently we cannot rescan existing devices; the inquiry string is
> > nailed to the sdev structure. The only way to really refresh the
> > information is to delete it and rescan it again.
> 
> How are distros handling 0x6/0x3f/0x0e (report luns changed) when it
> gets passed to userspace? Is everyone kicking off a new full (add and
> delete) scan to handle this or logging it? Is the driver returning this
> when the LUNs change?

Currently the udev rules we have to handle these events are installed
with a separate package, and only the REPORTED LUNS DATA HAS CHANGED
does anything, the others are commented out.  It turns out that e.g.
multipath stops using a path if it notices that the capacity has changed
and we need to do some more work there, it is under discussion.

We do not delete LUNs that disappear from the REPORT LUNS inventory,
although someone could write their own udev rule to do that if desired.

Beware the case where a LUN is remapped to a different LUN number, or
if LUN's WWID is used for a device with different data (e.g. a LUN
deleted and re-added and the WWID is the same although I don't know
if this actually happens).

Consider that the UA just provides notification to userspace of a
change -- lack of notification does not prevent someone from deciding
to rescan for new LUNs via sysfs any time they feel like it.  So
you can't just change the storage configuration and hope that no-one
notices until you are done making changes.

-Ewan

> 
> Also is the driver getting a 0x5/0x25/0 (invalid LUN) when the LUN does
> not exist, or is it just getting that SRB_STATUS_INVALID_LUN error code?
> --
> 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

Re: [PATCH 2/2] Drivers: scsi: storvsc: Force discovery of LUNs that may have been removed.

[Ewan Milne]

On Thu, 2014-08-28 at 21:42 -0500, Mike Christie wrote:
> On 08/27/2014 09:31 AM, Hannes Reinecke wrote:
> > On 08/19/2014 07:54 PM, Christoph Hellwig wrote:
> >> On Sat, Aug 16, 2014 at 08:09:48PM -0700, K. Y. Srinivasan wrote:
> >>> The host asks the guest to scan when a LUN is removed or added.
> >>> The only way a guest can identify the removed LUN is when an I/O is
> >>> attempted on a removed LUN - the SRB status code indicates that the LUN
> >>> is invalid. We currently handle this SRB status and remove the device.
> >>>
> >>> Rather than waiting for an I/O to remove the device, force the
> >>> discovery of
> >>> LUNs that may have been removed prior to discovering LUNs that may have
> >>> been added.
> >>
> >> This looks pretty reasonable to me, but I wonder if we should move this
> >> up to common code so that it happens for any host rescan triggered by
> >> sysfs or other drivers as well.
> >>
> > Not without proper testing.
> > Currently we cannot rescan existing devices; the inquiry string is
> > nailed to the sdev structure. The only way to really refresh the
> > information is to delete it and rescan it again.
> 
> How are distros handling 0x6/0x3f/0x0e (report luns changed) when it
> gets passed to userspace? Is everyone kicking off a new full (add and
> delete) scan to handle this or logging it? Is the driver returning this
> when the LUNs change?

Currently the udev rules we have to handle these events are installed
with a separate package, and only the REPORTED LUNS DATA HAS CHANGED
does anything, the others are commented out.  It turns out that e.g.
multipath stops using a path if it notices that the capacity has changed
and we need to do some more work there, it is under discussion.

We do not delete LUNs that disappear from the REPORT LUNS inventory,
although someone could write their own udev rule to do that if desired.

Beware the case where a LUN is remapped to a different LUN number, or
if LUN's WWID is used for a device with different data (e.g. a LUN
deleted and re-added and the WWID is the same although I don't know
if this actually happens).

Consider that the UA just provides notification to userspace of a
change -- lack of notification does not prevent someone from deciding
to rescan for new LUNs via sysfs any time they feel like it.  So
you can't just change the storage configuration and hope that no-one
notices until you are done making changes.

-Ewan

> 
> Also is the driver getting a 0x5/0x25/0 (invalid LUN) when the LUN does
> not exist, or is it just getting that SRB_STATUS_INVALID_LUN error code?
> --
> 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

Re: [PATCH 0/2] Drivers: scsi: storvsc: Fix issues with hot-add/remove of LUNs

[Christoph Hellwig]

So can anywayone give me a review for those two patches?  

On Sat, Aug 16, 2014 at 08:09:14PM -0700, K. Y. Srinivasan wrote:
> This patch-set addresses issues with LUN hot-add and remove. When the host
> notifies the guest that a scan is needed, scan the host. Also, prior to
> discovering new devices that may have been added, ensure we handle the
> LUN remove case first.
> 
> K. Y. Srinivasan (2):
>   Drivers: scsi: storvsc: In responce to a scan event, scan the host
>   Drivers: scsi: storvsc: Force discovery of LUNs that may have been
>     removed.
> 
>  drivers/scsi/storvsc_drv.c |   43 ++++++++++++++++++++++++++++++++-----------
>  1 files changed, 32 insertions(+), 11 deletions(-)
> 
> -- 
> 1.7.4.1
> 
> --
> 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
---end quoted text---

RE: [PATCH 0/2] Drivers: scsi: storvsc: Fix issues with hot-add/remove of LUNs

[KY Srinivasan]

> -----Original Message-----
> From: Christoph Hellwig [mailto:hch@infradead.org]
> Sent: Thursday, September 25, 2014 6:47 AM
> To: KY Srinivasan
> Cc: hare@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; ohering@suse.com;
> jbottomley@parallels.com; linux-scsi@vger.kernel.org
> Subject: Re: [PATCH 0/2] Drivers: scsi: storvsc: Fix issues with hot-
> add/remove of LUNs
> 
> So can anywayone give me a review for those two patches?

Olaf,

Could you review these patches.

K. Y
> 
> On Sat, Aug 16, 2014 at 08:09:14PM -0700, K. Y. Srinivasan wrote:
> > This patch-set addresses issues with LUN hot-add and remove. When the
> > host notifies the guest that a scan is needed, scan the host. Also,
> > prior to discovering new devices that may have been added, ensure we
> > handle the LUN remove case first.
> >
> > K. Y. Srinivasan (2):
> >   Drivers: scsi: storvsc: In responce to a scan event, scan the host
> >   Drivers: scsi: storvsc: Force discovery of LUNs that may have been
> >     removed.
> >
> >  drivers/scsi/storvsc_drv.c |   43 ++++++++++++++++++++++++++++++++-
> ----------
> >  1 files changed, 32 insertions(+), 11 deletions(-)
> >
> > --
> > 1.7.4.1
> >
> > --
> > 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
> ---end quoted text---

RE: [PATCH 0/2] Drivers: scsi: storvsc: Fix issues with hot-add/remove of LUNs

[KY Srinivasan]

> -----Original Message-----
> From: Christoph Hellwig [mailto:hch@infradead.org]
> Sent: Thursday, September 25, 2014 6:47 AM
> To: KY Srinivasan
> Cc: hare@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; ohering@suse.com;
> jbottomley@parallels.com; linux-scsi@vger.kernel.org
> Subject: Re: [PATCH 0/2] Drivers: scsi: storvsc: Fix issues with hot-
> add/remove of LUNs
> 
> So can anywayone give me a review for those two patches?

Olaf,

Could you review these patches.

K. Y
> 
> On Sat, Aug 16, 2014 at 08:09:14PM -0700, K. Y. Srinivasan wrote:
> > This patch-set addresses issues with LUN hot-add and remove. When the
> > host notifies the guest that a scan is needed, scan the host. Also,
> > prior to discovering new devices that may have been added, ensure we
> > handle the LUN remove case first.
> >
> > K. Y. Srinivasan (2):
> >   Drivers: scsi: storvsc: In responce to a scan event, scan the host
> >   Drivers: scsi: storvsc: Force discovery of LUNs that may have been
> >     removed.
> >
> >  drivers/scsi/storvsc_drv.c |   43 ++++++++++++++++++++++++++++++++-
> ----------
> >  1 files changed, 32 insertions(+), 11 deletions(-)
> >
> > --
> > 1.7.4.1
> >
> > --
> > 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
> ---end quoted text---

RE: [PATCH 0/2] Drivers: scsi: storvsc: Fix issues with hot-add/remove of LUNs

[KY Srinivasan]

> -----Original Message-----
> From: Christoph Hellwig [mailto:hch@infradead.org]
> Sent: Thursday, September 25, 2014 6:47 AM
> To: KY Srinivasan
> Cc: hare@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; ohering@suse.com;
> jbottomley@parallels.com; linux-scsi@vger.kernel.org
> Subject: Re: [PATCH 0/2] Drivers: scsi: storvsc: Fix issues with hot-
> add/remove of LUNs
> 
> So can anywayone give me a review for those two patches?

Did you get the reviews?

K. Y
> 
> On Sat, Aug 16, 2014 at 08:09:14PM -0700, K. Y. Srinivasan wrote:
> > This patch-set addresses issues with LUN hot-add and remove. When the
> > host notifies the guest that a scan is needed, scan the host. Also,
> > prior to discovering new devices that may have been added, ensure we
> > handle the LUN remove case first.
> >
> > K. Y. Srinivasan (2):
> >   Drivers: scsi: storvsc: In responce to a scan event, scan the host
> >   Drivers: scsi: storvsc: Force discovery of LUNs that may have been
> >     removed.
> >
> >  drivers/scsi/storvsc_drv.c |   43 ++++++++++++++++++++++++++++++++-
> ----------
> >  1 files changed, 32 insertions(+), 11 deletions(-)
> >
> > --
> > 1.7.4.1
> >
> > --
> > 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
> ---end quoted text---

RE: [PATCH 0/2] Drivers: scsi: storvsc: Fix issues with hot-add/remove of LUNs

[KY Srinivasan]

> -----Original Message-----
> From: Christoph Hellwig [mailto:hch@infradead.org]
> Sent: Thursday, September 25, 2014 6:47 AM
> To: KY Srinivasan
> Cc: hare@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; ohering@suse.com;
> jbottomley@parallels.com; linux-scsi@vger.kernel.org
> Subject: Re: [PATCH 0/2] Drivers: scsi: storvsc: Fix issues with hot-
> add/remove of LUNs
> 
> So can anywayone give me a review for those two patches?

Did you get the reviews?

K. Y
> 
> On Sat, Aug 16, 2014 at 08:09:14PM -0700, K. Y. Srinivasan wrote:
> > This patch-set addresses issues with LUN hot-add and remove. When the
> > host notifies the guest that a scan is needed, scan the host. Also,
> > prior to discovering new devices that may have been added, ensure we
> > handle the LUN remove case first.
> >
> > K. Y. Srinivasan (2):
> >   Drivers: scsi: storvsc: In responce to a scan event, scan the host
> >   Drivers: scsi: storvsc: Force discovery of LUNs that may have been
> >     removed.
> >
> >  drivers/scsi/storvsc_drv.c |   43 ++++++++++++++++++++++++++++++++-
> ----------
> >  1 files changed, 32 insertions(+), 11 deletions(-)
> >
> > --
> > 1.7.4.1
> >
> > --
> > 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
> ---end quoted text---

Re: [PATCH 0/2] Drivers: scsi: storvsc: Fix issues with hot-add/remove of LUNs

[Christoph Hellwig]

On Tue, Oct 07, 2014 at 05:44:38PM +0000, KY Srinivasan wrote:
> > So can anywayone give me a review for those two patches?
> 
> Did you get the reviews?

No.

Re: [PATCH 0/2] Drivers: scsi: storvsc: Fix issues with hot-add/remove of LUNs

[Christoph Hellwig]

On Tue, Oct 07, 2014 at 05:44:38PM +0000, KY Srinivasan wrote:
> > So can anywayone give me a review for those two patches?
> 
> Did you get the reviews?

No.