[RFC PATCH 0/4] Persistent device name using alias name

[RFC PATCH 0/4] Persistent device name using alias name

[Nao Nishijima]

Hi,

This patch series provides an "alias name" of the disk into kernel and procfs
messages. The user can assign a preferred name to an alias name of the device.

Based on previous discussion (*), I changed patches as follows
- This is "alias name"
- An "alias name" is stored in gendisk struct
- Add document to Documentation/ABI/testing/sysfs-block
- When the user changes an "alias name", kernel notifies udev

(*) http://marc.info/?l=linux-scsi&m=130812625531219&w=2


How to use:
1. Build and install the kernel with this series, and reboot with the kernel.

2. Check device names.

[localhost]# cat /proc/partitions
major minor  #blocks  name

   8        0   12582912 sda
   8        1   12582878 sda1
   8        0    8388608 sdb
   8        1     512000 sdb1
   8        2    7875584 sdb2


3. Make a script of get alias_name

[localhost]# vi /lib/udev/get_alias_name
#!/bin/sh -e
DEVNAME=`echo $1 | sed -e 's/[0-9]//g'`
echo "ALIAS=`cat /sys/block/$DEVNAME/alias_name`"
exit 0

And you should set an execute bit,
[localhost]# chmod +x /lib/udev/get_alias_name

4. Check disk's id
Here is an example to get the serial id and the path of the device.
Some devices have not the serial id. Therefore, to identify a device,
users need to get the path of the device.

[localhost]# udevadm info --query=property --path=/sys/block/sda \
| grep ID_SERIAL=
ID_SERIAL=0QEMU_QEMU_HARDDISK_drive-scsi0-0-1

or you can also use the path of the device

[localhost]# udevadm info --query=property --path=/sys/block/sr0 \
| grep ID_PATH=
ID_PATH=pci-0000:00:01.1-scsi-1:0:0:0


5. Write udev rules as follows
(The user assigns "foo" to sda and "bar" to sr0)
We use ENV{ID_SERIAL} or ENV{ID_PATH} (get by 3) to identify a disk.
And to assign automatically an "alias name", we use ATTR key.
If ENV{ALIAS} is empty, we use to get an "alias_name" by get_alias_name script.

[localhost]# vi /etc/udev/rules.d/70-alias_name.rules
SUBSYSTEM!="block", GOTO="end"

# write alias name for sdX
KERNEL=="sd*[!0-9]", ACTION=="add", ATTR{alias_name}="foo", \
ENV{ID_SERIAL}=="0QEMU_QEMU_HARDDISK_drive-scsi0-0-1"

# write alias name for srX
KERNEL=="sr[0-9]", ACTION=="add", ATTR{alias_name}="bar", \
ENV{ID_PATH}=="pci-0000:00:01.1-scsi-1:0:0:0"

# make symlink
ENV{DEVTYPE}=="disk", ENV{ALIAS}=="?*", SYMLINK+="disk/by-alias/$env{ALIAS}"
ENV{DEVTYPE}=="partition", ENV{ALIAS}=="", \
IMPORT{program}="/lib/udev/get_alias_name %k"
ENV{DEVTYPE}=="partition", ENV{ALIAS}=="?*", \
SYMLINK+="disk/by-alias/$env{ALIAS}%n"

LABEL="end"


6. reboot
After reboot, we can see alias name in kernel and procfs messages.

[localhost]# ls -l /dev/disk/by-alias/
total 0
lrwxrwxrwx. 1 root root  9 Jul  1 21:21 bar -> ../../sr0
lrwxrwxrwx. 1 root root  9 Jul  1 21:21 foo -> ../../sda
lrwxrwxrwx. 1 root root 10 Jul  1 21:21 foo1 -> ../../sda1

[localhost]# dmesg
...
sd 2:0:1:0: [sda] Attached SCSI disk
alias_name: assigned foo to sda
EXT4-fs (foo1): warning: maximal mount count reached, running e2fsck is recommended
EXT4-fs (foo1): mounted filesystem with ordered data mode. Opts: (null)
...

[localhost]# cat /proc/partitions
major minor  #blocks  name

   8        0   12582912 foo
   8        1   12582878 foo1
   8        0    8388608 sdb
   8        1     512000 sdb1
   8        2    7875584 sdb2



When a new device is added, the udev appends a new rule manually.
In the future, it is appended automatically, as like NIC.

TODO:
- Modify blkid to show "alias name"

Best Regards,

---

Nao Nishijima (4):
      sd: cleanup for alias name
      fs: modify disk_name() for alias name
      sd: modify printk for alias_name
      block: add a new attribute "alias name" in gendisk structure


 Documentation/ABI/testing/sysfs-block |   15 ++++++
 block/genhd.c                         |   85 +++++++++++++++++++++++++++++++++
 drivers/scsi/sd.c                     |    7 ++-
 drivers/scsi/sd.h                     |    2 -
 fs/partitions/check.c                 |    6 +-
 include/linux/genhd.h                 |    4 ++
 include/scsi/scsi_device.h            |    3 +
 7 files changed, 114 insertions(+), 8 deletions(-)


--
Nao Nishijima (nao.nishijima.xt@hitachi.com)

[RFC PATCH 1/4] block: add a new attribute "alias name" in gendisk structure

[Nao Nishijima]

This patch allows the user to set an "alias name" of the disk via sysfs interface.

A raw device name of a disk does not always point a same disk each boot-up time.
Therefore, users have to use persistent device names, which udev create to always
access the same disk. However, kernel messages still display device names.

This patch adds a new attribute "alias name" in gendisk structure. And if users
set their preferred name to an "alias name" of the disk, it would be appeared
in kernel messages. A disk can have an "alias name" which length is up to
255bytes. Users can use alphabets, numbers, '-' and '_' in alias name.

Suggested-by: James Bottomley <James.Bottomley@HansenPartnership.com>
Suggested-by: Jon Masters <jcm@redhat.com>
Signed-off-by: Nao Nishijima <nao.nishijima.xt@hitachi.com>
---

 Documentation/ABI/testing/sysfs-block |   15 ++++++
 block/genhd.c                         |   85 +++++++++++++++++++++++++++++++++
 include/linux/genhd.h                 |    4 ++
 3 files changed, 104 insertions(+), 0 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-block b/Documentation/ABI/testing/sysfs-block
index c1eb41c..07c4fcd 100644
--- a/Documentation/ABI/testing/sysfs-block
+++ b/Documentation/ABI/testing/sysfs-block
@@ -206,3 +206,18 @@ Description:
 		when a discarded area is read the discard_zeroes_data
 		parameter will be set to one. Otherwise it will be 0 and
 		the result of reading a discarded area is undefined.
+
+What:		/sys/block/<disk>/alias_name
+Date:		June 2011
+Contact:	Nao Nishijima <nao.nishijima.xt@hitachi.com>
+Description:
+		A raw device name of a disk does not always point a same disk
+		each boot-up time. Therefore, users have to use persistent
+		device names, which udev creates when the kernel finds a disk,
+		instead of raw device name. However, kernel doesn't show those
+		persistent names on its message (e.g. dmesg).
+		This file can store an alias name of the disk and it would be
+		appeared in kernel messages if it is set. A disk can have an
+		alias name which length is up to 255bytes. Users can use
+		use alphabets, numbers, "-" and "_" in alias name and can
+		change it anytime.
diff --git a/block/genhd.c b/block/genhd.c
index 3608289..4363947 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -19,6 +19,7 @@
 #include <linux/mutex.h>
 #include <linux/idr.h>
 #include <linux/log2.h>
+#include <linux/ctype.h>
 
 #include "blk.h"
 
@@ -909,6 +910,87 @@ static int __init genhd_device_init(void)
 
 subsys_initcall(genhd_device_init);
 
