From mboxrd@z Thu Jan  1 00:00:00 1970
From: Michael Tremer <michael.tremer@ipfire.org>
To: development@lists.ipfire.org
Subject: Re: [PATCH 2/2] grub-btrfs: New package
Date: Mon, 25 Mar 2024 10:38:47 +0000
Message-ID: <D08FA2D0-9361-40D6-9CBF-6C48DE0BF809@ipfire.org>
In-Reply-To: <20240324123953.633474-2-stefan.schantl@ipfire.org>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============3497638411549691364=="
List-Id: <development.lists.ipfire.org>

--===============3497638411549691364==
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

Hello,

> On 24 Mar 2024, at 12:39, Stefan Schantl <stefan.schantl(a)ipfire.org> wrot=
e:
>=20
> This kind of grub addon will extend the grub boot menu by a additional
> submenu where a BTRFS snapshot can be selected to directly use as root
> volume and boot into it.
>=20
> The grub-btrfsd daemon is using inotify(tools) to watch the snapshot direct=
ory for
> new or deleted snapshots and calls grub-mkconfig to adjust the snapshot gru=
b submenu
>=20
> Signed-off-by: Stefan Schantl <stefan.schantl(a)ipfire.org>
> ---
> config/rootfiles/common/grub-btrfs         | 14 ++++
> config/rootfiles/common/x86_64/initscripts |  4 ++
> lfs/grub-btrfs                             | 79 ++++++++++++++++++++++
> lfs/initscripts                            |  3 +
> make.sh                                    |  1 +
> src/initscripts/system/grub-btrfsd         | 61 +++++++++++++++++
> 6 files changed, 162 insertions(+)
> create mode 100644 config/rootfiles/common/grub-btrfs
> create mode 100644 lfs/grub-btrfs
> create mode 100644 src/initscripts/system/grub-btrfsd
>=20
> diff --git a/config/rootfiles/common/grub-btrfs b/config/rootfiles/common/g=
rub-btrfs
> new file mode 100644
> index 000000000..a612af065
> --- /dev/null
> +++ b/config/rootfiles/common/grub-btrfs
> @@ -0,0 +1,14 @@
> +boot/grub/grubenv
> +etc/default/grub-btrfs
> +etc/default/grub-btrfs/config
> +etc/grub.d/41_snapshots-btrfs
> +usr/bin/grub-btrfsd
> +#usr/lib/systemd/system/grub-btrfsd.service
> +#usr/share/doc/grub-btrfs
> +#usr/share/doc/grub-btrfs/README.md
> +#usr/share/doc/grub-btrfs/initramfs-overlayfs.md
> +#usr/share/licenses
> +#usr/share/licenses/grub-btrfs
> +#usr/share/licenses/grub-btrfs/LICENSE
> +#usr/share/man/man8/grub-btrfs.8.bz2
> +#usr/share/man/man8/grub-btrfsd.8.bz2
> diff --git a/config/rootfiles/common/x86_64/initscripts b/config/rootfiles/=
common/x86_64/initscripts
> index a5e1596fb..2d6fa85dc 100644
> --- a/config/rootfiles/common/x86_64/initscripts
> +++ b/config/rootfiles/common/x86_64/initscripts
> @@ -26,6 +26,7 @@ etc/rc.d/init.d/firewall
> etc/rc.d/init.d/firstsetup
> etc/rc.d/init.d/fsresize
> etc/rc.d/init.d/functions
> +etc/rc.d/init.d/grub-btrfsd
> etc/rc.d/init.d/halt
> etc/rc.d/init.d/ipsec
> etc/rc.d/init.d/leds
> @@ -92,6 +93,7 @@ etc/rc.d/init.d/vnstat
> etc/rc.d/init.d/waitdrives
> etc/rc.d/init.d/wlanclient
> #etc/rc.d/rc0.d
> +etc/rc.d/rc0.d/K01grub-btrfsd
> #etc/rc.d/rc0.d/K01imspetor
> #etc/rc.d/rc0.d/K01motion
> #etc/rc.d/rc0.d/K01vdradmin
> @@ -135,10 +137,12 @@ etc/rc.d/rc3.d/S32apache
> etc/rc.d/rc3.d/S40fcron
> etc/rc.d/rc3.d/S98rc.local
> #etc/rc.d/rc3.d/S98sslh
> +etc/rc.d/rc3.d/S99grub-btrfsd
> #etc/rc.d/rc3.d/S99imspetor
> #etc/rc.d/rc3.d/S99motion
> #etc/rc.d/rc3.d/S99vdradmin
> #etc/rc.d/rc6.d
> +etc/rc.d/rc6.d/K01grub-btrfsd
> #etc/rc.d/rc6.d/K01imspetor
> #etc/rc.d/rc6.d/K01motion
> #etc/rc.d/rc6.d/K01vdradmin
> diff --git a/lfs/grub-btrfs b/lfs/grub-btrfs
> new file mode 100644
> index 000000000..8dc0c4f3e
> --- /dev/null
> +++ b/lfs/grub-btrfs
> @@ -0,0 +1,79 @@
> +##########################################################################=
#####
> +#                                                                         =
    #
