* [PATCH] scsi: scsi_sysfs.c: Hide wwid sdev attr if VPD is not supported
@ 2019-06-12 2:08 ` Marcos Paulo de Souza
0 siblings, 0 replies; 7+ messages in thread
From: Marcos Paulo de Souza @ 2019-06-12 2:08 UTC (permalink / raw)
To: linux-kernel
Cc: Marcos Paulo de Souza, James E.J. Bottomley, Martin K. Petersen,
open list:SCSI SUBSYSTEM
WWID composed from VPD data from device, specifically page 0x83. So,
when a device does not have VPD support, for example USB storage devices
where VPD is specifically disabled, a read into <blk device>/device/wwid
file will always return ENXIO. To avoid this, change the
scsi_sdev_attr_is_visible function to hide wwid sysfs file when the
devices does not support VPD.
Signed-off-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com>
---
drivers/scsi/scsi_sysfs.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index dbb206c90ecf..bfd890fa0c69 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -1159,6 +1159,9 @@ static umode_t scsi_sdev_attr_is_visible(struct kobject *kobj,
struct device *dev = container_of(kobj, struct device, kobj);
struct scsi_device *sdev = to_scsi_device(dev);
+ /* do not present wwid if the device does not support VPD */
+ if (attr == &dev_attr_wwid.attr && sdev->skip_vpd_pages)
+ return 0;
if (attr == &dev_attr_queue_depth.attr &&
!sdev->host->hostt->change_queue_depth)
--
2.21.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH] scsi: scsi_sysfs.c: Hide wwid sdev attr if VPD is not supported
@ 2019-06-12 2:08 ` Marcos Paulo de Souza
0 siblings, 0 replies; 7+ messages in thread
From: Marcos Paulo de Souza @ 2019-06-12 2:08 UTC (permalink / raw)
To: linux-kernel
Cc: Marcos Paulo de Souza, James E.J. Bottomley, Martin K. Petersen,
open list:SCSI SUBSYSTEM
WWID composed from VPD data from device, specifically page 0x83. So,
when a device does not have VPD support, for example USB storage devices
where VPD is specifically disabled, a read into <blk device>/device/wwid
file will always return ENXIO. To avoid this, change the
scsi_sdev_attr_is_visible function to hide wwid sysfs file when the
devices does not support VPD.
Signed-off-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com>
---
drivers/scsi/scsi_sysfs.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index dbb206c90ecf..bfd890fa0c69 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -1159,6 +1159,9 @@ static umode_t scsi_sdev_attr_is_visible(struct kobject *kobj,
struct device *dev = container_of(kobj, struct device, kobj);
struct scsi_device *sdev = to_scsi_device(dev);
+ /* do not present wwid if the device does not support VPD */
+ if (attr == &dev_attr_wwid.attr && sdev->skip_vpd_pages)
+ return 0;
if (attr == &dev_attr_queue_depth.attr &&
!sdev->host->hostt->change_queue_depth)
--
2.21.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] scsi: scsi_sysfs.c: Hide wwid sdev attr if VPD is not supported
2019-06-12 2:08 ` Marcos Paulo de Souza
(?)
@ 2019-06-18 22:47 ` Marcos Paulo de Souza
-1 siblings, 0 replies; 7+ messages in thread
From: Marcos Paulo de Souza @ 2019-06-18 22:47 UTC (permalink / raw)
To: linux-kernel
Cc: James E.J. Bottomley, Martin K. Petersen, open list:SCSI SUBSYSTEM
ping? Can anybody take a look at this patch?
Thanks,
Marcos
On Tue, Jun 11, 2019 at 11:08:28PM -0300, Marcos Paulo de Souza wrote:
> WWID composed from VPD data from device, specifically page 0x83. So,
> when a device does not have VPD support, for example USB storage devices
> where VPD is specifically disabled, a read into <blk device>/device/wwid
> file will always return ENXIO. To avoid this, change the
> scsi_sdev_attr_is_visible function to hide wwid sysfs file when the
> devices does not support VPD.
>
> Signed-off-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com>
> ---
> drivers/scsi/scsi_sysfs.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
> index dbb206c90ecf..bfd890fa0c69 100644
> --- a/drivers/scsi/scsi_sysfs.c
> +++ b/drivers/scsi/scsi_sysfs.c
> @@ -1159,6 +1159,9 @@ static umode_t scsi_sdev_attr_is_visible(struct kobject *kobj,
> struct device *dev = container_of(kobj, struct device, kobj);
> struct scsi_device *sdev = to_scsi_device(dev);
>
> + /* do not present wwid if the device does not support VPD */
> + if (attr == &dev_attr_wwid.attr && sdev->skip_vpd_pages)
> + return 0;
>
> if (attr == &dev_attr_queue_depth.attr &&
> !sdev->host->hostt->change_queue_depth)
> --
> 2.21.0
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] scsi: scsi_sysfs.c: Hide wwid sdev attr if VPD is not supported
2019-06-12 2:08 ` Marcos Paulo de Souza
(?)
(?)
@ 2019-06-19 3:35 ` Martin K. Petersen
2019-06-19 6:34 ` Hannes Reinecke
-1 siblings, 1 reply; 7+ messages in thread
From: Martin K. Petersen @ 2019-06-19 3:35 UTC (permalink / raw)
To: Marcos Paulo de Souza
Cc: linux-kernel, James E.J. Bottomley, Martin K. Petersen,
linux-scsi, Hannes Reinecke
Marcos,
> WWID composed from VPD data from device, specifically page 0x83. So,
> when a device does not have VPD support, for example USB storage
> devices where VPD is specifically disabled, a read into <blk
> device>/device/wwid file will always return ENXIO. To avoid this,
> change the scsi_sdev_attr_is_visible function to hide wwid sysfs file
> when the devices does not support VPD.
Not a big fan of attribute files that come and go.
Why not just return an empty string? Hannes?
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] scsi: scsi_sysfs.c: Hide wwid sdev attr if VPD is not supported
2019-06-19 3:35 ` Martin K. Petersen
@ 2019-06-19 6:34 ` Hannes Reinecke
2019-06-19 9:52 ` Marcos Paulo de Souza
0 siblings, 1 reply; 7+ messages in thread
From: Hannes Reinecke @ 2019-06-19 6:34 UTC (permalink / raw)
To: Martin K. Petersen, Marcos Paulo de Souza
Cc: linux-kernel, James E.J. Bottomley, linux-scsi
On 6/19/19 5:35 AM, Martin K. Petersen wrote:
>
> Marcos,
>
>> WWID composed from VPD data from device, specifically page 0x83. So,
>> when a device does not have VPD support, for example USB storage
>> devices where VPD is specifically disabled, a read into <blk
>> device>/device/wwid file will always return ENXIO. To avoid this,
>> change the scsi_sdev_attr_is_visible function to hide wwid sysfs file
>> when the devices does not support VPD.
>
> Not a big fan of attribute files that come and go.
>
> Why not just return an empty string? Hannes?
>
Actually, the intention of the 'wwid' attribute was to have a common
place where one could look up the global id.
As such it actually serves a dual purpose, namely indicating that there
_is_ a global ID _and_ that this kernel (version) has support for 'wwid'
attribute. This is to resolve one big issue we have to udev nowadays,
which is figuring out if a specific sysfs attribute is actually
supported on this particular kernel.
Dynamic attributes are 'nicer' on a conceptual level, but make the above
test nearly impossible, as we now have _two_ possibilities why a
specific attribute is not present.
So making 'wwid' conditional would actually defeat its very purpose, and
we should leave it blank if not supported.
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
hare@suse.com +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] scsi: scsi_sysfs.c: Hide wwid sdev attr if VPD is not supported
2019-06-19 6:34 ` Hannes Reinecke
@ 2019-06-19 9:52 ` Marcos Paulo de Souza
2019-06-19 9:57 ` Hannes Reinecke
0 siblings, 1 reply; 7+ messages in thread
From: Marcos Paulo de Souza @ 2019-06-19 9:52 UTC (permalink / raw)
To: Hannes Reinecke
Cc: Martin K. Petersen, linux-kernel, James E.J. Bottomley, linux-scsi
On Wed, Jun 19, 2019 at 08:34:56AM +0200, Hannes Reinecke wrote:
> On 6/19/19 5:35 AM, Martin K. Petersen wrote:
> >
> > Marcos,
> >
> >> WWID composed from VPD data from device, specifically page 0x83. So,
> >> when a device does not have VPD support, for example USB storage
> >> devices where VPD is specifically disabled, a read into <blk
> >> device>/device/wwid file will always return ENXIO. To avoid this,
> >> change the scsi_sdev_attr_is_visible function to hide wwid sysfs file
> >> when the devices does not support VPD.
> >
> > Not a big fan of attribute files that come and go.
> >
> > Why not just return an empty string? Hannes?
> >
> Actually, the intention of the 'wwid' attribute was to have a common
> place where one could look up the global id.
> As such it actually serves a dual purpose, namely indicating that there
> _is_ a global ID _and_ that this kernel (version) has support for 'wwid'
> attribute. This is to resolve one big issue we have to udev nowadays,
> which is figuring out if a specific sysfs attribute is actually
> supported on this particular kernel.
> Dynamic attributes are 'nicer' on a conceptual level, but make the above
> test nearly impossible, as we now have _two_ possibilities why a
> specific attribute is not present.
> So making 'wwid' conditional would actually defeat its very purpose, and
> we should leave it blank if not supported.
My intention was to apply the same approach used for VPD pages, which currently
also hides the attributes if not supported by the device. So, if vpd pages are
hidden, there is no usage for wwid. But I also like the idea of the vpd pages
being blank if not supported by the device.
>
> Cheers,
>
> Hannes
> --
> Dr. Hannes Reinecke zSeries & Storage
> hare@suse.com +49 911 74053 688
> SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
> GF: F. Imendörffer, J. Smithard, D. Upmanyu, G. Norton
> HRB 21284 (AG Nürnberg)
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] scsi: scsi_sysfs.c: Hide wwid sdev attr if VPD is not supported
2019-06-19 9:52 ` Marcos Paulo de Souza
@ 2019-06-19 9:57 ` Hannes Reinecke
0 siblings, 0 replies; 7+ messages in thread
From: Hannes Reinecke @ 2019-06-19 9:57 UTC (permalink / raw)
To: Marcos Paulo de Souza
Cc: Martin K. Petersen, linux-kernel, James E.J. Bottomley, linux-scsi
On 6/19/19 11:52 AM, Marcos Paulo de Souza wrote:
> On Wed, Jun 19, 2019 at 08:34:56AM +0200, Hannes Reinecke wrote:
>> On 6/19/19 5:35 AM, Martin K. Petersen wrote:
>>>
>>> Marcos,
>>>
>>>> WWID composed from VPD data from device, specifically page 0x83. So,
>>>> when a device does not have VPD support, for example USB storage
>>>> devices where VPD is specifically disabled, a read into <blk
>>>> device>/device/wwid file will always return ENXIO. To avoid this,
>>>> change the scsi_sdev_attr_is_visible function to hide wwid sysfs file
>>>> when the devices does not support VPD.
>>>
>>> Not a big fan of attribute files that come and go.
>>>
>>> Why not just return an empty string? Hannes?
>>>
>> Actually, the intention of the 'wwid' attribute was to have a common
>> place where one could look up the global id.
>> As such it actually serves a dual purpose, namely indicating that there
>> _is_ a global ID _and_ that this kernel (version) has support for 'wwid'
>> attribute. This is to resolve one big issue we have to udev nowadays,
>> which is figuring out if a specific sysfs attribute is actually
>> supported on this particular kernel.
>> Dynamic attributes are 'nicer' on a conceptual level, but make the above
>> test nearly impossible, as we now have _two_ possibilities why a
>> specific attribute is not present.
>> So making 'wwid' conditional would actually defeat its very purpose, and
>> we should leave it blank if not supported.
>
> My intention was to apply the same approach used for VPD pages, which currently
> also hides the attributes if not supported by the device. So, if vpd pages are
> hidden, there is no usage for wwid. But I also like the idea of the vpd pages
> being blank if not supported by the device.
>
Not quite.
As outlined above, the non-existence of the vpd sysfs attribute doesn't
automatically imply that the device doesn't support VPD pages; we might
as well running on older kernels simply not supporting VPD pages in sysfs.
The whole idea of the wwid is that the attribute is _always_ present, so
we don't have to out-guess the kernel here; if the kernel supports the
wwid attribute it will be present, full stop.
(Background: we do was to avoid doing I/O from uevents, as the events
are handled asynchronously, so by the time the event is handled the
device might not be accesible anymore, leading to a stuck udev process.)
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
hare@suse.com +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-06-19 9:57 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-12 2:08 [PATCH] scsi: scsi_sysfs.c: Hide wwid sdev attr if VPD is not supported Marcos Paulo de Souza
2019-06-12 2:08 ` Marcos Paulo de Souza
2019-06-18 22:47 ` Marcos Paulo de Souza
2019-06-19 3:35 ` Martin K. Petersen
2019-06-19 6:34 ` Hannes Reinecke
2019-06-19 9:52 ` Marcos Paulo de Souza
2019-06-19 9:57 ` Hannes Reinecke
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.