* [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel. @ 2011-04-05 12:49 ` Nao Nishijima 0 siblings, 0 replies; 46+ messages in thread From: Nao Nishijima @ 2011-04-05 12:49 UTC (permalink / raw) To: linux-kernel, linux-scsi, linux-hotplug Cc: Greg KH, Kay Sievers, James Bottomley, Jon Masters, 2nddept-manager, Nao Nishijima This patch series provides a SCSI option for persistent device names in kernel. With this option, user can always assign a same device name (e.g. sda) to a specific device according to udev rules and device id. Issue: Recently, kernel registers block devices in parallel. As a result, different device names will be assigned at each boot time. This will confuse file-system mounter, thus we usually use persistent symbolic links provided by udev. However, dmesg and procfs outputs show device names instead of the symbolic link names. This causes a serious problem when managing multiple devices (e.g. on a large-scale storage), because usually, device errors are output with device names on dmesg. We also concern about some commands which output device names, as well as kernel messages. Solution: To assign a persistent device name, I create unnamed devices (not a block device, but an intermediate device. users cannot read/write this device). When scsi device driver finds a LU, the LU is registered as an unnamed device and inform to udev. After udev finds the unnamed device, udev assigns a persistent device name to a specific device according to udev rules and registers it as a block device. Hence, this is just a naming, not a renaming. Some users are satisfied with current udev solution. Therefore, my proposal is implemented as an option. If using this option, add the following kernel parameter. scsi_mod.persistent_name=1 Also, if you want to assign a device name with sda, add the following udev rules. SUBSYSTEM=="scsi_unnamed", ATTR{disk_id}=="xxx", PROGRAM="/bin/sh -c 'echo -n sda > /sys/%p/disk_name'" Todo: - usb support - hot-remove support I've already started discussion about device naming at LKML. https://lkml.org/lkml/2010/10/8/31 Signed-off-by: Nao Nishijima <nao.nishijima.xt@hitachi.com> --- drivers/scsi/Makefile | 1 drivers/scsi/scsi_sysfs.c | 26 +++ drivers/scsi/scsi_unnamed.c | 340 +++++++++++++++++++++++++++++++++++++++++++ drivers/scsi/scsi_unnamed.h | 25 +++ 4 files changed, 387 insertions(+), 5 deletions(-) create mode 100644 drivers/scsi/scsi_unnamed.c create mode 100644 drivers/scsi/scsi_unnamed.h diff --git a/drivers/scsi/Makefile b/drivers/scsi/Makefile index 7ad0b8a..782adc1 100644 --- a/drivers/scsi/Makefile +++ b/drivers/scsi/Makefile @@ -167,6 +167,7 @@ scsi_mod-$(CONFIG_SYSCTL) += scsi_sysctl.o scsi_mod-$(CONFIG_SCSI_PROC_FS) += scsi_proc.o scsi_mod-y += scsi_trace.o scsi_mod-$(CONFIG_PM) += scsi_pm.o +scsi_mod-y += scsi_unnamed.o scsi_tgt-y += scsi_tgt_lib.o scsi_tgt_if.o diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c index e44ff64..7959f5d 100644 --- a/drivers/scsi/scsi_sysfs.c +++ b/drivers/scsi/scsi_sysfs.c @@ -22,6 +22,7 @@ #include "scsi_priv.h" #include "scsi_logging.h" +#include "scsi_unnamed.h" static struct device_type scsi_dev_type; @@ -393,13 +394,27 @@ int scsi_sysfs_register(void) { int error; + error = scsi_unnamed_register(&scsi_bus_type); + if (error) + goto cleanup; + error = bus_register(&scsi_bus_type); - if (!error) { - error = class_register(&sdev_class); - if (error) - bus_unregister(&scsi_bus_type); - } + if (error) + goto cleanup_scsi_unnamed; + + error = class_register(&sdev_class); + if (error) + goto cleanup_bus; + + return 0; + +cleanup_bus: + bus_unregister(&scsi_bus_type); + +cleanup_scsi_unnamed: + scsi_unnamed_unregister(); +cleanup: return error; } @@ -407,6 +422,7 @@ void scsi_sysfs_unregister(void) { class_unregister(&sdev_class); bus_unregister(&scsi_bus_type); + scsi_unnamed_unregister(); } /* diff --git a/drivers/scsi/scsi_unnamed.c b/drivers/scsi/scsi_unnamed.c new file mode 100644 index 0000000..ed96945 --- /dev/null +++ b/drivers/scsi/scsi_unnamed.c @@ -0,0 +1,340 @@ +/* + * scsi_unnamed.c - SCSI driver for unnmaed device + * + * Copyright: Copyright (c) Hitachi, Ltd. 2011 + * Authors: Nao Nishijima <nao.nishijima.xt@hitachi.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111 USA + */ + +#include <linux/module.h> +#include <linux/err.h> +#include <linux/device.h> +#include <linux/slab.h> +#include <linux/kobject.h> +#include <linux/kdev_t.h> +#include <linux/sysdev.h> +#include <linux/list.h> +#include <linux/wait.h> + +#include <scsi/scsi_driver.h> +#include <scsi/scsi_device.h> +#include <scsi/scsi.h> + +#define MAX_BUFFER_LEN 256 +#define DISK_NAME_LEN 32 + +#define SCSI_ID_BINARY 1 +#define SCSI_ID_ASCII 2 +#define SCSI_ID_UTF8 3 + +static LIST_HEAD(su_list); +static DEFINE_MUTEX(su_list_lock); + +static int persistent_name; +MODULE_PARM_DESC(persistent_name, "SCSI unnamed device support"); +module_param(persistent_name, bool, S_IRUGO); + +static struct class su_sysfs_class = { + .name = "scsi_unnamed", +}; + +struct scsi_unnamed { + struct list_head list; + struct device dev; + char disk_name[DISK_NAME_LEN]; + char disk_lun[MAX_BUFFER_LEN]; + char disk_id[MAX_BUFFER_LEN]; + bool assigned; +}; + +#define to_scsi_unnamed(d) \ + container_of(d, struct scsi_unnamed, dev) + +/* Get device identification VPD page */ +static void store_disk_id(struct scsi_device *sdev, char *disk_id) +{ + unsigned char *page_83; + int page_len, id_len, j = 0, i = 8; + static const char hex_str[] = "0123456789abcdef"; + + page_83 = kmalloc(MAX_BUFFER_LEN, GFP_KERNEL); + if (!page_83) + return; + + if (scsi_get_vpd_page(sdev, 0x83, page_83, MAX_BUFFER_LEN)) + goto err; + + page_len = ((page_83[2] << 8) | page_83[3]) + 4; + if (page_len > 0) { + if ((page_83[5] & 0x30) != 0) + goto err; + + id_len = page_83[7]; + if (id_len > 0) { + switch (page_83[4] & 0x0f) { + case SCSI_ID_BINARY: + while (i < id_len) { + disk_id[j++] = hex_str[(page_83[i] + & 0xf0) >> 4]; + disk_id[j++] = hex_str[page_83[i] + & 0x0f]; + i++; + } + break; + case SCSI_ID_ASCII: + case SCSI_ID_UTF8: + strncpy(disk_id, strim(page_83 + 8), id_len); + break; + default: + break; + } + } + } + +err: + kfree(page_83); + return; +} + +static ssize_t show_disk_name(struct device *dev, + struct device_attribute *attr, char *buf) +{ + return sprintf(buf, "%s\n", to_scsi_unnamed(dev)->disk_name); +} + +static ssize_t show_disk_lun(struct device *dev, + struct device_attribute *attr, char *buf) +{ + return sprintf(buf, "%s\n", to_scsi_unnamed(dev)->disk_lun); +} + +static ssize_t show_disk_id(struct device *dev, + struct device_attribute *attr, char *buf) +{ + return sprintf(buf, "%s\n", to_scsi_unnamed(dev)->disk_id); +} + +/* Assign a persistent device name */ +static ssize_t store_disk_name(struct device *dev, + struct device_attribute *attr, char *buf, size_t count) +{ + struct scsi_unnamed *su = to_scsi_unnamed(dev); + struct scsi_unnamed *tmp; + struct device *lu = dev->parent; + int ret = -EINVAL; + + if (count >= DISK_NAME_LEN) { + printk(KERN_WARNING "su: Too long a persistent name!\n"); + return -EINVAL; + } + + if (su->assigned) { + printk(KERN_WARNING "su: Already assigned a persistent name!\n"); + return -EINVAL; + } + + if (strncmp(lu->driver->name, buf, 2)) { + printk(KERN_WARNING "su: Wrong prefix!\n"); + return -EINVAL; + } + + mutex_lock(&su_list_lock); + list_for_each_entry(tmp, &su_list, list) { + if (!strncmp(tmp->disk_name, buf, DISK_NAME_LEN)) { + printk(KERN_WARNING "su: Duplicate name exsists!\n"); + return -EINVAL; + } + } + strncpy(su->disk_name, buf, DISK_NAME_LEN); + mutex_unlock(&su_list_lock); + + if (!lu->driver->probe) + return -EINVAL; + + lu->init_name = buf; + + ret = lu->driver->probe(lu); + if (ret) { + lu->init_name = NULL; + su->disk_name[0] = '\0'; + return ret; + } + + su->assigned = true; + return count; +} + +static DEVICE_ATTR(disk_name, S_IRUGO|S_IWUSR, show_disk_name, + store_disk_name); +static DEVICE_ATTR(disk_lun, S_IRUGO, show_disk_lun, NULL); +static DEVICE_ATTR(disk_id, S_IRUGO, show_disk_id, NULL); + +/* Confirm the driver name and the device type */ +static int check_device_type(char type, const char *name) +{ + switch (type) { + case TYPE_DISK: + case TYPE_MOD: + case TYPE_RBC: + if (strncmp("sd", name, 2)) + return -ENODEV; + break; + case TYPE_ROM: + case TYPE_WORM: + if (strncmp("sr", name, 2)) + return -ENODEV; + break; + case TYPE_TAPE: + if (strncmp("st", name, 2)) + return -ENODEV; + break; + default: + break; + } + return 0; +} + +/** + * scsi_unnamed_probe - Setup the unnamed device. + * @dev: parent scsi device + * + * This function adds a unnamed device to the device model and + * gets a number of device information by scsi inquiry commands. + * Udev identify a device by those information. + * + * Unnamed devices: + * This device is not a block device and user can not read/write + * this device. But it can advertise device information in sysfs. + */ +int scsi_unnamed_probe(struct device *dev) +{ + struct scsi_unnamed *su; + struct scsi_device *sdev = to_scsi_device(dev); + int ret = -EINVAL; + static int i; + + if (check_device_type(sdev->type, dev->driver->name)) + return -ENODEV; + + su = kzalloc(sizeof(*su), GFP_KERNEL); + if (!su) + return -ENOMEM; + + device_initialize(&su->dev); + su->dev.parent = dev; + su->dev.class = &su_sysfs_class; + dev_set_name(&su->dev, "su%d", i++); + strncpy(su->disk_lun, dev_name(dev), MAX_BUFFER_LEN); + + ret = device_add(&su->dev); + if (ret) + goto err; + + ret = device_create_file(&su->dev, &dev_attr_disk_name); + if (ret) + goto err_disk_name; + + ret = device_create_file(&su->dev, &dev_attr_disk_lun); + if (ret) + goto err_disk_lun; + + store_disk_id(sdev, su->disk_id); + if (su->disk_id[0] != '\0') { + ret = device_create_file(&su->dev, &dev_attr_disk_id); + if (ret) + goto err_disk_id; + } + + su->assigned = false; + mutex_lock(&su_list_lock); + list_add(&su->list, &su_list); + mutex_unlock(&su_list_lock); + + return 0; + +err_disk_id: + device_remove_file(&su->dev, &dev_attr_disk_lun); + +err_disk_lun: + device_remove_file(&su->dev, &dev_attr_disk_name); + +err_disk_name: + device_del(&su->dev); + +err: + kfree(su); + return ret; +} + +/* Remove a unnamed device from su_list and release resources */ +void scsi_unnamed_remove(struct device *dev) +{ + struct scsi_unnamed *tmp; + + mutex_lock(&su_list_lock); + list_for_each_entry(tmp, &su_list, list) { + if (dev == tmp->dev.parent) { + list_del(&tmp->list); + break; + } + } + mutex_unlock(&su_list_lock); + + if (tmp->disk_id[0] != '\0') + device_remove_file(&tmp->dev, &dev_attr_disk_id); + device_remove_file(&tmp->dev, &dev_attr_disk_name); + device_remove_file(&tmp->dev, &dev_attr_disk_lun); + device_del(&tmp->dev); + + device_release_driver(dev); + + kfree(tmp); +} + +/** + * copy_persistent_name - Copy the device name + * @disk_name: allocate to + * @dev: scsi device + * + * Copy an initial name of the device to disk_name. + */ +int copy_persistent_name(char *disk_name, struct device *dev) +{ + if (persistent_name) { + strncpy(disk_name, dev->init_name, DISK_NAME_LEN); + return 1; + } + return 0; +} +EXPORT_SYMBOL(copy_persistent_name); + +int scsi_unnamed_register(struct bus_type *bus) +{ + if (persistent_name) { + bus->probe = scsi_unnamed_probe; + bus->remove = scsi_unnamed_remove; + return class_register(&su_sysfs_class); + } + return 0; +} +EXPORT_SYMBOL(scsi_unnamed_register); + +void scsi_unnamed_unregister(void) +{ + if (persistent_name) + class_unregister(&su_sysfs_class); +} +EXPORT_SYMBOL(scsi_unnamed_unregister); diff --git a/drivers/scsi/scsi_unnamed.h b/drivers/scsi/scsi_unnamed.h new file mode 100644 index 0000000..ca4e852 --- /dev/null +++ b/drivers/scsi/scsi_unnamed.h @@ -0,0 +1,25 @@ +/* + * scsi_unnamed.h - SCSI driver for unnmaed device + * + * Copyright: Copyright (c) Hitachi, Ltd. 2011 + * Authors: Nao Nishijima <nao.nishijima.xt@hitachi.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111 USA + */ + +extern int check_disk_name_prefix(char *, struct device *); +extern int copy_persistent_name(char *, struct device *); +extern int scsi_unnamed_register(struct bus_type *); +extern void scsi_unnamed_unregister(void); ^ permalink raw reply related [flat|nested] 46+ messages in thread
* [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in @ 2011-04-05 12:49 ` Nao Nishijima 0 siblings, 0 replies; 46+ messages in thread From: Nao Nishijima @ 2011-04-05 12:49 UTC (permalink / raw) To: linux-kernel, linux-scsi, linux-hotplug Cc: Greg KH, Kay Sievers, James Bottomley, Jon Masters, 2nddept-manager, Nao Nishijima This patch series provides a SCSI option for persistent device names in kernel. With this option, user can always assign a same device name (e.g. sda) to a specific device according to udev rules and device id. Issue: Recently, kernel registers block devices in parallel. As a result, different device names will be assigned at each boot time. This will confuse file-system mounter, thus we usually use persistent symbolic links provided by udev. However, dmesg and procfs outputs show device names instead of the symbolic link names. This causes a serious problem when managing multiple devices (e.g. on a large-scale storage), because usually, device errors are output with device names on dmesg. We also concern about some commands which output device names, as well as kernel messages. Solution: To assign a persistent device name, I create unnamed devices (not a block device, but an intermediate device. users cannot read/write this device). When scsi device driver finds a LU, the LU is registered as an unnamed device and inform to udev. After udev finds the unnamed device, udev assigns a persistent device name to a specific device according to udev rules and registers it as a block device. Hence, this is just a naming, not a renaming. Some users are satisfied with current udev solution. Therefore, my proposal is implemented as an option. If using this option, add the following kernel parameter. scsi_mod.persistent_name=1 Also, if you want to assign a device name with sda, add the following udev rules. SUBSYSTEM="scsi_unnamed", ATTR{disk_id}="xxx", PROGRAM="/bin/sh -c 'echo -n sda > /sys/%p/disk_name'" Todo: - usb support - hot-remove support I've already started discussion about device naming at LKML. https://lkml.org/lkml/2010/10/8/31 Signed-off-by: Nao Nishijima <nao.nishijima.xt@hitachi.com> --- drivers/scsi/Makefile | 1 drivers/scsi/scsi_sysfs.c | 26 +++ drivers/scsi/scsi_unnamed.c | 340 +++++++++++++++++++++++++++++++++++++++++++ drivers/scsi/scsi_unnamed.h | 25 +++ 4 files changed, 387 insertions(+), 5 deletions(-) create mode 100644 drivers/scsi/scsi_unnamed.c create mode 100644 drivers/scsi/scsi_unnamed.h diff --git a/drivers/scsi/Makefile b/drivers/scsi/Makefile index 7ad0b8a..782adc1 100644 --- a/drivers/scsi/Makefile +++ b/drivers/scsi/Makefile @@ -167,6 +167,7 @@ scsi_mod-$(CONFIG_SYSCTL) += scsi_sysctl.o scsi_mod-$(CONFIG_SCSI_PROC_FS) += scsi_proc.o scsi_mod-y += scsi_trace.o scsi_mod-$(CONFIG_PM) += scsi_pm.o +scsi_mod-y += scsi_unnamed.o scsi_tgt-y += scsi_tgt_lib.o scsi_tgt_if.o diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c index e44ff64..7959f5d 100644 --- a/drivers/scsi/scsi_sysfs.c +++ b/drivers/scsi/scsi_sysfs.c @@ -22,6 +22,7 @@ #include "scsi_priv.h" #include "scsi_logging.h" +#include "scsi_unnamed.h" static struct device_type scsi_dev_type; @@ -393,13 +394,27 @@ int scsi_sysfs_register(void) { int error; + error = scsi_unnamed_register(&scsi_bus_type); + if (error) + goto cleanup; + error = bus_register(&scsi_bus_type); - if (!error) { - error = class_register(&sdev_class); - if (error) - bus_unregister(&scsi_bus_type); - } + if (error) + goto cleanup_scsi_unnamed; + + error = class_register(&sdev_class); + if (error) + goto cleanup_bus; + + return 0; + +cleanup_bus: + bus_unregister(&scsi_bus_type); + +cleanup_scsi_unnamed: + scsi_unnamed_unregister(); +cleanup: return error; } @@ -407,6 +422,7 @@ void scsi_sysfs_unregister(void) { class_unregister(&sdev_class); bus_unregister(&scsi_bus_type); + scsi_unnamed_unregister(); } /* diff --git a/drivers/scsi/scsi_unnamed.c b/drivers/scsi/scsi_unnamed.c new file mode 100644 index 0000000..ed96945 --- /dev/null +++ b/drivers/scsi/scsi_unnamed.c @@ -0,0 +1,340 @@ +/* + * scsi_unnamed.c - SCSI driver for unnmaed device + * + * Copyright: Copyright (c) Hitachi, Ltd. 2011 + * Authors: Nao Nishijima <nao.nishijima.xt@hitachi.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111 USA + */ + +#include <linux/module.h> +#include <linux/err.h> +#include <linux/device.h> +#include <linux/slab.h> +#include <linux/kobject.h> +#include <linux/kdev_t.h> +#include <linux/sysdev.h> +#include <linux/list.h> +#include <linux/wait.h> + +#include <scsi/scsi_driver.h> +#include <scsi/scsi_device.h> +#include <scsi/scsi.h> + +#define MAX_BUFFER_LEN 256 +#define DISK_NAME_LEN 32 + +#define SCSI_ID_BINARY 1 +#define SCSI_ID_ASCII 2 +#define SCSI_ID_UTF8 3 + +static LIST_HEAD(su_list); +static DEFINE_MUTEX(su_list_lock); + +static int persistent_name; +MODULE_PARM_DESC(persistent_name, "SCSI unnamed device support"); +module_param(persistent_name, bool, S_IRUGO); + +static struct class su_sysfs_class = { + .name = "scsi_unnamed", +}; + +struct scsi_unnamed { + struct list_head list; + struct device dev; + char disk_name[DISK_NAME_LEN]; + char disk_lun[MAX_BUFFER_LEN]; + char disk_id[MAX_BUFFER_LEN]; + bool assigned; +}; + +#define to_scsi_unnamed(d) \ + container_of(d, struct scsi_unnamed, dev) + +/* Get device identification VPD page */ +static void store_disk_id(struct scsi_device *sdev, char *disk_id) +{ + unsigned char *page_83; + int page_len, id_len, j = 0, i = 8; + static const char hex_str[] = "0123456789abcdef"; + + page_83 = kmalloc(MAX_BUFFER_LEN, GFP_KERNEL); + if (!page_83) + return; + + if (scsi_get_vpd_page(sdev, 0x83, page_83, MAX_BUFFER_LEN)) + goto err; + + page_len = ((page_83[2] << 8) | page_83[3]) + 4; + if (page_len > 0) { + if ((page_83[5] & 0x30) != 0) + goto err; + + id_len = page_83[7]; + if (id_len > 0) { + switch (page_83[4] & 0x0f) { + case SCSI_ID_BINARY: + while (i < id_len) { + disk_id[j++] = hex_str[(page_83[i] + & 0xf0) >> 4]; + disk_id[j++] = hex_str[page_83[i] + & 0x0f]; + i++; + } + break; + case SCSI_ID_ASCII: + case SCSI_ID_UTF8: + strncpy(disk_id, strim(page_83 + 8), id_len); + break; + default: + break; + } + } + } + +err: + kfree(page_83); + return; +} + +static ssize_t show_disk_name(struct device *dev, + struct device_attribute *attr, char *buf) +{ + return sprintf(buf, "%s\n", to_scsi_unnamed(dev)->disk_name); +} + +static ssize_t show_disk_lun(struct device *dev, + struct device_attribute *attr, char *buf) +{ + return sprintf(buf, "%s\n", to_scsi_unnamed(dev)->disk_lun); +} + +static ssize_t show_disk_id(struct device *dev, + struct device_attribute *attr, char *buf) +{ + return sprintf(buf, "%s\n", to_scsi_unnamed(dev)->disk_id); +} + +/* Assign a persistent device name */ +static ssize_t store_disk_name(struct device *dev, + struct device_attribute *attr, char *buf, size_t count) +{ + struct scsi_unnamed *su = to_scsi_unnamed(dev); + struct scsi_unnamed *tmp; + struct device *lu = dev->parent; + int ret = -EINVAL; + + if (count >= DISK_NAME_LEN) { + printk(KERN_WARNING "su: Too long a persistent name!\n"); + return -EINVAL; + } + + if (su->assigned) { + printk(KERN_WARNING "su: Already assigned a persistent name!\n"); + return -EINVAL; + } + + if (strncmp(lu->driver->name, buf, 2)) { + printk(KERN_WARNING "su: Wrong prefix!\n"); + return -EINVAL; + } + + mutex_lock(&su_list_lock); + list_for_each_entry(tmp, &su_list, list) { + if (!strncmp(tmp->disk_name, buf, DISK_NAME_LEN)) { + printk(KERN_WARNING "su: Duplicate name exsists!\n"); + return -EINVAL; + } + } + strncpy(su->disk_name, buf, DISK_NAME_LEN); + mutex_unlock(&su_list_lock); + + if (!lu->driver->probe) + return -EINVAL; + + lu->init_name = buf; + + ret = lu->driver->probe(lu); + if (ret) { + lu->init_name = NULL; + su->disk_name[0] = '\0'; + return ret; + } + + su->assigned = true; + return count; +} + +static DEVICE_ATTR(disk_name, S_IRUGO|S_IWUSR, show_disk_name, + store_disk_name); +static DEVICE_ATTR(disk_lun, S_IRUGO, show_disk_lun, NULL); +static DEVICE_ATTR(disk_id, S_IRUGO, show_disk_id, NULL); + +/* Confirm the driver name and the device type */ +static int check_device_type(char type, const char *name) +{ + switch (type) { + case TYPE_DISK: + case TYPE_MOD: + case TYPE_RBC: + if (strncmp("sd", name, 2)) + return -ENODEV; + break; + case TYPE_ROM: + case TYPE_WORM: + if (strncmp("sr", name, 2)) + return -ENODEV; + break; + case TYPE_TAPE: + if (strncmp("st", name, 2)) + return -ENODEV; + break; + default: + break; + } + return 0; +} + +/** + * scsi_unnamed_probe - Setup the unnamed device. + * @dev: parent scsi device + * + * This function adds a unnamed device to the device model and + * gets a number of device information by scsi inquiry commands. + * Udev identify a device by those information. + * + * Unnamed devices: + * This device is not a block device and user can not read/write + * this device. But it can advertise device information in sysfs. + */ +int scsi_unnamed_probe(struct device *dev) +{ + struct scsi_unnamed *su; + struct scsi_device *sdev = to_scsi_device(dev); + int ret = -EINVAL; + static int i; + + if (check_device_type(sdev->type, dev->driver->name)) + return -ENODEV; + + su = kzalloc(sizeof(*su), GFP_KERNEL); + if (!su) + return -ENOMEM; + + device_initialize(&su->dev); + su->dev.parent = dev; + su->dev.class = &su_sysfs_class; + dev_set_name(&su->dev, "su%d", i++); + strncpy(su->disk_lun, dev_name(dev), MAX_BUFFER_LEN); + + ret = device_add(&su->dev); + if (ret) + goto err; + + ret = device_create_file(&su->dev, &dev_attr_disk_name); + if (ret) + goto err_disk_name; + + ret = device_create_file(&su->dev, &dev_attr_disk_lun); + if (ret) + goto err_disk_lun; + + store_disk_id(sdev, su->disk_id); + if (su->disk_id[0] != '\0') { + ret = device_create_file(&su->dev, &dev_attr_disk_id); + if (ret) + goto err_disk_id; + } + + su->assigned = false; + mutex_lock(&su_list_lock); + list_add(&su->list, &su_list); + mutex_unlock(&su_list_lock); + + return 0; + +err_disk_id: + device_remove_file(&su->dev, &dev_attr_disk_lun); + +err_disk_lun: + device_remove_file(&su->dev, &dev_attr_disk_name); + +err_disk_name: + device_del(&su->dev); + +err: + kfree(su); + return ret; +} + +/* Remove a unnamed device from su_list and release resources */ +void scsi_unnamed_remove(struct device *dev) +{ + struct scsi_unnamed *tmp; + + mutex_lock(&su_list_lock); + list_for_each_entry(tmp, &su_list, list) { + if (dev = tmp->dev.parent) { + list_del(&tmp->list); + break; + } + } + mutex_unlock(&su_list_lock); + + if (tmp->disk_id[0] != '\0') + device_remove_file(&tmp->dev, &dev_attr_disk_id); + device_remove_file(&tmp->dev, &dev_attr_disk_name); + device_remove_file(&tmp->dev, &dev_attr_disk_lun); + device_del(&tmp->dev); + + device_release_driver(dev); + + kfree(tmp); +} + +/** + * copy_persistent_name - Copy the device name + * @disk_name: allocate to + * @dev: scsi device + * + * Copy an initial name of the device to disk_name. + */ +int copy_persistent_name(char *disk_name, struct device *dev) +{ + if (persistent_name) { + strncpy(disk_name, dev->init_name, DISK_NAME_LEN); + return 1; + } + return 0; +} +EXPORT_SYMBOL(copy_persistent_name); + +int scsi_unnamed_register(struct bus_type *bus) +{ + if (persistent_name) { + bus->probe = scsi_unnamed_probe; + bus->remove = scsi_unnamed_remove; + return class_register(&su_sysfs_class); + } + return 0; +} +EXPORT_SYMBOL(scsi_unnamed_register); + +void scsi_unnamed_unregister(void) +{ + if (persistent_name) + class_unregister(&su_sysfs_class); +} +EXPORT_SYMBOL(scsi_unnamed_unregister); diff --git a/drivers/scsi/scsi_unnamed.h b/drivers/scsi/scsi_unnamed.h new file mode 100644 index 0000000..ca4e852 --- /dev/null +++ b/drivers/scsi/scsi_unnamed.h @@ -0,0 +1,25 @@ +/* + * scsi_unnamed.h - SCSI driver for unnmaed device + * + * Copyright: Copyright (c) Hitachi, Ltd. 2011 + * Authors: Nao Nishijima <nao.nishijima.xt@hitachi.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111 USA + */ + +extern int check_disk_name_prefix(char *, struct device *); +extern int copy_persistent_name(char *, struct device *); +extern int scsi_unnamed_register(struct bus_type *); +extern void scsi_unnamed_unregister(void); ^ permalink raw reply related [flat|nested] 46+ messages in thread
* [PATCH 2/2] SCSI: modify SCSI subsystem 2011-04-05 12:49 ` [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Nao Nishijima @ 2011-04-05 12:50 ` Nao Nishijima -1 siblings, 0 replies; 46+ messages in thread From: Nao Nishijima @ 2011-04-05 12:50 UTC (permalink / raw) To: linux-kernel, linux-scsi, linux-hotplug Cc: Greg KH, Kay Sievers, James Bottomley, Jon Masters, 2nddept-manager, Nao Nishijima Add a SCSI option for persistent device names in Kernel. If scsi_mod.persistent_name=1, device names is assigned by udev. If scsi_mod.persistent_name=0, device names is assigned the order of logical unit recognizing. Signed-off-by: Nao Nishijima <nao.nishijima.xt@hitachi.com> --- drivers/scsi/sd.c | 10 +++++++--- drivers/scsi/sr.c | 4 +++- drivers/scsi/st.c | 4 +++- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index b61ebec..94ad290 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -65,6 +65,7 @@ #include "sd.h" #include "scsi_logging.h" +#include "scsi_unnamed.h" MODULE_AUTHOR("Eric Youngdale"); MODULE_DESCRIPTION("SCSI disk (sd) driver"); @@ -2554,9 +2555,12 @@ static int sd_probe(struct device *dev) goto out_free_index; } - error = sd_format_disk_name("sd", index, gd->disk_name, DISK_NAME_LEN); - if (error) - goto out_free_index; + if (!copy_persistent_name(gd->disk_name, dev)) { + error = sd_format_disk_name("sd", index, + gd->disk_name, DISK_NAME_LEN); + if (error) + goto out_free_index; + } sdkp->device = sdp; sdkp->driver = &sd_template; diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c index aefadc6..682b1a4 100644 --- a/drivers/scsi/sr.c +++ b/drivers/scsi/sr.c @@ -58,6 +58,7 @@ #include "scsi_logging.h" #include "sr.h" +#include "scsi_unnamed.h" MODULE_DESCRIPTION("SCSI cdrom (sr) driver"); @@ -634,7 +635,8 @@ static int sr_probe(struct device *dev) disk->major = SCSI_CDROM_MAJOR; disk->first_minor = minor; - sprintf(disk->disk_name, "sr%d", minor); + if (!copy_persistent_name(disk->disk_name, dev)) + sprintf(disk->disk_name, "sr%d", minor); disk->fops = &sr_bdops; disk->flags = GENHD_FL_CD; disk->events = DISK_EVENT_MEDIA_CHANGE | DISK_EVENT_EJECT_REQUEST; diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c index 1871b8a..9acf8b2 100644 --- a/drivers/scsi/st.c +++ b/drivers/scsi/st.c @@ -74,6 +74,7 @@ static const char *verstr = "20101219"; #include "st_options.h" #include "st.h" +#include "scsi_unnamed.h" static DEFINE_MUTEX(st_mutex); static int buffer_kbs; @@ -4051,7 +4052,8 @@ static int st_probe(struct device *dev) } kref_init(&tpnt->kref); tpnt->disk = disk; - sprintf(disk->disk_name, "st%d", i); + if (!copy_persistent_name(disk->disk_name, dev)) + sprintf(disk->disk_name, "st%d", i); disk->private_data = &tpnt->driver; disk->queue = SDp->request_queue; tpnt->driver = &st_template; ^ permalink raw reply related [flat|nested] 46+ messages in thread
* [PATCH 2/2] SCSI: modify SCSI subsystem @ 2011-04-05 12:50 ` Nao Nishijima 0 siblings, 0 replies; 46+ messages in thread From: Nao Nishijima @ 2011-04-05 12:50 UTC (permalink / raw) To: linux-kernel, linux-scsi, linux-hotplug Cc: Greg KH, Kay Sievers, James Bottomley, Jon Masters, 2nddept-manager, Nao Nishijima Add a SCSI option for persistent device names in Kernel. If scsi_mod.persistent_name=1, device names is assigned by udev. If scsi_mod.persistent_name=0, device names is assigned the order of logical unit recognizing. Signed-off-by: Nao Nishijima <nao.nishijima.xt@hitachi.com> --- drivers/scsi/sd.c | 10 +++++++--- drivers/scsi/sr.c | 4 +++- drivers/scsi/st.c | 4 +++- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index b61ebec..94ad290 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -65,6 +65,7 @@ #include "sd.h" #include "scsi_logging.h" +#include "scsi_unnamed.h" MODULE_AUTHOR("Eric Youngdale"); MODULE_DESCRIPTION("SCSI disk (sd) driver"); @@ -2554,9 +2555,12 @@ static int sd_probe(struct device *dev) goto out_free_index; } - error = sd_format_disk_name("sd", index, gd->disk_name, DISK_NAME_LEN); - if (error) - goto out_free_index; + if (!copy_persistent_name(gd->disk_name, dev)) { + error = sd_format_disk_name("sd", index, + gd->disk_name, DISK_NAME_LEN); + if (error) + goto out_free_index; + } sdkp->device = sdp; sdkp->driver = &sd_template; diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c index aefadc6..682b1a4 100644 --- a/drivers/scsi/sr.c +++ b/drivers/scsi/sr.c @@ -58,6 +58,7 @@ #include "scsi_logging.h" #include "sr.h" +#include "scsi_unnamed.h" MODULE_DESCRIPTION("SCSI cdrom (sr) driver"); @@ -634,7 +635,8 @@ static int sr_probe(struct device *dev) disk->major = SCSI_CDROM_MAJOR; disk->first_minor = minor; - sprintf(disk->disk_name, "sr%d", minor); + if (!copy_persistent_name(disk->disk_name, dev)) + sprintf(disk->disk_name, "sr%d", minor); disk->fops = &sr_bdops; disk->flags = GENHD_FL_CD; disk->events = DISK_EVENT_MEDIA_CHANGE | DISK_EVENT_EJECT_REQUEST; diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c index 1871b8a..9acf8b2 100644 --- a/drivers/scsi/st.c +++ b/drivers/scsi/st.c @@ -74,6 +74,7 @@ static const char *verstr = "20101219"; #include "st_options.h" #include "st.h" +#include "scsi_unnamed.h" static DEFINE_MUTEX(st_mutex); static int buffer_kbs; @@ -4051,7 +4052,8 @@ static int st_probe(struct device *dev) } kref_init(&tpnt->kref); tpnt->disk = disk; - sprintf(disk->disk_name, "st%d", i); + if (!copy_persistent_name(disk->disk_name, dev)) + sprintf(disk->disk_name, "st%d", i); disk->private_data = &tpnt->driver; disk->queue = SDp->request_queue; tpnt->driver = &st_template; ^ permalink raw reply related [flat|nested] 46+ messages in thread
* Re: [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel. 2011-04-05 12:49 ` [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Nao Nishijima @ 2011-04-05 16:14 ` Greg KH -1 siblings, 0 replies; 46+ messages in thread From: Greg KH @ 2011-04-05 16:14 UTC (permalink / raw) To: Nao Nishijima Cc: linux-kernel, linux-scsi, linux-hotplug, Kay Sievers, James Bottomley, Jon Masters, 2nddept-manager On Tue, Apr 05, 2011 at 09:49:46PM +0900, Nao Nishijima wrote: > This patch series provides a SCSI option for persistent device > names in kernel. With this option, user can always assign a > same device name (e.g. sda) to a specific device according to > udev rules and device id. > > Issue: > Recently, kernel registers block devices in parallel. As a result, > different device names will be assigned at each boot time. This > will confuse file-system mounter, thus we usually use persistent > symbolic links provided by udev. Yes, that is what you should use if you care about this. > However, dmesg and procfs outputs > show device names instead of the symbolic link names. This causes > a serious problem when managing multiple devices (e.g. on a > large-scale storage), because usually, device errors are output > with device names on dmesg. Then fix your tools that read the output of these files to point to the proper persistent name instead of trying to get the kernel to solve the problem. > We also concern about some commands > which output device names, as well as kernel messages. What commands are you concerned about? > Solution: > To assign a persistent device name, I create unnamed devices (not > a block device, but an intermediate device. users cannot read/write > this device). When scsi device driver finds a LU, the LU is registered > as an unnamed device and inform to udev. After udev finds the unnamed > device, udev assigns a persistent device name to a specific device > according to udev rules and registers it as a block device. Hence, > this is just a naming, not a renaming. Ick, so the kernel waits for userspace before you are able to use the device? That sounds messy. > Some users are satisfied with current udev solution. Therefore, my > proposal is implemented as an option. > > If using this option, add the following kernel parameter. > > scsi_mod.persistent_name=1 > > Also, if you want to assign a device name with sda, add the > following udev rules. > > SUBSYSTEM=="scsi_unnamed", ATTR{disk_id}=="xxx", PROGRAM="/bin/sh > -c 'echo -n sda > /sys/%p/disk_name'" > > Todo: > - usb support Why? USB uses scsi, so why would it not work as-is today? What about libata devices? > - hot-remove support So once you have assigned a name and the device is removed bad things happen? What is the problem with this? Note, any review of the code below does not imply that I agree with the ideas here at all. In fact, I really don't like this idea at all, but I might as well review the code anyway for your future contributions :) > > I've already started discussion about device naming at LKML. > https://lkml.org/lkml/2010/10/8/31 > > Signed-off-by: Nao Nishijima <nao.nishijima.xt@hitachi.com> > --- > > drivers/scsi/Makefile | 1 > drivers/scsi/scsi_sysfs.c | 26 +++ > drivers/scsi/scsi_unnamed.c | 340 +++++++++++++++++++++++++++++++++++++++++++ > drivers/scsi/scsi_unnamed.h | 25 +++ > 4 files changed, 387 insertions(+), 5 deletions(-) > create mode 100644 drivers/scsi/scsi_unnamed.c > create mode 100644 drivers/scsi/scsi_unnamed.h > > diff --git a/drivers/scsi/Makefile b/drivers/scsi/Makefile > index 7ad0b8a..782adc1 100644 > --- a/drivers/scsi/Makefile > +++ b/drivers/scsi/Makefile > @@ -167,6 +167,7 @@ scsi_mod-$(CONFIG_SYSCTL) += scsi_sysctl.o > scsi_mod-$(CONFIG_SCSI_PROC_FS) += scsi_proc.o > scsi_mod-y += scsi_trace.o > scsi_mod-$(CONFIG_PM) += scsi_pm.o > +scsi_mod-y += scsi_unnamed.o > > scsi_tgt-y += scsi_tgt_lib.o scsi_tgt_if.o > > diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c > index e44ff64..7959f5d 100644 > --- a/drivers/scsi/scsi_sysfs.c > +++ b/drivers/scsi/scsi_sysfs.c > @@ -22,6 +22,7 @@ > > #include "scsi_priv.h" > #include "scsi_logging.h" > +#include "scsi_unnamed.h" > > static struct device_type scsi_dev_type; > > @@ -393,13 +394,27 @@ int scsi_sysfs_register(void) > { > int error; > > + error = scsi_unnamed_register(&scsi_bus_type); > + if (error) > + goto cleanup; > + > error = bus_register(&scsi_bus_type); > - if (!error) { > - error = class_register(&sdev_class); > - if (error) > - bus_unregister(&scsi_bus_type); > - } > + if (error) > + goto cleanup_scsi_unnamed; > + > + error = class_register(&sdev_class); > + if (error) > + goto cleanup_bus; > + > + return 0; > + > +cleanup_bus: > + bus_unregister(&scsi_bus_type); > + > +cleanup_scsi_unnamed: > + scsi_unnamed_unregister(); > > +cleanup: > return error; > } > > @@ -407,6 +422,7 @@ void scsi_sysfs_unregister(void) > { > class_unregister(&sdev_class); > bus_unregister(&scsi_bus_type); > + scsi_unnamed_unregister(); > } > > /* > diff --git a/drivers/scsi/scsi_unnamed.c b/drivers/scsi/scsi_unnamed.c > new file mode 100644 > index 0000000..ed96945 > --- /dev/null > +++ b/drivers/scsi/scsi_unnamed.c > @@ -0,0 +1,340 @@ > +/* > + * scsi_unnamed.c - SCSI driver for unnmaed device > + * > + * Copyright: Copyright (c) Hitachi, Ltd. 2011 > + * Authors: Nao Nishijima <nao.nishijima.xt@hitachi.com> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. Do you _REALLY_ mean "any later version? Are you sure about that? If so, fine, just be careful please. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program; if not, write to the Free Software > + * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111 USA Unless you wish to track the movements of the FSF's office space for the next 40+ years and keep this file up to date, just remove this paragraph please. > + */ > + > +#include <linux/module.h> > +#include <linux/err.h> > +#include <linux/device.h> > +#include <linux/slab.h> > +#include <linux/kobject.h> > +#include <linux/kdev_t.h> > +#include <linux/sysdev.h> > +#include <linux/list.h> > +#include <linux/wait.h> > + > +#include <scsi/scsi_driver.h> > +#include <scsi/scsi_device.h> > +#include <scsi/scsi.h> > + > +#define MAX_BUFFER_LEN 256 > +#define DISK_NAME_LEN 32 Why this limitation? > + > +#define SCSI_ID_BINARY 1 > +#define SCSI_ID_ASCII 2 > +#define SCSI_ID_UTF8 3 > + > +static LIST_HEAD(su_list); > +static DEFINE_MUTEX(su_list_lock); > + > +static int persistent_name; > +MODULE_PARM_DESC(persistent_name, "SCSI unnamed device support"); > +module_param(persistent_name, bool, S_IRUGO); Why is this a module parameter? > + > +static struct class su_sysfs_class = { > + .name = "scsi_unnamed", > +}; Why a static class? What is it used for? If you are adding sysfs files, you need to also add Documentation/ABI/ entries at the same time. Please do that if you really are adding new sysfs files. > + > +struct scsi_unnamed { > + struct list_head list; > + struct device dev; > + char disk_name[DISK_NAME_LEN]; > + char disk_lun[MAX_BUFFER_LEN]; > + char disk_id[MAX_BUFFER_LEN]; > + bool assigned; > +}; > + > +#define to_scsi_unnamed(d) \ > + container_of(d, struct scsi_unnamed, dev) > + > +/* Get device identification VPD page */ > +static void store_disk_id(struct scsi_device *sdev, char *disk_id) > +{ > + unsigned char *page_83; > + int page_len, id_len, j = 0, i = 8; > + static const char hex_str[] = "0123456789abcdef"; Don't we have functions that do this already? > + > + page_83 = kmalloc(MAX_BUFFER_LEN, GFP_KERNEL); > + if (!page_83) > + return; > + > + if (scsi_get_vpd_page(sdev, 0x83, page_83, MAX_BUFFER_LEN)) > + goto err; > + > + page_len = ((page_83[2] << 8) | page_83[3]) + 4; > + if (page_len > 0) { > + if ((page_83[5] & 0x30) != 0) > + goto err; > + > + id_len = page_83[7]; > + if (id_len > 0) { > + switch (page_83[4] & 0x0f) { > + case SCSI_ID_BINARY: > + while (i < id_len) { > + disk_id[j++] = hex_str[(page_83[i] > + & 0xf0) >> 4]; > + disk_id[j++] = hex_str[page_83[i] > + & 0x0f]; > + i++; > + } > + break; > + case SCSI_ID_ASCII: > + case SCSI_ID_UTF8: > + strncpy(disk_id, strim(page_83 + 8), id_len); > + break; > + default: > + break; > + } > + } > + } > + > +err: > + kfree(page_83); > + return; > +} > + > +static ssize_t show_disk_name(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + return sprintf(buf, "%s\n", to_scsi_unnamed(dev)->disk_name); > +} > + > +static ssize_t show_disk_lun(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + return sprintf(buf, "%s\n", to_scsi_unnamed(dev)->disk_lun); > +} > + > +static ssize_t show_disk_id(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + return sprintf(buf, "%s\n", to_scsi_unnamed(dev)->disk_id); > +} > + > +/* Assign a persistent device name */ > +static ssize_t store_disk_name(struct device *dev, > + struct device_attribute *attr, char *buf, size_t count) > +{ > + struct scsi_unnamed *su = to_scsi_unnamed(dev); > + struct scsi_unnamed *tmp; > + struct device *lu = dev->parent; > + int ret = -EINVAL; > + > + if (count >= DISK_NAME_LEN) { > + printk(KERN_WARNING "su: Too long a persistent name!\n"); > + return -EINVAL; > + } > + > + if (su->assigned) { > + printk(KERN_WARNING "su: Already assigned a persistent name!\n"); > + return -EINVAL; > + } > + > + if (strncmp(lu->driver->name, buf, 2)) { > + printk(KERN_WARNING "su: Wrong prefix!\n"); > + return -EINVAL; > + } > + > + mutex_lock(&su_list_lock); > + list_for_each_entry(tmp, &su_list, list) { > + if (!strncmp(tmp->disk_name, buf, DISK_NAME_LEN)) { > + printk(KERN_WARNING "su: Duplicate name exsists!\n"); > + return -EINVAL; > + } > + } > + strncpy(su->disk_name, buf, DISK_NAME_LEN); > + mutex_unlock(&su_list_lock); > + > + if (!lu->driver->probe) > + return -EINVAL; > + > + lu->init_name = buf; > + > + ret = lu->driver->probe(lu); > + if (ret) { > + lu->init_name = NULL; > + su->disk_name[0] = '\0'; > + return ret; > + } > + > + su->assigned = true; > + return count; > +} > + > +static DEVICE_ATTR(disk_name, S_IRUGO|S_IWUSR, show_disk_name, > + store_disk_name); > +static DEVICE_ATTR(disk_lun, S_IRUGO, show_disk_lun, NULL); > +static DEVICE_ATTR(disk_id, S_IRUGO, show_disk_id, NULL); > + > +/* Confirm the driver name and the device type */ > +static int check_device_type(char type, const char *name) > +{ > + switch (type) { > + case TYPE_DISK: > + case TYPE_MOD: > + case TYPE_RBC: > + if (strncmp("sd", name, 2)) > + return -ENODEV; > + break; > + case TYPE_ROM: > + case TYPE_WORM: > + if (strncmp("sr", name, 2)) > + return -ENODEV; > + break; > + case TYPE_TAPE: > + if (strncmp("st", name, 2)) > + return -ENODEV; > + break; > + default: > + break; > + } > + return 0; > +} > + > +/** > + * scsi_unnamed_probe - Setup the unnamed device. > + * @dev: parent scsi device > + * > + * This function adds a unnamed device to the device model and > + * gets a number of device information by scsi inquiry commands. > + * Udev identify a device by those information. > + * > + * Unnamed devices: > + * This device is not a block device and user can not read/write > + * this device. But it can advertise device information in sysfs. > + */ > +int scsi_unnamed_probe(struct device *dev) > +{ > + struct scsi_unnamed *su; > + struct scsi_device *sdev = to_scsi_device(dev); > + int ret = -EINVAL; > + static int i; > + > + if (check_device_type(sdev->type, dev->driver->name)) > + return -ENODEV; > + > + su = kzalloc(sizeof(*su), GFP_KERNEL); > + if (!su) > + return -ENOMEM; > + > + device_initialize(&su->dev); > + su->dev.parent = dev; > + su->dev.class = &su_sysfs_class; You just created a device that can not be removed from the system (i.e. you have no release function.) This is a major bug and as per the documentation for kobjects and the driver model in the kernel tree, I am allowed to mock this code mercilessly :) > + dev_set_name(&su->dev, "su%d", i++); > + strncpy(su->disk_lun, dev_name(dev), MAX_BUFFER_LEN); > + > + ret = device_add(&su->dev); > + if (ret) > + goto err; > + > + ret = device_create_file(&su->dev, &dev_attr_disk_name); Nope, you just raced with userspace here. NEVER create a sysfs file after you have added it to the system. Userspace just got notified of the device and is already starting to read the file. As it's not there yet, you just failed :( Use attributes properly to keep this race from happening. > + if (ret) > + goto err_disk_name; > + > + ret = device_create_file(&su->dev, &dev_attr_disk_lun); > + if (ret) > + goto err_disk_lun; > + > + store_disk_id(sdev, su->disk_id); > + if (su->disk_id[0] != '\0') { > + ret = device_create_file(&su->dev, &dev_attr_disk_id); > + if (ret) > + goto err_disk_id; > + } > + > + su->assigned = false; > + mutex_lock(&su_list_lock); > + list_add(&su->list, &su_list); > + mutex_unlock(&su_list_lock); > + > + return 0; > + > +err_disk_id: > + device_remove_file(&su->dev, &dev_attr_disk_lun); > + > +err_disk_lun: > + device_remove_file(&su->dev, &dev_attr_disk_name); > + > +err_disk_name: > + device_del(&su->dev); > + > +err: > + kfree(su); > + return ret; > +} > + > +/* Remove a unnamed device from su_list and release resources */ > +void scsi_unnamed_remove(struct device *dev) > +{ > + struct scsi_unnamed *tmp; > + > + mutex_lock(&su_list_lock); > + list_for_each_entry(tmp, &su_list, list) { > + if (dev == tmp->dev.parent) { > + list_del(&tmp->list); > + break; > + } > + } > + mutex_unlock(&su_list_lock); > + > + if (tmp->disk_id[0] != '\0') > + device_remove_file(&tmp->dev, &dev_attr_disk_id); > + device_remove_file(&tmp->dev, &dev_attr_disk_name); > + device_remove_file(&tmp->dev, &dev_attr_disk_lun); > + device_del(&tmp->dev); Your kerrnel just spit out some very nasty messages when this function was called, right? Why are you ignoring them? > + > + device_release_driver(dev); > + > + kfree(tmp); > +} > + > +/** > + * copy_persistent_name - Copy the device name > + * @disk_name: allocate to > + * @dev: scsi device > + * > + * Copy an initial name of the device to disk_name. > + */ > +int copy_persistent_name(char *disk_name, struct device *dev) > +{ > + if (persistent_name) { > + strncpy(disk_name, dev->init_name, DISK_NAME_LEN); > + return 1; > + } > + return 0; > +} > +EXPORT_SYMBOL(copy_persistent_name); That's a very bad global function name. EXPORT_SYMBOL_GPL? And why is it exported at all? Who would ever need to call this from a module? > + > +int scsi_unnamed_register(struct bus_type *bus) > +{ > + if (persistent_name) { > + bus->probe = scsi_unnamed_probe; > + bus->remove = scsi_unnamed_remove; > + return class_register(&su_sysfs_class); > + } > + return 0; > +} > +EXPORT_SYMBOL(scsi_unnamed_register); Same here, why is this exported? > + > +void scsi_unnamed_unregister(void) > +{ > + if (persistent_name) > + class_unregister(&su_sysfs_class); > +} > +EXPORT_SYMBOL(scsi_unnamed_unregister); And here. > diff --git a/drivers/scsi/scsi_unnamed.h b/drivers/scsi/scsi_unnamed.h > new file mode 100644 > index 0000000..ca4e852 > --- /dev/null > +++ b/drivers/scsi/scsi_unnamed.h > @@ -0,0 +1,25 @@ > +/* > + * scsi_unnamed.h - SCSI driver for unnmaed device > + * > + * Copyright: Copyright (c) Hitachi, Ltd. 2011 > + * Authors: Nao Nishijima <nao.nishijima.xt@hitachi.com> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. Same comment as above. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program; if not, write to the Free Software > + * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111 USA And again, same comment as above. thanks, greg k-h ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 1/2] SCSI: Add a SCSI option for persistent device names @ 2011-04-05 16:14 ` Greg KH 0 siblings, 0 replies; 46+ messages in thread From: Greg KH @ 2011-04-05 16:14 UTC (permalink / raw) To: Nao Nishijima Cc: linux-kernel, linux-scsi, linux-hotplug, Kay Sievers, James Bottomley, Jon Masters, 2nddept-manager On Tue, Apr 05, 2011 at 09:49:46PM +0900, Nao Nishijima wrote: > This patch series provides a SCSI option for persistent device > names in kernel. With this option, user can always assign a > same device name (e.g. sda) to a specific device according to > udev rules and device id. > > Issue: > Recently, kernel registers block devices in parallel. As a result, > different device names will be assigned at each boot time. This > will confuse file-system mounter, thus we usually use persistent > symbolic links provided by udev. Yes, that is what you should use if you care about this. > However, dmesg and procfs outputs > show device names instead of the symbolic link names. This causes > a serious problem when managing multiple devices (e.g. on a > large-scale storage), because usually, device errors are output > with device names on dmesg. Then fix your tools that read the output of these files to point to the proper persistent name instead of trying to get the kernel to solve the problem. > We also concern about some commands > which output device names, as well as kernel messages. What commands are you concerned about? > Solution: > To assign a persistent device name, I create unnamed devices (not > a block device, but an intermediate device. users cannot read/write > this device). When scsi device driver finds a LU, the LU is registered > as an unnamed device and inform to udev. After udev finds the unnamed > device, udev assigns a persistent device name to a specific device > according to udev rules and registers it as a block device. Hence, > this is just a naming, not a renaming. Ick, so the kernel waits for userspace before you are able to use the device? That sounds messy. > Some users are satisfied with current udev solution. Therefore, my > proposal is implemented as an option. > > If using this option, add the following kernel parameter. > > scsi_mod.persistent_name=1 > > Also, if you want to assign a device name with sda, add the > following udev rules. > > SUBSYSTEM="scsi_unnamed", ATTR{disk_id}="xxx", PROGRAM="/bin/sh > -c 'echo -n sda > /sys/%p/disk_name'" > > Todo: > - usb support Why? USB uses scsi, so why would it not work as-is today? What about libata devices? > - hot-remove support So once you have assigned a name and the device is removed bad things happen? What is the problem with this? Note, any review of the code below does not imply that I agree with the ideas here at all. In fact, I really don't like this idea at all, but I might as well review the code anyway for your future contributions :) > > I've already started discussion about device naming at LKML. > https://lkml.org/lkml/2010/10/8/31 > > Signed-off-by: Nao Nishijima <nao.nishijima.xt@hitachi.com> > --- > > drivers/scsi/Makefile | 1 > drivers/scsi/scsi_sysfs.c | 26 +++ > drivers/scsi/scsi_unnamed.c | 340 +++++++++++++++++++++++++++++++++++++++++++ > drivers/scsi/scsi_unnamed.h | 25 +++ > 4 files changed, 387 insertions(+), 5 deletions(-) > create mode 100644 drivers/scsi/scsi_unnamed.c > create mode 100644 drivers/scsi/scsi_unnamed.h > > diff --git a/drivers/scsi/Makefile b/drivers/scsi/Makefile > index 7ad0b8a..782adc1 100644 > --- a/drivers/scsi/Makefile > +++ b/drivers/scsi/Makefile > @@ -167,6 +167,7 @@ scsi_mod-$(CONFIG_SYSCTL) += scsi_sysctl.o > scsi_mod-$(CONFIG_SCSI_PROC_FS) += scsi_proc.o > scsi_mod-y += scsi_trace.o > scsi_mod-$(CONFIG_PM) += scsi_pm.o > +scsi_mod-y += scsi_unnamed.o > > scsi_tgt-y += scsi_tgt_lib.o scsi_tgt_if.o > > diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c > index e44ff64..7959f5d 100644 > --- a/drivers/scsi/scsi_sysfs.c > +++ b/drivers/scsi/scsi_sysfs.c > @@ -22,6 +22,7 @@ > > #include "scsi_priv.h" > #include "scsi_logging.h" > +#include "scsi_unnamed.h" > > static struct device_type scsi_dev_type; > > @@ -393,13 +394,27 @@ int scsi_sysfs_register(void) > { > int error; > > + error = scsi_unnamed_register(&scsi_bus_type); > + if (error) > + goto cleanup; > + > error = bus_register(&scsi_bus_type); > - if (!error) { > - error = class_register(&sdev_class); > - if (error) > - bus_unregister(&scsi_bus_type); > - } > + if (error) > + goto cleanup_scsi_unnamed; > + > + error = class_register(&sdev_class); > + if (error) > + goto cleanup_bus; > + > + return 0; > + > +cleanup_bus: > + bus_unregister(&scsi_bus_type); > + > +cleanup_scsi_unnamed: > + scsi_unnamed_unregister(); > > +cleanup: > return error; > } > > @@ -407,6 +422,7 @@ void scsi_sysfs_unregister(void) > { > class_unregister(&sdev_class); > bus_unregister(&scsi_bus_type); > + scsi_unnamed_unregister(); > } > > /* > diff --git a/drivers/scsi/scsi_unnamed.c b/drivers/scsi/scsi_unnamed.c > new file mode 100644 > index 0000000..ed96945 > --- /dev/null > +++ b/drivers/scsi/scsi_unnamed.c > @@ -0,0 +1,340 @@ > +/* > + * scsi_unnamed.c - SCSI driver for unnmaed device > + * > + * Copyright: Copyright (c) Hitachi, Ltd. 2011 > + * Authors: Nao Nishijima <nao.nishijima.xt@hitachi.com> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. Do you _REALLY_ mean "any later version? Are you sure about that? If so, fine, just be careful please. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program; if not, write to the Free Software > + * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111 USA Unless you wish to track the movements of the FSF's office space for the next 40+ years and keep this file up to date, just remove this paragraph please. > + */ > + > +#include <linux/module.h> > +#include <linux/err.h> > +#include <linux/device.h> > +#include <linux/slab.h> > +#include <linux/kobject.h> > +#include <linux/kdev_t.h> > +#include <linux/sysdev.h> > +#include <linux/list.h> > +#include <linux/wait.h> > + > +#include <scsi/scsi_driver.h> > +#include <scsi/scsi_device.h> > +#include <scsi/scsi.h> > + > +#define MAX_BUFFER_LEN 256 > +#define DISK_NAME_LEN 32 Why this limitation? > + > +#define SCSI_ID_BINARY 1 > +#define SCSI_ID_ASCII 2 > +#define SCSI_ID_UTF8 3 > + > +static LIST_HEAD(su_list); > +static DEFINE_MUTEX(su_list_lock); > + > +static int persistent_name; > +MODULE_PARM_DESC(persistent_name, "SCSI unnamed device support"); > +module_param(persistent_name, bool, S_IRUGO); Why is this a module parameter? > + > +static struct class su_sysfs_class = { > + .name = "scsi_unnamed", > +}; Why a static class? What is it used for? If you are adding sysfs files, you need to also add Documentation/ABI/ entries at the same time. Please do that if you really are adding new sysfs files. > + > +struct scsi_unnamed { > + struct list_head list; > + struct device dev; > + char disk_name[DISK_NAME_LEN]; > + char disk_lun[MAX_BUFFER_LEN]; > + char disk_id[MAX_BUFFER_LEN]; > + bool assigned; > +}; > + > +#define to_scsi_unnamed(d) \ > + container_of(d, struct scsi_unnamed, dev) > + > +/* Get device identification VPD page */ > +static void store_disk_id(struct scsi_device *sdev, char *disk_id) > +{ > + unsigned char *page_83; > + int page_len, id_len, j = 0, i = 8; > + static const char hex_str[] = "0123456789abcdef"; Don't we have functions that do this already? > + > + page_83 = kmalloc(MAX_BUFFER_LEN, GFP_KERNEL); > + if (!page_83) > + return; > + > + if (scsi_get_vpd_page(sdev, 0x83, page_83, MAX_BUFFER_LEN)) > + goto err; > + > + page_len = ((page_83[2] << 8) | page_83[3]) + 4; > + if (page_len > 0) { > + if ((page_83[5] & 0x30) != 0) > + goto err; > + > + id_len = page_83[7]; > + if (id_len > 0) { > + switch (page_83[4] & 0x0f) { > + case SCSI_ID_BINARY: > + while (i < id_len) { > + disk_id[j++] = hex_str[(page_83[i] > + & 0xf0) >> 4]; > + disk_id[j++] = hex_str[page_83[i] > + & 0x0f]; > + i++; > + } > + break; > + case SCSI_ID_ASCII: > + case SCSI_ID_UTF8: > + strncpy(disk_id, strim(page_83 + 8), id_len); > + break; > + default: > + break; > + } > + } > + } > + > +err: > + kfree(page_83); > + return; > +} > + > +static ssize_t show_disk_name(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + return sprintf(buf, "%s\n", to_scsi_unnamed(dev)->disk_name); > +} > + > +static ssize_t show_disk_lun(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + return sprintf(buf, "%s\n", to_scsi_unnamed(dev)->disk_lun); > +} > + > +static ssize_t show_disk_id(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + return sprintf(buf, "%s\n", to_scsi_unnamed(dev)->disk_id); > +} > + > +/* Assign a persistent device name */ > +static ssize_t store_disk_name(struct device *dev, > + struct device_attribute *attr, char *buf, size_t count) > +{ > + struct scsi_unnamed *su = to_scsi_unnamed(dev); > + struct scsi_unnamed *tmp; > + struct device *lu = dev->parent; > + int ret = -EINVAL; > + > + if (count >= DISK_NAME_LEN) { > + printk(KERN_WARNING "su: Too long a persistent name!\n"); > + return -EINVAL; > + } > + > + if (su->assigned) { > + printk(KERN_WARNING "su: Already assigned a persistent name!\n"); > + return -EINVAL; > + } > + > + if (strncmp(lu->driver->name, buf, 2)) { > + printk(KERN_WARNING "su: Wrong prefix!\n"); > + return -EINVAL; > + } > + > + mutex_lock(&su_list_lock); > + list_for_each_entry(tmp, &su_list, list) { > + if (!strncmp(tmp->disk_name, buf, DISK_NAME_LEN)) { > + printk(KERN_WARNING "su: Duplicate name exsists!\n"); > + return -EINVAL; > + } > + } > + strncpy(su->disk_name, buf, DISK_NAME_LEN); > + mutex_unlock(&su_list_lock); > + > + if (!lu->driver->probe) > + return -EINVAL; > + > + lu->init_name = buf; > + > + ret = lu->driver->probe(lu); > + if (ret) { > + lu->init_name = NULL; > + su->disk_name[0] = '\0'; > + return ret; > + } > + > + su->assigned = true; > + return count; > +} > + > +static DEVICE_ATTR(disk_name, S_IRUGO|S_IWUSR, show_disk_name, > + store_disk_name); > +static DEVICE_ATTR(disk_lun, S_IRUGO, show_disk_lun, NULL); > +static DEVICE_ATTR(disk_id, S_IRUGO, show_disk_id, NULL); > + > +/* Confirm the driver name and the device type */ > +static int check_device_type(char type, const char *name) > +{ > + switch (type) { > + case TYPE_DISK: > + case TYPE_MOD: > + case TYPE_RBC: > + if (strncmp("sd", name, 2)) > + return -ENODEV; > + break; > + case TYPE_ROM: > + case TYPE_WORM: > + if (strncmp("sr", name, 2)) > + return -ENODEV; > + break; > + case TYPE_TAPE: > + if (strncmp("st", name, 2)) > + return -ENODEV; > + break; > + default: > + break; > + } > + return 0; > +} > + > +/** > + * scsi_unnamed_probe - Setup the unnamed device. > + * @dev: parent scsi device > + * > + * This function adds a unnamed device to the device model and > + * gets a number of device information by scsi inquiry commands. > + * Udev identify a device by those information. > + * > + * Unnamed devices: > + * This device is not a block device and user can not read/write > + * this device. But it can advertise device information in sysfs. > + */ > +int scsi_unnamed_probe(struct device *dev) > +{ > + struct scsi_unnamed *su; > + struct scsi_device *sdev = to_scsi_device(dev); > + int ret = -EINVAL; > + static int i; > + > + if (check_device_type(sdev->type, dev->driver->name)) > + return -ENODEV; > + > + su = kzalloc(sizeof(*su), GFP_KERNEL); > + if (!su) > + return -ENOMEM; > + > + device_initialize(&su->dev); > + su->dev.parent = dev; > + su->dev.class = &su_sysfs_class; You just created a device that can not be removed from the system (i.e. you have no release function.) This is a major bug and as per the documentation for kobjects and the driver model in the kernel tree, I am allowed to mock this code mercilessly :) > + dev_set_name(&su->dev, "su%d", i++); > + strncpy(su->disk_lun, dev_name(dev), MAX_BUFFER_LEN); > + > + ret = device_add(&su->dev); > + if (ret) > + goto err; > + > + ret = device_create_file(&su->dev, &dev_attr_disk_name); Nope, you just raced with userspace here. NEVER create a sysfs file after you have added it to the system. Userspace just got notified of the device and is already starting to read the file. As it's not there yet, you just failed :( Use attributes properly to keep this race from happening. > + if (ret) > + goto err_disk_name; > + > + ret = device_create_file(&su->dev, &dev_attr_disk_lun); > + if (ret) > + goto err_disk_lun; > + > + store_disk_id(sdev, su->disk_id); > + if (su->disk_id[0] != '\0') { > + ret = device_create_file(&su->dev, &dev_attr_disk_id); > + if (ret) > + goto err_disk_id; > + } > + > + su->assigned = false; > + mutex_lock(&su_list_lock); > + list_add(&su->list, &su_list); > + mutex_unlock(&su_list_lock); > + > + return 0; > + > +err_disk_id: > + device_remove_file(&su->dev, &dev_attr_disk_lun); > + > +err_disk_lun: > + device_remove_file(&su->dev, &dev_attr_disk_name); > + > +err_disk_name: > + device_del(&su->dev); > + > +err: > + kfree(su); > + return ret; > +} > + > +/* Remove a unnamed device from su_list and release resources */ > +void scsi_unnamed_remove(struct device *dev) > +{ > + struct scsi_unnamed *tmp; > + > + mutex_lock(&su_list_lock); > + list_for_each_entry(tmp, &su_list, list) { > + if (dev = tmp->dev.parent) { > + list_del(&tmp->list); > + break; > + } > + } > + mutex_unlock(&su_list_lock); > + > + if (tmp->disk_id[0] != '\0') > + device_remove_file(&tmp->dev, &dev_attr_disk_id); > + device_remove_file(&tmp->dev, &dev_attr_disk_name); > + device_remove_file(&tmp->dev, &dev_attr_disk_lun); > + device_del(&tmp->dev); Your kerrnel just spit out some very nasty messages when this function was called, right? Why are you ignoring them? > + > + device_release_driver(dev); > + > + kfree(tmp); > +} > + > +/** > + * copy_persistent_name - Copy the device name > + * @disk_name: allocate to > + * @dev: scsi device > + * > + * Copy an initial name of the device to disk_name. > + */ > +int copy_persistent_name(char *disk_name, struct device *dev) > +{ > + if (persistent_name) { > + strncpy(disk_name, dev->init_name, DISK_NAME_LEN); > + return 1; > + } > + return 0; > +} > +EXPORT_SYMBOL(copy_persistent_name); That's a very bad global function name. EXPORT_SYMBOL_GPL? And why is it exported at all? Who would ever need to call this from a module? > + > +int scsi_unnamed_register(struct bus_type *bus) > +{ > + if (persistent_name) { > + bus->probe = scsi_unnamed_probe; > + bus->remove = scsi_unnamed_remove; > + return class_register(&su_sysfs_class); > + } > + return 0; > +} > +EXPORT_SYMBOL(scsi_unnamed_register); Same here, why is this exported? > + > +void scsi_unnamed_unregister(void) > +{ > + if (persistent_name) > + class_unregister(&su_sysfs_class); > +} > +EXPORT_SYMBOL(scsi_unnamed_unregister); And here. > diff --git a/drivers/scsi/scsi_unnamed.h b/drivers/scsi/scsi_unnamed.h > new file mode 100644 > index 0000000..ca4e852 > --- /dev/null > +++ b/drivers/scsi/scsi_unnamed.h > @@ -0,0 +1,25 @@ > +/* > + * scsi_unnamed.h - SCSI driver for unnmaed device > + * > + * Copyright: Copyright (c) Hitachi, Ltd. 2011 > + * Authors: Nao Nishijima <nao.nishijima.xt@hitachi.com> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. Same comment as above. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program; if not, write to the Free Software > + * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111 USA And again, same comment as above. thanks, greg k-h ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel. 2011-04-05 16:14 ` [PATCH 1/2] SCSI: Add a SCSI option for persistent device names Greg KH (?) @ 2011-04-08 14:12 ` Nao Nishijima -1 siblings, 0 replies; 46+ messages in thread From: Nao Nishijima @ 2011-04-08 14:12 UTC (permalink / raw) To: Greg KH Cc: linux-kernel, linux-scsi, linux-hotplug, Kay Sievers, James Bottomley, Jon Masters, 2nddept-manager Hi, (2011/04/06 1:14), Greg KH wrote: > On Tue, Apr 05, 2011 at 09:49:46PM +0900, Nao Nishijima wrote: >> This patch series provides a SCSI option for persistent device >> names in kernel. With this option, user can always assign a >> same device name (e.g. sda) to a specific device according to >> udev rules and device id. >> >> Issue: >> Recently, kernel registers block devices in parallel. As a result, >> different device names will be assigned at each boot time. This >> will confuse file-system mounter, thus we usually use persistent >> symbolic links provided by udev. > > Yes, that is what you should use if you care about this. > >> However, dmesg and procfs outputs >> show device names instead of the symbolic link names. This causes >> a serious problem when managing multiple devices (e.g. on a >> large-scale storage), because usually, device errors are output >> with device names on dmesg. > > Then fix your tools that read the output of these files to point to the > proper persistent name instead of trying to get the kernel to solve the > problem. > Yes, the tools should be revised if users get a device name using them. The problem I would like to discuss here is that users can not identify a disk from kernel messages when they DIRECTLY refer to these messages. For example, a device name is used instead of a symbolic link names in bootup messages, I/O devices errors and /proc/partitions …etc. In particular, users can not identify an appropriate device from a device name in syslog since different device name may be assigned to it at each boot time. My idea is able to fix this issue with small changes in scsi subsystem. Also, it is implemented as an option. Therefore, it does not affect users who do not select this option. >> We also concern about some commands >> which output device names, as well as kernel messages. > > What commands are you concerned about? > They are iostat, df, fdisk, parted and so on. For example, the following sdc did not point a same disk. # ls -l /dev/disk/by-id ... lrwxrwxrwx. 1 root root 9 Apr 8 11:17 wwn-0x50014ee057a61330 -> ../../sdc lrwxrwxrwx. 1 root root 9 Apr 8 11:17 wwn-0x50014ee204d3fcda -> ../../sdd # iostat ... Device: tps Blk_read/s Blk_wrtn/s Blk_read Blk_wrtn ... sdc 5.78 19.25 78.98 394124 1616821 sdd 7.66 893.45 0.86 18289518 17604 After rebooting, the same symbolic link name pointed different device name. # ls -l /dev/disk/by-id ... lrwxrwxrwx. 1 root root 9 Apr 8 11:23 wwn-0x50014ee204d3fcda -> ../../sdc lrwxrwxrwx. 1 root root 9 Apr 8 11:23 wwn-0x50014ee057a61330 -> ../../sdd # iostat ... Device: tps Blk_read/s Blk_wrtn/s Blk_read Blk_wrtn ... sdc 7.65 892.22 0.86 18289518 17604 sdd 5.78 19.23 78.90 394124 1617447 >> Solution: >> To assign a persistent device name, I create unnamed devices (not >> a block device, but an intermediate device. users cannot read/write >> this device). When scsi device driver finds a LU, the LU is registered >> as an unnamed device and inform to udev. After udev finds the unnamed >> device, udev assigns a persistent device name to a specific device >> according to udev rules and registers it as a block device. Hence, >> this is just a naming, not a renaming. > > Ick, so the kernel waits for userspace before you are able to use the > device? That sounds messy. > This option only affects the system where users selected it. Therefore, I think that the unnamed device should not be available before users give the name. >> Some users are satisfied with current udev solution. Therefore, my >> proposal is implemented as an option. >> >> If using this option, add the following kernel parameter. >> >> scsi_mod.persistent_name=1 >> >> Also, if you want to assign a device name with sda, add the >> following udev rules. >> >> SUBSYSTEM=="scsi_unnamed", ATTR{disk_id}=="xxx", PROGRAM="/bin/sh >> -c 'echo -n sda > /sys/%p/disk_name'" >> >> Todo: >> - usb support > > Why? USB uses scsi, so why would it not work as-is today? What about > libata devices? > Sorry, my explanation was not sufficient. It is required to get scsi id using a scsi inguiry command in order to create the udev rule, especially in ATTR(disk_id) item in it. The USB devices, however, do not respond to the command and we can not get their scsi id. >> - hot-remove support > > So once you have assigned a name and the device is removed bad things > happen? What is the problem with this? > I am deeply sorry for that I forgot including the release function in this patch. > Note, any review of the code below does not imply that I agree with the > ideas here at all. In fact, I really don't like this idea at all, but I > might as well review the code anyway for your future contributions :) > I am profoundly grateful for your review :) >> >> I've already started discussion about device naming at LKML. >> https://lkml.org/lkml/2010/10/8/31 >> >> Signed-off-by: Nao Nishijima <nao.nishijima.xt@hitachi.com> >> --- >> >> drivers/scsi/Makefile | 1 >> drivers/scsi/scsi_sysfs.c | 26 +++ >> drivers/scsi/scsi_unnamed.c | 340 +++++++++++++++++++++++++++++++++++++++++++ >> drivers/scsi/scsi_unnamed.h | 25 +++ >> 4 files changed, 387 insertions(+), 5 deletions(-) >> create mode 100644 drivers/scsi/scsi_unnamed.c >> create mode 100644 drivers/scsi/scsi_unnamed.h >> >> diff --git a/drivers/scsi/Makefile b/drivers/scsi/Makefile >> index 7ad0b8a..782adc1 100644 >> --- a/drivers/scsi/Makefile >> +++ b/drivers/scsi/Makefile >> @@ -167,6 +167,7 @@ scsi_mod-$(CONFIG_SYSCTL) += scsi_sysctl.o >> scsi_mod-$(CONFIG_SCSI_PROC_FS) += scsi_proc.o >> scsi_mod-y += scsi_trace.o >> scsi_mod-$(CONFIG_PM) += scsi_pm.o >> +scsi_mod-y += scsi_unnamed.o >> >> scsi_tgt-y += scsi_tgt_lib.o scsi_tgt_if.o >> >> diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c >> index e44ff64..7959f5d 100644 >> --- a/drivers/scsi/scsi_sysfs.c >> +++ b/drivers/scsi/scsi_sysfs.c >> @@ -22,6 +22,7 @@ >> >> #include "scsi_priv.h" >> #include "scsi_logging.h" >> +#include "scsi_unnamed.h" >> >> static struct device_type scsi_dev_type; >> >> @@ -393,13 +394,27 @@ int scsi_sysfs_register(void) >> { >> int error; >> >> + error = scsi_unnamed_register(&scsi_bus_type); >> + if (error) >> + goto cleanup; >> + >> error = bus_register(&scsi_bus_type); >> - if (!error) { >> - error = class_register(&sdev_class); >> - if (error) >> - bus_unregister(&scsi_bus_type); >> - } >> + if (error) >> + goto cleanup_scsi_unnamed; >> + >> + error = class_register(&sdev_class); >> + if (error) >> + goto cleanup_bus; >> + >> + return 0; >> + >> +cleanup_bus: >> + bus_unregister(&scsi_bus_type); >> + >> +cleanup_scsi_unnamed: >> + scsi_unnamed_unregister(); >> >> +cleanup: >> return error; >> } >> >> @@ -407,6 +422,7 @@ void scsi_sysfs_unregister(void) >> { >> class_unregister(&sdev_class); >> bus_unregister(&scsi_bus_type); >> + scsi_unnamed_unregister(); >> } >> >> /* >> diff --git a/drivers/scsi/scsi_unnamed.c b/drivers/scsi/scsi_unnamed.c >> new file mode 100644 >> index 0000000..ed96945 >> --- /dev/null >> +++ b/drivers/scsi/scsi_unnamed.c >> @@ -0,0 +1,340 @@ >> +/* >> + * scsi_unnamed.c - SCSI driver for unnmaed device >> + * >> + * Copyright: Copyright (c) Hitachi, Ltd. 2011 >> + * Authors: Nao Nishijima <nao.nishijima.xt@hitachi.com> >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License as published by >> + * the Free Software Foundation; either version 2 of the License, or >> + * (at your option) any later version. > > Do you _REALLY_ mean "any later version? Are you sure about that? If > so, fine, just be careful please. > OK, I will remove those paragraph. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public License >> + * along with this program; if not, write to the Free Software >> + * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111 USA > > Unless you wish to track the movements of the FSF's office space for the > next 40+ years and keep this file up to date, just remove this paragraph > please. > OK, I will remove those paragraph. >> + */ >> + >> +#include <linux/module.h> >> +#include <linux/err.h> >> +#include <linux/device.h> >> +#include <linux/slab.h> >> +#include <linux/kobject.h> >> +#include <linux/kdev_t.h> >> +#include <linux/sysdev.h> >> +#include <linux/list.h> >> +#include <linux/wait.h> >> + >> +#include <scsi/scsi_driver.h> >> +#include <scsi/scsi_device.h> >> +#include <scsi/scsi.h> >> + >> +#define MAX_BUFFER_LEN 256 >> +#define DISK_NAME_LEN 32 > > Why this limitation? > Device name limitation which used kernel is 32. include/linux/genhd.h ... #define DISK_NAME_LEN 32 ... struct gendisk { ... char disk_name[DISK_NAME_LEN]; /* name of major driver */ I will include genhd.h. >> + >> +#define SCSI_ID_BINARY 1 >> +#define SCSI_ID_ASCII 2 >> +#define SCSI_ID_UTF8 3 >> + >> +static LIST_HEAD(su_list); >> +static DEFINE_MUTEX(su_list_lock); >> + >> +static int persistent_name; >> +MODULE_PARM_DESC(persistent_name, "SCSI unnamed device support"); >> +module_param(persistent_name, bool, S_IRUGO); > > Why is this a module parameter? > I implemented this idea as an option. >> + >> +static struct class su_sysfs_class = { >> + .name = "scsi_unnamed", >> +}; > > Why a static class? What is it used for? If you are adding sysfs > files, you need to also add Documentation/ABI/ entries at the same time. > Please do that if you really are adding new sysfs files. > OK, I have a rethink on it. >> + >> +struct scsi_unnamed { >> + struct list_head list; >> + struct device dev; >> + char disk_name[DISK_NAME_LEN]; >> + char disk_lun[MAX_BUFFER_LEN]; >> + char disk_id[MAX_BUFFER_LEN]; >> + bool assigned; >> +}; >> + >> +#define to_scsi_unnamed(d) \ >> + container_of(d, struct scsi_unnamed, dev) >> + >> +/* Get device identification VPD page */ >> +static void store_disk_id(struct scsi_device *sdev, char *disk_id) >> +{ >> + unsigned char *page_83; >> + int page_len, id_len, j = 0, i = 8; >> + static const char hex_str[] = "0123456789abcdef"; > > Don't we have functions that do this already? > I will use pack_hex_byte(), thanks. >> + >> + page_83 = kmalloc(MAX_BUFFER_LEN, GFP_KERNEL); >> + if (!page_83) >> + return; >> + >> + if (scsi_get_vpd_page(sdev, 0x83, page_83, MAX_BUFFER_LEN)) >> + goto err; >> + >> + page_len = ((page_83[2] << 8) | page_83[3]) + 4; >> + if (page_len > 0) { >> + if ((page_83[5] & 0x30) != 0) >> + goto err; >> + >> + id_len = page_83[7]; >> + if (id_len > 0) { >> + switch (page_83[4] & 0x0f) { >> + case SCSI_ID_BINARY: >> + while (i < id_len) { >> + disk_id[j++] = hex_str[(page_83[i] >> + & 0xf0) >> 4]; >> + disk_id[j++] = hex_str[page_83[i] >> + & 0x0f]; >> + i++; >> + } >> + break; >> + case SCSI_ID_ASCII: >> + case SCSI_ID_UTF8: >> + strncpy(disk_id, strim(page_83 + 8), id_len); >> + break; >> + default: >> + break; >> + } >> + } >> + } >> + >> +err: >> + kfree(page_83); >> + return; >> +} >> + >> +static ssize_t show_disk_name(struct device *dev, >> + struct device_attribute *attr, char *buf) >> +{ >> + return sprintf(buf, "%s\n", to_scsi_unnamed(dev)->disk_name); >> +} >> + >> +static ssize_t show_disk_lun(struct device *dev, >> + struct device_attribute *attr, char *buf) >> +{ >> + return sprintf(buf, "%s\n", to_scsi_unnamed(dev)->disk_lun); >> +} >> + >> +static ssize_t show_disk_id(struct device *dev, >> + struct device_attribute *attr, char *buf) >> +{ >> + return sprintf(buf, "%s\n", to_scsi_unnamed(dev)->disk_id); >> +} >> + >> +/* Assign a persistent device name */ >> +static ssize_t store_disk_name(struct device *dev, >> + struct device_attribute *attr, char *buf, size_t count) >> +{ >> + struct scsi_unnamed *su = to_scsi_unnamed(dev); >> + struct scsi_unnamed *tmp; >> + struct device *lu = dev->parent; >> + int ret = -EINVAL; >> + >> + if (count >= DISK_NAME_LEN) { >> + printk(KERN_WARNING "su: Too long a persistent name!\n"); >> + return -EINVAL; >> + } >> + >> + if (su->assigned) { >> + printk(KERN_WARNING "su: Already assigned a persistent name!\n"); >> + return -EINVAL; >> + } >> + >> + if (strncmp(lu->driver->name, buf, 2)) { >> + printk(KERN_WARNING "su: Wrong prefix!\n"); >> + return -EINVAL; >> + } >> + >> + mutex_lock(&su_list_lock); >> + list_for_each_entry(tmp, &su_list, list) { >> + if (!strncmp(tmp->disk_name, buf, DISK_NAME_LEN)) { >> + printk(KERN_WARNING "su: Duplicate name exsists!\n"); >> + return -EINVAL; >> + } >> + } >> + strncpy(su->disk_name, buf, DISK_NAME_LEN); >> + mutex_unlock(&su_list_lock); >> + >> + if (!lu->driver->probe) >> + return -EINVAL; >> + >> + lu->init_name = buf; >> + >> + ret = lu->driver->probe(lu); >> + if (ret) { >> + lu->init_name = NULL; >> + su->disk_name[0] = '\0'; >> + return ret; >> + } >> + >> + su->assigned = true; >> + return count; >> +} >> + >> +static DEVICE_ATTR(disk_name, S_IRUGO|S_IWUSR, show_disk_name, >> + store_disk_name); >> +static DEVICE_ATTR(disk_lun, S_IRUGO, show_disk_lun, NULL); >> +static DEVICE_ATTR(disk_id, S_IRUGO, show_disk_id, NULL); >> + >> +/* Confirm the driver name and the device type */ >> +static int check_device_type(char type, const char *name) >> +{ >> + switch (type) { >> + case TYPE_DISK: >> + case TYPE_MOD: >> + case TYPE_RBC: >> + if (strncmp("sd", name, 2)) >> + return -ENODEV; >> + break; >> + case TYPE_ROM: >> + case TYPE_WORM: >> + if (strncmp("sr", name, 2)) >> + return -ENODEV; >> + break; >> + case TYPE_TAPE: >> + if (strncmp("st", name, 2)) >> + return -ENODEV; >> + break; >> + default: >> + break; >> + } >> + return 0; >> +} >> + >> +/** >> + * scsi_unnamed_probe - Setup the unnamed device. >> + * @dev: parent scsi device >> + * >> + * This function adds a unnamed device to the device model and >> + * gets a number of device information by scsi inquiry commands. >> + * Udev identify a device by those information. >> + * >> + * Unnamed devices: >> + * This device is not a block device and user can not read/write >> + * this device. But it can advertise device information in sysfs. >> + */ >> +int scsi_unnamed_probe(struct device *dev) >> +{ >> + struct scsi_unnamed *su; >> + struct scsi_device *sdev = to_scsi_device(dev); >> + int ret = -EINVAL; >> + static int i; >> + >> + if (check_device_type(sdev->type, dev->driver->name)) >> + return -ENODEV; >> + >> + su = kzalloc(sizeof(*su), GFP_KERNEL); >> + if (!su) >> + return -ENOMEM; >> + >> + device_initialize(&su->dev); >> + su->dev.parent = dev; >> + su->dev.class = &su_sysfs_class; > > You just created a device that can not be removed from the system (i.e. > you have no release function.) This is a major bug and as per the > documentation for kobjects and the driver model in the kernel tree, I am > allowed to mock this code mercilessly :) > Oh... sorry :( >> + dev_set_name(&su->dev, "su%d", i++); >> + strncpy(su->disk_lun, dev_name(dev), MAX_BUFFER_LEN); >> + >> + ret = device_add(&su->dev); >> + if (ret) >> + goto err; >> + >> + ret = device_create_file(&su->dev, &dev_attr_disk_name); > > Nope, you just raced with userspace here. NEVER create a sysfs file > after you have added it to the system. Userspace just got notified of > the device and is already starting to read the file. As it's not there > yet, you just failed :( > > Use attributes properly to keep this race from happening. > OK. I'll try to be more careful in future > >> + if (ret) >> + goto err_disk_name; >> + >> + ret = device_create_file(&su->dev, &dev_attr_disk_lun); >> + if (ret) >> + goto err_disk_lun; >> + >> + store_disk_id(sdev, su->disk_id); >> + if (su->disk_id[0] != '\0') { >> + ret = device_create_file(&su->dev, &dev_attr_disk_id); >> + if (ret) >> + goto err_disk_id; >> + } >> + >> + su->assigned = false; >> + mutex_lock(&su_list_lock); >> + list_add(&su->list, &su_list); >> + mutex_unlock(&su_list_lock); >> + >> + return 0; >> + >> +err_disk_id: >> + device_remove_file(&su->dev, &dev_attr_disk_lun); >> + >> +err_disk_lun: >> + device_remove_file(&su->dev, &dev_attr_disk_name); >> + >> +err_disk_name: >> + device_del(&su->dev); >> + >> +err: >> + kfree(su); >> + return ret; >> +} >> + >> +/* Remove a unnamed device from su_list and release resources */ >> +void scsi_unnamed_remove(struct device *dev) >> +{ >> + struct scsi_unnamed *tmp; >> + >> + mutex_lock(&su_list_lock); >> + list_for_each_entry(tmp, &su_list, list) { >> + if (dev == tmp->dev.parent) { >> + list_del(&tmp->list); >> + break; >> + } >> + } >> + mutex_unlock(&su_list_lock); >> + >> + if (tmp->disk_id[0] != '\0') >> + device_remove_file(&tmp->dev, &dev_attr_disk_id); >> + device_remove_file(&tmp->dev, &dev_attr_disk_name); >> + device_remove_file(&tmp->dev, &dev_attr_disk_lun); >> + device_del(&tmp->dev); > > Your kerrnel just spit out some very nasty messages when this function > was called, right? Why are you ignoring them? > Sorry. I will check it. >> + >> + device_release_driver(dev); >> + >> + kfree(tmp); >> +} >> + >> +/** >> + * copy_persistent_name - Copy the device name >> + * @disk_name: allocate to >> + * @dev: scsi device >> + * >> + * Copy an initial name of the device to disk_name. >> + */ >> +int copy_persistent_name(char *disk_name, struct device *dev) >> +{ >> + if (persistent_name) { >> + strncpy(disk_name, dev->init_name, DISK_NAME_LEN); >> + return 1; >> + } >> + return 0; >> +} >> +EXPORT_SYMBOL(copy_persistent_name); > > That's a very bad global function name. > I have a bad sense :( I will name this function scsi_unnamed_copy_persistent_name. > EXPORT_SYMBOL_GPL? And why is it exported at all? Who would ever need > to call this from a module? > Please see the patch 2/2. This function is called by external >> + >> +int scsi_unnamed_register(struct bus_type *bus) >> +{ >> + if (persistent_name) { >> + bus->probe = scsi_unnamed_probe; >> + bus->remove = scsi_unnamed_remove; >> + return class_register(&su_sysfs_class); >> + } >> + return 0; >> +} >> +EXPORT_SYMBOL(scsi_unnamed_register); > > Same here, why is this exported? > >> + >> +void scsi_unnamed_unregister(void) >> +{ >> + if (persistent_name) >> + class_unregister(&su_sysfs_class); >> +} >> +EXPORT_SYMBOL(scsi_unnamed_unregister); > > And here. > >> diff --git a/drivers/scsi/scsi_unnamed.h b/drivers/scsi/scsi_unnamed.h >> new file mode 100644 >> index 0000000..ca4e852 >> --- /dev/null >> +++ b/drivers/scsi/scsi_unnamed.h >> @@ -0,0 +1,25 @@ >> +/* >> + * scsi_unnamed.h - SCSI driver for unnmaed device >> + * >> + * Copyright: Copyright (c) Hitachi, Ltd. 2011 >> + * Authors: Nao Nishijima <nao.nishijima.xt@hitachi.com> >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License as published by >> + * the Free Software Foundation; either version 2 of the License, or >> + * (at your option) any later version. > > Same comment as above. > >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public License >> + * along with this program; if not, write to the Free Software >> + * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111 USA > > And again, same comment as above. > > thanks, > > greg k-h > Thanks, -- Nao NISHIJIMA Software Platform Research Dept. Linux Technology Center Hitachi, Ltd., YOKOHAMA Research Laboratory Email: nao.nishijima.xt@hitachi.com ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 1/2] SCSI: Add a SCSI option for persistent device names @ 2011-04-08 14:12 ` Nao Nishijima 0 siblings, 0 replies; 46+ messages in thread From: Nao Nishijima @ 2011-04-08 14:12 UTC (permalink / raw) To: Greg KH Cc: linux-kernel, linux-scsi, linux-hotplug, Kay Sievers, James Bottomley, Jon Masters, 2nddept-manager Hi, (2011/04/06 1:14), Greg KH wrote: > On Tue, Apr 05, 2011 at 09:49:46PM +0900, Nao Nishijima wrote: >> This patch series provides a SCSI option for persistent device >> names in kernel. With this option, user can always assign a >> same device name (e.g. sda) to a specific device according to >> udev rules and device id. >> >> Issue: >> Recently, kernel registers block devices in parallel. As a result, >> different device names will be assigned at each boot time. This >> will confuse file-system mounter, thus we usually use persistent >> symbolic links provided by udev. > > Yes, that is what you should use if you care about this. > >> However, dmesg and procfs outputs >> show device names instead of the symbolic link names. This causes >> a serious problem when managing multiple devices (e.g. on a >> large-scale storage), because usually, device errors are output >> with device names on dmesg. > > Then fix your tools that read the output of these files to point to the > proper persistent name instead of trying to get the kernel to solve the > problem. > Yes, the tools should be revised if users get a device name using them. The problem I would like to discuss here is that users can not identify a disk from kernel messages when they DIRECTLY refer to these messages. For example, a device name is used instead of a symbolic link names in bootup messages, I/O devices errors and /proc/partitions …etc. In particular, users can not identify an appropriate device from a device name in syslog since different device name may be assigned to it at each boot time. My idea is able to fix this issue with small changes in scsi subsystem. Also, it is implemented as an option. Therefore, it does not affect users who do not select this option. >> We also concern about some commands >> which output device names, as well as kernel messages. > > What commands are you concerned about? > They are iostat, df, fdisk, parted and so on. For example, the following sdc did not point a same disk. # ls -l /dev/disk/by-id ... lrwxrwxrwx. 1 root root 9 Apr 8 11:17 wwn-0x50014ee057a61330 -> ../../sdc lrwxrwxrwx. 1 root root 9 Apr 8 11:17 wwn-0x50014ee204d3fcda -> ../../sdd # iostat ... Device: tps Blk_read/s Blk_wrtn/s Blk_read Blk_wrtn ... sdc 5.78 19.25 78.98 394124 1616821 sdd 7.66 893.45 0.86 18289518 17604 After rebooting, the same symbolic link name pointed different device name. # ls -l /dev/disk/by-id ... lrwxrwxrwx. 1 root root 9 Apr 8 11:23 wwn-0x50014ee204d3fcda -> ../../sdc lrwxrwxrwx. 1 root root 9 Apr 8 11:23 wwn-0x50014ee057a61330 -> ../../sdd # iostat ... Device: tps Blk_read/s Blk_wrtn/s Blk_read Blk_wrtn ... sdc 7.65 892.22 0.86 18289518 17604 sdd 5.78 19.23 78.90 394124 1617447 >> Solution: >> To assign a persistent device name, I create unnamed devices (not >> a block device, but an intermediate device. users cannot read/write >> this device). When scsi device driver finds a LU, the LU is registered >> as an unnamed device and inform to udev. After udev finds the unnamed >> device, udev assigns a persistent device name to a specific device >> according to udev rules and registers it as a block device. Hence, >> this is just a naming, not a renaming. > > Ick, so the kernel waits for userspace before you are able to use the > device? That sounds messy. > This option only affects the system where users selected it. Therefore, I think that the unnamed device should not be available before users give the name. >> Some users are satisfied with current udev solution. Therefore, my >> proposal is implemented as an option. >> >> If using this option, add the following kernel parameter. >> >> scsi_mod.persistent_name=1 >> >> Also, if you want to assign a device name with sda, add the >> following udev rules. >> >> SUBSYSTEM="scsi_unnamed", ATTR{disk_id}="xxx", PROGRAM="/bin/sh >> -c 'echo -n sda > /sys/%p/disk_name'" >> >> Todo: >> - usb support > > Why? USB uses scsi, so why would it not work as-is today? What about > libata devices? > Sorry, my explanation was not sufficient. It is required to get scsi id using a scsi inguiry command in order to create the udev rule, especially in ATTR(disk_id) item in it. The USB devices, however, do not respond to the command and we can not get their scsi id. >> - hot-remove support > > So once you have assigned a name and the device is removed bad things > happen? What is the problem with this? > I am deeply sorry for that I forgot including the release function in this patch. > Note, any review of the code below does not imply that I agree with the > ideas here at all. In fact, I really don't like this idea at all, but I > might as well review the code anyway for your future contributions :) > I am profoundly grateful for your review :) >> >> I've already started discussion about device naming at LKML. >> https://lkml.org/lkml/2010/10/8/31 >> >> Signed-off-by: Nao Nishijima <nao.nishijima.xt@hitachi.com> >> --- >> >> drivers/scsi/Makefile | 1 >> drivers/scsi/scsi_sysfs.c | 26 +++ >> drivers/scsi/scsi_unnamed.c | 340 +++++++++++++++++++++++++++++++++++++++++++ >> drivers/scsi/scsi_unnamed.h | 25 +++ >> 4 files changed, 387 insertions(+), 5 deletions(-) >> create mode 100644 drivers/scsi/scsi_unnamed.c >> create mode 100644 drivers/scsi/scsi_unnamed.h >> >> diff --git a/drivers/scsi/Makefile b/drivers/scsi/Makefile >> index 7ad0b8a..782adc1 100644 >> --- a/drivers/scsi/Makefile >> +++ b/drivers/scsi/Makefile >> @@ -167,6 +167,7 @@ scsi_mod-$(CONFIG_SYSCTL) += scsi_sysctl.o >> scsi_mod-$(CONFIG_SCSI_PROC_FS) += scsi_proc.o >> scsi_mod-y += scsi_trace.o >> scsi_mod-$(CONFIG_PM) += scsi_pm.o >> +scsi_mod-y += scsi_unnamed.o >> >> scsi_tgt-y += scsi_tgt_lib.o scsi_tgt_if.o >> >> diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c >> index e44ff64..7959f5d 100644 >> --- a/drivers/scsi/scsi_sysfs.c >> +++ b/drivers/scsi/scsi_sysfs.c >> @@ -22,6 +22,7 @@ >> >> #include "scsi_priv.h" >> #include "scsi_logging.h" >> +#include "scsi_unnamed.h" >> >> static struct device_type scsi_dev_type; >> >> @@ -393,13 +394,27 @@ int scsi_sysfs_register(void) >> { >> int error; >> >> + error = scsi_unnamed_register(&scsi_bus_type); >> + if (error) >> + goto cleanup; >> + >> error = bus_register(&scsi_bus_type); >> - if (!error) { >> - error = class_register(&sdev_class); >> - if (error) >> - bus_unregister(&scsi_bus_type); >> - } >> + if (error) >> + goto cleanup_scsi_unnamed; >> + >> + error = class_register(&sdev_class); >> + if (error) >> + goto cleanup_bus; >> + >> + return 0; >> + >> +cleanup_bus: >> + bus_unregister(&scsi_bus_type); >> + >> +cleanup_scsi_unnamed: >> + scsi_unnamed_unregister(); >> >> +cleanup: >> return error; >> } >> >> @@ -407,6 +422,7 @@ void scsi_sysfs_unregister(void) >> { >> class_unregister(&sdev_class); >> bus_unregister(&scsi_bus_type); >> + scsi_unnamed_unregister(); >> } >> >> /* >> diff --git a/drivers/scsi/scsi_unnamed.c b/drivers/scsi/scsi_unnamed.c >> new file mode 100644 >> index 0000000..ed96945 >> --- /dev/null >> +++ b/drivers/scsi/scsi_unnamed.c >> @@ -0,0 +1,340 @@ >> +/* >> + * scsi_unnamed.c - SCSI driver for unnmaed device >> + * >> + * Copyright: Copyright (c) Hitachi, Ltd. 2011 >> + * Authors: Nao Nishijima <nao.nishijima.xt@hitachi.com> >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License as published by >> + * the Free Software Foundation; either version 2 of the License, or >> + * (at your option) any later version. > > Do you _REALLY_ mean "any later version? Are you sure about that? If > so, fine, just be careful please. > OK, I will remove those paragraph. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public License >> + * along with this program; if not, write to the Free Software >> + * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111 USA > > Unless you wish to track the movements of the FSF's office space for the > next 40+ years and keep this file up to date, just remove this paragraph > please. > OK, I will remove those paragraph. >> + */ >> + >> +#include <linux/module.h> >> +#include <linux/err.h> >> +#include <linux/device.h> >> +#include <linux/slab.h> >> +#include <linux/kobject.h> >> +#include <linux/kdev_t.h> >> +#include <linux/sysdev.h> >> +#include <linux/list.h> >> +#include <linux/wait.h> >> + >> +#include <scsi/scsi_driver.h> >> +#include <scsi/scsi_device.h> >> +#include <scsi/scsi.h> >> + >> +#define MAX_BUFFER_LEN 256 >> +#define DISK_NAME_LEN 32 > > Why this limitation? > Device name limitation which used kernel is 32. include/linux/genhd.h ... #define DISK_NAME_LEN 32 ... struct gendisk { ... char disk_name[DISK_NAME_LEN]; /* name of major driver */ I will include genhd.h. >> + >> +#define SCSI_ID_BINARY 1 >> +#define SCSI_ID_ASCII 2 >> +#define SCSI_ID_UTF8 3 >> + >> +static LIST_HEAD(su_list); >> +static DEFINE_MUTEX(su_list_lock); >> + >> +static int persistent_name; >> +MODULE_PARM_DESC(persistent_name, "SCSI unnamed device support"); >> +module_param(persistent_name, bool, S_IRUGO); > > Why is this a module parameter? > I implemented this idea as an option. >> + >> +static struct class su_sysfs_class = { >> + .name = "scsi_unnamed", >> +}; > > Why a static class? What is it used for? If you are adding sysfs > files, you need to also add Documentation/ABI/ entries at the same time. > Please do that if you really are adding new sysfs files. > OK, I have a rethink on it. >> + >> +struct scsi_unnamed { >> + struct list_head list; >> + struct device dev; >> + char disk_name[DISK_NAME_LEN]; >> + char disk_lun[MAX_BUFFER_LEN]; >> + char disk_id[MAX_BUFFER_LEN]; >> + bool assigned; >> +}; >> + >> +#define to_scsi_unnamed(d) \ >> + container_of(d, struct scsi_unnamed, dev) >> + >> +/* Get device identification VPD page */ >> +static void store_disk_id(struct scsi_device *sdev, char *disk_id) >> +{ >> + unsigned char *page_83; >> + int page_len, id_len, j = 0, i = 8; >> + static const char hex_str[] = "0123456789abcdef"; > > Don't we have functions that do this already? > I will use pack_hex_byte(), thanks. >> + >> + page_83 = kmalloc(MAX_BUFFER_LEN, GFP_KERNEL); >> + if (!page_83) >> + return; >> + >> + if (scsi_get_vpd_page(sdev, 0x83, page_83, MAX_BUFFER_LEN)) >> + goto err; >> + >> + page_len = ((page_83[2] << 8) | page_83[3]) + 4; >> + if (page_len > 0) { >> + if ((page_83[5] & 0x30) != 0) >> + goto err; >> + >> + id_len = page_83[7]; >> + if (id_len > 0) { >> + switch (page_83[4] & 0x0f) { >> + case SCSI_ID_BINARY: >> + while (i < id_len) { >> + disk_id[j++] = hex_str[(page_83[i] >> + & 0xf0) >> 4]; >> + disk_id[j++] = hex_str[page_83[i] >> + & 0x0f]; >> + i++; >> + } >> + break; >> + case SCSI_ID_ASCII: >> + case SCSI_ID_UTF8: >> + strncpy(disk_id, strim(page_83 + 8), id_len); >> + break; >> + default: >> + break; >> + } >> + } >> + } >> + >> +err: >> + kfree(page_83); >> + return; >> +} >> + >> +static ssize_t show_disk_name(struct device *dev, >> + struct device_attribute *attr, char *buf) >> +{ >> + return sprintf(buf, "%s\n", to_scsi_unnamed(dev)->disk_name); >> +} >> + >> +static ssize_t show_disk_lun(struct device *dev, >> + struct device_attribute *attr, char *buf) >> +{ >> + return sprintf(buf, "%s\n", to_scsi_unnamed(dev)->disk_lun); >> +} >> + >> +static ssize_t show_disk_id(struct device *dev, >> + struct device_attribute *attr, char *buf) >> +{ >> + return sprintf(buf, "%s\n", to_scsi_unnamed(dev)->disk_id); >> +} >> + >> +/* Assign a persistent device name */ >> +static ssize_t store_disk_name(struct device *dev, >> + struct device_attribute *attr, char *buf, size_t count) >> +{ >> + struct scsi_unnamed *su = to_scsi_unnamed(dev); >> + struct scsi_unnamed *tmp; >> + struct device *lu = dev->parent; >> + int ret = -EINVAL; >> + >> + if (count >= DISK_NAME_LEN) { >> + printk(KERN_WARNING "su: Too long a persistent name!\n"); >> + return -EINVAL; >> + } >> + >> + if (su->assigned) { >> + printk(KERN_WARNING "su: Already assigned a persistent name!\n"); >> + return -EINVAL; >> + } >> + >> + if (strncmp(lu->driver->name, buf, 2)) { >> + printk(KERN_WARNING "su: Wrong prefix!\n"); >> + return -EINVAL; >> + } >> + >> + mutex_lock(&su_list_lock); >> + list_for_each_entry(tmp, &su_list, list) { >> + if (!strncmp(tmp->disk_name, buf, DISK_NAME_LEN)) { >> + printk(KERN_WARNING "su: Duplicate name exsists!\n"); >> + return -EINVAL; >> + } >> + } >> + strncpy(su->disk_name, buf, DISK_NAME_LEN); >> + mutex_unlock(&su_list_lock); >> + >> + if (!lu->driver->probe) >> + return -EINVAL; >> + >> + lu->init_name = buf; >> + >> + ret = lu->driver->probe(lu); >> + if (ret) { >> + lu->init_name = NULL; >> + su->disk_name[0] = '\0'; >> + return ret; >> + } >> + >> + su->assigned = true; >> + return count; >> +} >> + >> +static DEVICE_ATTR(disk_name, S_IRUGO|S_IWUSR, show_disk_name, >> + store_disk_name); >> +static DEVICE_ATTR(disk_lun, S_IRUGO, show_disk_lun, NULL); >> +static DEVICE_ATTR(disk_id, S_IRUGO, show_disk_id, NULL); >> + >> +/* Confirm the driver name and the device type */ >> +static int check_device_type(char type, const char *name) >> +{ >> + switch (type) { >> + case TYPE_DISK: >> + case TYPE_MOD: >> + case TYPE_RBC: >> + if (strncmp("sd", name, 2)) >> + return -ENODEV; >> + break; >> + case TYPE_ROM: >> + case TYPE_WORM: >> + if (strncmp("sr", name, 2)) >> + return -ENODEV; >> + break; >> + case TYPE_TAPE: >> + if (strncmp("st", name, 2)) >> + return -ENODEV; >> + break; >> + default: >> + break; >> + } >> + return 0; >> +} >> + >> +/** >> + * scsi_unnamed_probe - Setup the unnamed device. >> + * @dev: parent scsi device >> + * >> + * This function adds a unnamed device to the device model and >> + * gets a number of device information by scsi inquiry commands. >> + * Udev identify a device by those information. >> + * >> + * Unnamed devices: >> + * This device is not a block device and user can not read/write >> + * this device. But it can advertise device information in sysfs. >> + */ >> +int scsi_unnamed_probe(struct device *dev) >> +{ >> + struct scsi_unnamed *su; >> + struct scsi_device *sdev = to_scsi_device(dev); >> + int ret = -EINVAL; >> + static int i; >> + >> + if (check_device_type(sdev->type, dev->driver->name)) >> + return -ENODEV; >> + >> + su = kzalloc(sizeof(*su), GFP_KERNEL); >> + if (!su) >> + return -ENOMEM; >> + >> + device_initialize(&su->dev); >> + su->dev.parent = dev; >> + su->dev.class = &su_sysfs_class; > > You just created a device that can not be removed from the system (i.e. > you have no release function.) This is a major bug and as per the > documentation for kobjects and the driver model in the kernel tree, I am > allowed to mock this code mercilessly :) > Oh... sorry :( >> + dev_set_name(&su->dev, "su%d", i++); >> + strncpy(su->disk_lun, dev_name(dev), MAX_BUFFER_LEN); >> + >> + ret = device_add(&su->dev); >> + if (ret) >> + goto err; >> + >> + ret = device_create_file(&su->dev, &dev_attr_disk_name); > > Nope, you just raced with userspace here. NEVER create a sysfs file > after you have added it to the system. Userspace just got notified of > the device and is already starting to read the file. As it's not there > yet, you just failed :( > > Use attributes properly to keep this race from happening. > OK. I'll try to be more careful in future > >> + if (ret) >> + goto err_disk_name; >> + >> + ret = device_create_file(&su->dev, &dev_attr_disk_lun); >> + if (ret) >> + goto err_disk_lun; >> + >> + store_disk_id(sdev, su->disk_id); >> + if (su->disk_id[0] != '\0') { >> + ret = device_create_file(&su->dev, &dev_attr_disk_id); >> + if (ret) >> + goto err_disk_id; >> + } >> + >> + su->assigned = false; >> + mutex_lock(&su_list_lock); >> + list_add(&su->list, &su_list); >> + mutex_unlock(&su_list_lock); >> + >> + return 0; >> + >> +err_disk_id: >> + device_remove_file(&su->dev, &dev_attr_disk_lun); >> + >> +err_disk_lun: >> + device_remove_file(&su->dev, &dev_attr_disk_name); >> + >> +err_disk_name: >> + device_del(&su->dev); >> + >> +err: >> + kfree(su); >> + return ret; >> +} >> + >> +/* Remove a unnamed device from su_list and release resources */ >> +void scsi_unnamed_remove(struct device *dev) >> +{ >> + struct scsi_unnamed *tmp; >> + >> + mutex_lock(&su_list_lock); >> + list_for_each_entry(tmp, &su_list, list) { >> + if (dev = tmp->dev.parent) { >> + list_del(&tmp->list); >> + break; >> + } >> + } >> + mutex_unlock(&su_list_lock); >> + >> + if (tmp->disk_id[0] != '\0') >> + device_remove_file(&tmp->dev, &dev_attr_disk_id); >> + device_remove_file(&tmp->dev, &dev_attr_disk_name); >> + device_remove_file(&tmp->dev, &dev_attr_disk_lun); >> + device_del(&tmp->dev); > > Your kerrnel just spit out some very nasty messages when this function > was called, right? Why are you ignoring them? > Sorry. I will check it. >> + >> + device_release_driver(dev); >> + >> + kfree(tmp); >> +} >> + >> +/** >> + * copy_persistent_name - Copy the device name >> + * @disk_name: allocate to >> + * @dev: scsi device >> + * >> + * Copy an initial name of the device to disk_name. >> + */ >> +int copy_persistent_name(char *disk_name, struct device *dev) >> +{ >> + if (persistent_name) { >> + strncpy(disk_name, dev->init_name, DISK_NAME_LEN); >> + return 1; >> + } >> + return 0; >> +} >> +EXPORT_SYMBOL(copy_persistent_name); > > That's a very bad global function name. > I have a bad sense :( I will name this function scsi_unnamed_copy_persistent_name. > EXPORT_SYMBOL_GPL? And why is it exported at all? Who would ever need > to call this from a module? > Please see the patch 2/2. This function is called by external >> + >> +int scsi_unnamed_register(struct bus_type *bus) >> +{ >> + if (persistent_name) { >> + bus->probe = scsi_unnamed_probe; >> + bus->remove = scsi_unnamed_remove; >> + return class_register(&su_sysfs_class); >> + } >> + return 0; >> +} >> +EXPORT_SYMBOL(scsi_unnamed_register); > > Same here, why is this exported? > >> + >> +void scsi_unnamed_unregister(void) >> +{ >> + if (persistent_name) >> + class_unregister(&su_sysfs_class); >> +} >> +EXPORT_SYMBOL(scsi_unnamed_unregister); > > And here. > >> diff --git a/drivers/scsi/scsi_unnamed.h b/drivers/scsi/scsi_unnamed.h >> new file mode 100644 >> index 0000000..ca4e852 >> --- /dev/null >> +++ b/drivers/scsi/scsi_unnamed.h >> @@ -0,0 +1,25 @@ >> +/* >> + * scsi_unnamed.h - SCSI driver for unnmaed device >> + * >> + * Copyright: Copyright (c) Hitachi, Ltd. 2011 >> + * Authors: Nao Nishijima <nao.nishijima.xt@hitachi.com> >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License as published by >> + * the Free Software Foundation; either version 2 of the License, or >> + * (at your option) any later version. > > Same comment as above. > >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public License >> + * along with this program; if not, write to the Free Software >> + * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111 USA > > And again, same comment as above. > > thanks, > > greg k-h > Thanks, -- Nao NISHIJIMA Software Platform Research Dept. Linux Technology Center Hitachi, Ltd., YOKOHAMA Research Laboratory Email: nao.nishijima.xt@hitachi.com ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel. @ 2011-04-08 14:12 ` Nao Nishijima 0 siblings, 0 replies; 46+ messages in thread From: Nao Nishijima @ 2011-04-08 14:12 UTC (permalink / raw) To: Greg KH Cc: linux-kernel, linux-scsi, linux-hotplug, Kay Sievers, James Bottomley, Jon Masters, 2nddept-manager Hi, (2011/04/06 1:14), Greg KH wrote: > On Tue, Apr 05, 2011 at 09:49:46PM +0900, Nao Nishijima wrote: >> This patch series provides a SCSI option for persistent device >> names in kernel. With this option, user can always assign a >> same device name (e.g. sda) to a specific device according to >> udev rules and device id. >> >> Issue: >> Recently, kernel registers block devices in parallel. As a result, >> different device names will be assigned at each boot time. This >> will confuse file-system mounter, thus we usually use persistent >> symbolic links provided by udev. > > Yes, that is what you should use if you care about this. > >> However, dmesg and procfs outputs >> show device names instead of the symbolic link names. This causes >> a serious problem when managing multiple devices (e.g. on a >> large-scale storage), because usually, device errors are output >> with device names on dmesg. > > Then fix your tools that read the output of these files to point to the > proper persistent name instead of trying to get the kernel to solve the > problem. > Yes, the tools should be revised if users get a device name using them. The problem I would like to discuss here is that users can not identify a disk from kernel messages when they DIRECTLY refer to these messages. For example, a device name is used instead of a symbolic link names in bootup messages, I/O devices errors and /proc/partitions …etc. In particular, users can not identify an appropriate device from a device name in syslog since different device name may be assigned to it at each boot time. My idea is able to fix this issue with small changes in scsi subsystem. Also, it is implemented as an option. Therefore, it does not affect users who do not select this option. >> We also concern about some commands >> which output device names, as well as kernel messages. > > What commands are you concerned about? > They are iostat, df, fdisk, parted and so on. For example, the following sdc did not point a same disk. # ls -l /dev/disk/by-id ... lrwxrwxrwx. 1 root root 9 Apr 8 11:17 wwn-0x50014ee057a61330 -> ../../sdc lrwxrwxrwx. 1 root root 9 Apr 8 11:17 wwn-0x50014ee204d3fcda -> ../../sdd # iostat ... Device: tps Blk_read/s Blk_wrtn/s Blk_read Blk_wrtn ... sdc 5.78 19.25 78.98 394124 1616821 sdd 7.66 893.45 0.86 18289518 17604 After rebooting, the same symbolic link name pointed different device name. # ls -l /dev/disk/by-id ... lrwxrwxrwx. 1 root root 9 Apr 8 11:23 wwn-0x50014ee204d3fcda -> ../../sdc lrwxrwxrwx. 1 root root 9 Apr 8 11:23 wwn-0x50014ee057a61330 -> ../../sdd # iostat ... Device: tps Blk_read/s Blk_wrtn/s Blk_read Blk_wrtn ... sdc 7.65 892.22 0.86 18289518 17604 sdd 5.78 19.23 78.90 394124 1617447 >> Solution: >> To assign a persistent device name, I create unnamed devices (not >> a block device, but an intermediate device. users cannot read/write >> this device). When scsi device driver finds a LU, the LU is registered >> as an unnamed device and inform to udev. After udev finds the unnamed >> device, udev assigns a persistent device name to a specific device >> according to udev rules and registers it as a block device. Hence, >> this is just a naming, not a renaming. > > Ick, so the kernel waits for userspace before you are able to use the > device? That sounds messy. > This option only affects the system where users selected it. Therefore, I think that the unnamed device should not be available before users give the name. >> Some users are satisfied with current udev solution. Therefore, my >> proposal is implemented as an option. >> >> If using this option, add the following kernel parameter. >> >> scsi_mod.persistent_name=1 >> >> Also, if you want to assign a device name with sda, add the >> following udev rules. >> >> SUBSYSTEM=="scsi_unnamed", ATTR{disk_id}=="xxx", PROGRAM="/bin/sh >> -c 'echo -n sda > /sys/%p/disk_name'" >> >> Todo: >> - usb support > > Why? USB uses scsi, so why would it not work as-is today? What about > libata devices? > Sorry, my explanation was not sufficient. It is required to get scsi id using a scsi inguiry command in order to create the udev rule, especially in ATTR(disk_id) item in it. The USB devices, however, do not respond to the command and we can not get their scsi id. >> - hot-remove support > > So once you have assigned a name and the device is removed bad things > happen? What is the problem with this? > I am deeply sorry for that I forgot including the release function in this patch. > Note, any review of the code below does not imply that I agree with the > ideas here at all. In fact, I really don't like this idea at all, but I > might as well review the code anyway for your future contributions :) > I am profoundly grateful for your review :) >> >> I've already started discussion about device naming at LKML. >> https://lkml.org/lkml/2010/10/8/31 >> >> Signed-off-by: Nao Nishijima <nao.nishijima.xt@hitachi.com> >> --- >> >> drivers/scsi/Makefile | 1 >> drivers/scsi/scsi_sysfs.c | 26 +++ >> drivers/scsi/scsi_unnamed.c | 340 +++++++++++++++++++++++++++++++++++++++++++ >> drivers/scsi/scsi_unnamed.h | 25 +++ >> 4 files changed, 387 insertions(+), 5 deletions(-) >> create mode 100644 drivers/scsi/scsi_unnamed.c >> create mode 100644 drivers/scsi/scsi_unnamed.h >> >> diff --git a/drivers/scsi/Makefile b/drivers/scsi/Makefile >> index 7ad0b8a..782adc1 100644 >> --- a/drivers/scsi/Makefile >> +++ b/drivers/scsi/Makefile >> @@ -167,6 +167,7 @@ scsi_mod-$(CONFIG_SYSCTL) += scsi_sysctl.o >> scsi_mod-$(CONFIG_SCSI_PROC_FS) += scsi_proc.o >> scsi_mod-y += scsi_trace.o >> scsi_mod-$(CONFIG_PM) += scsi_pm.o >> +scsi_mod-y += scsi_unnamed.o >> >> scsi_tgt-y += scsi_tgt_lib.o scsi_tgt_if.o >> >> diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c >> index e44ff64..7959f5d 100644 >> --- a/drivers/scsi/scsi_sysfs.c >> +++ b/drivers/scsi/scsi_sysfs.c >> @@ -22,6 +22,7 @@ >> >> #include "scsi_priv.h" >> #include "scsi_logging.h" >> +#include "scsi_unnamed.h" >> >> static struct device_type scsi_dev_type; >> >> @@ -393,13 +394,27 @@ int scsi_sysfs_register(void) >> { >> int error; >> >> + error = scsi_unnamed_register(&scsi_bus_type); >> + if (error) >> + goto cleanup; >> + >> error = bus_register(&scsi_bus_type); >> - if (!error) { >> - error = class_register(&sdev_class); >> - if (error) >> - bus_unregister(&scsi_bus_type); >> - } >> + if (error) >> + goto cleanup_scsi_unnamed; >> + >> + error = class_register(&sdev_class); >> + if (error) >> + goto cleanup_bus; >> + >> + return 0; >> + >> +cleanup_bus: >> + bus_unregister(&scsi_bus_type); >> + >> +cleanup_scsi_unnamed: >> + scsi_unnamed_unregister(); >> >> +cleanup: >> return error; >> } >> >> @@ -407,6 +422,7 @@ void scsi_sysfs_unregister(void) >> { >> class_unregister(&sdev_class); >> bus_unregister(&scsi_bus_type); >> + scsi_unnamed_unregister(); >> } >> >> /* >> diff --git a/drivers/scsi/scsi_unnamed.c b/drivers/scsi/scsi_unnamed.c >> new file mode 100644 >> index 0000000..ed96945 >> --- /dev/null >> +++ b/drivers/scsi/scsi_unnamed.c >> @@ -0,0 +1,340 @@ >> +/* >> + * scsi_unnamed.c - SCSI driver for unnmaed device >> + * >> + * Copyright: Copyright (c) Hitachi, Ltd. 2011 >> + * Authors: Nao Nishijima <nao.nishijima.xt@hitachi.com> >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License as published by >> + * the Free Software Foundation; either version 2 of the License, or >> + * (at your option) any later version. > > Do you _REALLY_ mean "any later version? Are you sure about that? If > so, fine, just be careful please. > OK, I will remove those paragraph. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public License >> + * along with this program; if not, write to the Free Software >> + * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111 USA > > Unless you wish to track the movements of the FSF's office space for the > next 40+ years and keep this file up to date, just remove this paragraph > please. > OK, I will remove those paragraph. >> + */ >> + >> +#include <linux/module.h> >> +#include <linux/err.h> >> +#include <linux/device.h> >> +#include <linux/slab.h> >> +#include <linux/kobject.h> >> +#include <linux/kdev_t.h> >> +#include <linux/sysdev.h> >> +#include <linux/list.h> >> +#include <linux/wait.h> >> + >> +#include <scsi/scsi_driver.h> >> +#include <scsi/scsi_device.h> >> +#include <scsi/scsi.h> >> + >> +#define MAX_BUFFER_LEN 256 >> +#define DISK_NAME_LEN 32 > > Why this limitation? > Device name limitation which used kernel is 32. include/linux/genhd.h ... #define DISK_NAME_LEN 32 ... struct gendisk { ... char disk_name[DISK_NAME_LEN]; /* name of major driver */ I will include genhd.h. >> + >> +#define SCSI_ID_BINARY 1 >> +#define SCSI_ID_ASCII 2 >> +#define SCSI_ID_UTF8 3 >> + >> +static LIST_HEAD(su_list); >> +static DEFINE_MUTEX(su_list_lock); >> + >> +static int persistent_name; >> +MODULE_PARM_DESC(persistent_name, "SCSI unnamed device support"); >> +module_param(persistent_name, bool, S_IRUGO); > > Why is this a module parameter? > I implemented this idea as an option. >> + >> +static struct class su_sysfs_class = { >> + .name = "scsi_unnamed", >> +}; > > Why a static class? What is it used for? If you are adding sysfs > files, you need to also add Documentation/ABI/ entries at the same time. > Please do that if you really are adding new sysfs files. > OK, I have a rethink on it. >> + >> +struct scsi_unnamed { >> + struct list_head list; >> + struct device dev; >> + char disk_name[DISK_NAME_LEN]; >> + char disk_lun[MAX_BUFFER_LEN]; >> + char disk_id[MAX_BUFFER_LEN]; >> + bool assigned; >> +}; >> + >> +#define to_scsi_unnamed(d) \ >> + container_of(d, struct scsi_unnamed, dev) >> + >> +/* Get device identification VPD page */ >> +static void store_disk_id(struct scsi_device *sdev, char *disk_id) >> +{ >> + unsigned char *page_83; >> + int page_len, id_len, j = 0, i = 8; >> + static const char hex_str[] = "0123456789abcdef"; > > Don't we have functions that do this already? > I will use pack_hex_byte(), thanks. >> + >> + page_83 = kmalloc(MAX_BUFFER_LEN, GFP_KERNEL); >> + if (!page_83) >> + return; >> + >> + if (scsi_get_vpd_page(sdev, 0x83, page_83, MAX_BUFFER_LEN)) >> + goto err; >> + >> + page_len = ((page_83[2] << 8) | page_83[3]) + 4; >> + if (page_len > 0) { >> + if ((page_83[5] & 0x30) != 0) >> + goto err; >> + >> + id_len = page_83[7]; >> + if (id_len > 0) { >> + switch (page_83[4] & 0x0f) { >> + case SCSI_ID_BINARY: >> + while (i < id_len) { >> + disk_id[j++] = hex_str[(page_83[i] >> + & 0xf0) >> 4]; >> + disk_id[j++] = hex_str[page_83[i] >> + & 0x0f]; >> + i++; >> + } >> + break; >> + case SCSI_ID_ASCII: >> + case SCSI_ID_UTF8: >> + strncpy(disk_id, strim(page_83 + 8), id_len); >> + break; >> + default: >> + break; >> + } >> + } >> + } >> + >> +err: >> + kfree(page_83); >> + return; >> +} >> + >> +static ssize_t show_disk_name(struct device *dev, >> + struct device_attribute *attr, char *buf) >> +{ >> + return sprintf(buf, "%s\n", to_scsi_unnamed(dev)->disk_name); >> +} >> + >> +static ssize_t show_disk_lun(struct device *dev, >> + struct device_attribute *attr, char *buf) >> +{ >> + return sprintf(buf, "%s\n", to_scsi_unnamed(dev)->disk_lun); >> +} >> + >> +static ssize_t show_disk_id(struct device *dev, >> + struct device_attribute *attr, char *buf) >> +{ >> + return sprintf(buf, "%s\n", to_scsi_unnamed(dev)->disk_id); >> +} >> + >> +/* Assign a persistent device name */ >> +static ssize_t store_disk_name(struct device *dev, >> + struct device_attribute *attr, char *buf, size_t count) >> +{ >> + struct scsi_unnamed *su = to_scsi_unnamed(dev); >> + struct scsi_unnamed *tmp; >> + struct device *lu = dev->parent; >> + int ret = -EINVAL; >> + >> + if (count >= DISK_NAME_LEN) { >> + printk(KERN_WARNING "su: Too long a persistent name!\n"); >> + return -EINVAL; >> + } >> + >> + if (su->assigned) { >> + printk(KERN_WARNING "su: Already assigned a persistent name!\n"); >> + return -EINVAL; >> + } >> + >> + if (strncmp(lu->driver->name, buf, 2)) { >> + printk(KERN_WARNING "su: Wrong prefix!\n"); >> + return -EINVAL; >> + } >> + >> + mutex_lock(&su_list_lock); >> + list_for_each_entry(tmp, &su_list, list) { >> + if (!strncmp(tmp->disk_name, buf, DISK_NAME_LEN)) { >> + printk(KERN_WARNING "su: Duplicate name exsists!\n"); >> + return -EINVAL; >> + } >> + } >> + strncpy(su->disk_name, buf, DISK_NAME_LEN); >> + mutex_unlock(&su_list_lock); >> + >> + if (!lu->driver->probe) >> + return -EINVAL; >> + >> + lu->init_name = buf; >> + >> + ret = lu->driver->probe(lu); >> + if (ret) { >> + lu->init_name = NULL; >> + su->disk_name[0] = '\0'; >> + return ret; >> + } >> + >> + su->assigned = true; >> + return count; >> +} >> + >> +static DEVICE_ATTR(disk_name, S_IRUGO|S_IWUSR, show_disk_name, >> + store_disk_name); >> +static DEVICE_ATTR(disk_lun, S_IRUGO, show_disk_lun, NULL); >> +static DEVICE_ATTR(disk_id, S_IRUGO, show_disk_id, NULL); >> + >> +/* Confirm the driver name and the device type */ >> +static int check_device_type(char type, const char *name) >> +{ >> + switch (type) { >> + case TYPE_DISK: >> + case TYPE_MOD: >> + case TYPE_RBC: >> + if (strncmp("sd", name, 2)) >> + return -ENODEV; >> + break; >> + case TYPE_ROM: >> + case TYPE_WORM: >> + if (strncmp("sr", name, 2)) >> + return -ENODEV; >> + break; >> + case TYPE_TAPE: >> + if (strncmp("st", name, 2)) >> + return -ENODEV; >> + break; >> + default: >> + break; >> + } >> + return 0; >> +} >> + >> +/** >> + * scsi_unnamed_probe - Setup the unnamed device. >> + * @dev: parent scsi device >> + * >> + * This function adds a unnamed device to the device model and >> + * gets a number of device information by scsi inquiry commands. >> + * Udev identify a device by those information. >> + * >> + * Unnamed devices: >> + * This device is not a block device and user can not read/write >> + * this device. But it can advertise device information in sysfs. >> + */ >> +int scsi_unnamed_probe(struct device *dev) >> +{ >> + struct scsi_unnamed *su; >> + struct scsi_device *sdev = to_scsi_device(dev); >> + int ret = -EINVAL; >> + static int i; >> + >> + if (check_device_type(sdev->type, dev->driver->name)) >> + return -ENODEV; >> + >> + su = kzalloc(sizeof(*su), GFP_KERNEL); >> + if (!su) >> + return -ENOMEM; >> + >> + device_initialize(&su->dev); >> + su->dev.parent = dev; >> + su->dev.class = &su_sysfs_class; > > You just created a device that can not be removed from the system (i.e. > you have no release function.) This is a major bug and as per the > documentation for kobjects and the driver model in the kernel tree, I am > allowed to mock this code mercilessly :) > Oh... sorry :( >> + dev_set_name(&su->dev, "su%d", i++); >> + strncpy(su->disk_lun, dev_name(dev), MAX_BUFFER_LEN); >> + >> + ret = device_add(&su->dev); >> + if (ret) >> + goto err; >> + >> + ret = device_create_file(&su->dev, &dev_attr_disk_name); > > Nope, you just raced with userspace here. NEVER create a sysfs file > after you have added it to the system. Userspace just got notified of > the device and is already starting to read the file. As it's not there > yet, you just failed :( > > Use attributes properly to keep this race from happening. > OK. I'll try to be more careful in future > >> + if (ret) >> + goto err_disk_name; >> + >> + ret = device_create_file(&su->dev, &dev_attr_disk_lun); >> + if (ret) >> + goto err_disk_lun; >> + >> + store_disk_id(sdev, su->disk_id); >> + if (su->disk_id[0] != '\0') { >> + ret = device_create_file(&su->dev, &dev_attr_disk_id); >> + if (ret) >> + goto err_disk_id; >> + } >> + >> + su->assigned = false; >> + mutex_lock(&su_list_lock); >> + list_add(&su->list, &su_list); >> + mutex_unlock(&su_list_lock); >> + >> + return 0; >> + >> +err_disk_id: >> + device_remove_file(&su->dev, &dev_attr_disk_lun); >> + >> +err_disk_lun: >> + device_remove_file(&su->dev, &dev_attr_disk_name); >> + >> +err_disk_name: >> + device_del(&su->dev); >> + >> +err: >> + kfree(su); >> + return ret; >> +} >> + >> +/* Remove a unnamed device from su_list and release resources */ >> +void scsi_unnamed_remove(struct device *dev) >> +{ >> + struct scsi_unnamed *tmp; >> + >> + mutex_lock(&su_list_lock); >> + list_for_each_entry(tmp, &su_list, list) { >> + if (dev == tmp->dev.parent) { >> + list_del(&tmp->list); >> + break; >> + } >> + } >> + mutex_unlock(&su_list_lock); >> + >> + if (tmp->disk_id[0] != '\0') >> + device_remove_file(&tmp->dev, &dev_attr_disk_id); >> + device_remove_file(&tmp->dev, &dev_attr_disk_name); >> + device_remove_file(&tmp->dev, &dev_attr_disk_lun); >> + device_del(&tmp->dev); > > Your kerrnel just spit out some very nasty messages when this function > was called, right? Why are you ignoring them? > Sorry. I will check it. >> + >> + device_release_driver(dev); >> + >> + kfree(tmp); >> +} >> + >> +/** >> + * copy_persistent_name - Copy the device name >> + * @disk_name: allocate to >> + * @dev: scsi device >> + * >> + * Copy an initial name of the device to disk_name. >> + */ >> +int copy_persistent_name(char *disk_name, struct device *dev) >> +{ >> + if (persistent_name) { >> + strncpy(disk_name, dev->init_name, DISK_NAME_LEN); >> + return 1; >> + } >> + return 0; >> +} >> +EXPORT_SYMBOL(copy_persistent_name); > > That's a very bad global function name. > I have a bad sense :( I will name this function scsi_unnamed_copy_persistent_name. > EXPORT_SYMBOL_GPL? And why is it exported at all? Who would ever need > to call this from a module? > Please see the patch 2/2. This function is called by external >> + >> +int scsi_unnamed_register(struct bus_type *bus) >> +{ >> + if (persistent_name) { >> + bus->probe = scsi_unnamed_probe; >> + bus->remove = scsi_unnamed_remove; >> + return class_register(&su_sysfs_class); >> + } >> + return 0; >> +} >> +EXPORT_SYMBOL(scsi_unnamed_register); > > Same here, why is this exported? > >> + >> +void scsi_unnamed_unregister(void) >> +{ >> + if (persistent_name) >> + class_unregister(&su_sysfs_class); >> +} >> +EXPORT_SYMBOL(scsi_unnamed_unregister); > > And here. > >> diff --git a/drivers/scsi/scsi_unnamed.h b/drivers/scsi/scsi_unnamed.h >> new file mode 100644 >> index 0000000..ca4e852 >> --- /dev/null >> +++ b/drivers/scsi/scsi_unnamed.h >> @@ -0,0 +1,25 @@ >> +/* >> + * scsi_unnamed.h - SCSI driver for unnmaed device >> + * >> + * Copyright: Copyright (c) Hitachi, Ltd. 2011 >> + * Authors: Nao Nishijima <nao.nishijima.xt@hitachi.com> >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License as published by >> + * the Free Software Foundation; either version 2 of the License, or >> + * (at your option) any later version. > > Same comment as above. > >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public License >> + * along with this program; if not, write to the Free Software >> + * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111 USA > > And again, same comment as above. > > thanks, > > greg k-h > Thanks, -- Nao NISHIJIMA Software Platform Research Dept. Linux Technology Center Hitachi, Ltd., YOKOHAMA Research Laboratory Email: nao.nishijima.xt@hitachi.com -- To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel. 2011-04-08 14:12 ` [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel Nao Nishijima (?) @ 2011-04-08 14:33 ` Hannes Reinecke -1 siblings, 0 replies; 46+ messages in thread From: Hannes Reinecke @ 2011-04-08 14:33 UTC (permalink / raw) To: Nao Nishijima Cc: Greg KH, linux-kernel, linux-scsi, linux-hotplug, Kay Sievers, James Bottomley, Jon Masters, 2nddept-manager On 04/08/2011 07:12 AM, Nao Nishijima wrote: > Hi, > > (2011/04/06 1:14), Greg KH wrote: >> On Tue, Apr 05, 2011 at 09:49:46PM +0900, Nao Nishijima wrote: >>> This patch series provides a SCSI option for persistent device >>> names in kernel. With this option, user can always assign a >>> same device name (e.g. sda) to a specific device according to >>> udev rules and device id. >>> >>> Issue: >>> Recently, kernel registers block devices in parallel. As a result, >>> different device names will be assigned at each boot time. This >>> will confuse file-system mounter, thus we usually use persistent >>> symbolic links provided by udev. >> >> Yes, that is what you should use if you care about this. >> >>> However, dmesg and procfs outputs >>> show device names instead of the symbolic link names. This causes >>> a serious problem when managing multiple devices (e.g. on a >>> large-scale storage), because usually, device errors are output >>> with device names on dmesg. >> >> Then fix your tools that read the output of these files to point to the >> proper persistent name instead of trying to get the kernel to solve the >> problem. >> > > Yes, the tools should be revised if users get a device name using them. > > The problem I would like to discuss here is that users can not identify > a disk from kernel messages when they DIRECTLY refer to these messages. > For example, a device name is used instead of a symbolic link names in > bootup messages, I/O devices errors and /proc/partitions …etc. > > In particular, users can not identify an appropriate device from a > device name in syslog since different device name may be assigned to it > at each boot time. > > My idea is able to fix this issue with small changes in scsi subsystem. > Also, it is implemented as an option. Therefore, it does not affect > users who do not select this option. > We have been discussing this problem several times in the past, and indeed on these very mailing list. The conclusion we arrived at is that the kernel-provided device node name is inherently unstable and impossible to fix within the existing 'sdX' naming scheme. So the choices have been to either move to a totally different naming scheme or keep the naming scheme and provide the required information by other means. We have decided on the latter, and agreed on using udev to provide persistent device names. We are fully aware that any kernel related messages are subject to chance after reboot, but then most kernel related messages are (PID number, timestamps, login tty etc). And also we are aware that any kernel messages need to be matched against the current system layout to figure out any hardware-related issue. But then basically all products requiring to filter out information from kernel messages already do so I don't see a problem with that. Just adding an in-kernel identifier to the LUN will only be an incomplete solution, as other identifiers will still be volatile. So I would prefer by keeping the in-kernel information as small as possible to reduce memory consumption and rely on out-of-band programs to provide the required mapping. Cheers, Hannes -- Dr. Hannes Reinecke zSeries & Storage hare@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Markus Rex, HRB 16746 (AG Nürnberg) ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 1/2] SCSI: Add a SCSI option for persistent device names @ 2011-04-08 14:33 ` Hannes Reinecke 0 siblings, 0 replies; 46+ messages in thread From: Hannes Reinecke @ 2011-04-08 14:33 UTC (permalink / raw) To: Nao Nishijima Cc: Greg KH, linux-kernel, linux-scsi, linux-hotplug, Kay Sievers, James Bottomley, Jon Masters, 2nddept-manager On 04/08/2011 07:12 AM, Nao Nishijima wrote: > Hi, > > (2011/04/06 1:14), Greg KH wrote: >> On Tue, Apr 05, 2011 at 09:49:46PM +0900, Nao Nishijima wrote: >>> This patch series provides a SCSI option for persistent device >>> names in kernel. With this option, user can always assign a >>> same device name (e.g. sda) to a specific device according to >>> udev rules and device id. >>> >>> Issue: >>> Recently, kernel registers block devices in parallel. As a result, >>> different device names will be assigned at each boot time. This >>> will confuse file-system mounter, thus we usually use persistent >>> symbolic links provided by udev. >> >> Yes, that is what you should use if you care about this. >> >>> However, dmesg and procfs outputs >>> show device names instead of the symbolic link names. This causes >>> a serious problem when managing multiple devices (e.g. on a >>> large-scale storage), because usually, device errors are output >>> with device names on dmesg. >> >> Then fix your tools that read the output of these files to point to the >> proper persistent name instead of trying to get the kernel to solve the >> problem. >> > > Yes, the tools should be revised if users get a device name using them. > > The problem I would like to discuss here is that users can not identify > a disk from kernel messages when they DIRECTLY refer to these messages. > For example, a device name is used instead of a symbolic link names in > bootup messages, I/O devices errors and /proc/partitions …etc. > > In particular, users can not identify an appropriate device from a > device name in syslog since different device name may be assigned to it > at each boot time. > > My idea is able to fix this issue with small changes in scsi subsystem. > Also, it is implemented as an option. Therefore, it does not affect > users who do not select this option. > We have been discussing this problem several times in the past, and indeed on these very mailing list. The conclusion we arrived at is that the kernel-provided device node name is inherently unstable and impossible to fix within the existing 'sdX' naming scheme. So the choices have been to either move to a totally different naming scheme or keep the naming scheme and provide the required information by other means. We have decided on the latter, and agreed on using udev to provide persistent device names. We are fully aware that any kernel related messages are subject to chance after reboot, but then most kernel related messages are (PID number, timestamps, login tty etc). And also we are aware that any kernel messages need to be matched against the current system layout to figure out any hardware-related issue. But then basically all products requiring to filter out information from kernel messages already do so I don't see a problem with that. Just adding an in-kernel identifier to the LUN will only be an incomplete solution, as other identifiers will still be volatile. So I would prefer by keeping the in-kernel information as small as possible to reduce memory consumption and rely on out-of-band programs to provide the required mapping. Cheers, Hannes -- Dr. Hannes Reinecke zSeries & Storage hare@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Markus Rex, HRB 16746 (AG Nürnberg) ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel. @ 2011-04-08 14:33 ` Hannes Reinecke 0 siblings, 0 replies; 46+ messages in thread From: Hannes Reinecke @ 2011-04-08 14:33 UTC (permalink / raw) To: Nao Nishijima Cc: Greg KH, linux-kernel, linux-scsi, linux-hotplug, Kay Sievers, James Bottomley, Jon Masters, 2nddept-manager On 04/08/2011 07:12 AM, Nao Nishijima wrote: > Hi, > > (2011/04/06 1:14), Greg KH wrote: >> On Tue, Apr 05, 2011 at 09:49:46PM +0900, Nao Nishijima wrote: >>> This patch series provides a SCSI option for persistent device >>> names in kernel. With this option, user can always assign a >>> same device name (e.g. sda) to a specific device according to >>> udev rules and device id. >>> >>> Issue: >>> Recently, kernel registers block devices in parallel. As a result, >>> different device names will be assigned at each boot time. This >>> will confuse file-system mounter, thus we usually use persistent >>> symbolic links provided by udev. >> >> Yes, that is what you should use if you care about this. >> >>> However, dmesg and procfs outputs >>> show device names instead of the symbolic link names. This causes >>> a serious problem when managing multiple devices (e.g. on a >>> large-scale storage), because usually, device errors are output >>> with device names on dmesg. >> >> Then fix your tools that read the output of these files to point to the >> proper persistent name instead of trying to get the kernel to solve the >> problem. >> > > Yes, the tools should be revised if users get a device name using them. > > The problem I would like to discuss here is that users can not identify > a disk from kernel messages when they DIRECTLY refer to these messages. > For example, a device name is used instead of a symbolic link names in > bootup messages, I/O devices errors and /proc/partitions …etc. > > In particular, users can not identify an appropriate device from a > device name in syslog since different device name may be assigned to it > at each boot time. > > My idea is able to fix this issue with small changes in scsi subsystem. > Also, it is implemented as an option. Therefore, it does not affect > users who do not select this option. > We have been discussing this problem several times in the past, and indeed on these very mailing list. The conclusion we arrived at is that the kernel-provided device node name is inherently unstable and impossible to fix within the existing 'sdX' naming scheme. So the choices have been to either move to a totally different naming scheme or keep the naming scheme and provide the required information by other means. We have decided on the latter, and agreed on using udev to provide persistent device names. We are fully aware that any kernel related messages are subject to chance after reboot, but then most kernel related messages are (PID number, timestamps, login tty etc). And also we are aware that any kernel messages need to be matched against the current system layout to figure out any hardware-related issue. But then basically all products requiring to filter out information from kernel messages already do so I don't see a problem with that. Just adding an in-kernel identifier to the LUN will only be an incomplete solution, as other identifiers will still be volatile. So I would prefer by keeping the in-kernel information as small as possible to reduce memory consumption and rely on out-of-band programs to provide the required mapping. Cheers, Hannes -- Dr. Hannes Reinecke zSeries & Storage hare@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Markus Rex, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel. 2011-04-08 14:33 ` [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel Hannes Reinecke (?) @ 2011-04-08 15:14 ` James Bottomley -1 siblings, 0 replies; 46+ messages in thread From: James Bottomley @ 2011-04-08 15:14 UTC (permalink / raw) To: Hannes Reinecke Cc: Nao Nishijima, Greg KH, linux-kernel, linux-scsi, linux-hotplug, Kay Sievers, Jon Masters, 2nddept-manager On Fri, 2011-04-08 at 07:33 -0700, Hannes Reinecke wrote: > > The problem I would like to discuss here is that users can not identify > > a disk from kernel messages when they DIRECTLY refer to these messages. > > For example, a device name is used instead of a symbolic link names in > > bootup messages, I/O devices errors and /proc/partitions …etc. > > > > In particular, users can not identify an appropriate device from a > > device name in syslog since different device name may be assigned to it > > at each boot time. > > > > My idea is able to fix this issue with small changes in scsi subsystem. > > Also, it is implemented as an option. Therefore, it does not affect > > users who do not select this option. > > > We have been discussing this problem several times in the past, and > indeed on these very mailing list. > > The conclusion we arrived at is that the kernel-provided device node > name is inherently unstable and impossible to fix within the existing > 'sdX' naming scheme. > So the choices have been to either move to a totally different naming > scheme or keep the naming scheme and provide the required information > by other means. > We have decided on the latter, and agreed on using udev to provide > persistent device names. > We are fully aware that any kernel related messages are subject to > chance after reboot, but then most kernel related messages are > (PID number, timestamps, login tty etc). > And also we are aware that any kernel messages need to be matched > against the current system layout to figure out any hardware-related > issue. > > But then basically all products requiring to filter out information > from kernel messages already do so I don't see a problem with that. > > Just adding an in-kernel identifier to the LUN will only be an > incomplete solution, as other identifiers will still be volatile. > > So I would prefer by keeping the in-kernel information as small > as possible to reduce memory consumption and rely on out-of-band > programs to provide the required mapping. So, while I agree totally with the above: udev and userspace is the way to go, I'm not totally opposed to having a non-invasive mechanism for indicating a user's preferred name for a device. I think there are a couple of ways to do this: 1. Entirely in userspace: just have udev consult a preferred name file and create say /dev/disk/by-preferred. Then have all the tools that normally output device information do the same (i.e. since real name to preferred name is 1:1, they could all do a reverse lookup). 2. have a writeable sysfs preferred_name field, either in the generic device or just in SCSI. The preferred name would be used by outbound only (i.e. kernel dev_printk messages and possibly /proc/partitions). All inbound uses of the device would come via the standard udev mechanisms (i.e. /dev/disk/by-preferred would be the usual symlink). This means from the kernel point of view, no renaming has happened. We'd just try to print out the preferred name in certain circumstances, which should solve most of the described problem. James ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 1/2] SCSI: Add a SCSI option for persistent device @ 2011-04-08 15:14 ` James Bottomley 0 siblings, 0 replies; 46+ messages in thread From: James Bottomley @ 2011-04-08 15:14 UTC (permalink / raw) To: Hannes Reinecke Cc: Nao Nishijima, Greg KH, linux-kernel, linux-scsi, linux-hotplug, Kay Sievers, Jon Masters, 2nddept-manager On Fri, 2011-04-08 at 07:33 -0700, Hannes Reinecke wrote: > > The problem I would like to discuss here is that users can not identify > > a disk from kernel messages when they DIRECTLY refer to these messages. > > For example, a device name is used instead of a symbolic link names in > > bootup messages, I/O devices errors and /proc/partitions …etc. > > > > In particular, users can not identify an appropriate device from a > > device name in syslog since different device name may be assigned to it > > at each boot time. > > > > My idea is able to fix this issue with small changes in scsi subsystem. > > Also, it is implemented as an option. Therefore, it does not affect > > users who do not select this option. > > > We have been discussing this problem several times in the past, and > indeed on these very mailing list. > > The conclusion we arrived at is that the kernel-provided device node > name is inherently unstable and impossible to fix within the existing > 'sdX' naming scheme. > So the choices have been to either move to a totally different naming > scheme or keep the naming scheme and provide the required information > by other means. > We have decided on the latter, and agreed on using udev to provide > persistent device names. > We are fully aware that any kernel related messages are subject to > chance after reboot, but then most kernel related messages are > (PID number, timestamps, login tty etc). > And also we are aware that any kernel messages need to be matched > against the current system layout to figure out any hardware-related > issue. > > But then basically all products requiring to filter out information > from kernel messages already do so I don't see a problem with that. > > Just adding an in-kernel identifier to the LUN will only be an > incomplete solution, as other identifiers will still be volatile. > > So I would prefer by keeping the in-kernel information as small > as possible to reduce memory consumption and rely on out-of-band > programs to provide the required mapping. So, while I agree totally with the above: udev and userspace is the way to go, I'm not totally opposed to having a non-invasive mechanism for indicating a user's preferred name for a device. I think there are a couple of ways to do this: 1. Entirely in userspace: just have udev consult a preferred name file and create say /dev/disk/by-preferred. Then have all the tools that normally output device information do the same (i.e. since real name to preferred name is 1:1, they could all do a reverse lookup). 2. have a writeable sysfs preferred_name field, either in the generic device or just in SCSI. The preferred name would be used by outbound only (i.e. kernel dev_printk messages and possibly /proc/partitions). All inbound uses of the device would come via the standard udev mechanisms (i.e. /dev/disk/by-preferred would be the usual symlink). This means from the kernel point of view, no renaming has happened. We'd just try to print out the preferred name in certain circumstances, which should solve most of the described problem. James ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel. @ 2011-04-08 15:14 ` James Bottomley 0 siblings, 0 replies; 46+ messages in thread From: James Bottomley @ 2011-04-08 15:14 UTC (permalink / raw) To: Hannes Reinecke Cc: Nao Nishijima, Greg KH, linux-kernel, linux-scsi, linux-hotplug, Kay Sievers, Jon Masters, 2nddept-manager On Fri, 2011-04-08 at 07:33 -0700, Hannes Reinecke wrote: > > The problem I would like to discuss here is that users can not identify > > a disk from kernel messages when they DIRECTLY refer to these messages. > > For example, a device name is used instead of a symbolic link names in > > bootup messages, I/O devices errors and /proc/partitions …etc. > > > > In particular, users can not identify an appropriate device from a > > device name in syslog since different device name may be assigned to it > > at each boot time. > > > > My idea is able to fix this issue with small changes in scsi subsystem. > > Also, it is implemented as an option. Therefore, it does not affect > > users who do not select this option. > > > We have been discussing this problem several times in the past, and > indeed on these very mailing list. > > The conclusion we arrived at is that the kernel-provided device node > name is inherently unstable and impossible to fix within the existing > 'sdX' naming scheme. > So the choices have been to either move to a totally different naming > scheme or keep the naming scheme and provide the required information > by other means. > We have decided on the latter, and agreed on using udev to provide > persistent device names. > We are fully aware that any kernel related messages are subject to > chance after reboot, but then most kernel related messages are > (PID number, timestamps, login tty etc). > And also we are aware that any kernel messages need to be matched > against the current system layout to figure out any hardware-related > issue. > > But then basically all products requiring to filter out information > from kernel messages already do so I don't see a problem with that. > > Just adding an in-kernel identifier to the LUN will only be an > incomplete solution, as other identifiers will still be volatile. > > So I would prefer by keeping the in-kernel information as small > as possible to reduce memory consumption and rely on out-of-band > programs to provide the required mapping. So, while I agree totally with the above: udev and userspace is the way to go, I'm not totally opposed to having a non-invasive mechanism for indicating a user's preferred name for a device. I think there are a couple of ways to do this: 1. Entirely in userspace: just have udev consult a preferred name file and create say /dev/disk/by-preferred. Then have all the tools that normally output device information do the same (i.e. since real name to preferred name is 1:1, they could all do a reverse lookup). 2. have a writeable sysfs preferred_name field, either in the generic device or just in SCSI. The preferred name would be used by outbound only (i.e. kernel dev_printk messages and possibly /proc/partitions). All inbound uses of the device would come via the standard udev mechanisms (i.e. /dev/disk/by-preferred would be the usual symlink). This means from the kernel point of view, no renaming has happened. We'd just try to print out the preferred name in certain circumstances, which should solve most of the described problem. James -- To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel. 2011-04-08 15:14 ` [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel James Bottomley @ 2011-04-08 16:14 ` Greg KH -1 siblings, 0 replies; 46+ messages in thread From: Greg KH @ 2011-04-08 16:14 UTC (permalink / raw) To: James Bottomley Cc: Hannes Reinecke, Nao Nishijima, linux-kernel, linux-scsi, linux-hotplug, Kay Sievers, Jon Masters, 2nddept-manager On Fri, Apr 08, 2011 at 08:14:12AM -0700, James Bottomley wrote: > So, while I agree totally with the above: udev and userspace is the way > to go, I'm not totally opposed to having a non-invasive mechanism for > indicating a user's preferred name for a device. I think there are a > couple of ways to do this: > > 1. Entirely in userspace: just have udev consult a preferred name > file and create say /dev/disk/by-preferred. Then have all the > tools that normally output device information do the same (i.e. > since real name to preferred name is 1:1, they could all do a > reverse lookup). > 2. have a writeable sysfs preferred_name field, either in the > generic device or just in SCSI. The preferred name would be > used by outbound only (i.e. kernel dev_printk messages and > possibly /proc/partitions). All inbound uses of the device > would come via the standard udev mechanisms > (i.e. /dev/disk/by-preferred would be the usual symlink). This > means from the kernel point of view, no renaming has happened. > We'd just try to print out the preferred name in certain > circumstances, which should solve most of the described problem. Either, or both, of those options are fine with me as well. thanks, greg k-h ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 1/2] SCSI: Add a SCSI option for persistent device names @ 2011-04-08 16:14 ` Greg KH 0 siblings, 0 replies; 46+ messages in thread From: Greg KH @ 2011-04-08 16:14 UTC (permalink / raw) To: James Bottomley Cc: Hannes Reinecke, Nao Nishijima, linux-kernel, linux-scsi, linux-hotplug, Kay Sievers, Jon Masters, 2nddept-manager On Fri, Apr 08, 2011 at 08:14:12AM -0700, James Bottomley wrote: > So, while I agree totally with the above: udev and userspace is the way > to go, I'm not totally opposed to having a non-invasive mechanism for > indicating a user's preferred name for a device. I think there are a > couple of ways to do this: > > 1. Entirely in userspace: just have udev consult a preferred name > file and create say /dev/disk/by-preferred. Then have all the > tools that normally output device information do the same (i.e. > since real name to preferred name is 1:1, they could all do a > reverse lookup). > 2. have a writeable sysfs preferred_name field, either in the > generic device or just in SCSI. The preferred name would be > used by outbound only (i.e. kernel dev_printk messages and > possibly /proc/partitions). All inbound uses of the device > would come via the standard udev mechanisms > (i.e. /dev/disk/by-preferred would be the usual symlink). This > means from the kernel point of view, no renaming has happened. > We'd just try to print out the preferred name in certain > circumstances, which should solve most of the described problem. Either, or both, of those options are fine with me as well. thanks, greg k-h ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel. 2011-04-08 16:14 ` [PATCH 1/2] SCSI: Add a SCSI option for persistent device names Greg KH (?) @ 2011-04-08 16:43 ` Kay Sievers -1 siblings, 0 replies; 46+ messages in thread From: Kay Sievers @ 2011-04-08 16:43 UTC (permalink / raw) To: Greg KH Cc: James Bottomley, Hannes Reinecke, Nao Nishijima, linux-kernel, linux-scsi, linux-hotplug, Jon Masters, 2nddept-manager On Fri, Apr 8, 2011 at 18:14, Greg KH <greg@kroah.com> wrote: > On Fri, Apr 08, 2011 at 08:14:12AM -0700, James Bottomley wrote: >> So, while I agree totally with the above: udev and userspace is the way >> to go, I'm not totally opposed to having a non-invasive mechanism for >> indicating a user's preferred name for a device. I think there are a >> couple of ways to do this: >> >> 1. Entirely in userspace: just have udev consult a preferred name >> file and create say /dev/disk/by-preferred. Then have all the >> tools that normally output device information do the same (i.e. >> since real name to preferred name is 1:1, they could all do a >> reverse lookup). >> 2. have a writeable sysfs preferred_name field, either in the >> generic device or just in SCSI. The preferred name would be >> used by outbound only (i.e. kernel dev_printk messages and >> possibly /proc/partitions). All inbound uses of the device >> would come via the standard udev mechanisms >> (i.e. /dev/disk/by-preferred would be the usual symlink). This >> means from the kernel point of view, no renaming has happened. >> We'd just try to print out the preferred name in certain >> circumstances, which should solve most of the described problem. > > Either, or both, of those options are fine with me as well. I guess a generic sysfs file, that can carry a user-given name for a device, and which is looked-up with dev_printk() macro, and maybe /proc/partitions is all what we would need if we want to involve the kernel here. Kay ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 1/2] SCSI: Add a SCSI option for persistent device names @ 2011-04-08 16:43 ` Kay Sievers 0 siblings, 0 replies; 46+ messages in thread From: Kay Sievers @ 2011-04-08 16:43 UTC (permalink / raw) To: Greg KH Cc: James Bottomley, Hannes Reinecke, Nao Nishijima, linux-kernel, linux-scsi, linux-hotplug, Jon Masters, 2nddept-manager On Fri, Apr 8, 2011 at 18:14, Greg KH <greg@kroah.com> wrote: > On Fri, Apr 08, 2011 at 08:14:12AM -0700, James Bottomley wrote: >> So, while I agree totally with the above: udev and userspace is the way >> to go, I'm not totally opposed to having a non-invasive mechanism for >> indicating a user's preferred name for a device. I think there are a >> couple of ways to do this: >> >> 1. Entirely in userspace: just have udev consult a preferred name >> file and create say /dev/disk/by-preferred. Then have all the >> tools that normally output device information do the same (i.e. >> since real name to preferred name is 1:1, they could all do a >> reverse lookup). >> 2. have a writeable sysfs preferred_name field, either in the >> generic device or just in SCSI. The preferred name would be >> used by outbound only (i.e. kernel dev_printk messages and >> possibly /proc/partitions). All inbound uses of the device >> would come via the standard udev mechanisms >> (i.e. /dev/disk/by-preferred would be the usual symlink). This >> means from the kernel point of view, no renaming has happened. >> We'd just try to print out the preferred name in certain >> circumstances, which should solve most of the described problem. > > Either, or both, of those options are fine with me as well. I guess a generic sysfs file, that can carry a user-given name for a device, and which is looked-up with dev_printk() macro, and maybe /proc/partitions is all what we would need if we want to involve the kernel here. Kay ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel. @ 2011-04-08 16:43 ` Kay Sievers 0 siblings, 0 replies; 46+ messages in thread From: Kay Sievers @ 2011-04-08 16:43 UTC (permalink / raw) To: Greg KH Cc: James Bottomley, Hannes Reinecke, Nao Nishijima, linux-kernel, linux-scsi, linux-hotplug, Jon Masters, 2nddept-manager On Fri, Apr 8, 2011 at 18:14, Greg KH <greg@kroah.com> wrote: > On Fri, Apr 08, 2011 at 08:14:12AM -0700, James Bottomley wrote: >> So, while I agree totally with the above: udev and userspace is the way >> to go, I'm not totally opposed to having a non-invasive mechanism for >> indicating a user's preferred name for a device. I think there are a >> couple of ways to do this: >> >> 1. Entirely in userspace: just have udev consult a preferred name >> file and create say /dev/disk/by-preferred. Then have all the >> tools that normally output device information do the same (i.e. >> since real name to preferred name is 1:1, they could all do a >> reverse lookup). >> 2. have a writeable sysfs preferred_name field, either in the >> generic device or just in SCSI. The preferred name would be >> used by outbound only (i.e. kernel dev_printk messages and >> possibly /proc/partitions). All inbound uses of the device >> would come via the standard udev mechanisms >> (i.e. /dev/disk/by-preferred would be the usual symlink). This >> means from the kernel point of view, no renaming has happened. >> We'd just try to print out the preferred name in certain >> circumstances, which should solve most of the described problem. > > Either, or both, of those options are fine with me as well. I guess a generic sysfs file, that can carry a user-given name for a device, and which is looked-up with dev_printk() macro, and maybe /proc/partitions is all what we would need if we want to involve the kernel here. Kay -- To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel. 2011-04-08 15:14 ` [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel James Bottomley (?) @ 2011-04-12 13:23 ` Nao Nishijima -1 siblings, 0 replies; 46+ messages in thread From: Nao Nishijima @ 2011-04-12 13:23 UTC (permalink / raw) To: James Bottomley Cc: Hannes Reinecke, Greg KH, linux-kernel, linux-scsi, linux-hotplug, Kay Sievers, Jon Masters, 2nddept-manager Hi, James (2011/04/09 0:14), James Bottomley wrote: > On Fri, 2011-04-08 at 07:33 -0700, Hannes Reinecke wrote: >>> The problem I would like to discuss here is that users can not identify >>> a disk from kernel messages when they DIRECTLY refer to these messages. >>> For example, a device name is used instead of a symbolic link names in >>> bootup messages, I/O devices errors and /proc/partitions …etc. >>> >>> In particular, users can not identify an appropriate device from a >>> device name in syslog since different device name may be assigned to it >>> at each boot time. >>> >>> My idea is able to fix this issue with small changes in scsi subsystem. >>> Also, it is implemented as an option. Therefore, it does not affect >>> users who do not select this option. >>> >> We have been discussing this problem several times in the past, and >> indeed on these very mailing list. >> >> The conclusion we arrived at is that the kernel-provided device node >> name is inherently unstable and impossible to fix within the existing >> 'sdX' naming scheme. >> So the choices have been to either move to a totally different naming >> scheme or keep the naming scheme and provide the required information >> by other means. >> We have decided on the latter, and agreed on using udev to provide >> persistent device names. >> We are fully aware that any kernel related messages are subject to >> chance after reboot, but then most kernel related messages are >> (PID number, timestamps, login tty etc). >> And also we are aware that any kernel messages need to be matched >> against the current system layout to figure out any hardware-related >> issue. >> >> But then basically all products requiring to filter out information >> from kernel messages already do so I don't see a problem with that. >> >> Just adding an in-kernel identifier to the LUN will only be an >> incomplete solution, as other identifiers will still be volatile. >> >> So I would prefer by keeping the in-kernel information as small >> as possible to reduce memory consumption and rely on out-of-band >> programs to provide the required mapping. > > So, while I agree totally with the above: udev and userspace is the way > to go, I'm not totally opposed to having a non-invasive mechanism for > indicating a user's preferred name for a device. I think there are a > couple of ways to do this: > > 1. Entirely in userspace: just have udev consult a preferred name > file and create say /dev/disk/by-preferred. Then have all the > tools that normally output device information do the same (i.e. > since real name to preferred name is 1:1, they could all do a > reverse lookup). > 2. have a writeable sysfs preferred_name field, either in the > generic device or just in SCSI. The preferred name would be > used by outbound only (i.e. kernel dev_printk messages and > possibly /proc/partitions). All inbound uses of the device > would come via the standard udev mechanisms > (i.e. /dev/disk/by-preferred would be the usual symlink). This > means from the kernel point of view, no renaming has happened. > We'd just try to print out the preferred name in certain > circumstances, which should solve most of the described problem. > > James > > > I have a question. Why is in-kernel device name necessary? The kernel can identify a device by major/miner number and udev can create a device node of a prefer name. Currently, device names are only used to show to users. Therefore I think that in-kernel device name is unnecessary if we introduce your /dev/disk/by-prefferd idea. thanks, -- Nao NISHIJIMA Software Platform Research Dept. Linux Technology Center Hitachi, Ltd., YOKOHAMA Research Laboratory Email: nao.nishijima.xt@hitachi.com ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 1/2] SCSI: Add a SCSI option for persistent device names @ 2011-04-12 13:23 ` Nao Nishijima 0 siblings, 0 replies; 46+ messages in thread From: Nao Nishijima @ 2011-04-12 13:23 UTC (permalink / raw) To: James Bottomley Cc: Hannes Reinecke, Greg KH, linux-kernel, linux-scsi, linux-hotplug, Kay Sievers, Jon Masters, 2nddept-manager Hi, James (2011/04/09 0:14), James Bottomley wrote: > On Fri, 2011-04-08 at 07:33 -0700, Hannes Reinecke wrote: >>> The problem I would like to discuss here is that users can not identify >>> a disk from kernel messages when they DIRECTLY refer to these messages. >>> For example, a device name is used instead of a symbolic link names in >>> bootup messages, I/O devices errors and /proc/partitions …etc. >>> >>> In particular, users can not identify an appropriate device from a >>> device name in syslog since different device name may be assigned to it >>> at each boot time. >>> >>> My idea is able to fix this issue with small changes in scsi subsystem. >>> Also, it is implemented as an option. Therefore, it does not affect >>> users who do not select this option. >>> >> We have been discussing this problem several times in the past, and >> indeed on these very mailing list. >> >> The conclusion we arrived at is that the kernel-provided device node >> name is inherently unstable and impossible to fix within the existing >> 'sdX' naming scheme. >> So the choices have been to either move to a totally different naming >> scheme or keep the naming scheme and provide the required information >> by other means. >> We have decided on the latter, and agreed on using udev to provide >> persistent device names. >> We are fully aware that any kernel related messages are subject to >> chance after reboot, but then most kernel related messages are >> (PID number, timestamps, login tty etc). >> And also we are aware that any kernel messages need to be matched >> against the current system layout to figure out any hardware-related >> issue. >> >> But then basically all products requiring to filter out information >> from kernel messages already do so I don't see a problem with that. >> >> Just adding an in-kernel identifier to the LUN will only be an >> incomplete solution, as other identifiers will still be volatile. >> >> So I would prefer by keeping the in-kernel information as small >> as possible to reduce memory consumption and rely on out-of-band >> programs to provide the required mapping. > > So, while I agree totally with the above: udev and userspace is the way > to go, I'm not totally opposed to having a non-invasive mechanism for > indicating a user's preferred name for a device. I think there are a > couple of ways to do this: > > 1. Entirely in userspace: just have udev consult a preferred name > file and create say /dev/disk/by-preferred. Then have all the > tools that normally output device information do the same (i.e. > since real name to preferred name is 1:1, they could all do a > reverse lookup). > 2. have a writeable sysfs preferred_name field, either in the > generic device or just in SCSI. The preferred name would be > used by outbound only (i.e. kernel dev_printk messages and > possibly /proc/partitions). All inbound uses of the device > would come via the standard udev mechanisms > (i.e. /dev/disk/by-preferred would be the usual symlink). This > means from the kernel point of view, no renaming has happened. > We'd just try to print out the preferred name in certain > circumstances, which should solve most of the described problem. > > James > > > I have a question. Why is in-kernel device name necessary? The kernel can identify a device by major/miner number and udev can create a device node of a prefer name. Currently, device names are only used to show to users. Therefore I think that in-kernel device name is unnecessary if we introduce your /dev/disk/by-prefferd idea. thanks, -- Nao NISHIJIMA Software Platform Research Dept. Linux Technology Center Hitachi, Ltd., YOKOHAMA Research Laboratory Email: nao.nishijima.xt@hitachi.com ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel. @ 2011-04-12 13:23 ` Nao Nishijima 0 siblings, 0 replies; 46+ messages in thread From: Nao Nishijima @ 2011-04-12 13:23 UTC (permalink / raw) To: James Bottomley Cc: Hannes Reinecke, Greg KH, linux-kernel, linux-scsi, linux-hotplug, Kay Sievers, Jon Masters, 2nddept-manager Hi, James (2011/04/09 0:14), James Bottomley wrote: > On Fri, 2011-04-08 at 07:33 -0700, Hannes Reinecke wrote: >>> The problem I would like to discuss here is that users can not identify >>> a disk from kernel messages when they DIRECTLY refer to these messages. >>> For example, a device name is used instead of a symbolic link names in >>> bootup messages, I/O devices errors and /proc/partitions …etc. >>> >>> In particular, users can not identify an appropriate device from a >>> device name in syslog since different device name may be assigned to it >>> at each boot time. >>> >>> My idea is able to fix this issue with small changes in scsi subsystem. >>> Also, it is implemented as an option. Therefore, it does not affect >>> users who do not select this option. >>> >> We have been discussing this problem several times in the past, and >> indeed on these very mailing list. >> >> The conclusion we arrived at is that the kernel-provided device node >> name is inherently unstable and impossible to fix within the existing >> 'sdX' naming scheme. >> So the choices have been to either move to a totally different naming >> scheme or keep the naming scheme and provide the required information >> by other means. >> We have decided on the latter, and agreed on using udev to provide >> persistent device names. >> We are fully aware that any kernel related messages are subject to >> chance after reboot, but then most kernel related messages are >> (PID number, timestamps, login tty etc). >> And also we are aware that any kernel messages need to be matched >> against the current system layout to figure out any hardware-related >> issue. >> >> But then basically all products requiring to filter out information >> from kernel messages already do so I don't see a problem with that. >> >> Just adding an in-kernel identifier to the LUN will only be an >> incomplete solution, as other identifiers will still be volatile. >> >> So I would prefer by keeping the in-kernel information as small >> as possible to reduce memory consumption and rely on out-of-band >> programs to provide the required mapping. > > So, while I agree totally with the above: udev and userspace is the way > to go, I'm not totally opposed to having a non-invasive mechanism for > indicating a user's preferred name for a device. I think there are a > couple of ways to do this: > > 1. Entirely in userspace: just have udev consult a preferred name > file and create say /dev/disk/by-preferred. Then have all the > tools that normally output device information do the same (i.e. > since real name to preferred name is 1:1, they could all do a > reverse lookup). > 2. have a writeable sysfs preferred_name field, either in the > generic device or just in SCSI. The preferred name would be > used by outbound only (i.e. kernel dev_printk messages and > possibly /proc/partitions). All inbound uses of the device > would come via the standard udev mechanisms > (i.e. /dev/disk/by-preferred would be the usual symlink). This > means from the kernel point of view, no renaming has happened. > We'd just try to print out the preferred name in certain > circumstances, which should solve most of the described problem. > > James > > > I have a question. Why is in-kernel device name necessary? The kernel can identify a device by major/miner number and udev can create a device node of a prefer name. Currently, device names are only used to show to users. Therefore I think that in-kernel device name is unnecessary if we introduce your /dev/disk/by-prefferd idea. thanks, -- Nao NISHIJIMA Software Platform Research Dept. Linux Technology Center Hitachi, Ltd., YOKOHAMA Research Laboratory Email: nao.nishijima.xt@hitachi.com -- To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel. 2011-04-12 13:23 ` [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel Nao Nishijima (?) @ 2011-04-12 13:29 ` James Bottomley -1 siblings, 0 replies; 46+ messages in thread From: James Bottomley @ 2011-04-12 13:29 UTC (permalink / raw) To: Nao Nishijima Cc: Hannes Reinecke, Greg KH, linux-kernel, linux-scsi, linux-hotplug, Kay Sievers, Jon Masters, 2nddept-manager On Tue, 2011-04-12 at 22:23 +0900, Nao Nishijima wrote: > (2011/04/09 0:14), James Bottomley wrote: > > On Fri, 2011-04-08 at 07:33 -0700, Hannes Reinecke wrote: > >>> The problem I would like to discuss here is that users can not identify > >>> a disk from kernel messages when they DIRECTLY refer to these messages. > >>> For example, a device name is used instead of a symbolic link names in > >>> bootup messages, I/O devices errors and /proc/partitions …etc. > >>> > >>> In particular, users can not identify an appropriate device from a > >>> device name in syslog since different device name may be assigned to it > >>> at each boot time. > >>> > >>> My idea is able to fix this issue with small changes in scsi subsystem. > >>> Also, it is implemented as an option. Therefore, it does not affect > >>> users who do not select this option. > >>> > >> We have been discussing this problem several times in the past, and > >> indeed on these very mailing list. > >> > >> The conclusion we arrived at is that the kernel-provided device node > >> name is inherently unstable and impossible to fix within the existing > >> 'sdX' naming scheme. > >> So the choices have been to either move to a totally different naming > >> scheme or keep the naming scheme and provide the required information > >> by other means. > >> We have decided on the latter, and agreed on using udev to provide > >> persistent device names. > >> We are fully aware that any kernel related messages are subject to > >> chance after reboot, but then most kernel related messages are > >> (PID number, timestamps, login tty etc). > >> And also we are aware that any kernel messages need to be matched > >> against the current system layout to figure out any hardware-related > >> issue. > >> > >> But then basically all products requiring to filter out information > >> from kernel messages already do so I don't see a problem with that. > >> > >> Just adding an in-kernel identifier to the LUN will only be an > >> incomplete solution, as other identifiers will still be volatile. > >> > >> So I would prefer by keeping the in-kernel information as small > >> as possible to reduce memory consumption and rely on out-of-band > >> programs to provide the required mapping. > > > > So, while I agree totally with the above: udev and userspace is the way > > to go, I'm not totally opposed to having a non-invasive mechanism for > > indicating a user's preferred name for a device. I think there are a > > couple of ways to do this: > > > > 1. Entirely in userspace: just have udev consult a preferred name > > file and create say /dev/disk/by-preferred. Then have all the > > tools that normally output device information do the same (i.e. > > since real name to preferred name is 1:1, they could all do a > > reverse lookup). > > 2. have a writeable sysfs preferred_name field, either in the > > generic device or just in SCSI. The preferred name would be > > used by outbound only (i.e. kernel dev_printk messages and > > possibly /proc/partitions). All inbound uses of the device > > would come via the standard udev mechanisms > > (i.e. /dev/disk/by-preferred would be the usual symlink). This > > means from the kernel point of view, no renaming has happened. > > We'd just try to print out the preferred name in certain > > circumstances, which should solve most of the described problem. > > > > James > > > > > > > > I have a question. Why is in-kernel device name necessary? The kernel > can identify a device by major/miner number and udev can create a > device node of a prefer name. Well, that's the inbound vs outbound name I referred to above. If all you care about is inbound, this is true. > Currently, device names are only used to show to users. Therefore I > think that in-kernel device name is unnecessary if we introduce your > /dev/disk/by-prefferd idea. So if you have no problem with the kernel still printing out sdX in logs and it appearing in /proc/partitions, I'm happy with this. James ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 1/2] SCSI: Add a SCSI option for persistent device @ 2011-04-12 13:29 ` James Bottomley 0 siblings, 0 replies; 46+ messages in thread From: James Bottomley @ 2011-04-12 13:29 UTC (permalink / raw) To: Nao Nishijima Cc: Hannes Reinecke, Greg KH, linux-kernel, linux-scsi, linux-hotplug, Kay Sievers, Jon Masters, 2nddept-manager On Tue, 2011-04-12 at 22:23 +0900, Nao Nishijima wrote: > (2011/04/09 0:14), James Bottomley wrote: > > On Fri, 2011-04-08 at 07:33 -0700, Hannes Reinecke wrote: > >>> The problem I would like to discuss here is that users can not identify > >>> a disk from kernel messages when they DIRECTLY refer to these messages. > >>> For example, a device name is used instead of a symbolic link names in > >>> bootup messages, I/O devices errors and /proc/partitions …etc. > >>> > >>> In particular, users can not identify an appropriate device from a > >>> device name in syslog since different device name may be assigned to it > >>> at each boot time. > >>> > >>> My idea is able to fix this issue with small changes in scsi subsystem. > >>> Also, it is implemented as an option. Therefore, it does not affect > >>> users who do not select this option. > >>> > >> We have been discussing this problem several times in the past, and > >> indeed on these very mailing list. > >> > >> The conclusion we arrived at is that the kernel-provided device node > >> name is inherently unstable and impossible to fix within the existing > >> 'sdX' naming scheme. > >> So the choices have been to either move to a totally different naming > >> scheme or keep the naming scheme and provide the required information > >> by other means. > >> We have decided on the latter, and agreed on using udev to provide > >> persistent device names. > >> We are fully aware that any kernel related messages are subject to > >> chance after reboot, but then most kernel related messages are > >> (PID number, timestamps, login tty etc). > >> And also we are aware that any kernel messages need to be matched > >> against the current system layout to figure out any hardware-related > >> issue. > >> > >> But then basically all products requiring to filter out information > >> from kernel messages already do so I don't see a problem with that. > >> > >> Just adding an in-kernel identifier to the LUN will only be an > >> incomplete solution, as other identifiers will still be volatile. > >> > >> So I would prefer by keeping the in-kernel information as small > >> as possible to reduce memory consumption and rely on out-of-band > >> programs to provide the required mapping. > > > > So, while I agree totally with the above: udev and userspace is the way > > to go, I'm not totally opposed to having a non-invasive mechanism for > > indicating a user's preferred name for a device. I think there are a > > couple of ways to do this: > > > > 1. Entirely in userspace: just have udev consult a preferred name > > file and create say /dev/disk/by-preferred. Then have all the > > tools that normally output device information do the same (i.e. > > since real name to preferred name is 1:1, they could all do a > > reverse lookup). > > 2. have a writeable sysfs preferred_name field, either in the > > generic device or just in SCSI. The preferred name would be > > used by outbound only (i.e. kernel dev_printk messages and > > possibly /proc/partitions). All inbound uses of the device > > would come via the standard udev mechanisms > > (i.e. /dev/disk/by-preferred would be the usual symlink). This > > means from the kernel point of view, no renaming has happened. > > We'd just try to print out the preferred name in certain > > circumstances, which should solve most of the described problem. > > > > James > > > > > > > > I have a question. Why is in-kernel device name necessary? The kernel > can identify a device by major/miner number and udev can create a > device node of a prefer name. Well, that's the inbound vs outbound name I referred to above. If all you care about is inbound, this is true. > Currently, device names are only used to show to users. Therefore I > think that in-kernel device name is unnecessary if we introduce your > /dev/disk/by-prefferd idea. So if you have no problem with the kernel still printing out sdX in logs and it appearing in /proc/partitions, I'm happy with this. James ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel. @ 2011-04-12 13:29 ` James Bottomley 0 siblings, 0 replies; 46+ messages in thread From: James Bottomley @ 2011-04-12 13:29 UTC (permalink / raw) To: Nao Nishijima Cc: Hannes Reinecke, Greg KH, linux-kernel, linux-scsi, linux-hotplug, Kay Sievers, Jon Masters, 2nddept-manager On Tue, 2011-04-12 at 22:23 +0900, Nao Nishijima wrote: > (2011/04/09 0:14), James Bottomley wrote: > > On Fri, 2011-04-08 at 07:33 -0700, Hannes Reinecke wrote: > >>> The problem I would like to discuss here is that users can not identify > >>> a disk from kernel messages when they DIRECTLY refer to these messages. > >>> For example, a device name is used instead of a symbolic link names in > >>> bootup messages, I/O devices errors and /proc/partitions …etc. > >>> > >>> In particular, users can not identify an appropriate device from a > >>> device name in syslog since different device name may be assigned to it > >>> at each boot time. > >>> > >>> My idea is able to fix this issue with small changes in scsi subsystem. > >>> Also, it is implemented as an option. Therefore, it does not affect > >>> users who do not select this option. > >>> > >> We have been discussing this problem several times in the past, and > >> indeed on these very mailing list. > >> > >> The conclusion we arrived at is that the kernel-provided device node > >> name is inherently unstable and impossible to fix within the existing > >> 'sdX' naming scheme. > >> So the choices have been to either move to a totally different naming > >> scheme or keep the naming scheme and provide the required information > >> by other means. > >> We have decided on the latter, and agreed on using udev to provide > >> persistent device names. > >> We are fully aware that any kernel related messages are subject to > >> chance after reboot, but then most kernel related messages are > >> (PID number, timestamps, login tty etc). > >> And also we are aware that any kernel messages need to be matched > >> against the current system layout to figure out any hardware-related > >> issue. > >> > >> But then basically all products requiring to filter out information > >> from kernel messages already do so I don't see a problem with that. > >> > >> Just adding an in-kernel identifier to the LUN will only be an > >> incomplete solution, as other identifiers will still be volatile. > >> > >> So I would prefer by keeping the in-kernel information as small > >> as possible to reduce memory consumption and rely on out-of-band > >> programs to provide the required mapping. > > > > So, while I agree totally with the above: udev and userspace is the way > > to go, I'm not totally opposed to having a non-invasive mechanism for > > indicating a user's preferred name for a device. I think there are a > > couple of ways to do this: > > > > 1. Entirely in userspace: just have udev consult a preferred name > > file and create say /dev/disk/by-preferred. Then have all the > > tools that normally output device information do the same (i.e. > > since real name to preferred name is 1:1, they could all do a > > reverse lookup). > > 2. have a writeable sysfs preferred_name field, either in the > > generic device or just in SCSI. The preferred name would be > > used by outbound only (i.e. kernel dev_printk messages and > > possibly /proc/partitions). All inbound uses of the device > > would come via the standard udev mechanisms > > (i.e. /dev/disk/by-preferred would be the usual symlink). This > > means from the kernel point of view, no renaming has happened. > > We'd just try to print out the preferred name in certain > > circumstances, which should solve most of the described problem. > > > > James > > > > > > > > I have a question. Why is in-kernel device name necessary? The kernel > can identify a device by major/miner number and udev can create a > device node of a prefer name. Well, that's the inbound vs outbound name I referred to above. If all you care about is inbound, this is true. > Currently, device names are only used to show to users. Therefore I > think that in-kernel device name is unnecessary if we introduce your > /dev/disk/by-prefferd idea. So if you have no problem with the kernel still printing out sdX in logs and it appearing in /proc/partitions, I'm happy with this. James -- To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel. 2011-04-08 14:33 ` [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel Hannes Reinecke (?) @ 2011-04-14 2:06 ` Nao Nishijima -1 siblings, 0 replies; 46+ messages in thread From: Nao Nishijima @ 2011-04-14 2:06 UTC (permalink / raw) To: Hannes Reinecke Cc: Greg KH, linux-kernel, linux-scsi, linux-hotplug, Kay Sievers, James Bottomley, Jon Masters, 2nddept-manager Hi Hannes (2011/04/08 23:33), Hannes Reinecke wrote: > On 04/08/2011 07:12 AM, Nao Nishijima wrote: >> Hi, >> >> (2011/04/06 1:14), Greg KH wrote: >>> On Tue, Apr 05, 2011 at 09:49:46PM +0900, Nao Nishijima wrote: >>>> This patch series provides a SCSI option for persistent device >>>> names in kernel. With this option, user can always assign a >>>> same device name (e.g. sda) to a specific device according to >>>> udev rules and device id. >>>> >>>> Issue: >>>> Recently, kernel registers block devices in parallel. As a result, >>>> different device names will be assigned at each boot time. This >>>> will confuse file-system mounter, thus we usually use persistent >>>> symbolic links provided by udev. >>> >>> Yes, that is what you should use if you care about this. >>> >>>> However, dmesg and procfs outputs >>>> show device names instead of the symbolic link names. This causes >>>> a serious problem when managing multiple devices (e.g. on a >>>> large-scale storage), because usually, device errors are output >>>> with device names on dmesg. >>> >>> Then fix your tools that read the output of these files to point to the >>> proper persistent name instead of trying to get the kernel to solve the >>> problem. >>> >> >> Yes, the tools should be revised if users get a device name using them. >> >> The problem I would like to discuss here is that users can not identify >> a disk from kernel messages when they DIRECTLY refer to these messages. >> For example, a device name is used instead of a symbolic link names in >> bootup messages, I/O devices errors and /proc/partitions …etc. >> >> In particular, users can not identify an appropriate device from a >> device name in syslog since different device name may be assigned to it >> at each boot time. >> >> My idea is able to fix this issue with small changes in scsi subsystem. >> Also, it is implemented as an option. Therefore, it does not affect >> users who do not select this option. >> > We have been discussing this problem several times in the past, and > indeed on these very mailing list. > > The conclusion we arrived at is that the kernel-provided device node > name is inherently unstable and impossible to fix within the existing > 'sdX' naming scheme. > So the choices have been to either move to a totally different naming > scheme or keep the naming scheme and provide the required information > by other means. > We have decided on the latter, and agreed on using udev to provide > persistent device names. Could you tell me why you chose the latter? > We are fully aware that any kernel related messages are subject to > chance after reboot, but then most kernel related messages are > (PID number, timestamps, login tty etc). > And also we are aware that any kernel messages need to be matched > against the current system layout to figure out any hardware-related > issue. > > But then basically all products requiring to filter out information > from kernel messages already do so I don't see a problem with that. > All users did not always use the products. Users can see directly kernel messages (dmesg, /proc/partitions). Therefore I think that kernel messages need to provide the required mapping. > Just adding an in-kernel identifier to the LUN will only be an > incomplete solution, as other identifiers will still be volatile. > > So I would prefer by keeping the in-kernel information as small > as possible to reduce memory consumption and rely on out-of-band > programs to provide the required mapping. > > Cheers, > Thanks, -- Nao NISHIJIMA Software Platform Research Dept. Linux Technology Center Hitachi, Ltd., YOKOHAMA Research Laboratory Email: nao.nishijima.xt@hitachi.com ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 1/2] SCSI: Add a SCSI option for persistent device names @ 2011-04-14 2:06 ` Nao Nishijima 0 siblings, 0 replies; 46+ messages in thread From: Nao Nishijima @ 2011-04-14 2:06 UTC (permalink / raw) To: Hannes Reinecke Cc: Greg KH, linux-kernel, linux-scsi, linux-hotplug, Kay Sievers, James Bottomley, Jon Masters, 2nddept-manager Hi Hannes (2011/04/08 23:33), Hannes Reinecke wrote: > On 04/08/2011 07:12 AM, Nao Nishijima wrote: >> Hi, >> >> (2011/04/06 1:14), Greg KH wrote: >>> On Tue, Apr 05, 2011 at 09:49:46PM +0900, Nao Nishijima wrote: >>>> This patch series provides a SCSI option for persistent device >>>> names in kernel. With this option, user can always assign a >>>> same device name (e.g. sda) to a specific device according to >>>> udev rules and device id. >>>> >>>> Issue: >>>> Recently, kernel registers block devices in parallel. As a result, >>>> different device names will be assigned at each boot time. This >>>> will confuse file-system mounter, thus we usually use persistent >>>> symbolic links provided by udev. >>> >>> Yes, that is what you should use if you care about this. >>> >>>> However, dmesg and procfs outputs >>>> show device names instead of the symbolic link names. This causes >>>> a serious problem when managing multiple devices (e.g. on a >>>> large-scale storage), because usually, device errors are output >>>> with device names on dmesg. >>> >>> Then fix your tools that read the output of these files to point to the >>> proper persistent name instead of trying to get the kernel to solve the >>> problem. >>> >> >> Yes, the tools should be revised if users get a device name using them. >> >> The problem I would like to discuss here is that users can not identify >> a disk from kernel messages when they DIRECTLY refer to these messages. >> For example, a device name is used instead of a symbolic link names in >> bootup messages, I/O devices errors and /proc/partitions …etc. >> >> In particular, users can not identify an appropriate device from a >> device name in syslog since different device name may be assigned to it >> at each boot time. >> >> My idea is able to fix this issue with small changes in scsi subsystem. >> Also, it is implemented as an option. Therefore, it does not affect >> users who do not select this option. >> > We have been discussing this problem several times in the past, and > indeed on these very mailing list. > > The conclusion we arrived at is that the kernel-provided device node > name is inherently unstable and impossible to fix within the existing > 'sdX' naming scheme. > So the choices have been to either move to a totally different naming > scheme or keep the naming scheme and provide the required information > by other means. > We have decided on the latter, and agreed on using udev to provide > persistent device names. Could you tell me why you chose the latter? > We are fully aware that any kernel related messages are subject to > chance after reboot, but then most kernel related messages are > (PID number, timestamps, login tty etc). > And also we are aware that any kernel messages need to be matched > against the current system layout to figure out any hardware-related > issue. > > But then basically all products requiring to filter out information > from kernel messages already do so I don't see a problem with that. > All users did not always use the products. Users can see directly kernel messages (dmesg, /proc/partitions). Therefore I think that kernel messages need to provide the required mapping. > Just adding an in-kernel identifier to the LUN will only be an > incomplete solution, as other identifiers will still be volatile. > > So I would prefer by keeping the in-kernel information as small > as possible to reduce memory consumption and rely on out-of-band > programs to provide the required mapping. > > Cheers, > Thanks, -- Nao NISHIJIMA Software Platform Research Dept. Linux Technology Center Hitachi, Ltd., YOKOHAMA Research Laboratory Email: nao.nishijima.xt@hitachi.com ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel. @ 2011-04-14 2:06 ` Nao Nishijima 0 siblings, 0 replies; 46+ messages in thread From: Nao Nishijima @ 2011-04-14 2:06 UTC (permalink / raw) To: Hannes Reinecke Cc: Greg KH, linux-kernel, linux-scsi, linux-hotplug, Kay Sievers, James Bottomley, Jon Masters, 2nddept-manager Hi Hannes (2011/04/08 23:33), Hannes Reinecke wrote: > On 04/08/2011 07:12 AM, Nao Nishijima wrote: >> Hi, >> >> (2011/04/06 1:14), Greg KH wrote: >>> On Tue, Apr 05, 2011 at 09:49:46PM +0900, Nao Nishijima wrote: >>>> This patch series provides a SCSI option for persistent device >>>> names in kernel. With this option, user can always assign a >>>> same device name (e.g. sda) to a specific device according to >>>> udev rules and device id. >>>> >>>> Issue: >>>> Recently, kernel registers block devices in parallel. As a result, >>>> different device names will be assigned at each boot time. This >>>> will confuse file-system mounter, thus we usually use persistent >>>> symbolic links provided by udev. >>> >>> Yes, that is what you should use if you care about this. >>> >>>> However, dmesg and procfs outputs >>>> show device names instead of the symbolic link names. This causes >>>> a serious problem when managing multiple devices (e.g. on a >>>> large-scale storage), because usually, device errors are output >>>> with device names on dmesg. >>> >>> Then fix your tools that read the output of these files to point to the >>> proper persistent name instead of trying to get the kernel to solve the >>> problem. >>> >> >> Yes, the tools should be revised if users get a device name using them. >> >> The problem I would like to discuss here is that users can not identify >> a disk from kernel messages when they DIRECTLY refer to these messages. >> For example, a device name is used instead of a symbolic link names in >> bootup messages, I/O devices errors and /proc/partitions …etc. >> >> In particular, users can not identify an appropriate device from a >> device name in syslog since different device name may be assigned to it >> at each boot time. >> >> My idea is able to fix this issue with small changes in scsi subsystem. >> Also, it is implemented as an option. Therefore, it does not affect >> users who do not select this option. >> > We have been discussing this problem several times in the past, and > indeed on these very mailing list. > > The conclusion we arrived at is that the kernel-provided device node > name is inherently unstable and impossible to fix within the existing > 'sdX' naming scheme. > So the choices have been to either move to a totally different naming > scheme or keep the naming scheme and provide the required information > by other means. > We have decided on the latter, and agreed on using udev to provide > persistent device names. Could you tell me why you chose the latter? > We are fully aware that any kernel related messages are subject to > chance after reboot, but then most kernel related messages are > (PID number, timestamps, login tty etc). > And also we are aware that any kernel messages need to be matched > against the current system layout to figure out any hardware-related > issue. > > But then basically all products requiring to filter out information > from kernel messages already do so I don't see a problem with that. > All users did not always use the products. Users can see directly kernel messages (dmesg, /proc/partitions). Therefore I think that kernel messages need to provide the required mapping. > Just adding an in-kernel identifier to the LUN will only be an > incomplete solution, as other identifiers will still be volatile. > > So I would prefer by keeping the in-kernel information as small > as possible to reduce memory consumption and rely on out-of-band > programs to provide the required mapping. > > Cheers, > Thanks, -- Nao NISHIJIMA Software Platform Research Dept. Linux Technology Center Hitachi, Ltd., YOKOHAMA Research Laboratory Email: nao.nishijima.xt@hitachi.com -- To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel. 2011-04-14 2:06 ` [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel Nao Nishijima @ 2011-04-14 2:18 ` Greg KH -1 siblings, 0 replies; 46+ messages in thread From: Greg KH @ 2011-04-14 2:18 UTC (permalink / raw) To: Nao Nishijima Cc: Hannes Reinecke, linux-kernel, linux-scsi, linux-hotplug, Kay Sievers, James Bottomley, Jon Masters, 2nddept-manager On Thu, Apr 14, 2011 at 11:06:33AM +0900, Nao Nishijima wrote: > Hi Hannes > > (2011/04/08 23:33), Hannes Reinecke wrote: > > On 04/08/2011 07:12 AM, Nao Nishijima wrote: > >> Hi, > >> > >> (2011/04/06 1:14), Greg KH wrote: > >>> On Tue, Apr 05, 2011 at 09:49:46PM +0900, Nao Nishijima wrote: > >>>> This patch series provides a SCSI option for persistent device > >>>> names in kernel. With this option, user can always assign a > >>>> same device name (e.g. sda) to a specific device according to > >>>> udev rules and device id. > >>>> > >>>> Issue: > >>>> Recently, kernel registers block devices in parallel. As a result, > >>>> different device names will be assigned at each boot time. This > >>>> will confuse file-system mounter, thus we usually use persistent > >>>> symbolic links provided by udev. > >>> > >>> Yes, that is what you should use if you care about this. > >>> > >>>> However, dmesg and procfs outputs > >>>> show device names instead of the symbolic link names. This causes > >>>> a serious problem when managing multiple devices (e.g. on a > >>>> large-scale storage), because usually, device errors are output > >>>> with device names on dmesg. > >>> > >>> Then fix your tools that read the output of these files to point to the > >>> proper persistent name instead of trying to get the kernel to solve the > >>> problem. > >>> > >> > >> Yes, the tools should be revised if users get a device name using them. > >> > >> The problem I would like to discuss here is that users can not identify > >> a disk from kernel messages when they DIRECTLY refer to these messages. > >> For example, a device name is used instead of a symbolic link names in > >> bootup messages, I/O devices errors and /proc/partitions …etc. > >> > >> In particular, users can not identify an appropriate device from a > >> device name in syslog since different device name may be assigned to it > >> at each boot time. > >> > >> My idea is able to fix this issue with small changes in scsi subsystem. > >> Also, it is implemented as an option. Therefore, it does not affect > >> users who do not select this option. > >> > > We have been discussing this problem several times in the past, and > > indeed on these very mailing list. > > > > The conclusion we arrived at is that the kernel-provided device node > > name is inherently unstable and impossible to fix within the existing > > 'sdX' naming scheme. > > So the choices have been to either move to a totally different naming > > scheme or keep the naming scheme and provide the required information > > by other means. > > We have decided on the latter, and agreed on using udev to provide > > persistent device names. > > Could you tell me why you chose the latter? > > > > We are fully aware that any kernel related messages are subject to > > chance after reboot, but then most kernel related messages are > > (PID number, timestamps, login tty etc). > > And also we are aware that any kernel messages need to be matched > > against the current system layout to figure out any hardware-related > > issue. > > > > But then basically all products requiring to filter out information > > from kernel messages already do so I don't see a problem with that. > > > > All users did not always use the products. Users can see directly > kernel messages (dmesg, /proc/partitions). Therefore I think that > kernel messages need to provide the required mapping. No they don't. Anyone who wants to look at those files "knows" what they are doing and the kernel name is fine to use. thanks, greg k-h ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 1/2] SCSI: Add a SCSI option for persistent device names @ 2011-04-14 2:18 ` Greg KH 0 siblings, 0 replies; 46+ messages in thread From: Greg KH @ 2011-04-14 2:18 UTC (permalink / raw) To: Nao Nishijima Cc: Hannes Reinecke, linux-kernel, linux-scsi, linux-hotplug, Kay Sievers, James Bottomley, Jon Masters, 2nddept-manager On Thu, Apr 14, 2011 at 11:06:33AM +0900, Nao Nishijima wrote: > Hi Hannes > > (2011/04/08 23:33), Hannes Reinecke wrote: > > On 04/08/2011 07:12 AM, Nao Nishijima wrote: > >> Hi, > >> > >> (2011/04/06 1:14), Greg KH wrote: > >>> On Tue, Apr 05, 2011 at 09:49:46PM +0900, Nao Nishijima wrote: > >>>> This patch series provides a SCSI option for persistent device > >>>> names in kernel. With this option, user can always assign a > >>>> same device name (e.g. sda) to a specific device according to > >>>> udev rules and device id. > >>>> > >>>> Issue: > >>>> Recently, kernel registers block devices in parallel. As a result, > >>>> different device names will be assigned at each boot time. This > >>>> will confuse file-system mounter, thus we usually use persistent > >>>> symbolic links provided by udev. > >>> > >>> Yes, that is what you should use if you care about this. > >>> > >>>> However, dmesg and procfs outputs > >>>> show device names instead of the symbolic link names. This causes > >>>> a serious problem when managing multiple devices (e.g. on a > >>>> large-scale storage), because usually, device errors are output > >>>> with device names on dmesg. > >>> > >>> Then fix your tools that read the output of these files to point to the > >>> proper persistent name instead of trying to get the kernel to solve the > >>> problem. > >>> > >> > >> Yes, the tools should be revised if users get a device name using them. > >> > >> The problem I would like to discuss here is that users can not identify > >> a disk from kernel messages when they DIRECTLY refer to these messages. > >> For example, a device name is used instead of a symbolic link names in > >> bootup messages, I/O devices errors and /proc/partitions …etc. > >> > >> In particular, users can not identify an appropriate device from a > >> device name in syslog since different device name may be assigned to it > >> at each boot time. > >> > >> My idea is able to fix this issue with small changes in scsi subsystem. > >> Also, it is implemented as an option. Therefore, it does not affect > >> users who do not select this option. > >> > > We have been discussing this problem several times in the past, and > > indeed on these very mailing list. > > > > The conclusion we arrived at is that the kernel-provided device node > > name is inherently unstable and impossible to fix within the existing > > 'sdX' naming scheme. > > So the choices have been to either move to a totally different naming > > scheme or keep the naming scheme and provide the required information > > by other means. > > We have decided on the latter, and agreed on using udev to provide > > persistent device names. > > Could you tell me why you chose the latter? > > > > We are fully aware that any kernel related messages are subject to > > chance after reboot, but then most kernel related messages are > > (PID number, timestamps, login tty etc). > > And also we are aware that any kernel messages need to be matched > > against the current system layout to figure out any hardware-related > > issue. > > > > But then basically all products requiring to filter out information > > from kernel messages already do so I don't see a problem with that. > > > > All users did not always use the products. Users can see directly > kernel messages (dmesg, /proc/partitions). Therefore I think that > kernel messages need to provide the required mapping. No they don't. Anyone who wants to look at those files "knows" what they are doing and the kernel name is fine to use. thanks, greg k-h ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel. 2011-04-08 14:12 ` [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel Nao Nishijima @ 2011-04-08 17:21 ` Stefan Richter -1 siblings, 0 replies; 46+ messages in thread From: Stefan Richter @ 2011-04-08 17:21 UTC (permalink / raw) To: Nao Nishijima Cc: Greg KH, linux-kernel, linux-scsi, linux-hotplug, Kay Sievers, James Bottomley, Jon Masters, 2nddept-manager On Apr 08 Nao Nishijima wrote: > (2011/04/06 1:14), Greg KH wrote: > > On Tue, Apr 05, 2011 at 09:49:46PM +0900, Nao Nishijima wrote: > >> Todo: > >> - usb support > > > > Why? USB uses scsi, so why would it not work as-is today? What about > > libata devices? > > > > Sorry, my explanation was not sufficient. > It is required to get scsi id using a scsi inguiry command in order to > create the udev rule, especially in ATTR(disk_id) item in it. The USB > devices, however, do not respond to the command and we can not get their > scsi id. Identifiers and names of target ports and of logical units (per SAM-2 and later, Annex A) --- whether they are persistent, in which scope they are unique, how they look like, and how they can be obtained --- are transport specific. Neither the INQUIRY command (per SPC) nor any other command is generally useful to obtain e.g. logical unit identifiers. Any tool that needs to obtain identifiers or names of target ports and of logical units must talk with the transport driver through driver-specific interfaces. Years ago it was suggested on this list that scsi-mod exposes standard sysfs attributes for target and LU identifiers and names for all devices, so that tools that need identfiers or names have a single place where they can look them up. (The read() method of these attributes would have to be provided kernel-internally by transport drivers; and tools that read these attributes need to be aware that there is not a single format for identifiers and names.) The SCSI folks were not interested in such a more standardized sysfs ABI at that time. (There was only a proposal anyway, not a patch.) Hence, tools have to dig at various places for these SAM-2 artifacts. These ABIs have been defined ad hoc by the transport driver implementers. Anyway. /How/ to obtain identifiers is just a side issue. Back to the proposal of letting userland tell the kernel how to name devices: > >> I create unnamed devices (not > >> a block device, but an intermediate device. [...] > >> After udev finds the unnamed > >> device, udev assigns a persistent device name to a specific device > >> according to udev rules and registers it as a block device. Hence, > >> this is just a naming, not a renaming. I disagree to this conclusion. The "unnamed" device most definitely will have a name before it can be shown to userland. This preliminary name fulfills the very same requirement that "s[drt][a-z]+[0-9]*" fulfill today: It is unique within the system during the lifetime of the device. So, this /is/ renaming, only that the first name is only exposed to special tools that know of this new ABI. It seems to me that today's procedure of /not/ renaming devices but of providing as many alternative additional names as anyone could ever need (symlinks within /dev/) is more robust. If the contents of /var/log/messages or the output of iostat etc. is not what is needed, how about simply filtering that through a grep/ sed based script that rewrites names? (This needs to be run during the lifetime of the device of course, otherwise a wrong name could be put in.) The fine symlinks that udev provides can be used in such a text filter. -- Stefan Richter -=====-==-== -=-- -=--- http://arcgraph.de/sr/ ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 1/2] SCSI: Add a SCSI option for persistent device names @ 2011-04-08 17:21 ` Stefan Richter 0 siblings, 0 replies; 46+ messages in thread From: Stefan Richter @ 2011-04-08 17:21 UTC (permalink / raw) To: Nao Nishijima Cc: Greg KH, linux-kernel, linux-scsi, linux-hotplug, Kay Sievers, James Bottomley, Jon Masters, 2nddept-manager On Apr 08 Nao Nishijima wrote: > (2011/04/06 1:14), Greg KH wrote: > > On Tue, Apr 05, 2011 at 09:49:46PM +0900, Nao Nishijima wrote: > >> Todo: > >> - usb support > > > > Why? USB uses scsi, so why would it not work as-is today? What about > > libata devices? > > > > Sorry, my explanation was not sufficient. > It is required to get scsi id using a scsi inguiry command in order to > create the udev rule, especially in ATTR(disk_id) item in it. The USB > devices, however, do not respond to the command and we can not get their > scsi id. Identifiers and names of target ports and of logical units (per SAM-2 and later, Annex A) --- whether they are persistent, in which scope they are unique, how they look like, and how they can be obtained --- are transport specific. Neither the INQUIRY command (per SPC) nor any other command is generally useful to obtain e.g. logical unit identifiers. Any tool that needs to obtain identifiers or names of target ports and of logical units must talk with the transport driver through driver-specific interfaces. Years ago it was suggested on this list that scsi-mod exposes standard sysfs attributes for target and LU identifiers and names for all devices, so that tools that need identfiers or names have a single place where they can look them up. (The read() method of these attributes would have to be provided kernel-internally by transport drivers; and tools that read these attributes need to be aware that there is not a single format for identifiers and names.) The SCSI folks were not interested in such a more standardized sysfs ABI at that time. (There was only a proposal anyway, not a patch.) Hence, tools have to dig at various places for these SAM-2 artifacts. These ABIs have been defined ad hoc by the transport driver implementers. Anyway. /How/ to obtain identifiers is just a side issue. Back to the proposal of letting userland tell the kernel how to name devices: > >> I create unnamed devices (not > >> a block device, but an intermediate device. [...] > >> After udev finds the unnamed > >> device, udev assigns a persistent device name to a specific device > >> according to udev rules and registers it as a block device. Hence, > >> this is just a naming, not a renaming. I disagree to this conclusion. The "unnamed" device most definitely will have a name before it can be shown to userland. This preliminary name fulfills the very same requirement that "s[drt][a-z]+[0-9]*" fulfill today: It is unique within the system during the lifetime of the device. So, this /is/ renaming, only that the first name is only exposed to special tools that know of this new ABI. It seems to me that today's procedure of /not/ renaming devices but of providing as many alternative additional names as anyone could ever need (symlinks within /dev/) is more robust. If the contents of /var/log/messages or the output of iostat etc. is not what is needed, how about simply filtering that through a grep/ sed based script that rewrites names? (This needs to be run during the lifetime of the device of course, otherwise a wrong name could be put in.) The fine symlinks that udev provides can be used in such a text filter. -- Stefan Richter -===-=-= -=-- -=--- http://arcgraph.de/sr/ ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel. 2011-04-08 17:21 ` [PATCH 1/2] SCSI: Add a SCSI option for persistent device names Stefan Richter (?) @ 2011-04-18 20:10 ` Jeremy Linton -1 siblings, 0 replies; 46+ messages in thread From: Jeremy Linton @ 2011-04-18 20:10 UTC (permalink / raw) To: Stefan Richter; +Cc: Nao Nishijima, linux-scsi > Identifiers and names of target ports and of logical units (per SAM-2 and > later, Annex A) --- whether they are persistent, in which scope they are > unique, how they look like, and how they can be obtained --- are transport > specific. > > Neither the INQUIRY command (per SPC) nor any other command is generally > useful to obtain e.g. logical unit identifiers. Any tool that needs to > obtain identifiers or names of target ports and of logical units must talk > with the transport driver through driver-specific interfaces. Inquiry page 0x83 has been mandatory for a while now, and while the name/id assignments are all over the place, it is getting better. Page 0x80 is also supported on nearly every device manufactured in the last 10 years. Parsing a few common cases covers the majority of the devices. Of course, the returned information may not be helpful (even when supported), but we are mostly just talking about messages being displayed to the user. Some of this information is already being stored in the kernel, printing something like "s[dgt]X (man/model/serial/portinfo)" instead of just a "sda" via sd_printk/etc would probably be quite helpful in a lot of situations. Finally, printing this information provides encouragement to the device manufactures to make sure its available and correct. ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel. 2011-04-05 12:49 ` [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Nao Nishijima @ 2011-04-05 16:14 ` Greg KH -1 siblings, 0 replies; 46+ messages in thread From: Greg KH @ 2011-04-05 16:14 UTC (permalink / raw) To: Nao Nishijima Cc: linux-kernel, linux-scsi, linux-hotplug, Kay Sievers, James Bottomley, Jon Masters, 2nddept-manager On Tue, Apr 05, 2011 at 09:49:46PM +0900, Nao Nishijima wrote: > This patch series provides a SCSI option for persistent device > names in kernel. With this option, user can always assign a > same device name (e.g. sda) to a specific device according to > udev rules and device id. > > Issue: > Recently, kernel registers block devices in parallel. As a result, > different device names will be assigned at each boot time. This > will confuse file-system mounter, thus we usually use persistent > symbolic links provided by udev. However, dmesg and procfs outputs > show device names instead of the symbolic link names. This causes > a serious problem when managing multiple devices (e.g. on a > large-scale storage), because usually, device errors are output > with device names on dmesg. We also concern about some commands > which output device names, as well as kernel messages. > > Solution: > To assign a persistent device name, I create unnamed devices (not > a block device, but an intermediate device. users cannot read/write > this device). When scsi device driver finds a LU, the LU is registered > as an unnamed device and inform to udev. After udev finds the unnamed > device, udev assigns a persistent device name to a specific device > according to udev rules and registers it as a block device. Hence, > this is just a naming, not a renaming. > > Some users are satisfied with current udev solution. Therefore, my > proposal is implemented as an option. > > If using this option, add the following kernel parameter. > > scsi_mod.persistent_name=1 > > Also, if you want to assign a device name with sda, add the > following udev rules. > > SUBSYSTEM=="scsi_unnamed", ATTR{disk_id}=="xxx", PROGRAM="/bin/sh > -c 'echo -n sda > /sys/%p/disk_name'" Also, where is the "real" program you have created to properly name devices from userspace? You need that to properly test this patch, right? thanks, greg k-h ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 1/2] SCSI: Add a SCSI option for persistent device names @ 2011-04-05 16:14 ` Greg KH 0 siblings, 0 replies; 46+ messages in thread From: Greg KH @ 2011-04-05 16:14 UTC (permalink / raw) To: Nao Nishijima Cc: linux-kernel, linux-scsi, linux-hotplug, Kay Sievers, James Bottomley, Jon Masters, 2nddept-manager On Tue, Apr 05, 2011 at 09:49:46PM +0900, Nao Nishijima wrote: > This patch series provides a SCSI option for persistent device > names in kernel. With this option, user can always assign a > same device name (e.g. sda) to a specific device according to > udev rules and device id. > > Issue: > Recently, kernel registers block devices in parallel. As a result, > different device names will be assigned at each boot time. This > will confuse file-system mounter, thus we usually use persistent > symbolic links provided by udev. However, dmesg and procfs outputs > show device names instead of the symbolic link names. This causes > a serious problem when managing multiple devices (e.g. on a > large-scale storage), because usually, device errors are output > with device names on dmesg. We also concern about some commands > which output device names, as well as kernel messages. > > Solution: > To assign a persistent device name, I create unnamed devices (not > a block device, but an intermediate device. users cannot read/write > this device). When scsi device driver finds a LU, the LU is registered > as an unnamed device and inform to udev. After udev finds the unnamed > device, udev assigns a persistent device name to a specific device > according to udev rules and registers it as a block device. Hence, > this is just a naming, not a renaming. > > Some users are satisfied with current udev solution. Therefore, my > proposal is implemented as an option. > > If using this option, add the following kernel parameter. > > scsi_mod.persistent_name=1 > > Also, if you want to assign a device name with sda, add the > following udev rules. > > SUBSYSTEM="scsi_unnamed", ATTR{disk_id}="xxx", PROGRAM="/bin/sh > -c 'echo -n sda > /sys/%p/disk_name'" Also, where is the "real" program you have created to properly name devices from userspace? You need that to properly test this patch, right? thanks, greg k-h ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel. 2011-04-05 16:14 ` [PATCH 1/2] SCSI: Add a SCSI option for persistent device names Greg KH @ 2011-04-08 14:07 ` Nao Nishijima -1 siblings, 0 replies; 46+ messages in thread From: Nao Nishijima @ 2011-04-08 14:07 UTC (permalink / raw) To: Greg KH Cc: linux-kernel, linux-scsi, linux-hotplug, Kay Sievers, James Bottomley, Jon Masters, 2nddept-manager Hi, (2011/04/06 1:14), Greg KH wrote: > On Tue, Apr 05, 2011 at 09:49:46PM +0900, Nao Nishijima wrote: >> This patch series provides a SCSI option for persistent device >> names in kernel. With this option, user can always assign a >> same device name (e.g. sda) to a specific device according to >> udev rules and device id. >> >> Issue: >> Recently, kernel registers block devices in parallel. As a result, >> different device names will be assigned at each boot time. This >> will confuse file-system mounter, thus we usually use persistent >> symbolic links provided by udev. However, dmesg and procfs outputs >> show device names instead of the symbolic link names. This causes >> a serious problem when managing multiple devices (e.g. on a >> large-scale storage), because usually, device errors are output >> with device names on dmesg. We also concern about some commands >> which output device names, as well as kernel messages. >> >> Solution: >> To assign a persistent device name, I create unnamed devices (not >> a block device, but an intermediate device. users cannot read/write >> this device). When scsi device driver finds a LU, the LU is registered >> as an unnamed device and inform to udev. After udev finds the unnamed >> device, udev assigns a persistent device name to a specific device >> according to udev rules and registers it as a block device. Hence, >> this is just a naming, not a renaming. >> >> Some users are satisfied with current udev solution. Therefore, my >> proposal is implemented as an option. >> >> If using this option, add the following kernel parameter. >> >> scsi_mod.persistent_name=1 >> >> Also, if you want to assign a device name with sda, add the >> following udev rules. >> >> SUBSYSTEM=="scsi_unnamed", ATTR{disk_id}=="xxx", PROGRAM="/bin/sh >> -c 'echo -n sda > /sys/%p/disk_name'" > > Also, where is the "real" program you have created to properly name > devices from userspace? You need that to properly test this patch, > right? > In the udev rule described above, notation “xxx” indicated by ATTR(disk_id) is scsi id given by disk. Then, when udev finds this rule, "/bin/sh -c 'echo -n sda> /sys/%p/disk_name'" indicated by PROGRAM is operated using xxx (scsi id) if udev find the disk with scic id xxx. Thus, persistent device name is assigned. I have tested this patch using the udev rule. and It works well. > thanks, > > greg k-h > Thanks, -- Nao NISHIJIMA Software Platform Research Dept. Linux Technology Center Hitachi, Ltd., YOKOHAMA Research Laboratory Email: nao.nishijima.xt@hitachi.com ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 1/2] SCSI: Add a SCSI option for persistent device names @ 2011-04-08 14:07 ` Nao Nishijima 0 siblings, 0 replies; 46+ messages in thread From: Nao Nishijima @ 2011-04-08 14:07 UTC (permalink / raw) To: Greg KH Cc: linux-kernel, linux-scsi, linux-hotplug, Kay Sievers, James Bottomley, Jon Masters, 2nddept-manager Hi, (2011/04/06 1:14), Greg KH wrote: > On Tue, Apr 05, 2011 at 09:49:46PM +0900, Nao Nishijima wrote: >> This patch series provides a SCSI option for persistent device >> names in kernel. With this option, user can always assign a >> same device name (e.g. sda) to a specific device according to >> udev rules and device id. >> >> Issue: >> Recently, kernel registers block devices in parallel. As a result, >> different device names will be assigned at each boot time. This >> will confuse file-system mounter, thus we usually use persistent >> symbolic links provided by udev. However, dmesg and procfs outputs >> show device names instead of the symbolic link names. This causes >> a serious problem when managing multiple devices (e.g. on a >> large-scale storage), because usually, device errors are output >> with device names on dmesg. We also concern about some commands >> which output device names, as well as kernel messages. >> >> Solution: >> To assign a persistent device name, I create unnamed devices (not >> a block device, but an intermediate device. users cannot read/write >> this device). When scsi device driver finds a LU, the LU is registered >> as an unnamed device and inform to udev. After udev finds the unnamed >> device, udev assigns a persistent device name to a specific device >> according to udev rules and registers it as a block device. Hence, >> this is just a naming, not a renaming. >> >> Some users are satisfied with current udev solution. Therefore, my >> proposal is implemented as an option. >> >> If using this option, add the following kernel parameter. >> >> scsi_mod.persistent_name=1 >> >> Also, if you want to assign a device name with sda, add the >> following udev rules. >> >> SUBSYSTEM="scsi_unnamed", ATTR{disk_id}="xxx", PROGRAM="/bin/sh >> -c 'echo -n sda > /sys/%p/disk_name'" > > Also, where is the "real" program you have created to properly name > devices from userspace? You need that to properly test this patch, > right? > In the udev rule described above, notation “xxx” indicated by ATTR(disk_id) is scsi id given by disk. Then, when udev finds this rule, "/bin/sh -c 'echo -n sda> /sys/%p/disk_name'" indicated by PROGRAM is operated using xxx (scsi id) if udev find the disk with scic id xxx. Thus, persistent device name is assigned. I have tested this patch using the udev rule. and It works well. > thanks, > > greg k-h > Thanks, -- Nao NISHIJIMA Software Platform Research Dept. Linux Technology Center Hitachi, Ltd., YOKOHAMA Research Laboratory Email: nao.nishijima.xt@hitachi.com ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel. 2011-04-08 14:07 ` [PATCH 1/2] SCSI: Add a SCSI option for persistent device names Nao Nishijima @ 2011-04-08 16:12 ` Greg KH -1 siblings, 0 replies; 46+ messages in thread From: Greg KH @ 2011-04-08 16:12 UTC (permalink / raw) To: Nao Nishijima Cc: linux-kernel, linux-scsi, linux-hotplug, Kay Sievers, James Bottomley, Jon Masters, 2nddept-manager On Fri, Apr 08, 2011 at 11:07:41PM +0900, Nao Nishijima wrote: > Hi, > > (2011/04/06 1:14), Greg KH wrote: > > On Tue, Apr 05, 2011 at 09:49:46PM +0900, Nao Nishijima wrote: > >> This patch series provides a SCSI option for persistent device > >> names in kernel. With this option, user can always assign a > >> same device name (e.g. sda) to a specific device according to > >> udev rules and device id. > >> > >> Issue: > >> Recently, kernel registers block devices in parallel. As a result, > >> different device names will be assigned at each boot time. This > >> will confuse file-system mounter, thus we usually use persistent > >> symbolic links provided by udev. However, dmesg and procfs outputs > >> show device names instead of the symbolic link names. This causes > >> a serious problem when managing multiple devices (e.g. on a > >> large-scale storage), because usually, device errors are output > >> with device names on dmesg. We also concern about some commands > >> which output device names, as well as kernel messages. > >> > >> Solution: > >> To assign a persistent device name, I create unnamed devices (not > >> a block device, but an intermediate device. users cannot read/write > >> this device). When scsi device driver finds a LU, the LU is registered > >> as an unnamed device and inform to udev. After udev finds the unnamed > >> device, udev assigns a persistent device name to a specific device > >> according to udev rules and registers it as a block device. Hence, > >> this is just a naming, not a renaming. > >> > >> Some users are satisfied with current udev solution. Therefore, my > >> proposal is implemented as an option. > >> > >> If using this option, add the following kernel parameter. > >> > >> scsi_mod.persistent_name=1 > >> > >> Also, if you want to assign a device name with sda, add the > >> following udev rules. > >> > >> SUBSYSTEM=="scsi_unnamed", ATTR{disk_id}=="xxx", PROGRAM="/bin/sh > >> -c 'echo -n sda > /sys/%p/disk_name'" > > > > Also, where is the "real" program you have created to properly name > > devices from userspace? You need that to properly test this patch, > > right? > > > > In the udev rule described above, notation “xxx” indicated by > ATTR(disk_id) is scsi id given by disk. Then, when udev finds this rule, > "/bin/sh -c 'echo -n sda> /sys/%p/disk_name'" indicated by PROGRAM is > operated using xxx (scsi id) if udev find the disk with scic id xxx. > Thus, persistent device name is assigned. > > I have tested this patch using the udev rule. and It works well. That's nice, but it's a "toy". What have you used to test this with 20000 scsi devices to properly work like it does today? Where is the program that will name them in a deterministic manner? We have that functionality today, you can't break it. thanks, greg k-h ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 1/2] SCSI: Add a SCSI option for persistent device names @ 2011-04-08 16:12 ` Greg KH 0 siblings, 0 replies; 46+ messages in thread From: Greg KH @ 2011-04-08 16:12 UTC (permalink / raw) To: Nao Nishijima Cc: linux-kernel, linux-scsi, linux-hotplug, Kay Sievers, James Bottomley, Jon Masters, 2nddept-manager On Fri, Apr 08, 2011 at 11:07:41PM +0900, Nao Nishijima wrote: > Hi, > > (2011/04/06 1:14), Greg KH wrote: > > On Tue, Apr 05, 2011 at 09:49:46PM +0900, Nao Nishijima wrote: > >> This patch series provides a SCSI option for persistent device > >> names in kernel. With this option, user can always assign a > >> same device name (e.g. sda) to a specific device according to > >> udev rules and device id. > >> > >> Issue: > >> Recently, kernel registers block devices in parallel. As a result, > >> different device names will be assigned at each boot time. This > >> will confuse file-system mounter, thus we usually use persistent > >> symbolic links provided by udev. However, dmesg and procfs outputs > >> show device names instead of the symbolic link names. This causes > >> a serious problem when managing multiple devices (e.g. on a > >> large-scale storage), because usually, device errors are output > >> with device names on dmesg. We also concern about some commands > >> which output device names, as well as kernel messages. > >> > >> Solution: > >> To assign a persistent device name, I create unnamed devices (not > >> a block device, but an intermediate device. users cannot read/write > >> this device). When scsi device driver finds a LU, the LU is registered > >> as an unnamed device and inform to udev. After udev finds the unnamed > >> device, udev assigns a persistent device name to a specific device > >> according to udev rules and registers it as a block device. Hence, > >> this is just a naming, not a renaming. > >> > >> Some users are satisfied with current udev solution. Therefore, my > >> proposal is implemented as an option. > >> > >> If using this option, add the following kernel parameter. > >> > >> scsi_mod.persistent_name=1 > >> > >> Also, if you want to assign a device name with sda, add the > >> following udev rules. > >> > >> SUBSYSTEM="scsi_unnamed", ATTR{disk_id}="xxx", PROGRAM="/bin/sh > >> -c 'echo -n sda > /sys/%p/disk_name'" > > > > Also, where is the "real" program you have created to properly name > > devices from userspace? You need that to properly test this patch, > > right? > > > > In the udev rule described above, notation “xxx” indicated by > ATTR(disk_id) is scsi id given by disk. Then, when udev finds this rule, > "/bin/sh -c 'echo -n sda> /sys/%p/disk_name'" indicated by PROGRAM is > operated using xxx (scsi id) if udev find the disk with scic id xxx. > Thus, persistent device name is assigned. > > I have tested this patch using the udev rule. and It works well. That's nice, but it's a "toy". What have you used to test this with 20000 scsi devices to properly work like it does today? Where is the program that will name them in a deterministic manner? We have that functionality today, you can't break it. thanks, greg k-h ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel. 2011-04-08 16:12 ` [PATCH 1/2] SCSI: Add a SCSI option for persistent device names Greg KH (?) @ 2011-04-14 8:15 ` Nao Nishijima -1 siblings, 0 replies; 46+ messages in thread From: Nao Nishijima @ 2011-04-14 8:15 UTC (permalink / raw) To: Greg KH Cc: linux-kernel, linux-scsi, linux-hotplug, Kay Sievers, James Bottomley, Jon Masters, 2nddept-manager Hi, (2011/04/09 1:12), Greg KH wrote: > On Fri, Apr 08, 2011 at 11:07:41PM +0900, Nao Nishijima wrote: >> Hi, >> >> (2011/04/06 1:14), Greg KH wrote: >>> On Tue, Apr 05, 2011 at 09:49:46PM +0900, Nao Nishijima wrote: >>>> This patch series provides a SCSI option for persistent device >>>> names in kernel. With this option, user can always assign a >>>> same device name (e.g. sda) to a specific device according to >>>> udev rules and device id. >>>> >>>> Issue: >>>> Recently, kernel registers block devices in parallel. As a result, >>>> different device names will be assigned at each boot time. This >>>> will confuse file-system mounter, thus we usually use persistent >>>> symbolic links provided by udev. However, dmesg and procfs outputs >>>> show device names instead of the symbolic link names. This causes >>>> a serious problem when managing multiple devices (e.g. on a >>>> large-scale storage), because usually, device errors are output >>>> with device names on dmesg. We also concern about some commands >>>> which output device names, as well as kernel messages. >>>> >>>> Solution: >>>> To assign a persistent device name, I create unnamed devices (not >>>> a block device, but an intermediate device. users cannot read/write >>>> this device). When scsi device driver finds a LU, the LU is registered >>>> as an unnamed device and inform to udev. After udev finds the unnamed >>>> device, udev assigns a persistent device name to a specific device >>>> according to udev rules and registers it as a block device. Hence, >>>> this is just a naming, not a renaming. >>>> >>>> Some users are satisfied with current udev solution. Therefore, my >>>> proposal is implemented as an option. >>>> >>>> If using this option, add the following kernel parameter. >>>> >>>> scsi_mod.persistent_name=1 >>>> >>>> Also, if you want to assign a device name with sda, add the >>>> following udev rules. >>>> >>>> SUBSYSTEM=="scsi_unnamed", ATTR{disk_id}=="xxx", PROGRAM="/bin/sh >>>> -c 'echo -n sda > /sys/%p/disk_name'" >>> >>> Also, where is the "real" program you have created to properly name >>> devices from userspace? You need that to properly test this patch, >>> right? >>> >> >> In the udev rule described above, notation “xxx” indicated by >> ATTR(disk_id) is scsi id given by disk. Then, when udev finds this rule, >> "/bin/sh -c 'echo -n sda> /sys/%p/disk_name'" indicated by PROGRAM is >> operated using xxx (scsi id) if udev find the disk with scic id xxx. >> Thus, persistent device name is assigned. >> >> I have tested this patch using the udev rule. and It works well. > > That's nice, but it's a "toy". What have you used to test this with > 20000 scsi devices to properly work like it does today? Where is the > program that will name them in a deterministic manner? > > We have that functionality today, you can't break it. > > thanks, Indeed, I have not (can not, of course) tested 20000 scsi devices to properly work. This feature targets only scsi disk/cdrom/tape devices. Not all devices. We expect the target users are system administrators who need to control all devices for maintenance. They know all devices connected to the server and those devices will tested before connecting. We expect that admin will enable this option after installation. This means that device names are assigned by current mechanism when installation. Admin enables the option and makes udev rules (we are planning to make shell script which generate udev rules use by-id and assigned device names). In other word, this feature will be used just for "fixing" the names. In our scenario, admin appends a new rule manually when a new LU is added. In the future, we are planning to enhance udev to append a new rule automatically as like as NIC. Thanks, -- Nao NISHIJIMA Software Platform Research Dept. Linux Technology Center Hitachi, Ltd., YOKOHAMA Research Laboratory Email: nao.nishijima.xt@hitachi.com ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 1/2] SCSI: Add a SCSI option for persistent device names @ 2011-04-14 8:15 ` Nao Nishijima 0 siblings, 0 replies; 46+ messages in thread From: Nao Nishijima @ 2011-04-14 8:15 UTC (permalink / raw) To: Greg KH Cc: linux-kernel, linux-scsi, linux-hotplug, Kay Sievers, James Bottomley, Jon Masters, 2nddept-manager Hi, (2011/04/09 1:12), Greg KH wrote: > On Fri, Apr 08, 2011 at 11:07:41PM +0900, Nao Nishijima wrote: >> Hi, >> >> (2011/04/06 1:14), Greg KH wrote: >>> On Tue, Apr 05, 2011 at 09:49:46PM +0900, Nao Nishijima wrote: >>>> This patch series provides a SCSI option for persistent device >>>> names in kernel. With this option, user can always assign a >>>> same device name (e.g. sda) to a specific device according to >>>> udev rules and device id. >>>> >>>> Issue: >>>> Recently, kernel registers block devices in parallel. As a result, >>>> different device names will be assigned at each boot time. This >>>> will confuse file-system mounter, thus we usually use persistent >>>> symbolic links provided by udev. However, dmesg and procfs outputs >>>> show device names instead of the symbolic link names. This causes >>>> a serious problem when managing multiple devices (e.g. on a >>>> large-scale storage), because usually, device errors are output >>>> with device names on dmesg. We also concern about some commands >>>> which output device names, as well as kernel messages. >>>> >>>> Solution: >>>> To assign a persistent device name, I create unnamed devices (not >>>> a block device, but an intermediate device. users cannot read/write >>>> this device). When scsi device driver finds a LU, the LU is registered >>>> as an unnamed device and inform to udev. After udev finds the unnamed >>>> device, udev assigns a persistent device name to a specific device >>>> according to udev rules and registers it as a block device. Hence, >>>> this is just a naming, not a renaming. >>>> >>>> Some users are satisfied with current udev solution. Therefore, my >>>> proposal is implemented as an option. >>>> >>>> If using this option, add the following kernel parameter. >>>> >>>> scsi_mod.persistent_name=1 >>>> >>>> Also, if you want to assign a device name with sda, add the >>>> following udev rules. >>>> >>>> SUBSYSTEM="scsi_unnamed", ATTR{disk_id}="xxx", PROGRAM="/bin/sh >>>> -c 'echo -n sda > /sys/%p/disk_name'" >>> >>> Also, where is the "real" program you have created to properly name >>> devices from userspace? You need that to properly test this patch, >>> right? >>> >> >> In the udev rule described above, notation “xxx” indicated by >> ATTR(disk_id) is scsi id given by disk. Then, when udev finds this rule, >> "/bin/sh -c 'echo -n sda> /sys/%p/disk_name'" indicated by PROGRAM is >> operated using xxx (scsi id) if udev find the disk with scic id xxx. >> Thus, persistent device name is assigned. >> >> I have tested this patch using the udev rule. and It works well. > > That's nice, but it's a "toy". What have you used to test this with > 20000 scsi devices to properly work like it does today? Where is the > program that will name them in a deterministic manner? > > We have that functionality today, you can't break it. > > thanks, Indeed, I have not (can not, of course) tested 20000 scsi devices to properly work. This feature targets only scsi disk/cdrom/tape devices. Not all devices. We expect the target users are system administrators who need to control all devices for maintenance. They know all devices connected to the server and those devices will tested before connecting. We expect that admin will enable this option after installation. This means that device names are assigned by current mechanism when installation. Admin enables the option and makes udev rules (we are planning to make shell script which generate udev rules use by-id and assigned device names). In other word, this feature will be used just for "fixing" the names. In our scenario, admin appends a new rule manually when a new LU is added. In the future, we are planning to enhance udev to append a new rule automatically as like as NIC. Thanks, -- Nao NISHIJIMA Software Platform Research Dept. Linux Technology Center Hitachi, Ltd., YOKOHAMA Research Laboratory Email: nao.nishijima.xt@hitachi.com ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel. @ 2011-04-14 8:15 ` Nao Nishijima 0 siblings, 0 replies; 46+ messages in thread From: Nao Nishijima @ 2011-04-14 8:15 UTC (permalink / raw) To: Greg KH Cc: linux-kernel, linux-scsi, linux-hotplug, Kay Sievers, James Bottomley, Jon Masters, 2nddept-manager Hi, (2011/04/09 1:12), Greg KH wrote: > On Fri, Apr 08, 2011 at 11:07:41PM +0900, Nao Nishijima wrote: >> Hi, >> >> (2011/04/06 1:14), Greg KH wrote: >>> On Tue, Apr 05, 2011 at 09:49:46PM +0900, Nao Nishijima wrote: >>>> This patch series provides a SCSI option for persistent device >>>> names in kernel. With this option, user can always assign a >>>> same device name (e.g. sda) to a specific device according to >>>> udev rules and device id. >>>> >>>> Issue: >>>> Recently, kernel registers block devices in parallel. As a result, >>>> different device names will be assigned at each boot time. This >>>> will confuse file-system mounter, thus we usually use persistent >>>> symbolic links provided by udev. However, dmesg and procfs outputs >>>> show device names instead of the symbolic link names. This causes >>>> a serious problem when managing multiple devices (e.g. on a >>>> large-scale storage), because usually, device errors are output >>>> with device names on dmesg. We also concern about some commands >>>> which output device names, as well as kernel messages. >>>> >>>> Solution: >>>> To assign a persistent device name, I create unnamed devices (not >>>> a block device, but an intermediate device. users cannot read/write >>>> this device). When scsi device driver finds a LU, the LU is registered >>>> as an unnamed device and inform to udev. After udev finds the unnamed >>>> device, udev assigns a persistent device name to a specific device >>>> according to udev rules and registers it as a block device. Hence, >>>> this is just a naming, not a renaming. >>>> >>>> Some users are satisfied with current udev solution. Therefore, my >>>> proposal is implemented as an option. >>>> >>>> If using this option, add the following kernel parameter. >>>> >>>> scsi_mod.persistent_name=1 >>>> >>>> Also, if you want to assign a device name with sda, add the >>>> following udev rules. >>>> >>>> SUBSYSTEM=="scsi_unnamed", ATTR{disk_id}=="xxx", PROGRAM="/bin/sh >>>> -c 'echo -n sda > /sys/%p/disk_name'" >>> >>> Also, where is the "real" program you have created to properly name >>> devices from userspace? You need that to properly test this patch, >>> right? >>> >> >> In the udev rule described above, notation “xxx” indicated by >> ATTR(disk_id) is scsi id given by disk. Then, when udev finds this rule, >> "/bin/sh -c 'echo -n sda> /sys/%p/disk_name'" indicated by PROGRAM is >> operated using xxx (scsi id) if udev find the disk with scic id xxx. >> Thus, persistent device name is assigned. >> >> I have tested this patch using the udev rule. and It works well. > > That's nice, but it's a "toy". What have you used to test this with > 20000 scsi devices to properly work like it does today? Where is the > program that will name them in a deterministic manner? > > We have that functionality today, you can't break it. > > thanks, Indeed, I have not (can not, of course) tested 20000 scsi devices to properly work. This feature targets only scsi disk/cdrom/tape devices. Not all devices. We expect the target users are system administrators who need to control all devices for maintenance. They know all devices connected to the server and those devices will tested before connecting. We expect that admin will enable this option after installation. This means that device names are assigned by current mechanism when installation. Admin enables the option and makes udev rules (we are planning to make shell script which generate udev rules use by-id and assigned device names). In other word, this feature will be used just for "fixing" the names. In our scenario, admin appends a new rule manually when a new LU is added. In the future, we are planning to enhance udev to append a new rule automatically as like as NIC. Thanks, -- Nao NISHIJIMA Software Platform Research Dept. Linux Technology Center Hitachi, Ltd., YOKOHAMA Research Laboratory Email: nao.nishijima.xt@hitachi.com -- To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel. 2011-04-14 8:15 ` [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel Nao Nishijima (?) @ 2011-04-14 20:07 ` Greg KH -1 siblings, 0 replies; 46+ messages in thread From: Greg KH @ 2011-04-14 20:07 UTC (permalink / raw) To: Nao Nishijima Cc: linux-kernel, linux-scsi, linux-hotplug, Kay Sievers, James Bottomley, Jon Masters, 2nddept-manager On Thu, Apr 14, 2011 at 05:15:36PM +0900, Nao Nishijima wrote: > Hi, > > (2011/04/09 1:12), Greg KH wrote: > > On Fri, Apr 08, 2011 at 11:07:41PM +0900, Nao Nishijima wrote: > >> Hi, > >> > >> (2011/04/06 1:14), Greg KH wrote: > >>> On Tue, Apr 05, 2011 at 09:49:46PM +0900, Nao Nishijima wrote: > >>>> This patch series provides a SCSI option for persistent device > >>>> names in kernel. With this option, user can always assign a > >>>> same device name (e.g. sda) to a specific device according to > >>>> udev rules and device id. > >>>> > >>>> Issue: > >>>> Recently, kernel registers block devices in parallel. As a result, > >>>> different device names will be assigned at each boot time. This > >>>> will confuse file-system mounter, thus we usually use persistent > >>>> symbolic links provided by udev. However, dmesg and procfs outputs > >>>> show device names instead of the symbolic link names. This causes > >>>> a serious problem when managing multiple devices (e.g. on a > >>>> large-scale storage), because usually, device errors are output > >>>> with device names on dmesg. We also concern about some commands > >>>> which output device names, as well as kernel messages. > >>>> > >>>> Solution: > >>>> To assign a persistent device name, I create unnamed devices (not > >>>> a block device, but an intermediate device. users cannot read/write > >>>> this device). When scsi device driver finds a LU, the LU is registered > >>>> as an unnamed device and inform to udev. After udev finds the unnamed > >>>> device, udev assigns a persistent device name to a specific device > >>>> according to udev rules and registers it as a block device. Hence, > >>>> this is just a naming, not a renaming. > >>>> > >>>> Some users are satisfied with current udev solution. Therefore, my > >>>> proposal is implemented as an option. > >>>> > >>>> If using this option, add the following kernel parameter. > >>>> > >>>> scsi_mod.persistent_name=1 > >>>> > >>>> Also, if you want to assign a device name with sda, add the > >>>> following udev rules. > >>>> > >>>> SUBSYSTEM=="scsi_unnamed", ATTR{disk_id}=="xxx", PROGRAM="/bin/sh > >>>> -c 'echo -n sda > /sys/%p/disk_name'" > >>> > >>> Also, where is the "real" program you have created to properly name > >>> devices from userspace? You need that to properly test this patch, > >>> right? > >>> > >> > >> In the udev rule described above, notation “xxx” indicated by > >> ATTR(disk_id) is scsi id given by disk. Then, when udev finds this rule, > >> "/bin/sh -c 'echo -n sda> /sys/%p/disk_name'" indicated by PROGRAM is > >> operated using xxx (scsi id) if udev find the disk with scic id xxx. > >> Thus, persistent device name is assigned. > >> > >> I have tested this patch using the udev rule. and It works well. > > > > That's nice, but it's a "toy". What have you used to test this with > > 20000 scsi devices to properly work like it does today? Where is the > > program that will name them in a deterministic manner? > > > > We have that functionality today, you can't break it. > > > > thanks, > > Indeed, I have not (can not, of course) tested 20000 scsi devices to > properly work. Why not? You should be able to do this easily with a laptop these days with the dummy scsi device code. > This feature targets only scsi disk/cdrom/tape devices. > Not all devices. We expect the target users are system administrators > who need to control all devices for maintenance. They know all devices > connected to the server and those devices will tested before connecting. But that's not the large majority of the people using Linux. You need a sane default, which this does not provide. > We expect that admin will enable this option after installation. This > means that device names are assigned by current mechanism when > installation. Admin enables the option and makes udev rules (we are > planning to make shell script which generate udev rules use by-id and > assigned device names). In other word, this feature will be used just > for "fixing" the names. So admins are going to write their own udev rules by hand? I find that very unlikely. > In our scenario, admin appends a new rule manually when a new LU is > added. In the future, we are planning to enhance udev to append a new > rule automatically as like as NIC. I recommend doing the other options that have been proposed please. thanks, greg k-h ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 1/2] SCSI: Add a SCSI option for persistent device names @ 2011-04-14 20:07 ` Greg KH 0 siblings, 0 replies; 46+ messages in thread From: Greg KH @ 2011-04-14 20:07 UTC (permalink / raw) To: Nao Nishijima Cc: linux-kernel, linux-scsi, linux-hotplug, Kay Sievers, James Bottomley, Jon Masters, 2nddept-manager On Thu, Apr 14, 2011 at 05:15:36PM +0900, Nao Nishijima wrote: > Hi, > > (2011/04/09 1:12), Greg KH wrote: > > On Fri, Apr 08, 2011 at 11:07:41PM +0900, Nao Nishijima wrote: > >> Hi, > >> > >> (2011/04/06 1:14), Greg KH wrote: > >>> On Tue, Apr 05, 2011 at 09:49:46PM +0900, Nao Nishijima wrote: > >>>> This patch series provides a SCSI option for persistent device > >>>> names in kernel. With this option, user can always assign a > >>>> same device name (e.g. sda) to a specific device according to > >>>> udev rules and device id. > >>>> > >>>> Issue: > >>>> Recently, kernel registers block devices in parallel. As a result, > >>>> different device names will be assigned at each boot time. This > >>>> will confuse file-system mounter, thus we usually use persistent > >>>> symbolic links provided by udev. However, dmesg and procfs outputs > >>>> show device names instead of the symbolic link names. This causes > >>>> a serious problem when managing multiple devices (e.g. on a > >>>> large-scale storage), because usually, device errors are output > >>>> with device names on dmesg. We also concern about some commands > >>>> which output device names, as well as kernel messages. > >>>> > >>>> Solution: > >>>> To assign a persistent device name, I create unnamed devices (not > >>>> a block device, but an intermediate device. users cannot read/write > >>>> this device). When scsi device driver finds a LU, the LU is registered > >>>> as an unnamed device and inform to udev. After udev finds the unnamed > >>>> device, udev assigns a persistent device name to a specific device > >>>> according to udev rules and registers it as a block device. Hence, > >>>> this is just a naming, not a renaming. > >>>> > >>>> Some users are satisfied with current udev solution. Therefore, my > >>>> proposal is implemented as an option. > >>>> > >>>> If using this option, add the following kernel parameter. > >>>> > >>>> scsi_mod.persistent_name=1 > >>>> > >>>> Also, if you want to assign a device name with sda, add the > >>>> following udev rules. > >>>> > >>>> SUBSYSTEM="scsi_unnamed", ATTR{disk_id}="xxx", PROGRAM="/bin/sh > >>>> -c 'echo -n sda > /sys/%p/disk_name'" > >>> > >>> Also, where is the "real" program you have created to properly name > >>> devices from userspace? You need that to properly test this patch, > >>> right? > >>> > >> > >> In the udev rule described above, notation “xxx” indicated by > >> ATTR(disk_id) is scsi id given by disk. Then, when udev finds this rule, > >> "/bin/sh -c 'echo -n sda> /sys/%p/disk_name'" indicated by PROGRAM is > >> operated using xxx (scsi id) if udev find the disk with scic id xxx. > >> Thus, persistent device name is assigned. > >> > >> I have tested this patch using the udev rule. and It works well. > > > > That's nice, but it's a "toy". What have you used to test this with > > 20000 scsi devices to properly work like it does today? Where is the > > program that will name them in a deterministic manner? > > > > We have that functionality today, you can't break it. > > > > thanks, > > Indeed, I have not (can not, of course) tested 20000 scsi devices to > properly work. Why not? You should be able to do this easily with a laptop these days with the dummy scsi device code. > This feature targets only scsi disk/cdrom/tape devices. > Not all devices. We expect the target users are system administrators > who need to control all devices for maintenance. They know all devices > connected to the server and those devices will tested before connecting. But that's not the large majority of the people using Linux. You need a sane default, which this does not provide. > We expect that admin will enable this option after installation. This > means that device names are assigned by current mechanism when > installation. Admin enables the option and makes udev rules (we are > planning to make shell script which generate udev rules use by-id and > assigned device names). In other word, this feature will be used just > for "fixing" the names. So admins are going to write their own udev rules by hand? I find that very unlikely. > In our scenario, admin appends a new rule manually when a new LU is > added. In the future, we are planning to enhance udev to append a new > rule automatically as like as NIC. I recommend doing the other options that have been proposed please. thanks, greg k-h ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel. @ 2011-04-14 20:07 ` Greg KH 0 siblings, 0 replies; 46+ messages in thread From: Greg KH @ 2011-04-14 20:07 UTC (permalink / raw) To: Nao Nishijima Cc: linux-kernel, linux-scsi, linux-hotplug, Kay Sievers, James Bottomley, Jon Masters, 2nddept-manager On Thu, Apr 14, 2011 at 05:15:36PM +0900, Nao Nishijima wrote: > Hi, > > (2011/04/09 1:12), Greg KH wrote: > > On Fri, Apr 08, 2011 at 11:07:41PM +0900, Nao Nishijima wrote: > >> Hi, > >> > >> (2011/04/06 1:14), Greg KH wrote: > >>> On Tue, Apr 05, 2011 at 09:49:46PM +0900, Nao Nishijima wrote: > >>>> This patch series provides a SCSI option for persistent device > >>>> names in kernel. With this option, user can always assign a > >>>> same device name (e.g. sda) to a specific device according to > >>>> udev rules and device id. > >>>> > >>>> Issue: > >>>> Recently, kernel registers block devices in parallel. As a result, > >>>> different device names will be assigned at each boot time. This > >>>> will confuse file-system mounter, thus we usually use persistent > >>>> symbolic links provided by udev. However, dmesg and procfs outputs > >>>> show device names instead of the symbolic link names. This causes > >>>> a serious problem when managing multiple devices (e.g. on a > >>>> large-scale storage), because usually, device errors are output > >>>> with device names on dmesg. We also concern about some commands > >>>> which output device names, as well as kernel messages. > >>>> > >>>> Solution: > >>>> To assign a persistent device name, I create unnamed devices (not > >>>> a block device, but an intermediate device. users cannot read/write > >>>> this device). When scsi device driver finds a LU, the LU is registered > >>>> as an unnamed device and inform to udev. After udev finds the unnamed > >>>> device, udev assigns a persistent device name to a specific device > >>>> according to udev rules and registers it as a block device. Hence, > >>>> this is just a naming, not a renaming. > >>>> > >>>> Some users are satisfied with current udev solution. Therefore, my > >>>> proposal is implemented as an option. > >>>> > >>>> If using this option, add the following kernel parameter. > >>>> > >>>> scsi_mod.persistent_name=1 > >>>> > >>>> Also, if you want to assign a device name with sda, add the > >>>> following udev rules. > >>>> > >>>> SUBSYSTEM=="scsi_unnamed", ATTR{disk_id}=="xxx", PROGRAM="/bin/sh > >>>> -c 'echo -n sda > /sys/%p/disk_name'" > >>> > >>> Also, where is the "real" program you have created to properly name > >>> devices from userspace? You need that to properly test this patch, > >>> right? > >>> > >> > >> In the udev rule described above, notation “xxx” indicated by > >> ATTR(disk_id) is scsi id given by disk. Then, when udev finds this rule, > >> "/bin/sh -c 'echo -n sda> /sys/%p/disk_name'" indicated by PROGRAM is > >> operated using xxx (scsi id) if udev find the disk with scic id xxx. > >> Thus, persistent device name is assigned. > >> > >> I have tested this patch using the udev rule. and It works well. > > > > That's nice, but it's a "toy". What have you used to test this with > > 20000 scsi devices to properly work like it does today? Where is the > > program that will name them in a deterministic manner? > > > > We have that functionality today, you can't break it. > > > > thanks, > > Indeed, I have not (can not, of course) tested 20000 scsi devices to > properly work. Why not? You should be able to do this easily with a laptop these days with the dummy scsi device code. > This feature targets only scsi disk/cdrom/tape devices. > Not all devices. We expect the target users are system administrators > who need to control all devices for maintenance. They know all devices > connected to the server and those devices will tested before connecting. But that's not the large majority of the people using Linux. You need a sane default, which this does not provide. > We expect that admin will enable this option after installation. This > means that device names are assigned by current mechanism when > installation. Admin enables the option and makes udev rules (we are > planning to make shell script which generate udev rules use by-id and > assigned device names). In other word, this feature will be used just > for "fixing" the names. So admins are going to write their own udev rules by hand? I find that very unlikely. > In our scenario, admin appends a new rule manually when a new LU is > added. In the future, we are planning to enhance udev to append a new > rule automatically as like as NIC. I recommend doing the other options that have been proposed please. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 46+ messages in thread
end of thread, other threads:[~2011-04-18 20:15 UTC | newest] Thread overview: 46+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-04-05 12:49 [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel Nao Nishijima 2011-04-05 12:49 ` [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Nao Nishijima 2011-04-05 12:50 ` [PATCH 2/2] SCSI: modify SCSI subsystem Nao Nishijima 2011-04-05 12:50 ` Nao Nishijima 2011-04-05 16:14 ` [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel Greg KH 2011-04-05 16:14 ` [PATCH 1/2] SCSI: Add a SCSI option for persistent device names Greg KH 2011-04-08 14:12 ` [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel Nao Nishijima 2011-04-08 14:12 ` [PATCH 1/2] SCSI: Add a SCSI option for persistent device names Nao Nishijima 2011-04-08 14:12 ` [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel Nao Nishijima 2011-04-08 14:33 ` Hannes Reinecke 2011-04-08 14:33 ` [PATCH 1/2] SCSI: Add a SCSI option for persistent device names Hannes Reinecke 2011-04-08 14:33 ` [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel Hannes Reinecke 2011-04-08 15:14 ` James Bottomley 2011-04-08 15:14 ` [PATCH 1/2] SCSI: Add a SCSI option for persistent device James Bottomley 2011-04-08 15:14 ` [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel James Bottomley 2011-04-08 16:14 ` Greg KH 2011-04-08 16:14 ` [PATCH 1/2] SCSI: Add a SCSI option for persistent device names Greg KH 2011-04-08 16:43 ` [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel Kay Sievers 2011-04-08 16:43 ` [PATCH 1/2] SCSI: Add a SCSI option for persistent device names Kay Sievers 2011-04-08 16:43 ` [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel Kay Sievers 2011-04-12 13:23 ` Nao Nishijima 2011-04-12 13:23 ` [PATCH 1/2] SCSI: Add a SCSI option for persistent device names Nao Nishijima 2011-04-12 13:23 ` [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel Nao Nishijima 2011-04-12 13:29 ` James Bottomley 2011-04-12 13:29 ` [PATCH 1/2] SCSI: Add a SCSI option for persistent device James Bottomley 2011-04-12 13:29 ` [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel James Bottomley 2011-04-14 2:06 ` Nao Nishijima 2011-04-14 2:06 ` [PATCH 1/2] SCSI: Add a SCSI option for persistent device names Nao Nishijima 2011-04-14 2:06 ` [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel Nao Nishijima 2011-04-14 2:18 ` Greg KH 2011-04-14 2:18 ` [PATCH 1/2] SCSI: Add a SCSI option for persistent device names Greg KH 2011-04-08 17:21 ` [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel Stefan Richter 2011-04-08 17:21 ` [PATCH 1/2] SCSI: Add a SCSI option for persistent device names Stefan Richter 2011-04-18 20:10 ` [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel Jeremy Linton 2011-04-05 16:14 ` Greg KH 2011-04-05 16:14 ` [PATCH 1/2] SCSI: Add a SCSI option for persistent device names Greg KH 2011-04-08 14:07 ` [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel Nao Nishijima 2011-04-08 14:07 ` [PATCH 1/2] SCSI: Add a SCSI option for persistent device names Nao Nishijima 2011-04-08 16:12 ` [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel Greg KH 2011-04-08 16:12 ` [PATCH 1/2] SCSI: Add a SCSI option for persistent device names Greg KH 2011-04-14 8:15 ` [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel Nao Nishijima 2011-04-14 8:15 ` [PATCH 1/2] SCSI: Add a SCSI option for persistent device names Nao Nishijima 2011-04-14 8:15 ` [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel Nao Nishijima 2011-04-14 20:07 ` Greg KH 2011-04-14 20:07 ` [PATCH 1/2] SCSI: Add a SCSI option for persistent device names Greg KH 2011-04-14 20:07 ` [PATCH 1/2] SCSI: Add a SCSI option for persistent device names in Kernel Greg KH
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.