| Message ID | 20161202101028.26736-1-daggs@gmx.com |
|---|---|
| State | Rejected |
| Headers | show |
Hello, On Fri, 2 Dec 2016 12:10:27 +0200, Dagg Stompler wrote: > this new package will resize the root fs to the max possible when > booting an image for the first time. > > Signed-off-by: Dagg Stompler <daggs@gmx.com> I think this is still way too use-case specific, and I'll explain below why your implementation only works in your specific case, but will fail in many other cases. > diff --git a/package/first_boot_rootfs_resizer/first_boot_rootfs_resizer.mk b/package/first_boot_rootfs_resizer/first_boot_rootfs_resizer.mk > new file mode 100644 > index 000000000..8752831ea > --- /dev/null > +++ b/package/first_boot_rootfs_resizer/first_boot_rootfs_resizer.mk > @@ -0,0 +1,23 @@ > +################################################################################ > +# > +# first_boot_rootfs_resizer > +# > +################################################################################ > + > +FIRST_BOOT_ROOTFS_RESIZER_LICENSE = unclear We clearly shouldn't have an "unclear" license for such a package. > +FIRST_BOOT_ROOTFS_RESIZER_PRIO = $(shell printf "%02u" $(BR2_PACKAGE_FIRST_BOOT_ROOTFS_RESIZER_SYSV_PRIORITY)) Don't make the priority configurable. > +++ b/package/first_boot_rootfs_resizer/resize_fs.sh > @@ -0,0 +1,44 @@ > +#!/bin/sh > +### BEGIN INIT INFO > +# Provides: resize_fs.sh > +# Required-Start: $remote_fs $all > +# Required-Stop: > +# Default-Start: 2 3 4 5 S > +# Default-Stop: > +# Short-Description: First boot system setup > +### END INIT INFO Not needed in Buildroot init scripts. > +PATH=/sbin:/usr/sbin:/bin:/usr/bin Not needed. > +ROOT=$(cat /proc/cmdline | tr ' ' '\n' | grep root= | cut -f 2 -d =) This will fail badly if you have a GPT partition table and root= is using the partition UUID, like: root=UUID=xyz > +DEV=$(echo ${ROOT} | sed 's/p[0-9]\+$//g') This only works for mmcblkXpY case, but not for sdXY > +if [ -f /.first_boot ]; then > + echo "Resizing fs, please wait... upon finish the system will be restarted" > + # ok, its the very first boot, we need to resize the disk. > + p2_start=`fdisk -l ${DEV} | grep ${PART} | awk '{print $2}'` > + p2_finish=`fdisk -l ${DEV} | grep sectors | awk '{printf $5}'` This will not work with the Busybox fdisk, which is the default in Buildroot: Disk /dev/sda: 256.0 GB, 256060514304 bytes 255 heads, 63 sectors/track, 31130 cylinders Units = cylinders of 16065 * 512 = 8225280 bytes Device Boot Start End Blocks Id System /dev/sda1 * 1 3648 29295616 83 Linux /dev/sda2 3648 4644 8000512 82 Linux swap /dev/sda3 4644 31131 212761600 83 Linux > + > + fdisk ${DEV} <<EOF > +p > +d > +2 > +n > +p > +2 > +$p2_start > +$p2_finish > +p > +w > +EOF Using sfdisk is probably more appropriate here, as it's meant to be used for scripting. > + rm -fr /.first_boot > + sync > + reboot > +else > + resize2fs ${ROOT} && rm -fr $0 This only works if the root filesystem is ext2/ext3/ext4. Perhaps your package should depend on BR2_TARGET_ROOTFS_EXT2. Or the script should check if the filesystem is really ext2, ext3 or ext4 before proceeding. Also, this logic doesn't work if the root filesystem is mounted read-only, since you expect to be able to remove .first_boot, and remove the script itself. > diff --git a/package/first_boot_rootfs_resizer/resizing b/package/first_boot_rootfs_resizer/resizing > new file mode 100644 > index 000000000..4a19ce40e > --- /dev/null > +++ b/package/first_boot_rootfs_resizer/resizing > @@ -0,0 +1,11 @@ > +#!/bin/sh > + > +case "$1" in > + start) > + if [ -f /usr/sbin/resize_fs.sh ]; then /usr/sbin/resize_fs.sh; fi > + ;; > + *) > + ;; > +esac Why wouldn't the S00resizefs script directly do the resizing work? Best regards, Thomas
Greetings, > Hello, > > On Fri, 2 Dec 2016 12:10:27 +0200, Dagg Stompler wrote: > > this new package will resize the root fs to the max possible when > > booting an image for the first time. > > > > Signed-off-by: Dagg Stompler <daggs@gmx.com> > > I think this is still way too use-case specific, and I'll explain below > why your implementation only works in your specific case, but will fail > in many other cases. > > > diff --git a/package/first_boot_rootfs_resizer/first_boot_rootfs_resizer.mk b/package/first_boot_rootfs_resizer/first_boot_rootfs_resizer.mk > > new file mode 100644 > > index 000000000..8752831ea > > --- /dev/null > > +++ b/package/first_boot_rootfs_resizer/first_boot_rootfs_resizer.mk > > @@ -0,0 +1,23 @@ > > +################################################################################ > > +# > > +# first_boot_rootfs_resizer > > +# > > +################################################################################ > > + > > +FIRST_BOOT_ROOTFS_RESIZER_LICENSE = unclear > > We clearly shouldn't have an "unclear" license for such a package. the file is ported from the odroidc2 buildroot, I've marked it unclear because it doesn't states it's license anywhere. > > > +FIRST_BOOT_ROOTFS_RESIZER_PRIO = $(shell printf "%02u" $(BR2_PACKAGE_FIRST_BOOT_ROOTFS_RESIZER_SYSV_PRIORITY)) > > Don't make the priority configurable. ok. > > > +++ b/package/first_boot_rootfs_resizer/resize_fs.sh > > @@ -0,0 +1,44 @@ > > +#!/bin/sh > > +### BEGIN INIT INFO > > +# Provides: resize_fs.sh > > +# Required-Start: $remote_fs $all > > +# Required-Stop: > > +# Default-Start: 2 3 4 5 S > > +# Default-Stop: > > +# Short-Description: First boot system setup > > +### END INIT INFO > > Not needed in Buildroot init scripts. > > > +PATH=/sbin:/usr/sbin:/bin:/usr/bin > > Not needed. will remove. > > > +ROOT=$(cat /proc/cmdline | tr ' ' '\n' | grep root= | cut -f 2 -d =) > > This will fail badly if you have a GPT partition table and root= is > using the partition UUID, like: > > root=UUID=xyz > I see, that should be the proper way to detect the rootfs file system? > > +DEV=$(echo ${ROOT} | sed 's/p[0-9]\+$//g') > > This only works for mmcblkXpY case, but not for sdXY correct, will see how I can generalize it more. > > > +if [ -f /.first_boot ]; then > > + echo "Resizing fs, please wait... upon finish the system will be restarted" > > + # ok, its the very first boot, we need to resize the disk. > > + p2_start=`fdisk -l ${DEV} | grep ${PART} | awk '{print $2}'` > > + p2_finish=`fdisk -l ${DEV} | grep sectors | awk '{printf $5}'` > > This will not work with the Busybox fdisk, which is the default in > Buildroot: > > Disk /dev/sda: 256.0 GB, 256060514304 bytes > 255 heads, 63 sectors/track, 31130 cylinders > Units = cylinders of 16065 * 512 = 8225280 bytes > > Device Boot Start End Blocks Id System > /dev/sda1 * 1 3648 29295616 83 Linux > /dev/sda2 3648 4644 8000512 82 Linux swap > /dev/sda3 4644 31131 212761600 83 Linux I'm pretty sure I've tested it with busybox's fdisk. > > > + > > + fdisk ${DEV} <<EOF > > +p > > +d > > +2 > > +n > > +p > > +2 > > +$p2_start > > +$p2_finish > > +p > > +w > > +EOF > > Using sfdisk is probably more appropriate here, as it's meant to be > used for scripting. ok, will check. > > > + rm -fr /.first_boot > > + sync > > + reboot > > +else > > + resize2fs ${ROOT} && rm -fr $0 > > This only works if the root filesystem is ext2/ext3/ext4. Perhaps your > package should depend on BR2_TARGET_ROOTFS_EXT2. Or the script should > check if the filesystem is really ext2, ext3 or ext4 before proceeding. ok, I think it is better for now to limit it to ext2. > > Also, this logic doesn't work if the root filesystem is mounted > read-only, since you expect to be able to remove .first_boot, and > remove the script itself. so essentally, if the fs is read only, there is no way to remove it. so the right way is to check every boot if the fs is maxed out to the boot dev and resize it if not. am I correct? > > > diff --git a/package/first_boot_rootfs_resizer/resizing b/package/first_boot_rootfs_resizer/resizing > > new file mode 100644 > > index 000000000..4a19ce40e > > --- /dev/null > > +++ b/package/first_boot_rootfs_resizer/resizing > > @@ -0,0 +1,11 @@ > > +#!/bin/sh > > + > > +case "$1" in > > + start) > > + if [ -f /usr/sbin/resize_fs.sh ]; then /usr/sbin/resize_fs.sh; fi > > + ;; > > + *) > > + ;; > > +esac > > Why wouldn't the S00resizefs script directly do the resizing work? as said above, I've ported it from the odroidc2 buildroot, so I didn't tried to change it too much. > > Best regards, > > Thomas > -- > Thomas Petazzoni, CTO, Free Electrons > Embedded Linux and Kernel engineering > http://free-electrons.com >
Hello, On Fri, 23 Dec 2016 14:41:59 +0100, daggs wrote: > > We clearly shouldn't have an "unclear" license for such a package. > the file is ported from the odroidc2 buildroot, I've marked it unclear because it doesn't states it's license anywhere. Would be good to get a clarified license, especially when the source code is inside Buildroot itself. > > > +ROOT=$(cat /proc/cmdline | tr ' ' '\n' | grep root= | cut -f 2 -d =) > > > > This will fail badly if you have a GPT partition table and root= is > > using the partition UUID, like: > > > > root=UUID=xyz > > > I see, that should be the proper way to detect the rootfs file system? One way would be to use the Busybox "rdev" applet, but it's not enabled in our default configuration. You could run "stat /" where the "Device" fields tells you the block device that was mounted as the root filesystem. But not that trivial to go from this (major/minor) to the actual /dev/. I'm not sure there's a good and simple way to achieve that I'm afraid. > > > +DEV=$(echo ${ROOT} | sed 's/p[0-9]\+$//g') > > > > This only works for mmcblkXpY case, but not for sdXY > correct, will see how I can generalize it more. It's actually not that easy to generalize, except of course if you restrict yourself to sdXY and mmcblkXpY. > > > +if [ -f /.first_boot ]; then > > > + echo "Resizing fs, please wait... upon finish the system will be restarted" > > > + # ok, its the very first boot, we need to resize the disk. > > > + p2_start=`fdisk -l ${DEV} | grep ${PART} | awk '{print $2}'` > > > + p2_finish=`fdisk -l ${DEV} | grep sectors | awk '{printf $5}'` > > > > This will not work with the Busybox fdisk, which is the default in > > Buildroot: > > > > Disk /dev/sda: 256.0 GB, 256060514304 bytes > > 255 heads, 63 sectors/track, 31130 cylinders > > Units = cylinders of 16065 * 512 = 8225280 bytes > > > > Device Boot Start End Blocks Id System > > /dev/sda1 * 1 3648 29295616 83 Linux > > /dev/sda2 3648 4644 8000512 82 Linux swap > > /dev/sda3 4644 31131 212761600 83 Linux > I'm pretty sure I've tested it with busybox's fdisk. Well, to find the size of the block device, you grep for "sectors". The only line matching with Busybox fdisk is: 255 heads, 63 sectors/track, 31130 cylinders Which doesn't contain the size (at least not the size in sectors) > > Using sfdisk is probably more appropriate here, as it's meant to be > > used for scripting. > ok, will check. But sfdisk is not in Busybox, so make sure to select the right package. > > Also, this logic doesn't work if the root filesystem is mounted > > read-only, since you expect to be able to remove .first_boot, and > > remove the script itself. > so essentally, if the fs is read only, there is no way to remove it. > so the right way is to check every boot if the fs is maxed out to the boot dev and resize it if not. > am I correct? Well, what if you're read-only but the root filesystem doesn't take the whole space? Perhaps you can check in 'mount | grep " / "' whether it's mounted read-only or not? > > Why wouldn't the S00resizefs script directly do the resizing work? > as said above, I've ported it from the odroidc2 buildroot, so I didn't tried to change it too much. Since the source code is part of Buildroot and we don't intend to resync with the odroidc2 stuff, please make it more Buildroot-ish. Thanks a lot! Thomas
Greetings, > > Hello, > > On Fri, 23 Dec 2016 14:41:59 +0100, daggs wrote: > > > > We clearly shouldn't have an "unclear" license for such a package. > > the file is ported from the odroidc2 buildroot, I've marked it unclear because it doesn't states it's license anywhere. > > Would be good to get a clarified license, especially when the source > code is inside Buildroot itself. maybe I'll rewrite it from scratch not depending on this, thus license is irrelevant. > > > > +ROOT=$(cat /proc/cmdline | tr ' ' '\n' | grep root= | cut -f 2 -d =) > > > > > > This will fail badly if you have a GPT partition table and root= is > > > using the partition UUID, like: > > > > > > root=UUID=xyz > > > > > I see, that should be the proper way to detect the rootfs file system? > > One way would be to use the Busybox "rdev" applet, but it's not enabled > in our default configuration. You could run "stat /" where the "Device" > fields tells you the block device that was mounted as the root > filesystem. But not that trivial to go from this (major/minor) to the > actual /dev/. > > I'm not sure there's a good and simple way to achieve that I'm afraid. I'll test the possibilities. > > > > > +DEV=$(echo ${ROOT} | sed 's/p[0-9]\+$//g') > > > > > > This only works for mmcblkXpY case, but not for sdXY > > correct, will see how I can generalize it more. > > It's actually not that easy to generalize, except of course if you > restrict yourself to sdXY and mmcblkXpY. beside sdXY, mmcblkXpY and hdaXY, are there any other possibilities? > > > > > +if [ -f /.first_boot ]; then > > > > + echo "Resizing fs, please wait... upon finish the system will be restarted" > > > > + # ok, its the very first boot, we need to resize the disk. > > > > + p2_start=`fdisk -l ${DEV} | grep ${PART} | awk '{print $2}'` > > > > + p2_finish=`fdisk -l ${DEV} | grep sectors | awk '{printf $5}'` > > > > > > This will not work with the Busybox fdisk, which is the default in > > > Buildroot: > > > > > > Disk /dev/sda: 256.0 GB, 256060514304 bytes > > > 255 heads, 63 sectors/track, 31130 cylinders > > > Units = cylinders of 16065 * 512 = 8225280 bytes > > > > > > Device Boot Start End Blocks Id System > > > /dev/sda1 * 1 3648 29295616 83 Linux > > > /dev/sda2 3648 4644 8000512 82 Linux swap > > > /dev/sda3 4644 31131 212761600 83 Linux > > I'm pretty sure I've tested it with busybox's fdisk. > > Well, to find the size of the block device, you grep for "sectors". The > only line matching with Busybox fdisk is: > > 255 heads, 63 sectors/track, 31130 cylinders > > Which doesn't contain the size (at least not the size in sectors) > > > > Using sfdisk is probably more appropriate here, as it's meant to be > > > used for scripting. > > ok, will check. > > But sfdisk is not in Busybox, so make sure to select the right package. will see what I can do. > > > > Also, this logic doesn't work if the root filesystem is mounted > > > read-only, since you expect to be able to remove .first_boot, and > > > remove the script itself. > > so essentally, if the fs is read only, there is no way to remove it. > > so the right way is to check every boot if the fs is maxed out to the boot dev and resize it if not. > > am I correct? > > Well, what if you're read-only but the root filesystem doesn't take the > whole space? Perhaps you can check in 'mount | grep " / "' whether it's > mounted read-only or not? not sure I follow, can you please elaborate more? > > > > Why wouldn't the S00resizefs script directly do the resizing work? > > as said above, I've ported it from the odroidc2 buildroot, so I didn't tried to change it too much. > > Since the source code is part of Buildroot and we don't intend to > resync with the odroidc2 stuff, please make it more Buildroot-ish. > > Thanks a lot! > > Thomas Dagg.
Hello, On Friday 02 December 2016 12:10:27 daggs wrote: [...] > + p2_start=`fdisk -l ${DEV} | grep ${PART} | awk '{print $2}'` > + p2_finish=`fdisk -l ${DEV} | grep sectors | awk '{printf $5}'` > + > + fdisk ${DEV} <<EOF > +p > +d > +2 > +n > +p > +2 > +$p2_start > +$p2_finish > +p > +w > +EOF This script always resize 2nd partition, isn't? Is it the expected behavior? I think it should resize last partition. For information, I wrote a similar script for a customer some time ago: for part in 4 3 2 1; do for file in ${device}${part} ${device}p${part}; do if [ -e $file ]; then echo ',+,' | sfdisk -q -L -D -uM -N $part $device [ $? -eq 0 ] || exit 1 resize2fs -p $file [ $? -eq 0 ] || exit 1 e2fsck -f $file [ $? -le 1 ] || exit 1 exit 0 fi done done Also note, we did use this script during flash procedure, not during first boot. Therefore, we do not need to detect first boot (and I think it is safer). BR,
Greetings, > Hello, > > On Friday 02 December 2016 12:10:27 daggs wrote: > [...] > > + p2_start=`fdisk -l ${DEV} | grep ${PART} | awk '{print $2}'` > > + p2_finish=`fdisk -l ${DEV} | grep sectors | awk '{printf $5}'` > > + > > + fdisk ${DEV} <<EOF > > +p > > +d > > +2 > > +n > > +p > > +2 > > +$p2_start > > +$p2_finish > > +p > > +w > > +EOF > > This script always resize 2nd partition, isn't? Is it the expected > behavior? I think it should resize last partition. > > For information, I wrote a similar script for a customer some time ago: > > for part in 4 3 2 1; do > for file in ${device}${part} ${device}p${part}; do > if [ -e $file ]; then > echo ',+,' | sfdisk -q -L -D -uM -N $part $device > [ $? -eq 0 ] || exit 1 > resize2fs -p $file > [ $? -eq 0 ] || exit 1 > e2fsck -f $file > [ $? -le 1 ] || exit 1 > exit 0 > fi > done > done > > Also note, we did use this script during flash procedure, not during > first boot. Therefore, we do not need to detect first boot (and I think > it is safer). > > BR, > Thanks, will take that into account on later stage. Dagg.
diff --git a/package/Config.in b/package/Config.in index 9ed296f2d..ed414fd3d 100644 --- a/package/Config.in +++ b/package/Config.in @@ -1408,6 +1408,7 @@ menu "Miscellaneous" source "package/collectd/Config.in" source "package/domoticz/Config.in" source "package/empty/Config.in" + source "package/first_boot_rootfs_resizer/Config.in" source "package/gnuradio/Config.in" source "package/googlefontdirectory/Config.in" source "package/gr-osmosdr/Config.in" diff --git a/package/first_boot_rootfs_resizer/Config.in b/package/first_boot_rootfs_resizer/Config.in new file mode 100644 index 000000000..cce62dd39 --- /dev/null +++ b/package/first_boot_rootfs_resizer/Config.in @@ -0,0 +1,17 @@ +config BR2_PACKAGE_FIRST_BOOT_ROOTFS_RESIZER + bool "first_boot_rootfs_resizer" + select BR2_PACKAGE_E2FSPROGS # for resizefs + select BR2_PACKAGE_E2FSPROGS_RESIZE2FS # for resizefs + depends on BR2_USE_WCHAR # util-linux + + help + installs a script that will resize the rootfs to the max possible consecutive size + unused from the end of rootfs. + +config BR2_PACKAGE_FIRST_BOOT_ROOTFS_RESIZER_SYSV_PRIORITY + int "SystemV priority" + depends on BR2_PACKAGE_FIRST_BOOT_ROOTFS_RESIZER + default "0" + + help + set the script's priority (defualt is 00) diff --git a/package/first_boot_rootfs_resizer/first_boot_rootfs_resizer.mk b/package/first_boot_rootfs_resizer/first_boot_rootfs_resizer.mk new file mode 100644 index 000000000..8752831ea --- /dev/null +++ b/package/first_boot_rootfs_resizer/first_boot_rootfs_resizer.mk @@ -0,0 +1,23 @@ +################################################################################ +# +# first_boot_rootfs_resizer +# +################################################################################ + +FIRST_BOOT_ROOTFS_RESIZER_LICENSE = unclear + +FIRST_BOOT_ROOTFS_RESIZER_PRIO = $(shell printf "%02u" $(BR2_PACKAGE_FIRST_BOOT_ROOTFS_RESIZER_SYSV_PRIORITY)) +FIRST_BOOT_ROOTFS_RESIZER_DEPENDENCIES = e2fsprogs + +define FIRST_BOOT_ROOTFS_RESIZER_INSTALL_TARGET_CMDS + $(INSTALL) -D -m 755 package/first_boot_rootfs_resizer/resize_fs.sh \ + $(TARGET_DIR)/usr/sbin/resize_fs.sh + touch $(TARGET_DIR)/.first_boot +endef + +define FIRST_BOOT_ROOTFS_RESIZER_INSTALL_INIT_SYSV + $(INSTALL) -D -m 755 package/first_boot_rootfs_resizer/resizing \ + $(TARGET_DIR)/etc/init.d/S$(FIRST_BOOT_ROOTFS_RESIZER_PRIO)resizing +endef + +$(eval $(generic-package)) diff --git a/package/first_boot_rootfs_resizer/resize_fs.sh b/package/first_boot_rootfs_resizer/resize_fs.sh new file mode 100644 index 000000000..be41753d8 --- /dev/null +++ b/package/first_boot_rootfs_resizer/resize_fs.sh @@ -0,0 +1,44 @@ +#!/bin/sh +### BEGIN INIT INFO +# Provides: resize_fs.sh +# Required-Start: $remote_fs $all +# Required-Stop: +# Default-Start: 2 3 4 5 S +# Default-Stop: +# Short-Description: First boot system setup +### END INIT INFO + +PATH=/sbin:/usr/sbin:/bin:/usr/bin +ROOT=$(cat /proc/cmdline | tr ' ' '\n' | grep root= | cut -f 2 -d =) +DEV=$(echo ${ROOT} | sed 's/p[0-9]\+$//g') +PART=$(basename ${ROOT}) + +if [ ! -z "${ROOT}" ]; then + echo "unable to locate root= entry in cmdline, resizing aborted" + exit 1 +fi + +if [ -f /.first_boot ]; then + echo "Resizing fs, please wait... upon finish the system will be restarted" + # ok, its the very first boot, we need to resize the disk. + p2_start=`fdisk -l ${DEV} | grep ${PART} | awk '{print $2}'` + p2_finish=`fdisk -l ${DEV} | grep sectors | awk '{printf $5}'` + + fdisk ${DEV} <<EOF +p +d +2 +n +p +2 +$p2_start +$p2_finish +p +w +EOF + rm -fr /.first_boot + sync + reboot +else + resize2fs ${ROOT} && rm -fr $0 +fi diff --git a/package/first_boot_rootfs_resizer/resizing b/package/first_boot_rootfs_resizer/resizing new file mode 100644 index 000000000..4a19ce40e --- /dev/null +++ b/package/first_boot_rootfs_resizer/resizing @@ -0,0 +1,11 @@ +#!/bin/sh + +case "$1" in + start) + if [ -f /usr/sbin/resize_fs.sh ]; then /usr/sbin/resize_fs.sh; fi + ;; + *) + ;; +esac + +exit $?
this new package will resize the root fs to the max possible when booting an image for the first time. Signed-off-by: Dagg Stompler <daggs@gmx.com> --- package/Config.in | 1 + package/first_boot_rootfs_resizer/Config.in | 17 +++++++++ .../first_boot_rootfs_resizer.mk | 23 +++++++++++ package/first_boot_rootfs_resizer/resize_fs.sh | 44 ++++++++++++++++++++++ package/first_boot_rootfs_resizer/resizing | 11 ++++++ 5 files changed, 96 insertions(+) create mode 100644 package/first_boot_rootfs_resizer/Config.in create mode 100644 package/first_boot_rootfs_resizer/first_boot_rootfs_resizer.mk create mode 100644 package/first_boot_rootfs_resizer/resize_fs.sh create mode 100644 package/first_boot_rootfs_resizer/resizing