[PATCH 1/1] ubi: Allow ubiblock devices nodes to be created by volume name instead of volume ID.

[PATCH 1/1] ubi: Allow ubiblock devices nodes to be created by volume name instead of volume ID.

[Patrick Doyle]

Add support for CONFIG_MTD_UBI_BLOCK_BY_NAME, which enables ubi block
devices to be named by their volume name: /dev/ubiblock%d_%s rather than
their volume ID /dev/ubiblock%d_%d, so that one can mount e.g. a root
filesystem by UBI name instead of volume ID.  UBI volumes can be renamed
on-the-fly in user space. This allows the root file system to be swapped
from an "A" volume to a "B" volume without having to change the mount
options.

Signed-off-by: Patrick Doyle <pdoyle@irobot.com>
---
 drivers/mtd/ubi/Kconfig | 12 ++++++++++++
 drivers/mtd/ubi/block.c |  4 ++++
 2 files changed, 16 insertions(+)

diff --git a/drivers/mtd/ubi/Kconfig b/drivers/mtd/ubi/Kconfig
index 43d131f..d9320f0 100644
--- a/drivers/mtd/ubi/Kconfig
+++ b/drivers/mtd/ubi/Kconfig
@@ -103,4 +103,16 @@ config MTD_UBI_BLOCK

        If in doubt, say "N".

+config MTD_UBI_BLOCK_BY_NAME
+       bool "Create ubi block device nodes by name instead of volume ID"
+       default n
+       depends on MTD_UBI_BLOCK
+       help
+         This option enables ubi block devices to be named by their volume name
+     /dev/ubiblock%d_%s rather than their volume ID /dev/ubiblock%d_%d, so that
+     one can mount e.g. a root filesystem by UBI name instead of volume ID.
+     UBI volumes can be renamed on-the-fly in user space. This allows the root
+     file system to be swapped from an "A" volume to a "B" volume
without having to
+     change the mount options.
+
 endif # MTD_UBI
diff --git a/drivers/mtd/ubi/block.c b/drivers/mtd/ubi/block.c
index d0b63bbf..ac2d480 100644
--- a/drivers/mtd/ubi/block.c
+++ b/drivers/mtd/ubi/block.c
@@ -399,7 +399,11 @@ int ubiblock_create(struct ubi_volume_info *vi)
         goto out_put_disk;
     }
     gd->private_data = dev;
+#ifdef CONFIG_MTD_UBI_BLOCK_BY_NAME
+    sprintf(gd->disk_name, "ubiblock%d_%s", dev->ubi_num, vi->name);
+#else
     sprintf(gd->disk_name, "ubiblock%d_%d", dev->ubi_num, dev->vol_id);
+#endif
     set_capacity(gd, disk_capacity);
     dev->gd = gd;

-- 
2.7.4

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 1/1] ubi: Allow ubiblock devices nodes to be created by volume name instead of volume ID.

[Richard Weinberger]

Am Donnerstag, 4. April 2019, 17:29:16 CEST schrieb Patrick Doyle:
> Add support for CONFIG_MTD_UBI_BLOCK_BY_NAME, which enables ubi block
> devices to be named by their volume name: /dev/ubiblock%d_%s rather than
> their volume ID /dev/ubiblock%d_%d, so that one can mount e.g. a root
> filesystem by UBI name instead of volume ID.  UBI volumes can be renamed
> on-the-fly in user space. This allows the root file system to be swapped
> from an "A" volume to a "B" volume without having to change the mount
> options.

Isn't this why we have udev, to create fancy by-id/by-path/... naming conventions?

Thanks,
//richard



______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 1/1] ubi: Allow ubiblock devices nodes to be created by volume name instead of volume ID.

[Emil Lenngren]

Hi,

Den tors 4 apr. 2019 kl 21:43 skrev Richard Weinberger <richard@nod.at>:
>
> Am Donnerstag, 4. April 2019, 17:29:16 CEST schrieb Patrick Doyle:
> > Add support for CONFIG_MTD_UBI_BLOCK_BY_NAME, which enables ubi block
> > devices to be named by their volume name: /dev/ubiblock%d_%s rather than
> > their volume ID /dev/ubiblock%d_%d, so that one can mount e.g. a root
> > filesystem by UBI name instead of volume ID.  UBI volumes can be renamed
> > on-the-fly in user space. This allows the root file system to be swapped
> > from an "A" volume to a "B" volume without having to change the mount
> > options.
>
> Isn't this why we have udev, to create fancy by-id/by-path/... naming conventions?
>
> Thanks,
> //richard

