Fix /sys to /dev node name translation

PATCH In review Stanislav Brabec 2 participants 6 messages May 18, 2015 – May 25, 2015

Subsystems (2)

Library code Util-Linux package

lore

Share & tools

lore

Stanislav Brabec

author

2015-05-18 19:31 UTC #0 lore raw

PATCH 1/1 Fix /sys to /dev node name translation

linux/drivers/base/core.c: device_get_devnode() defines a translation of '!' in sysfs nodes to '/' in /dev nodes. The same translation has to be done to properly support device nodes with slash (e. g. device nodes of cciss driver and several other drivers).

Replace '!' by '/' in sysfs_devno_to_devpath() exactly like kernel does.

The problem is spread across more places of the code.

One of the chunks fixes lsblk -f, other chunks fixes code that apparently assumes that /sys/block and /dev node names are the same, however I do not have any reproducer for them. I fixed all places I found, but it is possible that there are even more occurrences.

Signed-off-by Stanislav Brabec <[email protected]>

---
 lib/sysfs.c          | 14 ++++++++++++--
 libblkid/src/devno.c |  7 ++++++-
 misc-utils/lsblk.c   |  7 +++++++
 3 files changed, 25 insertions(+), 3 deletions(-)

lib/sysfs.c +12 -2

index 759d97b..ffdc8a1 100644
@@ -112,7 +112,7 @@ dev_t sysfs_devname_to_devno(const char *name, const char *parent)
 char *sysfs_devno_to_devpath(dev_t devno, char *buf, size_t bufsiz)
 {
 	struct sysfs_cxt cxt;
-	char *name;
+	char *name, *s;
 	size_t sz;
 	struct stat st;

@@ -130,6 +130,11 @@ char *sysfs_devno_to_devpath(dev_t devno, char *buf, size_t bufsiz)
 	if (sz + sizeof("/dev/") > bufsiz)
 		return NULL;
 
+	/* replace '!' in the name with '/', see
+	 * linux/drivers/base/core.c: device_get_devnode() */
+	while ((s = strchr(name, '!')))
+		s[0] = '/';
+
 	/* create the final "/dev/<name>" string */
 	memmove(buf + 5, name, sz + 1);
 	memcpy(buf, "/dev/", 5);

@@ -776,7 +781,7 @@ int sysfs_devno_to_wholedisk(dev_t dev, char *diskname,
          *  - basename ../../block/sda      = sda
          */
         char linkpath[PATH_MAX];
-        char *name;
+        char *name, *s;
 	ssize_t	linklen;
 
 	linklen = sysfs_readlink(&cxt, NULL, linkpath, sizeof(linkpath) - 1);

@@ -789,6 +794,11 @@ int sysfs_devno_to_wholedisk(dev_t dev, char *diskname,
         if (!name)
             goto err;
 
+	/* replace '!' in the name with '/', see
+	 * linux/drivers/base/core.c: device_get_devnode() */
+	while ((s = strchr(name, '!')))
+		s[0] = '/';
+
         if (diskname && len) {
             strncpy(diskname, name, len);
             diskname[len - 1] = '\0';

libblkid/src/devno.c +6 -1

index f4a36e4..cedd2db 100644
@@ -179,7 +179,7 @@ static const char *devdirs[] = { "/devices", "/devfs", "/dev", NULL };
 static char *scandev_devno_to_devpath(dev_t devno)
 {
 	struct dir_list *list = NULL, *new_list = NULL;
-	char *devname = NULL;
+	char *devname = NULL, *s;
 	const char **dir;
 
 	/*

@@ -208,6 +208,11 @@ static char *scandev_devno_to_devpath(dev_t devno)
 			new_list = NULL;
 		}
 	}
+	/* replace '!' in the name with '/', see
+	 * linux/drivers/base/core.c: device_get_devnode() */
+	if (devname)
+		while ((s = strchr(devname, '!')))
+			s[0] = '/';
 	free_dirlist(&list);
 	free_dirlist(&new_list);

misc-utils/lsblk.c +7 -0

index 1b4ffc1..5cdae7d 100644
@@ -413,6 +413,7 @@ static struct dirent *xreaddir(DIR *dp)
 static char *get_device_path(struct blkdev_cxt *cxt)
 {
 	char path[PATH_MAX];
+	char *s;
 
 	assert(cxt);
 	assert(cxt->name);

@@ -421,6 +422,12 @@ static char *get_device_path(struct blkdev_cxt *cxt)
 		return canonicalize_dm_name(cxt->name);
 
 	snprintf(path, sizeof(path), "/dev/%s", cxt->name);
+
+	/* replace '!' in the name with '/', see
+	 * linux/drivers/base/core.c: device_get_devnode() */
+	while ((s = strchr(path, '!')))
+		s[0] = '/';
+
 	return xstrdup(path);
 }

Signature · 13 lines

2.4.1

-- 
Best Regards / S pozdravem,

Stanislav Brabec
software developer
---------------------------------------------------------------------
SUSE LINUX, s. r. o.                          e-mail: [email protected]
Lihovarská 1060/12                            tel: +49 911 7405384547
190 00 Praha 9                                 fax:  +420 284 084 001
Czech Republic                                    http://www.suse.cz/
PGP: 830B 40D5 9E05 35D8 5E27 6FA3 717C 209F A04F CD76

Karel Zak

maintainer

2015-05-19 08:49 UTC #1 lore raw

Quoted 4 lines · Stanislav Brabec

linux/drivers/base/core.c: device_get_devnode() defines a translation of '!' in sysfs nodes to '/' in /dev nodes. The same translation has to be done to properly support device nodes with slash (e. g. device nodes of cciss driver and several other drivers).

Strange feature.

lib/sysfs.c char *sysfs_devno_to_devpath(dev_t devno, char *buf, size_t bufsiz) · line 136

Quoted 4 lines

lib/sysfs.c +4 -0

+	/* replace '!' in the name with '/', see
+	 * linux/drivers/base/core.c: device_get_devnode() */
+	while ((s = strchr(name, '!')))
+		s[0] = '/';

What about to add small inline function sysfs_fix_devname() to include/sysfs.h ?

Karel

Signature · 2 lines

 Karel Zak  <[email protected]>
 http://karelzak.blogspot.com

Stanislav Brabec

author

2015-05-19 12:35 UTC #2 lore raw

Quoted 7 lines · Karel Zak

Quoted 4 lines · Stanislav Brabec
linux/drivers/base/core.c: device_get_devnode() defines a translation of '!' in sysfs nodes to '/' in /dev nodes. The same translation has to be done to properly support device nodes with slash (e. g. device nodes of cciss driver and several other drivers).
Strange feature.

It was probably introduced to support drivers that already used /dev sub-directories in time of introduction /sys.

Quoted 8 lines

lib/sysfs.c char *sysfs_devno_to_devpath(dev_t devno, char *buf, size_t bufsiz) · line 136
Quoted 4 lines
lib/sysfs.c +4 -0
+	/* replace '!' in the name with '/', see
+	 * linux/drivers/base/core.c: device_get_devnode() */
+	while ((s = strchr(name, '!')))
+		s[0] = '/';
What about to add small inline function sysfs_fix_devname() to include/sysfs.h ?

Yes, it makes sense.

Do you have an idea how to locate all parts of code that make assumptions basename (sysfs_block_path) == basename (dev_path).

Signature · 10 lines

Best Regards / S pozdravem,

Stanislav Brabec
software developer
---------------------------------------------------------------------
SUSE LINUX, s. r. o.                          e-mail: [email protected]
Lihovarská 1060/12                            tel: +49 911 7405384547
190 00 Praha 9                                 fax:  +420 284 084 001
Czech Republic                                    http://www.suse.cz/
PGP: 830B 40D5 9E05 35D8 5E27 6FA3 717C 209F A04F CD76

Karel Zak

maintainer

2015-05-21 11:11 UTC #3 lore raw

Quoted 21 lines · Stanislav Brabec

Quoted 7 lines · Karel Zak
Quoted 4 lines · Stanislav Brabec
linux/drivers/base/core.c: device_get_devnode() defines a translation of '!' in sysfs nodes to '/' in /dev nodes. The same translation has to be done to properly support device nodes with slash (e. g. device nodes of cciss driver and several other drivers).
Strange feature.
It was probably introduced to support drivers that already used /dev sub-directories in time of introduction /sys.
Quoted 8 lines
lib/sysfs.c char *sysfs_devno_to_devpath(dev_t devno, char *buf, size_t bufsiz) · line 136
Quoted 4 lines
lib/sysfs.c +4 -0
+	/* replace '!' in the name with '/', see
+	 * linux/drivers/base/core.c: device_get_devnode() */
+	while ((s = strchr(name, '!')))
+		s[0] = '/';
What about to add small inline function sysfs_fix_devname() to include/sysfs.h ?

Quoted 4 lines

Yes, it makes sense.
Do you have an idea how to locate all parts of code that make assumptions basename (sysfs_block_path) == basename (dev_path).

I hope we use lib/sysfs.c code always when possible. It should be possible to detect all /sys code by

git grep _PATH_SYS_

disk-utils/partx.c
lib/loopdev.c
lib/sysfs.c
libblkid/src/devname.c
misc-utils/lsblk.c

the rest is irrelevant lscpu..

Karel

Signature · 2 lines

 Karel Zak  <[email protected]>
 http://karelzak.blogspot.com

Stanislav Brabec

author

2015-05-22 13:52 UTC #4 lore raw

Quoted 5 lines · Karel Zak

I hope we use lib/sysfs.c code always when possible. It should be possible to detect all /sys code by
git grep _PATH_SYS_

Quoted 8 lines

so
disk-utils/partx.c
lib/loopdev.c
lib/sysfs.c
libblkid/src/devname.c
misc-utils/lsblk.c

I went through these files and none of them seems to do such conversion.

So only the three places in the last patch needs to be fixed.

I already have a new patch introducing static inline void sysfs_devname_to_dev_name (char *name) helper for in-place conversion of '!' to '/', but I am waiting for a chance to make a test on a real hardware next week.

Signature · 10 lines

Best Regards / S pozdravem,

Stanislav Brabec
software developer
---------------------------------------------------------------------
SUSE LINUX, s. r. o.                          e-mail: [email protected]
Lihovarská 1060/12                            tel: +49 911 7405384547
190 00 Praha 9                                 fax:  +420 284 084 001
Czech Republic                                    http://www.suse.cz/
PGP: 830B 40D5 9E05 35D8 5E27 6FA3 717C 209F A04F CD76

Stanislav Brabec

author

2015-05-25 16:21 UTC #5 lore raw

PATCH 1/1 Fix /sys to /dev node name translation

Introduce new helper sysfs_devname_to_devno() and use it where appropriate.

Fixes for example lsblk -f on devices using cciss driver.

Signed-off-by Stanislav Brabec <[email protected]>

---
 include/sysfs.h      | 18 ++++++++++++++++++
 lib/sysfs.c          |  2 ++
 libblkid/src/devno.c |  1 +
 misc-utils/lsblk.c   |  1 +
 4 files changed, 22 insertions(+)

include/sysfs.h +18 -0

index 1de624a..4564124 100644
@@ -91,4 +91,22 @@ extern int sysfs_scsi_host_is(struct sysfs_cxt *cxt, const char *type);
 extern int sysfs_scsi_has_attribute(struct sysfs_cxt *cxt, const char *attr);
 extern int sysfs_scsi_path_contains(struct sysfs_cxt *cxt, const char *pattern);
 
+/**
+ * sysfs_devname_to_dev_name:
+ * @name: devname to be converted in place
+ *
+ * Linux kernel linux/drivers/base/core.c: device_get_devnode()
+ * defines a replacement of '!' in the /sys device name by '/' in the
+ * /dev device name. This helper replaces all ocurrences of '!' in
+ * @name by '/'.
+ */
+static inline void sysfs_devname_to_dev_name (char *name)
+{
+	char *c;
+
+	if (name)
+		while ((c = strchr(name, '!')))
+			c[0] = '/';
+}
+
 #endif /* UTIL_LINUX_SYSFS_H */

lib/sysfs.c +2 -0

index 759d97b..8417d2d 100644
@@ -131,6 +131,7 @@ char *sysfs_devno_to_devpath(dev_t devno, char *buf, size_t bufsiz)
 		return NULL;
 
 	/* create the final "/dev/<name>" string */
+	sysfs_devname_to_dev_name(name);
 	memmove(buf + 5, name, sz + 1);
 	memcpy(buf, "/dev/", 5);

@@ -789,6 +790,7 @@ int sysfs_devno_to_wholedisk(dev_t dev, char *diskname,
         if (!name)
             goto err;
 
+	sysfs_devname_to_dev_name(name);
         if (diskname && len) {
             strncpy(diskname, name, len);
             diskname[len - 1] = '\0';

libblkid/src/devno.c +1 -0

index f4a36e4..3c08227 100644
@@ -208,6 +208,7 @@ static char *scandev_devno_to_devpath(dev_t devno)
 			new_list = NULL;
 		}
 	}
+	sysfs_devname_to_dev_name(devname);
 	free_dirlist(&list);
 	free_dirlist(&new_list);

misc-utils/lsblk.c +1 -0

index 1b4ffc1..d826c77 100644
@@ -421,6 +421,7 @@ static char *get_device_path(struct blkdev_cxt *cxt)
 		return canonicalize_dm_name(cxt->name);
 
 	snprintf(path, sizeof(path), "/dev/%s", cxt->name);
+	sysfs_devname_to_dev_name(path);
 	return xstrdup(path);
 }

Signature · 13 lines

2.4.1

-- 
Best Regards / S pozdravem,

Stanislav Brabec
software developer
---------------------------------------------------------------------
SUSE LINUX, s. r. o.                          e-mail: [email protected]
Lihovarská 1060/12                            tel: +49 911 7405384547
190 00 Praha 9                                 fax:  +420 284 084 001
Czech Republic                                    http://www.suse.cz/
PGP: 830B 40D5 9E05 35D8 5E27 6FA3 717C 209F A04F CD76