> +# IPFire.org - A linux based firewall                                     =
    #
> +# Copyright (C) 2007-2023  IPFire Team  <info(a)ipfire.org>               =
      #
> +#                                                                         =
    #
> +# 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 3 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, see <http://www.gnu.org/licenses/>.   =
    #
> +#                                                                         =
    #
> +##########################################################################=
#####
> +
> +##########################################################################=
#####
> +# Definitions
> +##########################################################################=
#####
> +
> +include Config
> +
> +VER        =3D 4.13
> +
> +THISAPP    =3D grub-btrfs-$(VER)
> +DL_FILE    =3D $(THISAPP).tar.gz
> +DL_FROM    =3D $(URL_IPFIRE)
> +DIR_APP    =3D $(DIR_SRC)/$(THISAPP)
> +TARGET     =3D $(DIR_INFO)/$(THISAPP)
> +
> +##########################################################################=
#####
> +# Top-level Rules
> +##########################################################################=
#####
> +
> +objects =3D $(DL_FILE)
> +
> +$(DL_FILE) =3D $(DL_FROM)/$(DL_FILE)
> +
> +$(DL_FILE)_BLAKE2 =3D e2f11a0a8282e3ec8ff8223e3bad70b5d5c5e81b4d740a1bf355=
4db412dbe48a8a0f216f4e6c65ae1d11c01a27b8b92bae9b470c60d2389505ce089511536e1a
> +
> +install : $(TARGET)
> +
> +check : $(patsubst %,$(DIR_CHK)/%,$(objects))
> +
> +download :$(patsubst %,$(DIR_DL)/%,$(objects))
> +
> +b2 : $(subst %,%_BLAKE2,$(objects))
> +
> +dist:
> + @$(PAK)
> +
> +##########################################################################=
#####
> +# Downloading, checking, b2sum
> +##########################################################################=
#####
> +
> +$(patsubst %,$(DIR_CHK)/%,$(objects)) :
> + @$(CHECK)
> +
> +$(patsubst %,$(DIR_DL)/%,$(objects)) :
> + @$(LOAD)
> +
> +$(subst %,%_BLAKE2,$(objects)) :
> + @$(B2SUM)
> +
> +##########################################################################=
#####
> +# Installation Details
> +##########################################################################=
#####
> +
> +$(TARGET) : $(patsubst %,$(DIR_DL)/%,$(objects))
> + @$(PREBUILD)
> + @rm -rf $(DIR_APP) && cd $(DIR_SRC) && tar axf $(DIR_DL)/$(DL_FILE)
> + $(UPDATE_AUTOMAKE)
> + cd $(DIR_APP) && make install
> + @rm -rf $(DIR_APP)
> + @$(POSTBUILD)
> diff --git a/lfs/initscripts b/lfs/initscripts
> index e078632ab..4d67ee0c4 100644
> --- a/lfs/initscripts
> +++ b/lfs/initscripts
> @@ -91,6 +91,7 @@ $(TARGET) :
> install -v -m 755 $$i /etc/rc.d/helper/; \
> done
>=20
> + ln -sf ../init.d/grub-btrfsd /etc/rc.d/rc0.d/K01grub-btrfsd
> ln -sf ../init.d/imspetor /etc/rc.d/rc0.d/K01imspetor
> ln -sf ../init.d/motion /etc/rc.d/rc0.d/K01motion
> ln -sf ../init.d/vdradmin /etc/rc.d/rc0.d/K01vdradmin
> @@ -132,10 +133,12 @@ $(TARGET) :
> ln -sf ../init.d/fcron /etc/rc.d/rc3.d/S40fcron
> ln -sf ../../sysconfig/rc.local /etc/rc.d/rc3.d/S98rc.local
> ln -sf ../init.d/sslh /etc/rc.d/rc3.d/S98sslh
> + ln -sf ../init.d/grub-btrfsd /etc/rc.d/rc3.d/S99grub-btrfsd
> ln -sf ../init.d/imspetor /etc/rc.d/rc3.d/S99imspetor
> ln -sf ../init.d/motion /etc/rc.d/rc3.d/S99motion
> ln -sf ../init.d/vdradmin /etc/rc.d/rc3.d/S99vdradmin
>=20
> + ln -sf ../init.d/grub-btrfsd /etc/rc.d/rc6.d/K01grub-btrfsd
> ln -sf ../init.d/imspetor /etc/rc.d/rc6.d/K01imspetor
> ln -sf ../init.d/motion /etc/rc.d/rc6.d/K01motion
> ln -sf ../init.d/vdradmin /etc/rc.d/rc6.d/K01vdradmin
> diff --git a/make.sh b/make.sh
> index 1834b927a..adc03f4e6 100755
> --- a/make.sh
> +++ b/make.sh
> @@ -1714,6 +1714,7 @@ buildipfire() {
>   lfsmake2 wsdd
>   lfsmake2 btrfs-progs
>   lfsmake2 inotify-tools
> +  lfsmake2 grub-btrfs
>=20
>   # Kernelbuild ... current we have no platform that need
>   # multi kernel builds so KCFG is empty
> diff --git a/src/initscripts/system/grub-btrfsd b/src/initscripts/system/gr=
ub-btrfsd
> new file mode 100644
> index 000000000..dc5e32ad4
> --- /dev/null
> +++ b/src/initscripts/system/grub-btrfsd
> @@ -0,0 +1,61 @@
> +#!/bin/sh
> +##########################################################################=
#####
> +#                                                                         =
    #
> +# IPFire.org - A linux based firewall                                     =
    #
> +# Copyright (C) 2007-2024  IPFire Team  <info(a)ipfire.org>               =
      #
> +#                                                                         =
    #
> +# 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 3 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, see <http://www.gnu.org/licenses/>.   =
    #
> +#                                                                         =
    #
> +##########################################################################=
#####
> +
> +. /etc/sysconfig/rc
> +. ${rc_functions}
> +
> +SNAPSHOTDIR=3D"/.snapshots"
> +PIDFILE=3D"/run/grub-btrfsd.pid"
> +
> +root_is_btrfs() {
> +    [ "$(stat -f --format=3D"%T" /)" =3D=3D "btrfs" ] || return 1
> +
> +    return 0
> +}

I think we should have this function available in the =E2=80=9Cfunctions=E2=
=80=9D file as I can see other things needing this, too.

Recently I have been touching a lot of code that had the same functionality i=
mplemented several times (at least two, sometimes even three or four times) w=
hich causes many problems. So I would like to be more strict in what we accep=
t in the code so that things don=E2=80=99t become even messier than they are =
now. Not that this function here is the biggest problem, but I have been put =
in a really bad mood about a lot of OpenVPN and web UI code=E2=80=A6

I think we can even solve this a lot smarter. Can we have the function simply=
 return the filesystem type of the root filesystem? You can then write code l=
ike this:

case =E2=80=9C$(root_fs_type)=E2=80=9D in
  btrfs)
    =E2=80=A6
    ;;