I actually implemented the same change some time ago, for the exact
same reason (swapping two volumes and then reboot). Referring to an
ubi volume by name is more convenient than volume numbers, since names
can be changed and numbers can't.
Is it maybe possible to define both /dev/ubiblock%d_%d and
/dev/ubiblock%d_%s at the same time?
How would this be done with udev instead to get "fancy by-id"?

/Emil

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 1/1] ubi: Allow ubiblock devices nodes to be created by volume name instead of volume ID.

[Patrick Doyle]

On Wed, Aug 14, 2019 at 8:34 AM Emil Lenngren <emil.lenngren@gmail.com> wrote:
> Hi,
> Den tors 4 apr. 2019 kl 21:43 skrev Richard Weinberger <richard@nod.at>:
> > Am Donnerstag, 4. April 2019, 17:29:16 CEST schrieb Patrick Doyle:
> > > Add support for CONFIG_MTD_UBI_BLOCK_BY_NAME, which enables ubi block

> >
> > Isn't this why we have udev, to create fancy by-id/by-path/... naming conventions?
> >
> > Thanks,
> > //richard
>
I never saw //richard's response.  I added it so I could use
root=/dev/ubiblock0_rootfs in my command line.  When I update my
software, I write the new rootfs to a volume named "rootfs_new", and
use UBI's fabulous atomic rename facility to swap "rootfs" and
"rootfs_new", reboot, and I have my new rootfs.

--wpd

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 1/1] ubi: Allow ubiblock devices nodes to be created by volume name instead of volume ID.

[Richard Weinberger]

On Wed, Aug 14, 2019 at 8:07 PM Patrick Doyle <wpdster@gmail.com> wrote:
> I never saw //richard's response.  I added it so I could use
> root=/dev/ubiblock0_rootfs in my command line.  When I update my
> software, I write the new rootfs to a volume named "rootfs_new", and
> use UBI's fabulous atomic rename facility to swap "rootfs" and
> "rootfs_new", reboot, and I have my new rootfs.

Did your spam filter eat my mail? :-(
Your use case can also be addressed with udev rules.

-- 
Thanks,
//richard

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 1/1] ubi: Allow ubiblock devices nodes to be created by volume name instead of volume ID.

[Richard Weinberger]

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

On Wed, Aug 14, 2019 at 2:34 PM Emil Lenngren <emil.lenngren@gmail.com> wrote:
> > Isn't this why we have udev, to create fancy by-id/by-path/... naming conventions?
> >
> > Thanks,
> > //richard
>
> I actually implemented the same change some time ago, for the exact
> same reason (swapping two volumes and then reboot). Referring to an
> ubi volume by name is more convenient than volume numbers, since names
> can be changed and numbers can't.
> Is it maybe possible to define both /dev/ubiblock%d_%d and
> /dev/ubiblock%d_%s at the same time?
> How would this be done with udev instead to get "fancy by-id"?

Thanks for bringing this topic up again.
udev allows rules and even helper (scripts) to gain device
attributes. These can then be used to name/symlink devices as you want.

For example, if you want to have device symlinks by name for UBI
volumes, you can create a rule like that:
KERNEL=="ubi?_?", IMPORT{program}="/etc/udev/rules.d/ubi_probe
$devnode", SYMLINK="ubi%E{MTD_NUM}_%E{VOL_NAME}"

With ubi_probe being:
#!/bin/sh
VOL_NAME="$(cat /sys/${DEVPATH}/name)"
MTD_NUM="$(cat /sys/${DEVPATH}/../mtd_num)"
echo "VOL_NAME=${VOL_NAME}"
echo "MTD_NUM=${MTD_NUM}"

