diff mbox series

revert async probing of VMBus scsi device

Message ID 20190605185205.12583-1-sthemmin@microsoft.com (mailing list archive)
State Rejected
Headers show
Series revert async probing of VMBus scsi device | expand

Commit Message

Stephen Hemminger June 5, 2019, 6:52 p.m. UTC
Doing asynchronous probing can lead to reordered device names
which is leads to failed mounts.

Fixes: af0a5646cb8d ("use the new async probing feature for the hyperv drivers")
Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
 drivers/scsi/storvsc_drv.c | 3 ---
 1 file changed, 3 deletions(-)

Comments

Haiyang Zhang June 5, 2019, 6:54 p.m. UTC | #1
> -----Original Message-----
> From: linux-hyperv-owner@vger.kernel.org <linux-hyperv-
> owner@vger.kernel.org> On Behalf Of Stephen Hemminger
> Sent: Wednesday, June 5, 2019 2:52 PM
> To: linux-scsi@vger.kernel.org
> Cc: linux-hyperv@vger.kernel.org; Stephen Hemminger
> <sthemmin@microsoft.com>
> Subject: [PATCH] revert async probing of VMBus scsi device
> 
> Doing asynchronous probing can lead to reordered device names which is
> leads to failed mounts.
> 
> Fixes: af0a5646cb8d ("use the new async probing feature for the hyperv
> drivers")
> Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
> ---

Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
Christoph Hellwig June 5, 2019, 6:56 p.m. UTC | #2
On Wed, Jun 05, 2019 at 11:52:05AM -0700, Stephen Hemminger wrote:
> Doing asynchronous probing can lead to reordered device names
> which is leads to failed mounts.

Which is true for every device, and why we use UUIDs or label for
mounts that are supposed to be stable.
Stephen Hemminger June 5, 2019, 7:06 p.m. UTC | #3
On Wed, 5 Jun 2019 11:56:37 -0700
Christoph Hellwig <hch@infradead.org> wrote:

> On Wed, Jun 05, 2019 at 11:52:05AM -0700, Stephen Hemminger wrote:
> > Doing asynchronous probing can lead to reordered device names
> > which is leads to failed mounts.  
> 
> Which is true for every device, and why we use UUIDs or label for
> mounts that are supposed to be stable.

Not everyone is smart enough to do that.
Christoph Hellwig June 5, 2019, 7:07 p.m. UTC | #4
On Wed, Jun 05, 2019 at 12:06:40PM -0700, Stephen Hemminger wrote:
> > Which is true for every device, and why we use UUIDs or label for
> > mounts that are supposed to be stable.
> 
> Not everyone is smart enough to do that.

Sure.  But they should not get a way out for just one specific driver.
Stephen Hemminger June 5, 2019, 7:10 p.m. UTC | #5
On Wed, 5 Jun 2019 12:07:23 -0700
Christoph Hellwig <hch@infradead.org> wrote:

> On Wed, Jun 05, 2019 at 12:06:40PM -0700, Stephen Hemminger wrote:
> > > Which is true for every device, and why we use UUIDs or label for
> > > mounts that are supposed to be stable.  
> > 
> > Not everyone is smart enough to do that.  
> 
> Sure.  But they should not get a way out for just one specific driver.

There are people running new kernels on 6 year old distributions.
Was every distribution smart enough then? If you think so, then
this not necessary.
Christoph Hellwig June 5, 2019, 7:26 p.m. UTC | #6
On Wed, Jun 05, 2019 at 12:10:20PM -0700, Stephen Hemminger wrote:
> > Sure.  But they should not get a way out for just one specific driver.
> 
> There are people running new kernels on 6 year old distributions.
> Was every distribution smart enough then? If you think so, then
> this not necessary.

I think you are missing my point.  If we want a way to disable this,
we:

 a) want it opt-in
 b) it needs to for the whole SCSI layer and not just one driver
David Miller June 5, 2019, 7:27 p.m. UTC | #7
From: Christoph Hellwig <hch@infradead.org>
Date: Wed, 5 Jun 2019 12:07:23 -0700

> On Wed, Jun 05, 2019 at 12:06:40PM -0700, Stephen Hemminger wrote:
>> > Which is true for every device, and why we use UUIDs or label for
>> > mounts that are supposed to be stable.
>> 
>> Not everyone is smart enough to do that.
> 
> Sure.  But they should not get a way out for just one specific driver.

Agreed.
Stephen Hemminger June 5, 2019, 8:06 p.m. UTC | #8
On Wed, 5 Jun 2019 12:26:47 -0700
Christoph Hellwig <hch@infradead.org> wrote:

> On Wed, Jun 05, 2019 at 12:10:20PM -0700, Stephen Hemminger wrote:
> > > Sure.  But they should not get a way out for just one specific driver.  
> > 
> > There are people running new kernels on 6 year old distributions.
> > Was every distribution smart enough then? If you think so, then
> > this not necessary.  
> 
> I think you are missing my point.  If we want a way to disable this,
> we:
> 
>  a) want it opt-in
>  b) it needs to for the whole SCSI layer and not just one driver

There is some possible issues with how initial images are deployed
with temporary disks.  The temp disks come pre-formatted with a blank
NTFS and cloudinit reformats them to ext4 the first time.

Not sure if ordering matters, or if cloudinit is smart enough to do
the right thing. I am trying to get an answer.

It might just be that the first boot should turn off async probing
for the whole scsi layer via kernel cmdline. Or it might not be an issue
if cloudinit was written by developers smart enough to handle moving
disks.
Stephen Hemminger June 5, 2019, 9:20 p.m. UTC | #9
On Wed, 5 Jun 2019 12:26:47 -0700
Christoph Hellwig <hch@infradead.org> wrote:

> On Wed, Jun 05, 2019 at 12:10:20PM -0700, Stephen Hemminger wrote:
> > > Sure.  But they should not get a way out for just one specific driver.  
> > 
> > There are people running new kernels on 6 year old distributions.
> > Was every distribution smart enough then? If you think so, then
> > this not necessary.  
> 
> I think you are missing my point.  If we want a way to disable this,
> we:
> 
>  a) want it opt-in
>  b) it needs to for the whole SCSI layer and not just one driver

Based on feedback from the Program Manager who handles the distros
the patch for storvsc is not needed.

SCSI device naming was never deterministic anyway.  
Cloud-init relies on the same mechanism as the Azure agent to detect the OS and ephemeral disk.
diff mbox series

Patch

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 8472de1007ff..56dcaa43b652 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -1942,9 +1942,6 @@  static struct hv_driver storvsc_drv = {
 	.id_table = id_table,
 	.probe = storvsc_probe,
 	.remove = storvsc_remove,
-	.driver = {
-		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
-	},
 };
 
 #if IS_ENABLED(CONFIG_SCSI_FC_ATTRS)