| Message ID | CAF_dkJA2DFr3Vgk5ie=V5YitZr8HaiXWuQ+SKsHzzmLBFnhyeg@mail.gmail.com |
|---|---|
| State | Rejected |
| Delegated to: | Richard Weinberger |
| Headers | show |
| Series | [1/1] ubi: Allow ubiblock devices nodes to be created by volume name instead of volume ID. | expand |
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
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
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
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.
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.
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
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. :-)
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
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
----- 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
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
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
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.
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;
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(+)