I took the chance and reviewed UBI's sysfs interface and found a few
things which need
improvement. With these issues addressed you can also work with
ubiblock by name:
1. UBI does not export the UBI device number via sysfs. In 99% of all
cases mtd_num will be correct,
but you can select the UBI number upon attach time.
2. UBI does not emit a change kevent when an volume is renamed. So
udev does not see the rename command.
3. ubiblock does not set the UBI volume as parent device.

So we can then have a rule like:
KERNEL=="ubiblock?_?",
IMPORT{program}="/etc/udev/rules.d/ubiblock_probe $devnode"
SYMLINK="ubiblock$number_%E{VOL_NAME}"

With ubiblock_probe being:
#!/bin/sh
VOL_NAME="$(cat /sys/${DEVPATH}/device/name)"
echo "VOL_NAME=${VOL_NAME}"

# ls -ltr /dev/ubiblock*
brw-rw---- 1 root disk 251, 0 18. Aug 22:41 /dev/ubiblock7_0
lrwxrwxrwx 1 root root     11 18. Aug 22:41 /dev/ubiblock0_test -> ubiblock7_0
# ubirename /dev/ubi7 test lala
# ls -ltr /dev/ubiblock*
brw-rw---- 1 root disk 251, 0 18. Aug 22:42 /dev/ubiblock7_0
lrwxrwxrwx 1 root root     11 18. Aug 22:42 /dev/ubiblock0_lala -> ubiblock7_0

Does this help? If you need other/more sysfs changes, please tell. :-)
Attached you can find my WIP patch for these changes. I need to double check
a few things first before I will send a formal patch.

I think it would even make sense to integrate a more powerful
ubi_probe (as C program) into systemd-udev
like we already have for mtd itself.

-- 
Thanks,
//richard

