[PATCH] sysfs host name support

[PATCH] sysfs host name support

[Kurt Garloff]

[-- Attachment #1.1: Type: text/plain, Size: 759 bytes --]

Hi,

in scsidev, the host adapter name is one of the possible attributes to
match when providing persistent device names.
And the module name is useful to deduce the list of modules that is 
needed when booting.
The proc_name was used previously for /proc/scsi/PROC_NAME/X/ and is
still part of the host template. We might consider ripping is out
or think about syncing it with the module name.

Unfortunately neither is visible in sysfs currently.

Please apply the attached patch.
It exports the 3 names to sysfs under /sys/classes/scsi_hosts/hostX/
name, proc_name, module_name.

Regards,
-- 
Kurt Garloff  <garloff@suse.de>                            Cologne, DE 
SUSE LINUX AG, Nuernberg, DE                          SUSE Labs (Head)

[-- Attachment #1.2: scsi_sysfs-addnames.diff --]
[-- Type: text/plain, Size: 2035 bytes --]

--- drivers/scsi/scsi_sysfs.c.orig	2004-01-09 07:59:10.000000000 +0100
+++ drivers/scsi/scsi_sysfs.c	2004-02-09 16:29:30.226170112 +0100
@@ -58,21 +58,24 @@
  * shost_show_function: macro to create an attr function that can be used to
  * show a non-bit field.
  */
-#define shost_show_function(field, format_string)			\
+#define shost_show_function(name, field, format_string)			\
 static ssize_t								\
-show_##field (struct class_device *class_dev, char *buf)		\
+show_##name (struct class_device *class_dev, char *buf)			\
 {									\
 	struct Scsi_Host *shost = class_to_shost(class_dev);		\
-	return snprintf (buf, 20, format_string, shost->field);	\
+	return snprintf (buf, 20, format_string, shost->field);		\
 }
 
 /*
  * shost_rd_attr: macro to create a function and attribute variable for a
  * read only field.
  */
-#define shost_rd_attr(field, format_string)				\
-	shost_show_function(field, format_string)			\
-static CLASS_DEVICE_ATTR(field, S_IRUGO, show_##field, NULL)
+#define shost_rd_attr2(name, field, format_string)			\
+	shost_show_function(name, field, format_string)			\
+static CLASS_DEVICE_ATTR(name, S_IRUGO, show_##name, NULL)
+
+#define shost_rd_attr(field, format_string) \
+shost_rd_attr2(field, field, format_string)
 
 /*
  * Create the actual show/store functions and data structures.
@@ -96,6 +99,9 @@
 shost_rd_attr(cmd_per_lun, "%hd\n");
 shost_rd_attr(sg_tablesize, "%hu\n");
 shost_rd_attr(unchecked_isa_dma, "%d\n");
+shost_rd_attr2(name, hostt->name, "%s\n");
+shost_rd_attr2(proc_name, hostt->proc_name, "%s\n");
+shost_rd_attr2(module_name, hostt->module->name, "%s\n");
 
 static struct class_device_attribute *scsi_sysfs_shost_attrs[] = {
 	&class_device_attr_unique_id,
@@ -103,6 +109,9 @@
 	&class_device_attr_cmd_per_lun,
 	&class_device_attr_sg_tablesize,
 	&class_device_attr_unchecked_isa_dma,
+	&class_device_attr_name,
+	&class_device_attr_proc_name,
+	&class_device_attr_module_name,
 	&class_device_attr_scan,
 	NULL
 };

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] sysfs host name support

[Christoph Hellwig]

On Mon, Feb 09, 2004 at 04:50:46PM +0100, Kurt Garloff wrote:
> Please apply the attached patch.
> It exports the 3 names to sysfs under /sys/classes/scsi_hosts/hostX/
> name, proc_name, module_name.

name is optional (you can use ->info instead), so please don't export it.
As for proc_name vs module_name - I think we should just export one of them,
I'm pretty open which one.  proc_name probably makes more sense.

> +shost_rd_attr2(module_name, hostt->module->name, "%s\n");

How does this work for builtin modules?  Last time I checked hostt->module
was NULL for those.

Re: [PATCH] sysfs host name support

[James Bottomley]

On Mon, 2004-02-09 at 11:05, Christoph Hellwig wrote:
> name is optional (you can use ->info instead), so please don't export it.
> As for proc_name vs module_name - I think we should just export one of them,
> I'm pretty open which one.  proc_name probably makes more sense.

Actually, no, I'd like to see module name as well.

We need a more generic than SCSI solution for this, I think, but if you
look at most initrd systems, they go through a lot of pattern matching
to try to translate the proc name into the module name.  This would
allow every initrd maker to eliminate those tiresome tables.

James

Re: [PATCH] sysfs host name support

[Christoph Hellwig]

On Mon, Feb 09, 2004 at 11:09:20AM -0500, James Bottomley wrote:
> On Mon, 2004-02-09 at 11:05, Christoph Hellwig wrote:
> > name is optional (you can use ->info instead), so please don't export it.
> > As for proc_name vs module_name - I think we should just export one of them,
> > I'm pretty open which one.  proc_name probably makes more sense.
> 
> Actually, no, I'd like to see module name as well.
> We need a more generic than SCSI solution for this, I think,

Agreed, this makes sense in common code.  In fact I remember the
driver model people were talking about it a while ago.

Re: [PATCH] sysfs host name support

[Kurt Garloff]

[-- Attachment #1.1: Type: text/plain, Size: 874 bytes --]

Hi,

On Mon, Feb 09, 2004 at 04:20:36PM +0000, Christoph Hellwig wrote:
> On Mon, Feb 09, 2004 at 11:09:20AM -0500, James Bottomley wrote:
> > Actually, no, I'd like to see module name as well.
> > We need a more generic than SCSI solution for this, I think,
> 
> Agreed, this makes sense in common code.  In fact I remember the
> driver model people were talking about it a while ago.

OK, let's export the proc_name then. It's some name that can be used
to identify the driver, works for code both compiled statically or
as module, is reasonably short and very often resembles the module
name.

For the module_name, let's wait for a generic sysfs solution. 

Please merge the attached patch.

Regards,
-- 
Kurt Garloff  <garloff@suse.de>                            Cologne, DE 
SUSE LINUX AG, Nuernberg, DE                          SUSE Labs (Head)

[-- Attachment #1.2: scsi_sysfs-addnames2.diff --]
[-- Type: text/plain, Size: 1867 bytes --]

--- drivers/scsi/scsi_sysfs.c.orig	2004-01-09 07:59:10.000000000 +0100
+++ drivers/scsi/scsi_sysfs.c	2004-02-10 01:29:36.903202660 +0100
@@ -58,21 +58,24 @@
  * shost_show_function: macro to create an attr function that can be used to
  * show a non-bit field.
  */
-#define shost_show_function(field, format_string)			\
+#define shost_show_function(name, field, format_string)			\
 static ssize_t								\
-show_##field (struct class_device *class_dev, char *buf)		\
+show_##name (struct class_device *class_dev, char *buf)			\
 {									\
 	struct Scsi_Host *shost = class_to_shost(class_dev);		\
-	return snprintf (buf, 20, format_string, shost->field);	\
+	return snprintf (buf, 20, format_string, shost->field);		\
 }
 
 /*
  * shost_rd_attr: macro to create a function and attribute variable for a
  * read only field.
  */
-#define shost_rd_attr(field, format_string)				\
-	shost_show_function(field, format_string)			\
-static CLASS_DEVICE_ATTR(field, S_IRUGO, show_##field, NULL)
+#define shost_rd_attr2(name, field, format_string)			\
+	shost_show_function(name, field, format_string)			\
+static CLASS_DEVICE_ATTR(name, S_IRUGO, show_##name, NULL)
+
+#define shost_rd_attr(field, format_string) \
+shost_rd_attr2(field, field, format_string)
 
 /*
  * Create the actual show/store functions and data structures.
@@ -96,6 +99,7 @@
 shost_rd_attr(cmd_per_lun, "%hd\n");
 shost_rd_attr(sg_tablesize, "%hu\n");
 shost_rd_attr(unchecked_isa_dma, "%d\n");
+shost_rd_attr2(proc_name, hostt->proc_name, "%s\n");
 
 static struct class_device_attribute *scsi_sysfs_shost_attrs[] = {
 	&class_device_attr_unique_id,
@@ -103,6 +107,7 @@
 	&class_device_attr_cmd_per_lun,
 	&class_device_attr_sg_tablesize,
 	&class_device_attr_unchecked_isa_dma,
+	&class_device_attr_proc_name,
 	&class_device_attr_scan,
 	NULL
 };

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] sysfs host name support

[Kurt Garloff]

[-- Attachment #1.1: Type: text/plain, Size: 629 bytes --]

On Tue, Feb 10, 2004 at 01:33:32AM +0100, Kurt Garloff wrote:
> OK, let's export the proc_name then. It's some name that can be used
> to identify the driver, works for code both compiled statically or
> as module, is reasonably short and very often resembles the module
> name.
> 
> For the module_name, let's wait for a generic sysfs solution. 
> 
> Please merge the attached patch.

Any consensus about this one?

I'd certainly like to see it merged.

Regards,
-- 
Kurt Garloff  <garloff@suse.de>                            Cologne, DE 
SUSE LINUX AG, Nuernberg, DE                          SUSE Labs (Head)

[-- Attachment #1.2: scsi_sysfs-procname-2.diff --]
[-- Type: text/plain, Size: 1879 bytes --]

--- linux/drivers/scsi/scsi_sysfs.c.orig	2004-01-09 07:59:10.000000000 +0100
+++ linux/drivers/scsi/scsi_sysfs.c	2004-02-10 01:29:36.903202660 +0100
@@ -58,21 +58,24 @@
  * shost_show_function: macro to create an attr function that can be used to
  * show a non-bit field.
  */
-#define shost_show_function(field, format_string)			\
+#define shost_show_function(name, field, format_string)			\
 static ssize_t								\
-show_##field (struct class_device *class_dev, char *buf)		\
+show_##name (struct class_device *class_dev, char *buf)			\
 {									\
 	struct Scsi_Host *shost = class_to_shost(class_dev);		\
-	return snprintf (buf, 20, format_string, shost->field);	\
+	return snprintf (buf, 20, format_string, shost->field);		\
 }
 
 /*
  * shost_rd_attr: macro to create a function and attribute variable for a
  * read only field.
  */
-#define shost_rd_attr(field, format_string)				\
-	shost_show_function(field, format_string)			\
-static CLASS_DEVICE_ATTR(field, S_IRUGO, show_##field, NULL)
+#define shost_rd_attr2(name, field, format_string)			\
+	shost_show_function(name, field, format_string)			\
+static CLASS_DEVICE_ATTR(name, S_IRUGO, show_##name, NULL)
+
+#define shost_rd_attr(field, format_string) \
+shost_rd_attr2(field, field, format_string)
 
 /*
  * Create the actual show/store functions and data structures.
@@ -96,6 +99,7 @@
 shost_rd_attr(cmd_per_lun, "%hd\n");
 shost_rd_attr(sg_tablesize, "%hu\n");
 shost_rd_attr(unchecked_isa_dma, "%d\n");
+shost_rd_attr2(proc_name, hostt->proc_name, "%s\n");
 
 static struct class_device_attribute *scsi_sysfs_shost_attrs[] = {
 	&class_device_attr_unique_id,
@@ -103,6 +107,7 @@
 	&class_device_attr_cmd_per_lun,
 	&class_device_attr_sg_tablesize,
 	&class_device_attr_unchecked_isa_dma,
+	&class_device_attr_proc_name,
 	&class_device_attr_scan,
 	NULL
 };

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] sysfs host name support

[Kurt Garloff]

[-- Attachment #1: Type: text/plain, Size: 1457 bytes --]

Hi Christoph,

On Mon, Feb 09, 2004 at 04:05:27PM +0000, Christoph Hellwig wrote:
> On Mon, Feb 09, 2004 at 04:50:46PM +0100, Kurt Garloff wrote:
> > Please apply the attached patch.
> > It exports the 3 names to sysfs under /sys/classes/scsi_hosts/hostX/
> > name, proc_name, module_name.
> 
> name is optional (you can use ->info instead), so please don't export it.

Hmm, I think it's still mandatory, except that it will be overridden
by info(). It's still useful info in any case, I'd still like to export it.

> As for proc_name vs module_name - I think we should just export one of them,
> I'm pretty open which one.  proc_name probably makes more sense.

I can do with just one name. 

Normally, the module name would look most useful, as it can be used by
installers etc. who need to figure what modules are needed.
I would prefer if proc_name would be identical to module name, and
then export the proc_name, which would even work for statically compiled
in drivers.

> > +shost_rd_attr2(module_name, hostt->module->name, "%s\n");
> 
> How does this work for builtin modules?  Last time I checked hostt->module
> was NULL for those.

Nice null-ptr deref Oops. Looks like we need to handcode a function 
for this one. Maybe with fallback to proc_name ...

Regards,
-- 
Kurt Garloff  <garloff@suse.de>                            Cologne, DE 
SUSE LINUX AG, Nuernberg, DE                          SUSE Labs (Head)

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]