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

[Stefan Schantl <stefan.schantl@ipfire.org>]

[-- Attachment #1: Type: text/plain, Size: 3075 bytes --]

Am Montag, dem 18.03.2024 um 16:11 +0000 schrieb Michael Tremer:
> 
> 
> > On 15 Mar 2024, at 19:14, Stefan Schantl
> > <stefan.schantl(a)ipfire.org> wrote:
> > 
> > Signed-off-by: Stefan Schantl <stefan.schantl(a)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
> > 
>