[-- Attachment #2: ubi_sysfs_wip.diff --]
[-- Type: text/x-patch, Size: 5625 bytes --]

diff --git a/drivers/mtd/ubi/block.c b/drivers/mtd/ubi/block.c
index 6025398955a2..6caab85907bd 100644
--- a/drivers/mtd/ubi/block.c
+++ b/drivers/mtd/ubi/block.c
@@ -345,7 +345,7 @@ static const struct blk_mq_ops ubiblock_mq_ops = {
 	.init_request	= ubiblock_init_request,
 };
 
-int ubiblock_create(struct ubi_volume_info *vi)
+int ubiblock_create(struct ubi_volume_info *vi, struct device *ubi_dev)
 {
 	struct ubiblock *dev;
 	struct gendisk *gd;
@@ -433,7 +433,7 @@ int ubiblock_create(struct ubi_volume_info *vi)
 	list_add_tail(&dev->list, &ubiblock_devices);
 
 	/* Must be the last step: anyone can call file ops from now on */
-	add_disk(dev->gd);
+	device_add_disk(ubi_dev, dev->gd, NULL);
 	dev_info(disk_to_dev(dev->gd), "created from ubi%d:%d(%s)",
 		 dev->ubi_num, dev->vol_id, vi->name);
 	mutex_unlock(&devices_mutex);
@@ -504,6 +504,23 @@ int ubiblock_remove(struct ubi_volume_info *vi)
 	return ret;
 }
 
+static void ubiblock_notify_rename(struct ubi_volume_info *vi)
+{
+	struct ubiblock *dev;
+
+	mutex_lock(&devices_mutex);
+	dev = find_dev_nolock(vi->ubi_num, vi->vol_id);
+	if (!dev) {
+		mutex_unlock(&devices_mutex);
+		return;
+	}
+
+	mutex_lock(&dev->dev_mutex);
+	kobject_uevent(&disk_to_dev(dev->gd)->kobj, KOBJ_CHANGE);
+	mutex_unlock(&dev->dev_mutex);
+	mutex_unlock(&devices_mutex);
+}
+
 static int ubiblock_resize(struct ubi_volume_info *vi)
 {
 	struct ubiblock *dev;
@@ -565,6 +582,8 @@ static int ubiblock_notify(struct notifier_block *nb,
 		if (nt->vi.vol_type == UBI_STATIC_VOLUME)
 			ubiblock_resize(&nt->vi);
 		break;
+	case UBI_VOLUME_RENAMED:
+		ubiblock_notify_rename(&nt->vi);
 	default:
 		break;
 	}
@@ -613,9 +632,8 @@ static void __init ubiblock_create_from_param(void)
 		}
 
 		ubi_get_volume_info(desc, &vi);
+		ret = ubiblock_create(&vi, &desc->vol->dev);
 		ubi_close_volume(desc);
-
-		ret = ubiblock_create(&vi);
 		if (ret) {
 			pr_err(
 			       "UBI: block: can't add '%s' volume on ubi%d_%d, err=%d\n",
diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
index d636bbe214cb..134492a1718c 100644
--- a/drivers/mtd/ubi/build.c
+++ b/drivers/mtd/ubi/build.c
@@ -140,6 +140,8 @@ static struct device_attribute dev_mtd_num =
 	__ATTR(mtd_num, S_IRUGO, dev_attribute_show, NULL);
 static struct device_attribute dev_ro_mode =
 	__ATTR(ro_mode, S_IRUGO, dev_attribute_show, NULL);
+static struct device_attribute dev_ubi_num =
+	__ATTR(ubi_num, S_IRUGO, dev_attribute_show, NULL);
 
 /**
  * ubi_volume_notify - send a volume change notification.
@@ -378,6 +380,8 @@ static ssize_t dev_attribute_show(struct device *dev,
 		ret = sprintf(buf, "%d\n", ubi->mtd->index);
 	else if (attr == &dev_ro_mode)
 		ret = sprintf(buf, "%d\n", ubi->ro_mode);
+	else if (attr == &dev_ubi_num)
+		ret = sprintf(buf, "%d\n", ubi->ubi_num);
 	else
 		ret = -EINVAL;
 
@@ -398,6 +402,7 @@ static struct attribute *ubi_dev_attrs[] = {
 	&dev_bgt_enabled.attr,
 	&dev_mtd_num.attr,
 	&dev_ro_mode.attr,
+	&dev_ubi_num.attr,
 	NULL
 };
 ATTRIBUTE_GROUPS(ubi_dev);
diff --git a/drivers/mtd/ubi/cdev.c b/drivers/mtd/ubi/cdev.c
index 1b77fff9f892..a7ac50cecc05 100644
--- a/drivers/mtd/ubi/cdev.c
+++ b/drivers/mtd/ubi/cdev.c
@@ -562,7 +562,7 @@ static long vol_cdev_ioctl(struct file *file, unsigned int cmd,
 		struct ubi_volume_info vi;
 
 		ubi_get_volume_info(desc, &vi);
-		err = ubiblock_create(&vi);
+		err = ubiblock_create(&vi, &desc->vol->dev);
 		break;
 	}
 
diff --git a/drivers/mtd/ubi/ubi.h b/drivers/mtd/ubi/ubi.h
index 721b6aa7936c..cb90d61f5f07 100644
--- a/drivers/mtd/ubi/ubi.h
+++ b/drivers/mtd/ubi/ubi.h
@@ -976,12 +976,12 @@ static inline void ubi_fastmap_destroy_checkmap(struct ubi_volume *vol) {}
 #ifdef CONFIG_MTD_UBI_BLOCK
 int ubiblock_init(void);
 void ubiblock_exit(void);
-int ubiblock_create(struct ubi_volume_info *vi);
+int ubiblock_create(struct ubi_volume_info *vi, struct device *dev);
 int ubiblock_remove(struct ubi_volume_info *vi);
 #else
 static inline int ubiblock_init(void) { return 0; }
 static inline void ubiblock_exit(void) {}
-static inline int ubiblock_create(struct ubi_volume_info *vi)
+static inline int ubiblock_create(struct ubi_volume_info *vi, struct device *dev)
 {
 	return -ENOSYS;
 }
diff --git a/drivers/mtd/ubi/vmt.c b/drivers/mtd/ubi/vmt.c
index 139ee132bfbc..c18b5739ca8e 100644
--- a/drivers/mtd/ubi/vmt.c
+++ b/drivers/mtd/ubi/vmt.c
@@ -38,6 +38,8 @@ static struct device_attribute attr_vol_data_bytes =
 	__ATTR(data_bytes, S_IRUGO, vol_attribute_show, NULL);
 static struct device_attribute attr_vol_upd_marker =
 	__ATTR(upd_marker, S_IRUGO, vol_attribute_show, NULL);
+static struct device_attribute attr_vol_id =
+	__ATTR(id, S_IRUGO, vol_attribute_show, NULL);
 
 /*
  * "Show" method for files in '/<sysfs>/class/ubi/ubiX_Y/'.
@@ -94,6 +96,8 @@ static ssize_t vol_attribute_show(struct device *dev,
 		ret = sprintf(buf, "%lld\n", vol->used_bytes);
 	else if (attr == &attr_vol_upd_marker)
 		ret = sprintf(buf, "%d\n", vol->upd_marker);
+	else if (attr == &attr_vol_id)
+		ret = sprintf(buf, "%d\n", vol->vol_id);
 	else
 		/* This must be a bug */
 		ret = -EINVAL;
@@ -116,6 +120,7 @@ static struct attribute *volume_dev_attrs[] = {
 	&attr_vol_usable_eb_size.attr,
 	&attr_vol_data_bytes.attr,
 	&attr_vol_upd_marker.attr,
+	&attr_vol_id.attr,
 	NULL
 };
 ATTRIBUTE_GROUPS(volume_dev);
@@ -555,6 +560,7 @@ int ubi_rename_volumes(struct ubi_device *ubi, struct list_head *rename_list)
 			memcpy(vol->name, re->new_name, re->new_name_len + 1);
 			spin_unlock(&ubi->volumes_lock);
 			ubi_volume_notify(ubi, vol, UBI_VOLUME_RENAMED);
+			kobject_uevent(&vol->dev.kobj, KOBJ_CHANGE);
 		}
 	}
 

[-- Attachment #3: Type: text/plain, Size: 144 bytes --]

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 1/1] ubi: Allow ubiblock devices nodes to be created by volume name instead of volume ID.

[Emil Lenngren]

Hi Richard,

Den sön 18 aug. 2019 kl 22:50 skrev Richard Weinberger
<richard.weinberger@gmail.com>:
>
> On Wed, Aug 14, 2019 at 2:34 PM Emil Lenngren <emil.lenngren@gmail.com> wrote:
> > > Isn't this why we have udev, to create fancy by-id/by-path/... naming conventions?
> > >
> > > Thanks,
> > > //richard
> >
> > I actually implemented the same change some time ago, for the exact
> > same reason (swapping two volumes and then reboot). Referring to an
> > ubi volume by name is more convenient than volume numbers, since names
> > can be changed and numbers can't.
> > Is it maybe possible to define both /dev/ubiblock%d_%d and
> > /dev/ubiblock%d_%s at the same time?
> > How would this be done with udev instead to get "fancy by-id"?
>
> Thanks for bringing this topic up again.
> udev allows rules and even helper (scripts) to gain device
> attributes. These can then be used to name/symlink devices as you want.
>
> For example, if you want to have device symlinks by name for UBI
> volumes, you can create a rule like that:
> KERNEL=="ubi?_?", IMPORT{program}="/etc/udev/rules.d/ubi_probe
> $devnode", SYMLINK="ubi%E{MTD_NUM}_%E{VOL_NAME}"
>
> With ubi_probe being:
> #!/bin/sh
> VOL_NAME="$(cat /sys/${DEVPATH}/name)"
> MTD_NUM="$(cat /sys/${DEVPATH}/../mtd_num)"
> echo "VOL_NAME=${VOL_NAME}"
> echo "MTD_NUM=${MTD_NUM}"
>
> I took the chance and reviewed UBI's sysfs interface and found a few
> things which need
> improvement. With these issues addressed you can also work with
> ubiblock by name:
> 1. UBI does not export the UBI device number via sysfs. In 99% of all
> cases mtd_num will be correct,
> but you can select the UBI number upon attach time.
> 2. UBI does not emit a change kevent when an volume is renamed. So
> udev does not see the rename command.
> 3. ubiblock does not set the UBI volume as parent device.
>
> So we can then have a rule like:
> KERNEL=="ubiblock?_?",
> IMPORT{program}="/etc/udev/rules.d/ubiblock_probe $devnode"
> SYMLINK="ubiblock$number_%E{VOL_NAME}"
>
> With ubiblock_probe being:
> #!/bin/sh
> VOL_NAME="$(cat /sys/${DEVPATH}/device/name)"
> echo "VOL_NAME=${VOL_NAME}"
>
> # ls -ltr /dev/ubiblock*
> brw-rw---- 1 root disk 251, 0 18. Aug 22:41 /dev/ubiblock7_0
> lrwxrwxrwx 1 root root     11 18. Aug 22:41 /dev/ubiblock0_test -> ubiblock7_0
> # ubirename /dev/ubi7 test lala
> # ls -ltr /dev/ubiblock*
> brw-rw---- 1 root disk 251, 0 18. Aug 22:42 /dev/ubiblock7_0
> lrwxrwxrwx 1 root root     11 18. Aug 22:42 /dev/ubiblock0_lala -> ubiblock7_0
>
> Does this help? If you need other/more sysfs changes, please tell. :-)
> Attached you can find my WIP patch for these changes. I need to double check
> a few things first before I will send a formal patch.
>
> I think it would even make sense to integrate a more powerful
> ubi_probe (as C program) into systemd-udev
> like we already have for mtd itself.

