[PATCH] Fix /sys to /dev node name translation

[PATCH] Fix /sys to /dev node name translation

[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).

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 <sbrabec@suse.cz>
---
 lib/sysfs.c          | 14 ++++++++++++--
 libblkid/src/devno.c |  7 ++++++-
 misc-utils/lsblk.c   |  7 +++++++
 3 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/lib/sysfs.c b/lib/sysfs.c
index 759d97b..ffdc8a1 100644
--- a/lib/sysfs.c
+++ b/lib/sysfs.c
@@ -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';
diff --git a/libblkid/src/devno.c b/libblkid/src/devno.c
index f4a36e4..cedd2db 100644
--- a/libblkid/src/devno.c
+++ b/libblkid/src/devno.c
@@ -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);
 
diff --git a/misc-utils/lsblk.c b/misc-utils/lsblk.c
index 1b4ffc1..5cdae7d 100644
--- a/misc-utils/lsblk.c
+++ b/misc-utils/lsblk.c
@@ -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);
 }
 
-- 
2.4.1

-- 
Best Regards / S pozdravem,

Stanislav Brabec
software developer
---------------------------------------------------------------------
SUSE LINUX, s. r. o.                          e-mail: sbrabec@suse.cz
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

Re: [PATCH] Fix /sys to /dev node name translation

[Karel Zak]

On Mon, May 18, 2015 at 09:31:34PM +0200, Stanislav Brabec wrote:
> 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.

> +	/* 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


-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH] Fix /sys to /dev node name translation

[Stanislav Brabec]

Karel Zak wrote:
> On Mon, May 18, 2015 at 09:31:34PM +0200, Stanislav Brabec wrote:
>> 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.

>> +	/* 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).

-- 
Best Regards / S pozdravem,

Stanislav Brabec
software developer
---------------------------------------------------------------------
SUSE LINUX, s. r. o.                          e-mail: sbrabec@suse.cz
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

Re: [PATCH] Fix /sys to /dev node name translation

[Karel Zak]

On Tue, May 19, 2015 at 02:35:54PM +0200, Stanislav Brabec wrote:
> Karel Zak wrote:
> >On Mon, May 18, 2015 at 09:31:34PM +0200, Stanislav Brabec wrote:
> >>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.
> 
> >>+	/* 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).

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_

so

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

 the rest is irrelevant lscpu..

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH] Fix /sys to /dev node name translation

[Stanislav Brabec]

Karel Zak wrote:
> 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_
>
> 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.

-- 
Best Regards / S pozdravem,

Stanislav Brabec
software developer
---------------------------------------------------------------------
SUSE LINUX, s. r. o.                          e-mail: sbrabec@suse.cz
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

[PATCH] Fix /sys to /dev node name translation

[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).

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 <sbrabec@suse.cz>
---
 include/sysfs.h      | 18 ++++++++++++++++++
 lib/sysfs.c          |  2 ++
 libblkid/src/devno.c |  1 +
 misc-utils/lsblk.c   |  1 +
 4 files changed, 22 insertions(+)

diff --git a/include/sysfs.h b/include/sysfs.h
index 1de624a..4564124 100644
--- a/include/sysfs.h
+++ b/include/sysfs.h
@@ -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 */
diff --git a/lib/sysfs.c b/lib/sysfs.c
index 759d97b..8417d2d 100644
--- a/lib/sysfs.c
+++ b/lib/sysfs.c
@@ -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';
diff --git a/libblkid/src/devno.c b/libblkid/src/devno.c
index f4a36e4..3c08227 100644
--- a/libblkid/src/devno.c
+++ b/libblkid/src/devno.c
@@ -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);
 
diff --git a/misc-utils/lsblk.c b/misc-utils/lsblk.c
index 1b4ffc1..d826c77 100644
--- a/misc-utils/lsblk.c
+++ b/misc-utils/lsblk.c
@@ -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);
 }
 
-- 
2.4.1

-- 
Best Regards / S pozdravem,

Stanislav Brabec
software developer
---------------------------------------------------------------------
SUSE LINUX, s. r. o.                          e-mail: sbrabec@suse.cz
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