+static ssize_t alias_name_show(struct device *dev,
+			       struct device_attribute *attr, char *buf)
+{
+	struct gendisk *disk = dev_to_disk(dev);
+	ssize_t ret = 0;
+
+	if (disk->alias_name)
+		ret = snprintf(buf, ALIAS_NAME_LEN + 1, "%s\n",
+			       disk->alias_name);
+	return ret;
+}
+
+static ssize_t alias_name_store(struct device *dev,
+				struct device_attribute *attr, const char *buf,
+				size_t count)
+{
+	struct gendisk *disk = dev_to_disk(dev);
+	struct disk_part_iter piter;
+	struct hd_struct *part;
+
+	char *new_alias_name;
+	char *envp[] = { NULL, NULL };
+	unsigned char c;
+	int idx;
+	ssize_t ret = count;
+
+	if (!count)
+		return -EINVAL;
+
+	if (count >= ALIAS_NAME_LEN) {
+		printk(KERN_ERR "alias_name: alias name is too long\n");
+		return -EINVAL;
+	}
+
+	/* Validation check */
+	for (idx = 0; idx < count; idx++) {
+		c = buf[idx];
+		if (idx == count - 1 && c == '\n')
+			break;
+		if (!isalnum(c) && c != '_' && c != '-') {
+			printk(KERN_ERR "alias_name: invalid alias name\n");
+			return -EINVAL;
+		}
+	}
+
+	new_alias_name = kasprintf(GFP_KERNEL, "%s", buf);
+	if (!new_alias_name)
+		return -ENOMEM;
+
+	if (new_alias_name[count - 1] == '\n')
+		new_alias_name[count - 1] = '\0';
+
+	envp[0] = kasprintf(GFP_KERNEL, "ALIAS=%s", new_alias_name);
+	if (!envp[0]) {
+		kfree(new_alias_name);
+		return -ENOMEM;
+	}
+
+	if (disk->alias_name[0] != '\0')
+		printk(KERN_INFO "alias_name: assigned %s to %s (previous "
+		       "alias_name was %s)\n",
+		       new_alias_name, disk->disk_name, disk->alias_name);
+	else
+		printk(KERN_INFO "alias_name: assigned %s to %s\n",
+		       new_alias_name, disk->disk_name);
+
+	snprintf(disk->alias_name, ALIAS_NAME_LEN, "%s", new_alias_name);
+
+	kobject_uevent_env(&dev->kobj, KOBJ_CHANGE, envp);
+
+	/* announce possible partitions */
+	disk_part_iter_init(&piter, disk, 0);
+	while ((part = disk_part_iter_next(&piter)))
+		kobject_uevent_env(&part_to_dev(part)->kobj, KOBJ_CHANGE, envp);
+	disk_part_iter_exit(&piter);
+
+	kfree(envp[0]);
+	kfree(new_alias_name);
+	return ret;
+}
+
 static ssize_t disk_range_show(struct device *dev,
 			       struct device_attribute *attr, char *buf)
 {
@@ -968,6 +1050,8 @@ static ssize_t disk_discard_alignment_show(struct device *dev,
 	return sprintf(buf, "%d\n", queue_discard_alignment(disk->queue));
 }
 
+static DEVICE_ATTR(alias_name, S_IRUGO|S_IWUSR, alias_name_show,
+		   alias_name_store);
 static DEVICE_ATTR(range, S_IRUGO, disk_range_show, NULL);
 static DEVICE_ATTR(ext_range, S_IRUGO, disk_ext_range_show, NULL);
 static DEVICE_ATTR(removable, S_IRUGO, disk_removable_show, NULL);
@@ -990,6 +1074,7 @@ static struct device_attribute dev_attr_fail_timeout =
 #endif
 
 static struct attribute *disk_attrs[] = {
+	&dev_attr_alias_name.attr,
 	&dev_attr_range.attr,
 	&dev_attr_ext_range.attr,
 	&dev_attr_removable.attr,
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 300d758..a3c219d 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -21,6 +21,8 @@
 #define dev_to_part(device)	container_of((device), struct hd_struct, __dev)
 #define disk_to_dev(disk)	(&(disk)->part0.__dev)
 #define part_to_dev(part)	(&((part)->__dev))
+#define alias_name(disk)	((disk)->alias_name[0] != '\0' \
+				   ? (disk)->alias_name : (disk)->disk_name)
 
 extern struct device_type part_type;
 extern struct kobject *block_depr;
@@ -58,6 +60,7 @@ enum {
 
 #define DISK_MAX_PARTS			256
 #define DISK_NAME_LEN			32
+#define ALIAS_NAME_LEN			256
 
 #include <linux/major.h>
 #include <linux/device.h>
@@ -162,6 +165,7 @@ struct gendisk {
                                          * disks that can't be partitioned. */
 
 	char disk_name[DISK_NAME_LEN];	/* name of major driver */
+	char alias_name[ALIAS_NAME_LEN];/* alias name of disk */
 	char *(*devnode)(struct gendisk *gd, mode_t *mode);
 
 	unsigned int events;		/* supported events */

[RFC PATCH 2/4] sd: modify printk for alias_name

[Nao Nishijima]

This patch modify sd_printk() and scmd_printk() to use alias_name. If user set
an alias_name, those print an alias_name instead of a disk_name.

Suggested-by: James Bottomley <James.Bottomley@HansenPartnership.com>
Suggested-by: Jon Masters <jonathan@jonmasters.org>
Signed-off-by: Nao Nishijima <nao.nishijima.xt@hitachi.com>
Reviewed-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---

 drivers/scsi/sd.h          |    2 +-
 include/scsi/scsi_device.h |    3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
index 6ad798b..66c7ae8 100644
--- a/drivers/scsi/sd.h
+++ b/drivers/scsi/sd.h
@@ -91,7 +91,7 @@ static inline struct scsi_disk *scsi_disk(struct gendisk *disk)
 #define sd_printk(prefix, sdsk, fmt, a...)				\
         (sdsk)->disk ?							\
 	sdev_printk(prefix, (sdsk)->device, "[%s] " fmt,		\
-		    (sdsk)->disk->disk_name, ##a) :			\
+		    alias_name((sdsk)->disk), ##a) :			\
 	sdev_printk(prefix, (sdsk)->device, fmt, ##a)
 
 /*
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index dd82e02..125c05c 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -6,6 +6,7 @@
 #include <linux/spinlock.h>
 #include <linux/workqueue.h>
 #include <linux/blkdev.h>
+#include <linux/genhd.h>
 #include <scsi/scsi.h>
 #include <asm/atomic.h>
 
@@ -219,7 +220,7 @@ struct scsi_dh_data {
 #define scmd_printk(prefix, scmd, fmt, a...)				\
         (scmd)->request->rq_disk ?					\
 	sdev_printk(prefix, (scmd)->device, "[%s] " fmt,		\
-		    (scmd)->request->rq_disk->disk_name, ##a) :		\
+		    alias_name((scmd)->request->rq_disk), ##a) :	\
 	sdev_printk(prefix, (scmd)->device, fmt, ##a)
 
 enum scsi_target_state {

[PATCH] scsi: Make functions out of logging macros.

[Joe Perches]

Reduce size of code and text of scmd_printk and sd_printk
macros by converting to the macros to functions and using
vsprintf extension %pV.  This moves the code out-of-line
and centralizes the code used to emit additional arguments.

Save ~32KB of space in an x86 allyesconfig.

$ size drivers/scsi/built-in.o*
   text	   data	    bss	    dec	    hex	filename
5860439	 135396	1393024	7388859	 70bebb	drivers/scsi/built-in.o.new
5882368	 135396	1395848	7413612	 711f6c	drivers/scsi/built-in.o.old
5887100	 135396	1397264	7419760	 713770	drivers/scsi/built-in.o.with_patch_1_and_2

Signed-off-by: Joe Perches <joe@perches.com>

---

Re: [RFC PATCH 2/4] sd: modify printk for alias_name

On Fri, 2011-07-08 at 17:46 +0900, Nao Nishijima wrote: 
> This patch modify sd_printk() and scmd_printk() to use alias_name. If user set
> an alias_name, those print an alias_name instead of a disk_name.
 
Instead of larding more function/macros into these
relatively heavily used logging macros, how about
converting the logging macros into functions?

 drivers/scsi/scsi_lib.c    |   26 ++++++++++++++++++++++++++
 drivers/scsi/sd.c          |   23 +++++++++++++++++++++++
 drivers/scsi/sd.h          |    8 +++-----
 include/scsi/scsi_device.h |    8 +++-----
 4 files changed, 55 insertions(+), 10 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index ec1803a..249c54c 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2571,3 +2571,29 @@ void scsi_kunmap_atomic_sg(void *virt)
 	kunmap_atomic(virt, KM_BIO_SRC_IRQ);
 }
 EXPORT_SYMBOL(scsi_kunmap_atomic_sg);
+
+/* Logging utilities */
+
+int scmd_printk(const char *prefix, const struct scsi_cmnd *scmd,
+		const char *format, ...)
+{
+	struct va_format vaf;
+	va_list args;
+	int r;
+
+	va_start(args, format);
+
+	vaf.fmt = format;
+	vaf.va = &args;
+
+	if (scmd->request->rq_disk)
+		r = sdev_printk(prefix, scmd->device, "[%s] %pV",
+				alias_name(scmd->request->rq_disk), &vaf);
+	else
+		r = sdev_printk(prefix, scmd->device, "%pV", &vaf);
+
+	va_end(args);
+
+	return r;
+}
+EXPORT_SYMBOL(scmd_printk);
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 953773c..2251e01 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -2888,3 +2888,26 @@ static void sd_print_result(struct scsi_disk *sdkp, int result)
 	scsi_show_result(result);
 }
 
+int sd_printk(const char *prefix, const struct scsi_disk *sdsk,
+	      const char *format, ...)
+{
+	struct va_format vaf;
+	va_list args;
+	int r;
+
+	va_start(args, format);
+
+	vaf.fmt = format;
+	vaf.va = &args;
+
+	if (sdsk->disk)
+		r = sdev_printk(prefix, sdsk->device, "[%s] %pV",
+				alias_name(sdsk->disk), &vaf);
+	else
+		r = sdev_printk(prefix, sdsk->device, "%pV", &vaf);
+
+	va_end(args);
+
+	return r;
+}
+EXPORT_SYMBOL(sd_printk);
diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
index 6ad798b..46aa748 100644
--- a/drivers/scsi/sd.h
+++ b/drivers/scsi/sd.h
@@ -88,11 +88,9 @@ static inline struct scsi_disk *scsi_disk(struct gendisk *disk)
 	return container_of(disk->private_data, struct scsi_disk, driver);
 }
 
-#define sd_printk(prefix, sdsk, fmt, a...)				\
-        (sdsk)->disk ?							\
-	sdev_printk(prefix, (sdsk)->device, "[%s] " fmt,		\
-		    (sdsk)->disk->disk_name, ##a) :			\
-	sdev_printk(prefix, (sdsk)->device, fmt, ##a)
+extern __attribute__((format (printf, 3, 4)))
+int sd_printk(const char *prefix, const struct scsi_disk *sdsk,
+	      const char *format, ...);
 
 /*
  * A DIF-capable target device can be formatted with different
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index dd82e02..c79631b 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -216,11 +216,9 @@ struct scsi_dh_data {
 #define sdev_printk(prefix, sdev, fmt, a...)	\
 	dev_printk(prefix, &(sdev)->sdev_gendev, fmt, ##a)
 
-#define scmd_printk(prefix, scmd, fmt, a...)				\
-        (scmd)->request->rq_disk ?					\
-	sdev_printk(prefix, (scmd)->device, "[%s] " fmt,		\
-		    (scmd)->request->rq_disk->disk_name, ##a) :		\
-	sdev_printk(prefix, (scmd)->device, fmt, ##a)
+extern __attribute__((format (printf, 3, 4)))
+int scmd_printk(const char *prefix, const struct scsi_cmnd *scmd,
+		const char *format, ...);
 
 enum scsi_target_state {
 	STARGET_CREATED = 1,
-- 
1.7.6.131.g99019

Re: [PATCH] scsi: Make functions out of logging macros.

[Nao Nishijima]

Hi, Joe

Thank you for looking at this patch.
Your patch sounds great. I will test the patch.

Best regards,

(2011/07/09 14:42), Joe Perches wrote:
> Reduce size of code and text of scmd_printk and sd_printk
> macros by converting to the macros to functions and using
> vsprintf extension %pV.  This moves the code out-of-line
> and centralizes the code used to emit additional arguments.
> 
> Save ~32KB of space in an x86 allyesconfig.
> 
> $ size drivers/scsi/built-in.o*
>    text	   data	    bss	    dec	    hex	filename
> 5860439	 135396	1393024	7388859	 70bebb	drivers/scsi/built-in.o.new
> 5882368	 135396	1395848	7413612	 711f6c	drivers/scsi/built-in.o.old
> 5887100	 135396	1397264	7419760	 713770	drivers/scsi/built-in.o.with_patch_1_and_2
> 
> Signed-off-by: Joe Perches <joe@perches.com>
> 
> ---
> 
> Re: [RFC PATCH 2/4] sd: modify printk for alias_name
> 
> On Fri, 2011-07-08 at 17:46 +0900, Nao Nishijima wrote: 
>> This patch modify sd_printk() and scmd_printk() to use alias_name. If user set
>> an alias_name, those print an alias_name instead of a disk_name.
>  
> Instead of larding more function/macros into these
> relatively heavily used logging macros, how about
> converting the logging macros into functions?
> 
>  drivers/scsi/scsi_lib.c    |   26 ++++++++++++++++++++++++++
>  drivers/scsi/sd.c          |   23 +++++++++++++++++++++++
>  drivers/scsi/sd.h          |    8 +++-----
>  include/scsi/scsi_device.h |    8 +++-----
>  4 files changed, 55 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index ec1803a..249c54c 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -2571,3 +2571,29 @@ void scsi_kunmap_atomic_sg(void *virt)
>  	kunmap_atomic(virt, KM_BIO_SRC_IRQ);
>  }
>  EXPORT_SYMBOL(scsi_kunmap_atomic_sg);
> +
> +/* Logging utilities */
> +
> +int scmd_printk(const char *prefix, const struct scsi_cmnd *scmd,
> +		const char *format, ...)
> +{
> +	struct va_format vaf;
> +	va_list args;
> +	int r;
> +
> +	va_start(args, format);
> +
> +	vaf.fmt = format;
> +	vaf.va = &args;
> +
> +	if (scmd->request->rq_disk)
> +		r = sdev_printk(prefix, scmd->device, "[%s] %pV",
> +				alias_name(scmd->request->rq_disk), &vaf);
> +	else
> +		r = sdev_printk(prefix, scmd->device, "%pV", &vaf);
> +
> +	va_end(args);
> +
> +	return r;
> +}
> +EXPORT_SYMBOL(scmd_printk);
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index 953773c..2251e01 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -2888,3 +2888,26 @@ static void sd_print_result(struct scsi_disk *sdkp, int result)
>  	scsi_show_result(result);
>  }
>  
> +int sd_printk(const char *prefix, const struct scsi_disk *sdsk,
> +	      const char *format, ...)
> +{
> +	struct va_format vaf;
> +	va_list args;
> +	int r;
> +
> +	va_start(args, format);
> +
> +	vaf.fmt = format;
> +	vaf.va = &args;
> +
> +	if (sdsk->disk)
> +		r = sdev_printk(prefix, sdsk->device, "[%s] %pV",
> +				alias_name(sdsk->disk), &vaf);
> +	else
> +		r = sdev_printk(prefix, sdsk->device, "%pV", &vaf);
> +
> +	va_end(args);
> +
> +	return r;
> +}
> +EXPORT_SYMBOL(sd_printk);
> diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
> index 6ad798b..46aa748 100644
> --- a/drivers/scsi/sd.h
> +++ b/drivers/scsi/sd.h
> @@ -88,11 +88,9 @@ static inline struct scsi_disk *scsi_disk(struct gendisk *disk)
>  	return container_of(disk->private_data, struct scsi_disk, driver);
>  }
>  
> -#define sd_printk(prefix, sdsk, fmt, a...)				\
> -        (sdsk)->disk ?							\
> -	sdev_printk(prefix, (sdsk)->device, "[%s] " fmt,		\
> -		    (sdsk)->disk->disk_name, ##a) :			\
> -	sdev_printk(prefix, (sdsk)->device, fmt, ##a)
> +extern __attribute__((format (printf, 3, 4)))
> +int sd_printk(const char *prefix, const struct scsi_disk *sdsk,
> +	      const char *format, ...);
>  
>  /*
>   * A DIF-capable target device can be formatted with different
> diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
> index dd82e02..c79631b 100644
> --- a/include/scsi/scsi_device.h
> +++ b/include/scsi/scsi_device.h
> @@ -216,11 +216,9 @@ struct scsi_dh_data {
>  #define sdev_printk(prefix, sdev, fmt, a...)	\
>  	dev_printk(prefix, &(sdev)->sdev_gendev, fmt, ##a)
>  
> -#define scmd_printk(prefix, scmd, fmt, a...)				\
> -        (scmd)->request->rq_disk ?					\
> -	sdev_printk(prefix, (scmd)->device, "[%s] " fmt,		\
> -		    (scmd)->request->rq_disk->disk_name, ##a) :		\
> -	sdev_printk(prefix, (scmd)->device, fmt, ##a)
> +extern __attribute__((format (printf, 3, 4)))
> +int scmd_printk(const char *prefix, const struct scsi_cmnd *scmd,
> +		const char *format, ...);
>  
>  enum scsi_target_state {
>  	STARGET_CREATED = 1,


-- 
Nao NISHIJIMA
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., YOKOHAMA Research  Laboratory
Email： nao.nishijima.xt@hitachi.com

[RFC PATCH 3/4] fs: modify disk_name() for alias name

[Nao Nishijima]

Make disk_name() return alais_name instead of disk_name when alias_name is set.
disk_name() is used in /proc/{partitions, diskstats}. Therefore, those files
show alias_name.

Signed-off-by: Nao Nishijima <nao.nishijima.xt@hitachi.com>
Reviewed-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---

 Documentation/ABI/testing/sysfs-block |    8 ++++----
 fs/partitions/check.c                 |    6 +++---
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-block b/Documentation/ABI/testing/sysfs-block
index 07c4fcd..432140c 100644
--- a/Documentation/ABI/testing/sysfs-block
+++ b/Documentation/ABI/testing/sysfs-block
@@ -217,7 +217,7 @@ Description:
 		instead of raw device name. However, kernel doesn't show those
 		persistent names on its message (e.g. dmesg).
 		This file can store an alias name of the disk and it would be
-		appeared in kernel messages if it is set. A disk can have an
-		alias name which length is up to 255bytes. Users can use
-		use alphabets, numbers, "-" and "_" in alias name and can
-		change it anytime.
+		appeared in kernel messages and procfs if it is set. A disk
+		can have an alias name which length is up to 255bytes. Users
+		can use use alphabets, numbers, "-" and "_" in alias name and
+		can change it anytime.
diff --git a/fs/partitions/check.c b/fs/partitions/check.c
index d545e97..d0bfe56 100644
--- a/fs/partitions/check.c
+++ b/fs/partitions/check.c
@@ -125,11 +125,11 @@ static int (*check_part[])(struct parsed_partitions *) = {
 char *disk_name(struct gendisk *hd, int partno, char *buf)
 {
 	if (!partno)
-		snprintf(buf, BDEVNAME_SIZE, "%s", hd->disk_name);
+		snprintf(buf, BDEVNAME_SIZE, "%s", alias_name(hd));
 	else if (isdigit(hd->disk_name[strlen(hd->disk_name)-1]))
-		snprintf(buf, BDEVNAME_SIZE, "%sp%d", hd->disk_name, partno);
+		snprintf(buf, BDEVNAME_SIZE, "%sp%d", alias_name(hd), partno);
 	else
-		snprintf(buf, BDEVNAME_SIZE, "%s%d", hd->disk_name, partno);
+		snprintf(buf, BDEVNAME_SIZE, "%s%d", alias_name(hd), partno);
 
 	return buf;
 }

[RFC PATCH 4/4] sd: cleanup for alias name

[Nao Nishijima]

Use sd_printk() instead of printk().

Signed-off-by: Nao Nishijima <nao.nishijima.xt@hitachi.com>
Reviewed-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---

 drivers/scsi/sd.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 953773c..e434e5f 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -1066,12 +1066,13 @@ static int sd_ioctl(struct block_device *bdev, fmode_t mode,
 		    unsigned int cmd, unsigned long arg)
 {
 	struct gendisk *disk = bdev->bd_disk;
-	struct scsi_device *sdp = scsi_disk(disk)->device;
+	struct scsi_disk *sdkp = scsi_disk(disk);
+	struct scsi_device *sdp = sdkp->device;
 	void __user *p = (void __user *)arg;
 	int error;
     
-	SCSI_LOG_IOCTL(1, printk("sd_ioctl: disk=%s, cmd=0x%x\n",
-						disk->disk_name, cmd));
+	SCSI_LOG_IOCTL(1, sd_printk(KERN_INFO, sdkp, "sd_ioctl: disk=%s,"
+		       " cmd=0x%x\n", disk->disk_name, cmd));
 
 	/*
 	 * If we are in the middle of error recovery, don't let anyone

Re: [RFC PATCH 0/4] Persistent device name using alias name

[Greg KH]

On Fri, Jul 08, 2011 at 05:45:47PM +0900, Nao Nishijima wrote:
> Hi,
> 
> This patch series provides an "alias name" of the disk into kernel and procfs
> messages. The user can assign a preferred name to an alias name of the device.
> 
> Based on previous discussion (*), I changed patches as follows
> - This is "alias name"
> - An "alias name" is stored in gendisk struct
> - Add document to Documentation/ABI/testing/sysfs-block
> - When the user changes an "alias name", kernel notifies udev
> 
> (*) http://marc.info/?l=linux-scsi&m=130812625531219&w=2

I don't like it and I don't think it will really solve the root problem
you are trying to address, but as the patches don't touch any code I
maintain, there's not much I can do to object to it.

greg k-h

Re: [RFC PATCH 0/4] Persistent device name using alias name

[Kay Sievers]

On Fri, Jul 8, 2011 at 16:54, Greg KH <greg@kroah.com> wrote:
> On Fri, Jul 08, 2011 at 05:45:47PM +0900, Nao Nishijima wrote:
>> This patch series provides an "alias name" of the disk into kernel and procfs
>> messages. The user can assign a preferred name to an alias name of the device.
>>
>> Based on previous discussion (*), I changed patches as follows
>> - This is "alias name"
>> - An "alias name" is stored in gendisk struct
>> - Add document to Documentation/ABI/testing/sysfs-block
>> - When the user changes an "alias name", kernel notifies udev
>>
>> (*) http://marc.info/?l=linux-scsi&m=130812625531219&w=2
>
> I don't like it and I don't think it will really solve the root problem
> you are trying to address, but as the patches don't touch any code I
> maintain, there's not much I can do to object to it.

I can only repeat what I already wrote in detail earlier:

This approach seems to papers over the problem which emitting and
parsing free-text printk() messages with much-too-dumb tools cause. It
seems to fix the symptoms not the cause.

You can already write a udev rule today that logs _all_ symlinks of a
device at discovery time, and any later kernel message can safely be
associated with all possible names of the blockdev. No kernel changes
needed, all possible names are covered. That also works good enough
with our current stone-age tools for anybody who is able to scroll
back to the last log udev message in that same log file.

There can be by-definition no default udev rules assigning a proper
single name to a block device. There is never a valid single name for
a disks, so udev can not ship anything like that in the default setup,
so this stays as a custom hack.

We absolutely need _structured_ data for logging and error reporting,
not only to solve this problem. Along with the current free-text
printk(), we would be able to attach classifications, device
error/sense data, firmware register dumps and anything
interesting-for-debug to the messages.

We can't solve that problem in the kernel alone. Structured data from
the kernel will need to feed a smarter userspace logger that can index
and classify messages, merge current userspace data into it, and
provides hooks for the system management to act on critical failures
and raise notifications.

Structured logging seems like the solution for this and also to many
other problems in this area. Single custom names pushed into the
kernel might cover some rather exotic use cases, but I think, is not
what we are looking for.

Kay

Re: [RFC PATCH 0/4] Persistent device name using alias name

[Greg KH]

On Fri, Jul 08, 2011 at 05:41:36PM +0200, Kay Sievers wrote:
> On Fri, Jul 8, 2011 at 16:54, Greg KH <greg@kroah.com> wrote:
> > On Fri, Jul 08, 2011 at 05:45:47PM +0900, Nao Nishijima wrote:
> >> This patch series provides an "alias name" of the disk into kernel and procfs
> >> messages. The user can assign a preferred name to an alias name of the device.
> >>
> >> Based on previous discussion (*), I changed patches as follows
> >> - This is "alias name"
> >> - An "alias name" is stored in gendisk struct
> >> - Add document to Documentation/ABI/testing/sysfs-block
> >> - When the user changes an "alias name", kernel notifies udev
> >>
> >> (*) http://marc.info/?l=linux-scsi&m=130812625531219&w=2
> >
> > I don't like it and I don't think it will really solve the root problem
> > you are trying to address, but as the patches don't touch any code I
> > maintain, there's not much I can do to object to it.
> 
> I can only repeat what I already wrote in detail earlier:
> 
> This approach seems to papers over the problem which emitting and
> parsing free-text printk() messages with much-too-dumb tools cause. It
> seems to fix the symptoms not the cause.
> 
> You can already write a udev rule today that logs _all_ symlinks of a
> device at discovery time, and any later kernel message can safely be
> associated with all possible names of the blockdev. No kernel changes
> needed, all possible names are covered. That also works good enough
> with our current stone-age tools for anybody who is able to scroll
> back to the last log udev message in that same log file.
> 
> There can be by-definition no default udev rules assigning a proper
> single name to a block device. There is never a valid single name for
> a disks, so udev can not ship anything like that in the default setup,
> so this stays as a custom hack.
> 
> We absolutely need _structured_ data for logging and error reporting,
> not only to solve this problem. Along with the current free-text
> printk(), we would be able to attach classifications, device
> error/sense data, firmware register dumps and anything
> interesting-for-debug to the messages.
> 
> We can't solve that problem in the kernel alone. Structured data from
> the kernel will need to feed a smarter userspace logger that can index
> and classify messages, merge current userspace data into it, and
> provides hooks for the system management to act on critical failures
> and raise notifications.
> 
> Structured logging seems like the solution for this and also to many
> other problems in this area. Single custom names pushed into the
> kernel might cover some rather exotic use cases, but I think, is not
> what we are looking for.

I totally agree, but hey, no one listens to us :)

greg k-h

Re: [RFC PATCH 0/4] Persistent device name using alias name

[James Bottomley]

On Fri, 2011-07-08 at 08:47 -0700, Greg KH wrote:
> On Fri, Jul 08, 2011 at 05:41:36PM +0200, Kay Sievers wrote:
> > On Fri, Jul 8, 2011 at 16:54, Greg KH <greg@kroah.com> wrote:
> > > On Fri, Jul 08, 2011 at 05:45:47PM +0900, Nao Nishijima wrote:
> > >> This patch series provides an "alias name" of the disk into kernel and procfs
> > >> messages. The user can assign a preferred name to an alias name of the device.
> > >>
> > >> Based on previous discussion (*), I changed patches as follows
> > >> - This is "alias name"
> > >> - An "alias name" is stored in gendisk struct
> > >> - Add document to Documentation/ABI/testing/sysfs-block
> > >> - When the user changes an "alias name", kernel notifies udev
> > >>
> > >> (*) http://marc.info/?l=linux-scsi&m=130812625531219&w=2
> > >
> > > I don't like it and I don't think it will really solve the root problem
> > > you are trying to address, but as the patches don't touch any code I
> > > maintain, there's not much I can do to object to it.
> > 
> > I can only repeat what I already wrote in detail earlier:
> > 
> > This approach seems to papers over the problem which emitting and
> > parsing free-text printk() messages with much-too-dumb tools cause. It
> > seems to fix the symptoms not the cause.
> > 
> > You can already write a udev rule today that logs _all_ symlinks of a
> > device at discovery time, and any later kernel message can safely be
> > associated with all possible names of the blockdev. No kernel changes
> > needed, all possible names are covered. That also works good enough
> > with our current stone-age tools for anybody who is able to scroll
> > back to the last log udev message in that same log file.
> > 
> > There can be by-definition no default udev rules assigning a proper
> > single name to a block device. There is never a valid single name for
> > a disks, so udev can not ship anything like that in the default setup,
> > so this stays as a custom hack.
> > 
> > We absolutely need _structured_ data for logging and error reporting,
> > not only to solve this problem. Along with the current free-text
> > printk(), we would be able to attach classifications, device
> > error/sense data, firmware register dumps and anything
> > interesting-for-debug to the messages.
> > 
> > We can't solve that problem in the kernel alone. Structured data from
> > the kernel will need to feed a smarter userspace logger that can index
> > and classify messages, merge current userspace data into it, and
> > provides hooks for the system management to act on critical failures
> > and raise notifications.
> > 
> > Structured logging seems like the solution for this and also to many
> > other problems in this area. Single custom names pushed into the
> > kernel might cover some rather exotic use cases, but I think, is not
> > what we are looking for.
> 
> I totally agree, but hey, no one listens to us :)

Yes, we did.  Everyone agrees structured logging would be the best long
term solution.  However, it's at least 10x the work presented here, plus
it would be a long process getting everyone to agree.  This looks like a
good 95% interim solution and it can be removed when structured logging
makes everything "just work(tm)".

I have also seen a couple of other attempts at structured logging which
both failed when the people proposing the patches realised how much work
it actually was, so I'm a bit sceptical we'll ever get there.  But hey,
you have the enthusiasm, propose it as a KS topic to get agreement that
we should do it and what the format should be and we can go from there.

James

Re: [RFC PATCH 0/4] Persistent device name using alias name

[Greg KH]

On Fri, Jul 08, 2011 at 10:54:11AM -0500, James Bottomley wrote:
> On Fri, 2011-07-08 at 08:47 -0700, Greg KH wrote:
> > On Fri, Jul 08, 2011 at 05:41:36PM +0200, Kay Sievers wrote:
> > > On Fri, Jul 8, 2011 at 16:54, Greg KH <greg@kroah.com> wrote:
> > > > On Fri, Jul 08, 2011 at 05:45:47PM +0900, Nao Nishijima wrote:
> > > >> This patch series provides an "alias name" of the disk into kernel and procfs
> > > >> messages. The user can assign a preferred name to an alias name of the device.
> > > >>
> > > >> Based on previous discussion (*), I changed patches as follows
> > > >> - This is "alias name"
> > > >> - An "alias name" is stored in gendisk struct
> > > >> - Add document to Documentation/ABI/testing/sysfs-block
> > > >> - When the user changes an "alias name", kernel notifies udev
> > > >>
> > > >> (*) http://marc.info/?l=linux-scsi&m=130812625531219&w=2
> > > >
> > > > I don't like it and I don't think it will really solve the root problem
> > > > you are trying to address, but as the patches don't touch any code I
> > > > maintain, there's not much I can do to object to it.
> > > 
> > > I can only repeat what I already wrote in detail earlier:
> > > 
> > > This approach seems to papers over the problem which emitting and
> > > parsing free-text printk() messages with much-too-dumb tools cause. It
> > > seems to fix the symptoms not the cause.
> > > 
> > > You can already write a udev rule today that logs _all_ symlinks of a
> > > device at discovery time, and any later kernel message can safely be
> > > associated with all possible names of the blockdev. No kernel changes
> > > needed, all possible names are covered. That also works good enough
> > > with our current stone-age tools for anybody who is able to scroll
> > > back to the last log udev message in that same log file.
> > > 
> > > There can be by-definition no default udev rules assigning a proper
> > > single name to a block device. There is never a valid single name for
> > > a disks, so udev can not ship anything like that in the default setup,
> > > so this stays as a custom hack.
> > > 
> > > We absolutely need _structured_ data for logging and error reporting,
> > > not only to solve this problem. Along with the current free-text
> > > printk(), we would be able to attach classifications, device
> > > error/sense data, firmware register dumps and anything
> > > interesting-for-debug to the messages.
> > > 
> > > We can't solve that problem in the kernel alone. Structured data from
> > > the kernel will need to feed a smarter userspace logger that can index
> > > and classify messages, merge current userspace data into it, and
> > > provides hooks for the system management to act on critical failures
> > > and raise notifications.
> > > 
> > > Structured logging seems like the solution for this and also to many
> > > other problems in this area. Single custom names pushed into the
> > > kernel might cover some rather exotic use cases, but I think, is not
> > > what we are looking for.
> > 
> > I totally agree, but hey, no one listens to us :)
> 
> Yes, we did.  Everyone agrees structured logging would be the best long
> term solution.  However, it's at least 10x the work presented here, plus
> it would be a long process getting everyone to agree.  This looks like a
> good 95% interim solution and it can be removed when structured logging
> makes everything "just work(tm)".

Do you seriously think that this sysfs attribute can be removed someday
in the future if we get structured logging implemented?  It can't,
sorry, people will start depending on this and their jury-rigged
implementations will require it no matter what we do otherwise.

But that's still not the main issue here.

The main issue that I see is that this really doesn't solve the problem
that is trying to be addressed, and the authors admit that.  So for that
very reason alone I feel it should be rejected, but again, it's not my
subsystem to maintain or control.

best of luck,

greg k-h

Re: [RFC PATCH 0/4] Persistent device name using alias name

[James Bottomley]

On Fri, 2011-07-08 at 09:04 -0700, Greg KH wrote:
> On Fri, Jul 08, 2011 at 10:54:11AM -0500, James Bottomley wrote:
> > On Fri, 2011-07-08 at 08:47 -0700, Greg KH wrote:
> > > On Fri, Jul 08, 2011 at 05:41:36PM +0200, Kay Sievers wrote:
> > > > On Fri, Jul 8, 2011 at 16:54, Greg KH <greg@kroah.com> wrote:
> > > > > On Fri, Jul 08, 2011 at 05:45:47PM +0900, Nao Nishijima wrote:
> > > > >> This patch series provides an "alias name" of the disk into kernel and procfs
> > > > >> messages. The user can assign a preferred name to an alias name of the device.
> > > > >>
> > > > >> Based on previous discussion (*), I changed patches as follows
> > > > >> - This is "alias name"
> > > > >> - An "alias name" is stored in gendisk struct
> > > > >> - Add document to Documentation/ABI/testing/sysfs-block
> > > > >> - When the user changes an "alias name", kernel notifies udev
> > > > >>
> > > > >> (*) http://marc.info/?l=linux-scsi&m=130812625531219&w=2
> > > > >
> > > > > I don't like it and I don't think it will really solve the root problem
> > > > > you are trying to address, but as the patches don't touch any code I
> > > > > maintain, there's not much I can do to object to it.
> > > > 
> > > > I can only repeat what I already wrote in detail earlier:
> > > > 
> > > > This approach seems to papers over the problem which emitting and
> > > > parsing free-text printk() messages with much-too-dumb tools cause. It
> > > > seems to fix the symptoms not the cause.
> > > > 
> > > > You can already write a udev rule today that logs _all_ symlinks of a
> > > > device at discovery time, and any later kernel message can safely be
> > > > associated with all possible names of the blockdev. No kernel changes
> > > > needed, all possible names are covered. That also works good enough
> > > > with our current stone-age tools for anybody who is able to scroll
> > > > back to the last log udev message in that same log file.
> > > > 
> > > > There can be by-definition no default udev rules assigning a proper
> > > > single name to a block device. There is never a valid single name for
> > > > a disks, so udev can not ship anything like that in the default setup,
> > > > so this stays as a custom hack.
> > > > 
> > > > We absolutely need _structured_ data for logging and error reporting,
> > > > not only to solve this problem. Along with the current free-text
> > > > printk(), we would be able to attach classifications, device
> > > > error/sense data, firmware register dumps and anything
> > > > interesting-for-debug to the messages.
> > > > 
> > > > We can't solve that problem in the kernel alone. Structured data from
> > > > the kernel will need to feed a smarter userspace logger that can index
> > > > and classify messages, merge current userspace data into it, and
> > > > provides hooks for the system management to act on critical failures
> > > > and raise notifications.
> > > > 
> > > > Structured logging seems like the solution for this and also to many
> > > > other problems in this area. Single custom names pushed into the
> > > > kernel might cover some rather exotic use cases, but I think, is not
> > > > what we are looking for.
> > > 
> > > I totally agree, but hey, no one listens to us :)
> > 
> > Yes, we did.  Everyone agrees structured logging would be the best long
> > term solution.  However, it's at least 10x the work presented here, plus
> > it would be a long process getting everyone to agree.  This looks like a
> > good 95% interim solution and it can be removed when structured logging
> > makes everything "just work(tm)".
> 
> Do you seriously think that this sysfs attribute can be removed someday
> in the future if we get structured logging implemented?  It can't,
> sorry, people will start depending on this and their jury-rigged
> implementations will require it no matter what we do otherwise.

Yes, it can.  The way we do that is to make udev the interface to this
and the sysfs file becomes an internal udev implementation, not an
external ABI.  Any program using it rather than udev that breaks, well,
tough, it used the wrong interface.

> But that's still not the main issue here.
> 
> The main issue that I see is that this really doesn't solve the problem
> that is trying to be addressed, and the authors admit that.  So for that
> very reason alone I feel it should be rejected, but again, it's not my
> subsystem to maintain or control.
> 
> best of luck,

Thanks,

James

Re: [RFC PATCH 0/4] Persistent device name using alias name

[Greg KH]

On Fri, Jul 08, 2011 at 11:17:36AM -0500, James Bottomley wrote:
> On Fri, 2011-07-08 at 09:04 -0700, Greg KH wrote:
> > Do you seriously think that this sysfs attribute can be removed someday
> > in the future if we get structured logging implemented?  It can't,
> > sorry, people will start depending on this and their jury-rigged
> > implementations will require it no matter what we do otherwise.
> 
> Yes, it can.  The way we do that is to make udev the interface to this
> and the sysfs file becomes an internal udev implementation, not an
> external ABI.

No, sorry, you just described the "external" abi in the
Documentation/ABI directory.

> Any program using it rather than udev that breaks, well, tough, it
> used the wrong interface.

Again, no, we can't do that as you should well know.

And see Kay's response as to how udev will not be using this interface
by default anyway, so I don't see how you will ever be able to remove
this sysfs file once it is added.

greg k-h

Re: [RFC PATCH 0/4] Persistent device name using alias name

[Kay Sievers]

On Fri, Jul 8, 2011 at 17:54, James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:

> Yes, we did.  Everyone agrees structured logging would be the best long
> term solution.  However, it's at least 10x the work presented here, plus
> it would be a long process getting everyone to agree.

Maybe even 100 times :)

> This looks like a
> good 95% interim solution and it can be removed when structured logging
> makes everything "just work(tm)".

I don't really see that. I think it does not add any real value on top
of a simple udev logging at device discovery.

You would see that in the log:
  [78441.742634]: udevd: sdc: \
  disk/by-id/ata-INTEL_SSDSA1M080G2GN_CVPO0363030V080EGN \
  disk/by-id/scsi-SATA_INTEL_SSDSA1M08CVPO0363030V080EGN \
  disk/by-path/pci-0000:00:1f.2-scsi-0:0:0:0 \
  disk/by-id/wwn-0x50015179593f3038

and know exactly _all_ names and all identifiers of the disk at that time.

Then you see this:
  [78441.742634] storage: really bad things happened to: sdc

and with the earlier message we have all we need to know without any
pretty-name hacks.

All that can be done today already with a single udev rule, even on
many years old distros.

> I have also seen a couple of other attempts at structured logging which
> both failed when the people proposing the patches realised how much work
> it actually was, so I'm a bit sceptical we'll ever get there.

The more paper-over we add the more unlikely it gets. That's what I fear. :)

> But hey,
> you have the enthusiasm, propose it as a KS topic to get agreement that
> we should do it and what the format should be and we can go from there.

Sure, I'll do that. If needed, I can even make half or a third of it
possible I guess.

Thanks,
Kay

Re: [RFC PATCH 0/4] Persistent device name using alias name

[Kay Sievers]

On Fri, Jul 8, 2011 at 18:15, Kay Sievers <kay.sievers@vrfy.org> wrote:
> On Fri, Jul 8, 2011 at 17:54, James Bottomley

>> But hey,
>> you have the enthusiasm, propose it as a KS topic to get agreement that
>> we should do it and what the format should be and we can go from there.
>
> Sure, I'll do that. If needed, I can even make half or a third of it
> possible I guess.

Submitted it a minute ago.

Thanks,
Kay

Re: [RFC PATCH 0/4] Persistent device name using alias name

[Hannes Reinecke]

On 07/08/2011 06:38 PM, Kay Sievers wrote:
> On Fri, Jul 8, 2011 at 18:15, Kay Sievers<kay.sievers@vrfy.org>  wrote:
>> On Fri, Jul 8, 2011 at 17:54, James Bottomley
>
>>> But hey,
>>> you have the enthusiasm, propose it as a KS topic to get agreement that
>>> we should do it and what the format should be and we can go from there.
>>
>> Sure, I'll do that. If needed, I can even make half or a third of it
>> possible I guess.
>
> Submitted it a minute ago.
>
I'd be willing to working on the design here, as it ties in rather 
neatly with my goal of updating the SCSI debugging facilities.

printk() is easy to write, but basically impossible to impose any 
type of checking/format whatever.
My all-time favorite here:

                 printk(KERN_INFO "error 1\n");

At least it got a 'KERN_INFO' thrown in there ...

So first step would be to reach an agreement _what_ printk() et al 
is supposed to convey to userland.
Informal only? Informal, but traceable to the origin?
Should the origin be the internal 
structure/subsystem/device/whatever generating the error?
Should the origin be persistent across reboots?

And, tied to that, what the supposed audience for printk() is:
Programs? Syslog? Humans?

Once we have an agreement here we can then go about and code the 
required pieces. But currently the main problem is that there are 
different ideas of what printk() is supposed to do.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)

Re: [RFC PATCH 0/4] Persistent device name using alias name

[Masami Hiramatsu]

(2011/07/09 1:15), Kay Sievers wrote:
> On Fri, Jul 8, 2011 at 17:54, James Bottomley
> <James.Bottomley@hansenpartnership.com> wrote:
> 
>> Yes, we did.  Everyone agrees structured logging would be the best long
>> term solution.  However, it's at least 10x the work presented here, plus
>> it would be a long process getting everyone to agree.
> 
> Maybe even 100 times :)
> 
>> This looks like a
>> good 95% interim solution and it can be removed when structured logging
>> makes everything "just work(tm)".
> 
> I don't really see that. I think it does not add any real value on top
> of a simple udev logging at device discovery.
> 
> You would see that in the log:
>   [78441.742634]: udevd: sdc: \
>   disk/by-id/ata-INTEL_SSDSA1M080G2GN_CVPO0363030V080EGN \
>   disk/by-id/scsi-SATA_INTEL_SSDSA1M08CVPO0363030V080EGN \
>   disk/by-path/pci-0000:00:1f.2-scsi-0:0:0:0 \
>   disk/by-id/wwn-0x50015179593f3038
> 
> and know exactly _all_ names and all identifiers of the disk at that time.
> 
> Then you see this:
>   [78441.742634] storage: really bad things happened to: sdc
> 
> and with the earlier message we have all we need to know without any
> pretty-name hacks.

Hmm, yes, we CAN do that, if user carefully sets the udev log-level
high, and saves udev log every boot or device change.

I think the main point of this attempt is to reduce the cost of
those operation, as like as users have done with old un*x way.

If we can use a same device name for same device and can see the
same name on the kernel log, we don't need to pay additional cost
for searching and picking correct entry from udev log.
Of course, it needs updates on some tools too.


> All that can be done today already with a single udev rule, even on
> many years old distros.
> 
>> I have also seen a couple of other attempts at structured logging which
>> both failed when the people proposing the patches realised how much work
>> it actually was, so I'm a bit sceptical we'll ever get there.
> 
> The more paper-over we add the more unlikely it gets. That's what I fear. :)
> 
>> But hey,
>> you have the enthusiasm, propose it as a KS topic to get agreement that
>> we should do it and what the format should be and we can go from there.
> 
> Sure, I'll do that. If needed, I can even make half or a third of it
> possible I guess.

BTW, I'm also interested in that structured error events, from the long
term view and viewpoint of tracers :)
I think we could expand current TRACE_EVENT macro to define those error
events.

Thank you,

-- 
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com

Re: [RFC PATCH 0/4] Persistent device name using alias name

[Borislav Petkov]

On Sat, Jul 09, 2011 at 03:11:45PM +0900, Masami Hiramatsu wrote:
> BTW, I'm also interested in that structured error events, from the long
> term view and viewpoint of tracers :)

Let me chime in a "mee too" from the HW errors perspective.

> I think we could expand current TRACE_EVENT macro to define those
> error events.

Concerning structure, we can use the format file in debugfs which
currently describes the fields of a tracepoint:

/sys/kernel/debug/tracing/events/../../format

and expand those to generic kernel events. Userspace parses the format
and knows exactly at what data it is looking at. Also, the idea is to
move that events hierarchy into sysfs so it will be present on every
system.

Hmm...

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

Re: [RFC PATCH 0/4] Persistent device name using alias name

[Masami Hiramatsu]

(2011/08/04 2:16), Borislav Petkov wrote:
> On Sat, Jul 09, 2011 at 03:11:45PM +0900, Masami Hiramatsu wrote:
>> BTW, I'm also interested in that structured error events, from the long
>> term view and viewpoint of tracers :)
>
> Let me chime in a "mee too" from the HW errors perspective.

Of course, :) and I guess this may be related to Fredrdic's work.


>> I think we could expand current TRACE_EVENT macro to define those
>> error events.
>
> Concerning structure, we can use the format file in debugfs which
> currently describes the fields of a tracepoint:
>
> /sys/kernel/debug/tracing/events/../../format
>
> and expand those to generic kernel events. Userspace parses the format
> and knows exactly at what data it is looking at. Also, the idea is to
> move that events hierarchy into sysfs so it will be present on every
> system.

Yeah, that is exactly what I've thought. Current trace events
strongly depends on ftrace and perf. And both of them are not
enough for handling events as error monitoring. Maybe we have to
prepare another monitor for this purpose, because, usually,
error monitoring should be run always on the machine, separately
from (and concurrently with) performance tuning or debugging tools.

Even though, I think the trace-event macros (include/trace/events/)
are good starting point.

Thank you,

-- 
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com

Re: [RFC PATCH 0/4] Persistent device name using alias name

[Karel Zak]

On Fri, Jul 08, 2011 at 05:45:47PM +0900, Nao Nishijima wrote:
> This patch series provides an "alias name" of the disk into kernel and procfs
> messages. The user can assign a preferred name to an alias name of the device.
> 
> Based on previous discussion (*), I changed patches as follows
> - This is "alias name"
> - An "alias name" is stored in gendisk struct
> - Add document to Documentation/ABI/testing/sysfs-block
> - When the user changes an "alias name", kernel notifies udev
> 
> (*) http://marc.info/?l=linux-scsi&m=130812625531219&w=2
> 

[...]

> [localhost]# cat /proc/partitions
> major minor  #blocks  name
> 
>    8        0   12582912 foo
>    8        1   12582878 foo1
>    8        0    8388608 sdb
>    8        1     512000 sdb1
>    8        2    7875584 sdb2

 If there is not /dev/foo and /sys/block/foo then the patch introduces
 a REGRESSION. 
 
 The names from /proc/partitions are used in many applications
 (libblkid, fdisk, ...) for many many years. The applications will not
 work as expected.

 It's crazy to assume that all the applications will be improved to
 translate the "pretty name" from /proc/partitions by /sys/block/<maj>:<min>.

 Note, it's pretty naive to expect that people will use the pretty
 names for printk()/dmesg only. I'm absolutely sure that it's will be
 expected (by end-users) in /etc/fstab, mount, df, fdisk, ...

 It's will be necessary to modify all these utils. 

 The patch is opening Pandora's box :-(

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

Re: [RFC PATCH 0/4] Persistent device name using alias name

[Greg KH]

On Fri, Jul 08, 2011 at 09:45:01PM +0200, Karel Zak wrote:
> On Fri, Jul 08, 2011 at 05:45:47PM +0900, Nao Nishijima wrote:
> > This patch series provides an "alias name" of the disk into kernel and procfs
> > messages. The user can assign a preferred name to an alias name of the device.
> > 
> > Based on previous discussion (*), I changed patches as follows
> > - This is "alias name"
> > - An "alias name" is stored in gendisk struct
> > - Add document to Documentation/ABI/testing/sysfs-block
> > - When the user changes an "alias name", kernel notifies udev
> > 
> > (*) http://marc.info/?l=linux-scsi&m=130812625531219&w=2
> > 
> 
> [...]
> 
> > [localhost]# cat /proc/partitions
> > major minor  #blocks  name
> > 
> >    8        0   12582912 foo
> >    8        1   12582878 foo1
> >    8        0    8388608 sdb
> >    8        1     512000 sdb1
> >    8        2    7875584 sdb2
> 
>  If there is not /dev/foo and /sys/block/foo then the patch introduces
>  a REGRESSION. 
>  
>  The names from /proc/partitions are used in many applications
>  (libblkid, fdisk, ...) for many many years. The applications will not
>  work as expected.
> 
>  It's crazy to assume that all the applications will be improved to
>  translate the "pretty name" from /proc/partitions by /sys/block/<maj>:<min>.
> 
>  Note, it's pretty naive to expect that people will use the pretty
>  names for printk()/dmesg only. I'm absolutely sure that it's will be
>  expected (by end-users) in /etc/fstab, mount, df, fdisk, ...
> 
>  It's will be necessary to modify all these utils. 

And it would be even easier, to just modify the utilities in the first
place to handle the symlink names in /dev/disk that are there today, and
leave the kernel alone.

With the above information you provided, I don't see how we can accept
this patch at all as it will break existing userspace utilities which is
not allowed.

thanks,

greg k-h

Re: [RFC PATCH 0/4] Persistent device name using alias name

[Nao Nishijima]

Hi, karel

(2011/07/09 4:45), Karel Zak wrote:
> On Fri, Jul 08, 2011 at 05:45:47PM +0900, Nao Nishijima wrote:
>> This patch series provides an "alias name" of the disk into kernel and procfs
>> messages. The user can assign a preferred name to an alias name of the device.
>>
>> Based on previous discussion (*), I changed patches as follows
>> - This is "alias name"
>> - An "alias name" is stored in gendisk struct
>> - Add document to Documentation/ABI/testing/sysfs-block
>> - When the user changes an "alias name", kernel notifies udev
>>
>> (*) http://marc.info/?l=linux-scsi&m=130812625531219&w=2
>>
> 
> [...]
> 
>> [localhost]# cat /proc/partitions
>> major minor  #blocks  name
>>
>>    8        0   12582912 foo
>>    8        1   12582878 foo1
>>    8        0    8388608 sdb
>>    8        1     512000 sdb1
>>    8        2    7875584 sdb2
> 
>  If there is not /dev/foo and /sys/block/foo then the patch introduces
>  a REGRESSION. 
>  
>  The names from /proc/partitions are used in many applications
>  (libblkid, fdisk, ...) for many many years. The applications will not
>  work as expected.
> 

I think that it is not a regression when users do not set an alias name.
Of course, I am going to modify all these utils so that users can use
alias names.

The purpose of alias names is to unify the name of device which users
operate and see. Therefore, I think that users would like to get an
alias name from it, because currently they get a device name form it.

However, for who wants to use alias name only for dmesg, I have an
enhancement idea that users can choose alias names or device names in
procfs by sysctl (default is device names).

I would like to hear your opinion about this.

>  It's crazy to assume that all the applications will be improved to
>  translate the "pretty name" from /proc/partitions by /sys/block/<maj>:<min>.
> 
>  Note, it's pretty naive to expect that people will use the pretty
>  names for printk()/dmesg only. I'm absolutely sure that it's will be
>  expected (by end-users) in /etc/fstab, mount, df, fdisk, ...
> 
>  It's will be necessary to modify all these utils. 
> 

Yes, I am going to modify all these utils to support udev persistent
names(including alias name).

I had looked over some commands where those get a device name from.
Most of them get a device name from /proc/partitions, also other
commands get it from /etc/fstab, /sys/block/ or own device database.

I am going to modify those commands to get a device name from /dev/disk.

>  The patch is opening Pandora's box :-(
> 

Best regards,

-- 
Nao NISHIJIMA
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., YOKOHAMA Research  Laboratory
Email： nao.nishijima.xt@hitachi.com

Re: [RFC PATCH 0/4] Persistent device name using alias name

[Karel Zak]

On Fri, Jul 15, 2011 at 03:55:02PM +0900, Nao Nishijima wrote:
> >  If there is not /dev/foo and /sys/block/foo then the patch introduces
> >  a REGRESSION. 
> >  
> >  The names from /proc/partitions are used in many applications
> >  (libblkid, fdisk, ...) for many many years. The applications will not
> >  work as expected.
> > 
> 
> I think that it is not a regression when users do not set an alias name.
> Of course, I am going to modify all these utils so that users can use
> alias names.
> 
> The purpose of alias names is to unify the name of device which users
> operate and see. Therefore, I think that users would like to get an
> alias name from it, because currently they get a device name form it.
> 
> However, for who wants to use alias name only for dmesg, I have an
> enhancement idea that users can choose alias names or device names in
> procfs by sysctl (default is device names).

>From my point of view

    dmesg | grep <alias>

seems a little bit problematic, because for example initial messages
for the device will be invisible. In the log will be messages with
original canonical names and another (later) messages with aliases, you
can also change the alias, etc. Seems like a mess. The log should be
consistent...

I think it would be better to use always only canonical names in the
kernel log and translate to something more user-friendly in userspace.
For example if you add some meta-information to the kernel log then we
can improve dmesg(1) to translate the canonical names to aliases.

 printk(KERN_INFO "this is info about %{device}s", device);

 <5>[105221.774534]{device=sda} this is info about sda

or whatever... (this is a nice topic with many colors for the bike
shed:-)

> I would like to hear your opinion about this.

If you want to modify all the userspace utils then you don't have to
modify /proc/partitions :-)

You can keep the standard names in /proc/partitions (so the file will
be compatible with /sys and /dev) and you can modify all the utils to
translate the canonical device names to aliases. The result will be
the same, except "cat /proc/partitions" -- but I think that instead of
"cat" you can use "lsblk".

If you add only the attribute (alias name) to the /sys then you don't
have to care if all the utils are already modified.

Anyway, I still agree with Kay and Greg -- the proper solution
is to improve printk() and dmesg(1). The aliases could be implemented 
in userspace by udev /dev/disk/by-alias/<cool_name> symlinks.

    Karel

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

Re: [RFC PATCH 0/4] Persistent device name using alias name

[Nao Nishijima]

Hi, Karel

Thank you for your comments.

(2011/07/15 21:48), Karel Zak wrote:
> On Fri, Jul 15, 2011 at 03:55:02PM +0900, Nao Nishijima wrote:
>>>  If there is not /dev/foo and /sys/block/foo then the patch introduces
>>>  a REGRESSION. 
>>>  
>>>  The names from /proc/partitions are used in many applications
>>>  (libblkid, fdisk, ...) for many many years. The applications will not
>>>  work as expected.
>>>
>>
>> I think that it is not a regression when users do not set an alias name.
>> Of course, I am going to modify all these utils so that users can use
>> alias names.
>>
>> The purpose of alias names is to unify the name of device which users
>> operate and see. Therefore, I think that users would like to get an
>> alias name from it, because currently they get a device name form it.
>>
>> However, for who wants to use alias name only for dmesg, I have an
>> enhancement idea that users can choose alias names or device names in
>> procfs by sysctl (default is device names).
> 
>>From my point of view
> 
>     dmesg | grep <alias>
> 
> seems a little bit problematic, because for example initial messages
> for the device will be invisible. In the log will be messages with
> original canonical names and another (later) messages with aliases, you
> can also change the alias, etc. Seems like a mess. The log should be
> consistent...
> 
> I think it would be better to use always only canonical names in the
> kernel log and translate to something more user-friendly in userspace.
> For example if you add some meta-information to the kernel log then we
> can improve dmesg(1) to translate the canonical names to aliases.
> 
>  printk(KERN_INFO "this is info about %{device}s", device);
> 
>  <5>[105221.774534]{device=sda} this is info about sda
> 
> or whatever... (this is a nice topic with many colors for the bike
> shed:-)
> 
>> I would like to hear your opinion about this.
> 
> If you want to modify all the userspace utils then you don't have to
> modify /proc/partitions :-)
> 
> You can keep the standard names in /proc/partitions (so the file will
> be compatible with /sys and /dev) and you can modify all the utils to
> translate the canonical device names to aliases. The result will be
> the same, except "cat /proc/partitions" -- but I think that instead of
> "cat" you can use "lsblk".
> 

"lsblk" is great. (Sorry, I didn't know this command until now)

I think it would be good that users can get alias name from BOTH of
"lsblk" and "/proc/partitions", because users expect to see same name in
kernel output and command output　as they can see now.

In kernel message, I think that "alias name" is necessary. Because those
logs are output when kernel crashes, it outputs kernel message to serial
console. In that case, it's hard to convert canonical device name to
alias name.

> If you add only the attribute (alias name) to the /sys then you don't
> have to care if all the utils are already modified.
> 
> Anyway, I still agree with Kay and Greg -- the proper solution
> is to improve printk() and dmesg(1). The aliases could be implemented 
> in userspace by udev /dev/disk/by-alias/<cool_name> symlinks.
> 

Best regards,

-- 
Nao NISHIJIMA
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., YOKOHAMA Research  Laboratory
Email： nao.nishijima.xt@hitachi.com