Will this really work when passing the rootfs to the kernel command
line like "root=/dev/ubiblock0_rootfs"? If the udev rules that set up
the symbolic link /dev/ubiblock0_rootfs are stored in a file on the
rootfs itself, I guess that symlink can't be made available before the
rootfs is mounted...

/Emil

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 1/1] ubi: Allow ubiblock devices nodes to be created by volume name instead of volume ID.

[Richard Weinberger]

On Wed, Aug 28, 2019 at 3:39 PM Emil Lenngren <emil.lenngren@gmail.com> wrote:
> Will this really work when passing the rootfs to the kernel command
> line like "root=/dev/ubiblock0_rootfs"? If the udev rules that set up
> the symbolic link /dev/ubiblock0_rootfs are stored in a file on the
> rootfs itself, I guess that symlink can't be made available before the
> rootfs is mounted...

No, this will not work directly from the kernel command line.
For any kind non-trivial root you need an initramfs,
that's the whole point of initramfs. :-)

-- 
Thanks,
//richard

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 1/1] ubi: Allow ubiblock devices nodes to be created by volume name instead of volume ID.

[Jan Kardell]

Richard Weinberger skrev:
> On Wed, Aug 28, 2019 at 3:39 PM Emil Lenngren <emil.lenngren@gmail.com> wrote:
>> Will this really work when passing the rootfs to the kernel command
>> line like "root=/dev/ubiblock0_rootfs"? If the udev rules that set up
>> the symbolic link /dev/ubiblock0_rootfs are stored in a file on the
>> rootfs itself, I guess that symlink can't be made available before the
>> rootfs is mounted...
> No, this will not work directly from the kernel command line.
> For any kind non-trivial root you need an initramfs,
> that's the whole point of initramfs. :-)
>

