All of lore.kernel.org
 help / color / mirror / Atom feed
From: daggs <daggs@gmx.com>
To: buildroot@busybox.net
Subject: [Buildroot] [PATCH 1/2] first_boot_rootfs_resizer: New Package.
Date: Fri, 23 Dec 2016 14:41:59 +0100	[thread overview]
Message-ID: <trinity-0083f4f6-b9ec-4e8f-83e5-87b64224abf7-1482500518932@3capp-mailcom-bs05> (raw)
In-Reply-To: <20161217165039.6169d54c@free-electrons.com>

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
> 

  reply	other threads:[~2016-12-23 13:41 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-02 10:10 [Buildroot] [PATCH 1/2] first_boot_rootfs_resizer: New Package Dagg Stompler
2016-12-02 10:10 ` [Buildroot] [PATCH 2/2] odroidc2: use first boot fs resize pkg Dagg Stompler
2016-12-17 15:51   ` Thomas Petazzoni
2016-12-23 13:43     ` daggs
2016-12-17 15:50 ` [Buildroot] [PATCH 1/2] first_boot_rootfs_resizer: New Package Thomas Petazzoni
2016-12-23 13:41   ` daggs [this message]
2016-12-23 14:00     ` Thomas Petazzoni
2016-12-23 19:08       ` daggs
2017-01-03 11:39 ` [Buildroot] [1/2] " Jérôme Pouiller
2017-02-25 10:54   ` daggs

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=trinity-0083f4f6-b9ec-4e8f-83e5-87b64224abf7-1482500518932@3capp-mailcom-bs05 \
    --to=daggs@gmx.com \
    --cc=buildroot@busybox.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.