esac

If there is any necessity to deal with other filesystem the case could match =
something else, too without introducing another function for another filesyst=
em.

I think this could already become handy when deprecating reiserfs.

This would become absolute gold if you would accept the mountpoint so that we=
 can even check for other filesystems as they might use reiferfs, too.

Best,
-Michael

> +
> +case "$1" in
> +        start)
> + root_is_btrfs || exit 0
> +
> + boot_mesg "Starting Grub/Btrfs snapshot manager..."
> + loadproc -b -p "$PIDFILE" /usr/bin/grub-btrfsd --syslog "$SNAPSHOTDIR"
> +
> + # Store the gained PID to the defined pidfile
> + echo "$!" > "$PIDFILE"

loadproc should do this automatically already. If not, then something is brok=
en.

It is also enough to define PIDFILE globally as loadproc/killproc etc. will a=
utomatically use it.

> + ;;
> +
> + stop)
> + boot_mesg "Stopping grub-btrfsd..."
> + killproc -p "$PIDFILE" /usr/bin/grub-btrfsd
> + sleep 1;
> + ;;
> +
> + restart)
> + $0 stop
> + sleep 2
> + $0 start
> + ;;
> +
> + *)
> + echo "Usage: $0 {start|stop|restart}"
> + exit 1
> + ;;
> +esac
> --=20
> 2.39.2
>=20


--===============3497638411549691364==--