I have this on my cmdline:

root=ubi0:rootfs rw ubi.mtd=rootfs,2048 rootfstype=ubifs

Maybe a bit confusing that both the MTD partition and the UBI volume are 
named rootfs:-) The board uses a very old 3.14 kernel from OpenWrt, I 
believe the MTD parts is vanilla. Doesn't this work on a more recent kernel?

//Jan

-- 

"I have always wished for my computer to be as easy to use as my telephone; my wish has come true because I can no longer figure out how to use my telephone."
- Bjarne Stroustrup


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 1/1] ubi: Allow ubiblock devices nodes to be created by volume name instead of volume ID.

[Jan Kardell]

Richard Weinberger skrev:
> On Wed, Aug 28, 2019 at 3:39 PM Emil Lenngren <emil.lenngren@gmail.com> wrote:
>> Will this really work when passing the rootfs to the kernel command
>> line like "root=/dev/ubiblock0_rootfs"? If the udev rules that set up
>> the symbolic link /dev/ubiblock0_rootfs are stored in a file on the
>> rootfs itself, I guess that symlink can't be made available before the
>> rootfs is mounted...
> No, this will not work directly from the kernel command line.
> For any kind non-trivial root you need an initramfs,
> that's the whole point of initramfs. :-)
>

I have this on my cmdline:

root=ubi0:rootfs rw ubi.mtd=rootfs,2048 rootfstype=ubifs

Maybe a bit confusing that both the MTD partition and the UBI volume are 
named rootfs:-) The board uses a very old 3.14 kernel from OpenWrt, I 
believe the MTD parts is vanilla. Doesn't this work on a more recent kernel?

//Jan

-- 

"I have always wished for my computer to be as easy to use as my telephone; my wish has come true because I can no longer figure out how to use my telephone."
- Bjarne Stroustrup


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 1/1] ubi: Allow ubiblock devices nodes to be created by volume name instead of volume ID.

[Richard Weinberger]

----- Ursprüngliche Mail -----
> Von: "Jan Kardell" <jan.kardell@telliq.com>
> I have this on my cmdline:
> 
> root=ubi0:rootfs rw ubi.mtd=rootfs,2048 rootfstype=ubifs
> 
> Maybe a bit confusing that both the MTD partition and the UBI volume are
> named rootfs:-) The board uses a very old 3.14 kernel from OpenWrt, I
> believe the MTD parts is vanilla. Doesn't this work on a more recent kernel?

How is this related to ubiblock?

Anyway, your cmdline should work.
If it used to work on an older (vanilla!) kernel and is now broken,
it is a regression which needs fixing. :-)

Thanks,
//richard

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 1/1] ubi: Allow ubiblock devices nodes to be created by volume name instead of volume ID.

[Ezequiel Garcia]

Hello Emil, Patrick, Richard:

I just came across this patch.

On Wed, 14 Aug 2019 at 09:35, Emil Lenngren <emil.lenngren@gmail.com> wrote:
>
> Hi,
>
> Den tors 4 apr. 2019 kl 21:43 skrev Richard Weinberger <richard@nod.at>:
> >
> > Am Donnerstag, 4. April 2019, 17:29:16 CEST schrieb Patrick Doyle:
> > > Add support for CONFIG_MTD_UBI_BLOCK_BY_NAME, which enables ubi block
> > > devices to be named by their volume name: /dev/ubiblock%d_%s rather than
> > > their volume ID /dev/ubiblock%d_%d, so that one can mount e.g. a root
> > > filesystem by UBI name instead of volume ID.  UBI volumes can be renamed
> > > on-the-fly in user space. This allows the root file system to be swapped
> > > from an "A" volume to a "B" volume without having to change the mount
> > > options.
> >
> > Isn't this why we have udev, to create fancy by-id/by-path/... naming conventions?
> >
> > Thanks,
> > //richard
>
> I actually implemented the same change some time ago, for the exact
> same reason (swapping two volumes and then reboot). Referring to an
> ubi volume by name is more convenient than volume numbers, since names
> can be changed and numbers can't.

Arggh, it is such a shame that we missed this when we
designed UBI block!

As you have noticed, atomically changing UBI volume names
is an important UBI feature, extensively used for implementing
safe firmware upgrades.

So, naturally, the device choice should have been based on the
name, and not on the volume ID.

Regarding, the "use a initramfs" I've been there, carrying an
initramfs for the sole purpose of finding the volume with the
name you want, mounting and pivoting. Looking back,
I can say this was a big, fat PITA.

And it's _not_ about boot time or anything like that.
It's about increasing the number of components
(in this case, an entire rootfs) that system integrators
have to maintain, keep track of, and worry about.

I'm not at all surprised hacking a downstream is easier for
embedded developers, and I'm sorry we didn't see thought
about this use case back then.

> Is it maybe possible to define both /dev/ubiblock%d_%d and
> /dev/ubiblock%d_%s at the same time?

Nope, this won't fly.

The sad news is that it's not easy to fix. The patch proposed
by Patrick is a no-go, because we don't want to increase
the number of configuration options for something like this.

Configuration options increase the combinations that you
need to test, and so we try to avoid them as much as possible.

We can't change any default behavior either, as it will
have an impact on existing systems.

What we _could_ do, is extend the current syntax, passing
a format string to the kernel. Something like this, provided
a ubi0_0 volume, named "rootA".

ubi.block=0,0,ubiblock${dev.id}_${vol.id}

