From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michael Tremer To: development@lists.ipfire.org Subject: Re: [PATCH 05/17] installer: Add code to create a BTRFS subvolume layout. Date: Mon, 18 Mar 2024 16:09:25 +0000 Message-ID: <1CB5662A-5B1D-4DEF-A655-EA72B9E6B9F7@ipfire.org> In-Reply-To: <20240315191442.3951-6-stefan.schantl@ipfire.org> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1049603496697888341==" List-Id: --===============1049603496697888341== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable > On 15 Mar 2024, at 19:14, Stefan Schantl wrot= e: >=20 > Signed-off-by: Stefan Schantl > --- > src/installer/hw.c | 70 ++++++++++++++++++++++++++++++++++++++++++++-- > src/installer/hw.h | 5 ++++ > 2 files changed, 73 insertions(+), 2 deletions(-) >=20 > diff --git a/src/installer/hw.c b/src/installer/hw.c > index 81be87471..420feaca7 100644 > --- a/src/installer/hw.c > +++ b/src/installer/hw.c > @@ -44,6 +44,20 @@ >=20 > #include "hw.h" >=20 > +// Array which contains the subvolumes which will be created when installi= ng > +// IPFire on a BTRFS. > +const char* btrfs_subvolumes[7][2] =3D { > + {"@root" ,"/"}, > + {"@snapshots", "/.snapshots"}, > + {"@home", "/home"}, > + {"@cache", "/var/cache"}, > + {"@lib", "/var/lib"}, > + {"@logs", "/var/log"}, > + {"@mails", "/var/mail"} > +}; I would prefer to not allocate these arrays to a specific size. You leave the= square brackets empty and the compiler will figure out how much space it nee= ds. That avoids any problems when something is being added later. You should also NULL-terminate the array. > + > +#define LEN(arr) ((int) (sizeof (arr) / sizeof (arr)[0])) > + > static int system_chroot(const char* output, const char* path, const char* = cmd) { > char chroot_cmd[STRING_SIZE]; >=20 > @@ -805,6 +819,7 @@ int hw_create_partitions(struct hw_destination* dest, c= onst char* output) { >=20 > static int hw_format_filesystem(const char* path, int fs, const char* outpu= t) { > char cmd[STRING_SIZE] =3D "\0"; > + int r; >=20 > // Swap > if (fs =3D=3D HW_FS_SWAP) { > @@ -824,7 +839,9 @@ static int hw_format_filesystem(const char* path, int f= s, const char* output) { >=20 > // BTRFS > } else if (fs =3D=3D HW_FS_BTRFS) { > - snprintf(cmd, sizeof(cmd), "/usr/bin/mkfs.btrfs -L rootfs -f %s", path); > + r =3D hw_create_btrfs_layout(path, output); > + > + return r; >=20 > // FAT32 > } else if (fs =3D=3D HW_FS_FAT32) { > @@ -833,7 +850,7 @@ static int hw_format_filesystem(const char* path, int f= s, const char* output) { >=20 > assert(*cmd); >=20 > - int r =3D mysystem(output, cmd); > + r =3D mysystem(output, cmd); >=20 > return r; > } > @@ -870,6 +887,43 @@ int hw_create_filesystems(struct hw_destination* dest,= const char* output) { > return 0; > } >=20 > +int hw_create_btrfs_layout(const char* path, const char* output) { > + char cmd[STRING_SIZE]; > + char subvolume[STRING_SIZE]; > + > + // Create the main BTRFS. > + snprintf(cmd, sizeof(cmd), "/usr/bin/mkfs.btrfs -L IPFire -f %s", path); Again, you are using a label here. Is that a requirement? > + int r =3D mysystem(output, cmd); > + > + if (r) > + return r; > + > + // We need to mount the FS in order to create any subvolumes. > + r =3D hw_mount(path, DESTINATION_MOUNT_PATH, "btrfs", 0); > + > + if (r) > + return r; > + > + // Loop through the array of subvolumes to create. > + for ( int i =3D 0; i < LEN(btrfs_subvolumes); i++ ) { If you NULL-terminate your array, you won=E2=80=99t need the LEN() macro whic= h I personally don=E2=80=99t consider good style. > + snprintf(subvolume, sizeof(subvolume), "%s", btrfs_subvolumes[i][0]); I know that the installer isn=E2=80=99t doing that very much, but you should = check the return code of snprintf(). > + // Call function to create the subvolume > + r =3D hw_create_btrfs_subvolume(output, subvolume); > + > + if (r) > + return r; > + } > + > + // Umount the main BTRFS after subvolume creation. > + r =3D hw_umount(path, 0); > + > + if (r) > + return r; > + > + return 0; > +} > + > int hw_mount_filesystems(struct hw_destination* dest, const char* prefix) { > char target[STRING_SIZE]; >=20 > @@ -1219,3 +1273,15 @@ int hw_restore_backup(const char* output, const char= * backup_path, const char* d >=20 > return 0; > } > + > +int hw_create_btrfs_subvolume(const char* output, const char* subvolume) { > + char command [STRING_SIZE]; > + snprintf(command, sizeof(command), "/usr/bin/btrfs subvolume create %s/%= s", DESTINATION_MOUNT_PATH, subvolume); Likewise. > + > + int r =3D mysystem(output, command); > + > + if (r) > + return -1; > + > + return 0; > +} > diff --git a/src/installer/hw.h b/src/installer/hw.h > index e5ee65a6d..2de73a3be 100644 > --- a/src/installer/hw.h > +++ b/src/installer/hw.h > @@ -104,6 +104,8 @@ struct hw_destination { > unsigned long long size_root; > }; >=20 > +extern const char* btrfs_subvolumes[][2]; Please declare the array statically and you won=E2=80=99t need this. > + > struct hw* hw_init(); > void hw_free(struct hw* hw); >=20 > @@ -143,4 +145,7 @@ int hw_start_networking(const char* output); >=20 > void hw_sync(); >=20 > +int hw_create_btrfs_layout(const char* output, const char* subvolume); > +int hw_create_btrfs_subvolume(const char* output, const char* subvolume); > + > #endif /* HEADER_HW_H */ > --=20 > 2.39.2 >=20 --===============1049603496697888341==--