[08/17] installer: Mount BTRFS layout before installing the system

Message ID 20240315191442.3951-9-stefan.schantl@ipfire.org
State Superseded
Headers
Series BTRFS support on IPFire 2.x (experimental) |

Commit Message

Stefan Schantl March 15, 2024, 7:14 p.m. UTC
  Signed-off-by: Stefan Schantl <stefan.schantl@ipfire.org>
---
 src/installer/hw.c | 48 +++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 45 insertions(+), 3 deletions(-)
  

Comments

Michael Tremer March 18, 2024, 4:11 p.m. UTC | #1
> On 15 Mar 2024, at 19:14, Stefan Schantl <stefan.schantl@ipfire.org> wrote:
> 
> Signed-off-by: Stefan Schantl <stefan.schantl@ipfire.org>
> ---
> src/installer/hw.c | 48 +++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 45 insertions(+), 3 deletions(-)
> 
> diff --git a/src/installer/hw.c b/src/installer/hw.c
> index cb6a8ab81..e520fb687 100644
> --- a/src/installer/hw.c
> +++ b/src/installer/hw.c
> @@ -926,6 +926,7 @@ int hw_create_btrfs_layout(const char* path, const char* output) {
> 
> int hw_mount_filesystems(struct hw_destination* dest, const char* prefix) {
> char target[STRING_SIZE];
> + int r;
> 
> assert(*prefix == '/');
> 
> @@ -953,9 +954,17 @@ int hw_mount_filesystems(struct hw_destination* dest, const char* prefix) {
> }
> 
> // root
> - int r = hw_mount(dest->part_root, prefix, filesystem, 0);
> - if (r)
> - return r;
> + if (dest->filesystem == HW_FS_BTRFS) {
> + r = hw_mount_btrfs_subvolumes(dest->part_root);
> +
> + if (r)
> + return r;
> + } else {
> + r = hw_mount(dest->part_root, prefix, filesystem, 0);
> +
> + if (r)
> + return r;
> + }
> 
> // boot
> snprintf(target, sizeof(target), "%s%s", prefix, HW_PATH_BOOT);
> @@ -1019,6 +1028,39 @@ int hw_mount_filesystems(struct hw_destination* dest, const char* prefix) {
> return 0;
> }
> 
> +int hw_mount_btrfs_subvolumes(const char* source) {
> + int r;
> + char path[STRING_SIZE];
> + char options[STRING_SIZE];
> +
> + // Mount the root subvolume
> + snprintf(options, sizeof(options), "subvol=%s,%s", btrfs_subvolumes[0][0], BTRFS_MOUNT_OPTIONS);
> + r = mount(source, DESTINATION_MOUNT_PATH, "btrfs", NULL, options);

Why is this not inside the loop when it is just a regular subvolume?

> +
> + if (r)
> + return r;
> +
> + // Loop through the remain array of known subvolumes.
> + for ( int i = 1; i < LEN(btrfs_subvolumes); i++ ) {
> + snprintf(path, sizeof(path), "%s%s", DESTINATION_MOUNT_PATH, btrfs_subvolumes[i][1]);
> + snprintf(options, sizeof(options), "subvol=%s,", btrfs_subvolumes[i][0], BTRFS_MOUNT_OPTIONS);

Likewise, please check the return code of snprintf.

> +
> + // Create the directory.
> + r = hw_mkdir(path, S_IRWXU|S_IRWXG|S_IRWXO);
> +
> + if(r != 0 && errno != EEXIST)
> + return r;
> +
> + // Try to mount the subvolume.
> + r = mount(source, path, "btrfs", NULL, options);
> +
> + if (r)
> + return r;
> + }
> + 
> + return 0;
> +}
> +
> int hw_umount_filesystems(struct hw_destination* dest, const char* prefix) {
> int r;
> char target[STRING_SIZE];
> -- 
> 2.39.2
>
  
Stefan Schantl March 19, 2024, 8:09 p.m. UTC | #2
Am Montag, dem 18.03.2024 um 16:11 +0000 schrieb Michael Tremer:
> 
> 
> > On 15 Mar 2024, at 19:14, Stefan Schantl
> > <stefan.schantl@ipfire.org> wrote:
> > 
> > Signed-off-by: Stefan Schantl <stefan.schantl@ipfire.org>
> > ---
> > src/installer/hw.c | 48
> > +++++++++++++++++++++++++++++++++++++++++++---
> > 1 file changed, 45 insertions(+), 3 deletions(-)
> > 
> > diff --git a/src/installer/hw.c b/src/installer/hw.c
> > index cb6a8ab81..e520fb687 100644
> > --- a/src/installer/hw.c
> > +++ b/src/installer/hw.c
> > @@ -926,6 +926,7 @@ int hw_create_btrfs_layout(const char* path,
> > const char* output) {
> > 
> > int hw_mount_filesystems(struct hw_destination* dest, const char*
> > prefix) {
> > char target[STRING_SIZE];
> > + int r;
> > 
> > assert(*prefix == '/');
> > 
> > @@ -953,9 +954,17 @@ int hw_mount_filesystems(struct
> > hw_destination* dest, const char* prefix) {
> > }
> > 
> > // root
> > - int r = hw_mount(dest->part_root, prefix, filesystem, 0);
> > - if (r)
> > - return r;
> > + if (dest->filesystem == HW_FS_BTRFS) {
> > + r = hw_mount_btrfs_subvolumes(dest->part_root);
> > +
> > + if (r)
> > + return r;
> > + } else {
> > + r = hw_mount(dest->part_root, prefix, filesystem, 0);
> > +
> > + if (r)
> > + return r;
> > + }
> > 
> > // boot
> > snprintf(target, sizeof(target), "%s%s", prefix, HW_PATH_BOOT);
> > @@ -1019,6 +1028,39 @@ int hw_mount_filesystems(struct
> > hw_destination* dest, const char* prefix) {
> > return 0;
> > }
> > 
> > +int hw_mount_btrfs_subvolumes(const char* source) {
> > + int r;
> > + char path[STRING_SIZE];
> > + char options[STRING_SIZE];
> > +
> > + // Mount the root subvolume
> > + snprintf(options, sizeof(options), "subvol=%s,%s",
> > btrfs_subvolumes[0][0], BTRFS_MOUNT_OPTIONS);
> > + r = mount(source, DESTINATION_MOUNT_PATH, "btrfs", NULL,
> > options);
> 
> Why is this not inside the loop when it is just a regular subvolume?

I think my intention, was to be sure that the main root is always
mounted before mounting the remaining subvolumes. But as you pointed
out the first subvolume in the array would be the root one, so this is
some kind of over-engineered and can be joined together.

> 
> > +
> > + if (r)
> > + return r;
> > +
> > + // Loop through the remain array of known subvolumes.
> > + for ( int i = 1; i < LEN(btrfs_subvolumes); i++ ) {
> > + snprintf(path, sizeof(path), "%s%s", DESTINATION_MOUNT_PATH,
> > btrfs_subvolumes[i][1]);
> > + snprintf(options, sizeof(options), "subvol=%s,",
> > btrfs_subvolumes[i][0], BTRFS_MOUNT_OPTIONS);
> 
> Likewise, please check the return code of snprintf.
> 
> > +
> > + // Create the directory.
> > + r = hw_mkdir(path, S_IRWXU|S_IRWXG|S_IRWXO);
> > +
> > + if(r != 0 && errno != EEXIST)
> > + return r;
> > +
> > + // Try to mount the subvolume.
> > + r = mount(source, path, "btrfs", NULL, options);
> > +
> > + if (r)
> > + return r;
> > + }
> > + 
> > + return 0;
> > +}
> > +
> > int hw_umount_filesystems(struct hw_destination* dest, const char*
> > prefix) {
> > int r;
> > char target[STRING_SIZE];
> > -- 
> > 2.39.2
> > 
>
  

Patch

diff --git a/src/installer/hw.c b/src/installer/hw.c
index cb6a8ab81..e520fb687 100644
--- a/src/installer/hw.c
+++ b/src/installer/hw.c
@@ -926,6 +926,7 @@  int hw_create_btrfs_layout(const char* path, const char* output) {
 
 int hw_mount_filesystems(struct hw_destination* dest, const char* prefix) {
 	char target[STRING_SIZE];
+	int r;
 
 	assert(*prefix == '/');
 
@@ -953,9 +954,17 @@  int hw_mount_filesystems(struct hw_destination* dest, const char* prefix) {
 	}
 
 	// root
-	int r = hw_mount(dest->part_root, prefix, filesystem, 0);
-	if (r)
-		return r;
+	if (dest->filesystem == HW_FS_BTRFS) {
+		r = hw_mount_btrfs_subvolumes(dest->part_root);
+
+		if (r)
+			return r;
+	} else {
+		r = hw_mount(dest->part_root, prefix, filesystem, 0);
+
+		if (r)
+			return r;
+	}
 
 	// boot
 	snprintf(target, sizeof(target), "%s%s", prefix, HW_PATH_BOOT);
@@ -1019,6 +1028,39 @@  int hw_mount_filesystems(struct hw_destination* dest, const char* prefix) {
 	return 0;
 }
 
+int hw_mount_btrfs_subvolumes(const char* source) {
+	int r;
+	char path[STRING_SIZE];
+	char options[STRING_SIZE];
+
+	// Mount the root subvolume
+	snprintf(options, sizeof(options), "subvol=%s,%s", btrfs_subvolumes[0][0], BTRFS_MOUNT_OPTIONS);
+	r = mount(source, DESTINATION_MOUNT_PATH, "btrfs", NULL, options);
+
+	if (r)
+		return r;
+
+	// Loop through the remain array of known subvolumes.
+	for ( int i = 1; i < LEN(btrfs_subvolumes); i++ ) {
+		snprintf(path, sizeof(path), "%s%s", DESTINATION_MOUNT_PATH, btrfs_subvolumes[i][1]);
+		snprintf(options, sizeof(options), "subvol=%s,", btrfs_subvolumes[i][0], BTRFS_MOUNT_OPTIONS);
+
+		// Create the directory.
+		r = hw_mkdir(path, S_IRWXU|S_IRWXG|S_IRWXO);
+
+		if(r != 0 && errno != EEXIST)
+			return r;
+
+		// Try to mount the subvolume.
+		r = mount(source, path, "btrfs", NULL, options);
+
+		if (r)
+			return r;
+	}
+	
+	return 0;
+}
+
 int hw_umount_filesystems(struct hw_destination* dest, const char* prefix) {
 	int r;
 	char target[STRING_SIZE];