would create block device as "/dev/ubiblock0_0".

Where as, ubi.block=0,0,ubiblock${dev.id}${vol.name}

would create block device as "/dev/ubiblock0_rootA".

Knowing Richard's stand in the initramfs camp, I'm sure
he's eyes are in flames right now :-)

Does this make any sense... or it is pure insanity?

Thanks (also sorry),
Ezequiel

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 1/1] ubi: Allow ubiblock devices nodes to be created by volume name instead of volume ID.

[Patrick Doyle]

On Mon, Dec 30, 2019 at 12:32 PM Ezequiel Garcia
<ezequiel@vanguardiasur.com.ar> wrote:
> Hello Emil, Patrick, Richard:
>
> What we _could_ do, is extend the current syntax, passing
> a format string to the kernel. Something like this, provided
> a ubi0_0 volume, named "rootA".
>
> ubi.block=0,0,ubiblock${dev.id}_${vol.id}
>
> would create block device as "/dev/ubiblock0_0".
>
> Where as, ubi.block=0,0,ubiblock${dev.id}${vol.name}
>
> would create block device as "/dev/ubiblock0_rootA".
> Does this make any sense... or it is pure insanity?
That would work for me... as it is... the first project where I needed
this got shelved, and the second one grew an initramfs so I no longer
need the patch.  I would be happy to see others benefit from it, or
from some other, better mechanism.

--wpd

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 1/1] ubi: Allow ubiblock devices nodes to be created by volume name instead of volume ID.

[Richard Weinberger]

On Mon, Dec 30, 2019 at 6:32 PM Ezequiel Garcia
<ezequiel@vanguardiasur.com.ar> wrote:
> Regarding, the "use a initramfs" I've been there, carrying an
> initramfs for the sole purpose of finding the volume with the
> name you want, mounting and pivoting. Looking back,
> I can say this was a big, fat PITA.

It all depends on your tooling. With yocto creating and maintaining
and initramfs is easy.
But maybe I'm a little biased since every single embedded system
I create needs an initramfs due to crypto foo..

> And it's _not_ about boot time or anything like that.
> It's about increasing the number of components
> (in this case, an entire rootfs) that system integrators
> have to maintain, keep track of, and worry about.
>
> I'm not at all surprised hacking a downstream is easier for
> embedded developers, and I'm sorry we didn't see thought
> about this use case back then.
>
> > Is it maybe possible to define both /dev/ubiblock%d_%d and
> > /dev/ubiblock%d_%s at the same time?
>
> Nope, this won't fly.
>
> The sad news is that it's not easy to fix. The patch proposed
> by Patrick is a no-go, because we don't want to increase
> the number of configuration options for something like this.
>
> Configuration options increase the combinations that you
> need to test, and so we try to avoid them as much as possible.
>
> We can't change any default behavior either, as it will
> have an impact on existing systems.
>
> What we _could_ do, is extend the current syntax, passing
> a format string to the kernel. Something like this, provided
> a ubi0_0 volume, named "rootA".
>
> ubi.block=0,0,ubiblock${dev.id}_${vol.id}
>
> would create block device as "/dev/ubiblock0_0".
>
> Where as, ubi.block=0,0,ubiblock${dev.id}${vol.name}
>
> would create block device as "/dev/ubiblock0_rootA".
>
> Knowing Richard's stand in the initramfs camp, I'm sure
> he's eyes are in flames right now :-)
>
> Does this make any sense... or it is pure insanity?

We need to consider also the volume rename case.
If the kernel creates a named ubiblock device we have to keep
the names consistent also across renames.
Again, udev helps here.

That said, I understand that not everyone wants to have an initramfs.
As maintainer I have to be neutral even when I personally don't like a solution.

I'll happily accept such a change if it:
a) is not an ugly hack and/or makes the existing ubi-naming logic worse
b) is is fine by block layer folks and does not violate the Linux device model

Within UBI we can do whatever we want. That's why the whole ubi name
parsing at attach time exists. IMHO doing this in the kernel was a mistake
and we should have enforced an initramfs back then.
ubiblock is a block device stacked ontop of ubi and we have to obey their rules.

-- 
Thanks,
//richard

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/