Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Btrfs2 #787

Merged
merged 17 commits into from
Dec 31, 2021
Merged

Btrfs2 #787

merged 17 commits into from
Dec 31, 2021

Conversation

wllacer
Copy link
Contributor

@wllacer wllacer commented Dec 6, 2021

Contains all the changes to make btrfs subvolumes work correctly in archinstall (thus solving #718 and paving the way for things like #781 .
I hope you'all cant test it and get whatever is missing / wrong.
Things missing, because I need help in this areas

  • An UI to define subvolumes. As it is now, subvolume definition has to be done by disk_layouts file.
  • have only tested it in non encripted environments. I feel this path is still incomplete
  • Only tested with systemd-bootctl. For unexplained reasons I've been unable to boot wih Btrfs, Grub and loop files, together. The code for grub needs changes for sure
  • I haven't still implemented code for handling btrfs mount options, although it is needed and has some twists. Real Soon Now (done)

The faster way of testing would be to download my branch wllacer/archinstall/btrfs2 , and use the script only_hd, which only manages partition creation and mounting

Edit 20/12 Grub works without any manual intervention under VM. The problem seems to be with loop files and grub

…o two points;

the handling of the addressing of subvolumes re. physical partitions, and the small changes at the bootloader level
@wllacer wllacer requested a review from Torxed as a code owner December 6, 2021 11:00
@wllacer
Copy link
Contributor Author

wllacer commented Dec 6, 2021

This is one example of the partition part of disk_layouts.json of btrfs partition with subvolumes
WARNING don't use this example as a pattern for anything. It is just a sample for testing syntactic features, and contains several errors and atrocities

{
    "btrfs": {
        "subvolumes": {
            "@":"/",
            "@.snapshots": null,
            "@home": "/home",
            "@log": "/var/log",
            "@pkgs": "/var/cache/pacman/pkg",
            "@/home": "/home2",
            "@/log": "/var/log2",
            "@/pkgs": "/var/cache/pacman/pkg",
            "root": null,
            "home": null,
            "sieso/total":null,
            "/sieso/parcial":"/opt"
        }
    },
    "encrypted": false,
    "filesystem": {
        "format": "btrfs"
    },
    "format": true,
    "mountpoint": null,
    "size": "100%",
    "start": "518MiB",
    "type": "primary"
}

Special entries

The first interesting entry is mountpoint

It can contain either None (null) or a string representing a mount point in the target system .
If we give some value to it , we will mount the whole partition there, and will ignore any other mount directive (cf. infra) information inside the entry. This is the current behaviour. All subvolume directories will be physically found under this point in the target system, with the name we gave them (the key of the dictionary)[¹]
The rationale for this behaviour is simple. If we mount the full partition all subvolumes are accessible in the file system tree. To mount them again is innecesary and can be the cause of problems. In this setup, subvolumes are exclusively for internal btrfs use (snapshoting mainly)
If we let this argument null, the next section applies

The second point of interest is the dictionary [btrfs][subvolumes]

It is a series of entries subvolume name:mount point, where mount point can be null. As you can see, this last follows the usual UNIX filesystem notation. As subvolumes are materialized as directories, the subvolume name is also a directory name , but has some semantic twists:

  • In our implementation [²] the names are relative to the "root" of the filesystem in the partition as their starting point. This is not necesarily equal to "/" in the operating system . Better asume it is its own hierarchy.
  • The at sign @ is a mere convention to indicate where the OS root partition will be. It's also customary that its siblings also start with the at sign. Caveat Some tools assume this convention as a standard
  • The slash is a directory level delimiter, as in the OS. If you look at the example @home is a sibling of @ (flat structure); but @/home creates a subvolume home inside @ (nested subvolumes).
    When naming a subvolume remember that it is also used as identifier with the partiton name, during the mount operation and the boot procedure.

What the current implementation is missing.

We miss a place where to document btrfs and mount options for subvolumes. As probably is need to have backward compatibility we change the structure as:
**subvolume name: None | mountpoint | options_structure **
Most of the options will be routed to the mount procedure, but AFAIK nodatacow might need specially handling.

When to define a mount point and when not.

Some tips. We have no plan to implement them as rules, for the time being.

  • Don't define mountpoints to nested subvolumes when a parent has a mount point. If you need some convoluted structure, better use symbolic links
  • If you need the subvolume only for backup reasons (include or exclude from snapshots), there is no need to mount, but be certain that the sname is nested within a subvolume that reflects the OS hierarchy. Supose you want to exclude /var/log/archinstall from the snapshot, and /var/log is the @VarLog subvolume; you need to name the subvolume @varlog/archinstall
  • If you need special btrfs features for your subvolume (e.g. compression, nodatacow,...)[³], it usually better/the only way to separatelly mount from its parent. edited, see note
  • If you have the need to be able to switch rapidly certain areas of the system (soipecially from snapshots) to isolate them in mounted subvolumes makes it a very fast operation.

[1] Actually, the current implementation does not behaves that way. It uses the content of the item. We feel it is an error as we prefer to have separted namespaces for subvolumes and the FSH
[2] We create all the subvolumes from the root of the partition. In general usage, subvolumes are created relative to the current directory where is executed. Absolute adresses are from the "/" mountpoint of the OS image.
[3] WARNING, Nowadays, both options cited can not be applied to subvolumes at mount time. We have solved it for nodatacow using the extended attribute +C. We are currently thinking about doing the same for compression, although it has some limitations

@Torxed
Copy link
Member

Torxed commented Dec 6, 2021

A bit of a big change and I saw some things that looked strange (it looked like some master changes were undone). I'll have a proper look when I have some time to sit down in peace a and quiet and look at this! Might take a few days.

@wllacer
Copy link
Contributor Author

wllacer commented Dec 6, 2021

I hope not.
What i am working on this patch is more or less orthogonal to the most important changes on the master branch lately (encription and TUI).
There is a small chance of collision on Installer.add_bootloader (lines 635 passim in my version) I moved part of the management of the options line to a variable defined before the selection of encrypted or normal root partition. As it is written now, my patch only affects the behavior of the NON encripted part, and far as I can see i haven't altered the behaviour with encryption (if not is a bug)
I believe lines 645-648 should be moved after line 638, but as i have not the slightest experience with encrypted partitions i would prefer the opinion of someone more knowledgable in this concrete point.

https://github.com/wllacer/archinstall/blob/a105a0ff00bddd27306c839ab72a9fcb9fea9642/archinstall/lib/installer.py#L635-L650

The biggest departure from master now is the script, which is a descendent from the original guided.py, prior to the TUI fication,

@wllacer
Copy link
Contributor Author

wllacer commented Dec 6, 2021

Most of the changes in Partition.py and disk/helper.py are due to the same reason. For subvolumes the path the system exposes is /dev/partition[/subvolume]. I've created two new properties (device_path and bind_name) which separates its components and have changed all uses of Partition.path regarding lsblk and findmnt to Partititon.device_path (which for normal partitions would be equal to Partition.path). For the few cases outside a partition instance i've created a function which does the same (split_bind_path).
The changes in get_mount_info are of the same kind, but have to use a bit of a list comprehension trick, because I had to guarantee unicity and correction in the return(lines 156-157 in my version)
The changes in the installer are more serious. For one i had to add the "rootflags=subvolume=" option to the options line of the bootloader (as I recall, only got to work for systemd-bootctl ).
On the other hand, i had to create the subvolumes and made them able to be mounted. The relevant code is in the manage_btrf_subvolumes at disk/btrf.py. It is the dirtiest piece of code of the whole bunch. First I had to mount the whole partition first and then operate (and unmount it at the end) to be able to create the subvolumes. To mount them what i did was to extented the mounpoints dictionary used in Installer.mount_orderded_layout, creating a "fake partition" for each subvolume which has to be mounted (with the corresponding data massage and creation of a Partition instance "ad hoc") and adding it to mountpoints. After this the general procedure of the installer can mount and manage the subvolumes as "partitions". And it works fine ;-). I admit is ugly as sin, but it is the most minimal intervention possible, while being still maintenable and basically non invasive(well i ought to better document the routine). One of this days I'll have to make a proposal for a different object model for the disk part that avoids this "incoherence". But i can understand that it is not perhaps the best moment now

@wllacer
Copy link
Contributor Author

wllacer commented Dec 8, 2021

I've added an hipercommented version of the manage_btrfs_subvolumes function as it is probably the most "hard" part of my processing.
IN between I've managed to add an options entry for the btrfs subvolume. Instead of having only the mountpoint, i process a dictionary with optionally the mountpoint and a list of possible mount options, like in this example

                "btrfs": {
                    "subvolumes": {
                        "root": "/",
                        "home": "/home",
                        "var": {
                            "mountpoint":"/var",
                            "options":["space_cache=v2","nodatacow"]
                        }
                    }
                },

The nodatacow option has an special treatment (it shouldn't be a mount option), and the rest of the options are functionally available.
Edit: Mount options are available and working. Some of those particular to btrfs are not avaliable at subvolume level (like compress)

the conflict at partition solves by using device_path instead of path
at helper is a bit simpler but needs the split_bind_ function
@wllacer
Copy link
Contributor Author

wllacer commented Dec 19, 2021

Now there are a couple of known conflicts with master. I have accepted the changes in master, using the new properties i defined in partition, and the patch for path (in complex situations) i have for the same situations when not inside a partition.
Preliminary checks don't show problems ... let's hope

@timakro
Copy link

timakro commented Dec 25, 2021

This is exactly what I was asking for when the original Btrfs PR was merged. Great work!

@timakro
Copy link

timakro commented Dec 25, 2021

As for the default, it's probably best to go with what other major distros are doing. (source)

  • Ubuntu's installer uses the flat layout, with @ and @home, mounted at / and /home, respectively
  • Manjaro's installer uses the flat layout, identical to the Ubuntu one.
  • Fedora's Installer uses the flat layout, with a different naming scheme (root and home AFAIK)

And make home optional with a separate prompt.

Being able to fully configure this on the command line would be even more awesome. Maybe offer

  1. @ and @home
  2. Only @
  3. Custom

Not sure how "custom" would work. Does archinstall have an option to custom configure regular partitions which could serve as inspiration? Otherwise gdisk/fdisk can serve as inspiration.

@wllacer
Copy link
Contributor Author

wllacer commented Dec 26, 2021

@timakro Sadly it is still incomplete. The biggest missing piece (aside from UI) is the integration with encryption handling. As the code stays now it is incompatible with the current implementation. AFAIK I think i have the sore spot(s) located, but i'm still researching on the solution. A lot has to do with my own lack of experience with encryption, so any help would be very very welcome
Edited. Just achieved compatibility with the current implementation. (will upload in some minutes)

@wllacer
Copy link
Contributor Author

wllacer commented Dec 29, 2021

We are now able to use an encrypted btrfs partition with subvolume mounts.
Only tested with systemd-bootctl. And still not checked for compatibility.
For the time being, it uses no keyfiles, just passwords
Code is just good (tricky) enough to work

@DrymarchonShaun
Copy link

DrymarchonShaun commented Dec 31, 2021

As for the default, it's probably best to go with what other major distros are doing. (source)

Mentioning this here so it's seen before a decision is made on this - If you see my comment on the aforementioned issue, that is not the only layout used by major distros. Opensuse uses something different that is the only fully compatible layout for snapper, which is one of the two main btrfs snapshot programs / wrappers, the other obviously being timeshift. (I'd say snapper has more features out of the box. It's also CLI only, with a separate GUI for people to install if they want.) snap-pac also integrates snapper in into pacman for pre/post snapshots, and I'm not sure timeshift has something like that that's working and actively being developed.

Long winded way of saying that Opensuse was the one that really mainstreamed btrfs when they made it the default for all installations. They have used it as a core component of their distro for a while, rather than having it as an optional feature (as Ubuntu, Manjaro, etc. do), meaning they probably have a better idea of it's limits and the best ways to configure it, and for that reason, I think their layout should be considered for being default as much as the other suggestion in that issue, whether as the only default, or as an option alongside the original suggestion.

(Something else to look at is possibly adding an extra Y/N if the Opensuse layout is selected to also install and set up snapper and snap-pac with snapper rollback functioning.)

Sorry if this whole comment is out of line, or if this isn't the right place, I'm entirely new to the.. contributor? side of github, and I know there's usually some sort of etiquette with these kind of things. I'm happy to help test and answer any questions I can on what would have to be done for it to function correctly. I barely know any python, or I would do it myself. If I did managed to code something that worked, it would almost certainly be below the standards for being officially added.

@Torxed
Copy link
Member

Torxed commented Dec 31, 2021

I'm running and debugging this now :) It's a major change so it's going to take me some time.
But this is the last remaining fix before v2.3.1 is released. This PR is made against master so I'll have to pull it in to master first and then force it into v2.3.1-dev, which is fine because otherwise I'd had to do it the other way around anyway. But it will take some time :) Major work on this, good job!

And I get that BRFS layouts are something that are very different depending on who you talk to and where you look. But it's a good step towards supporting more complex and different designs.

@Torxed
Copy link
Member

Torxed commented Dec 31, 2021

screenshot

I couldn't push the latest changes from master into this branch for some reason.

@Torxed Torxed merged commit 7f9b799 into archlinux:master Dec 31, 2021
@Torxed
Copy link
Member

Torxed commented Dec 31, 2021

Tested and it seems to work.
The default layout is still preserved in a "simple state", which is nice since it doesn't add complexity there, but allows for more complicated layouts. We can consider moving the default layout after this.

Torxed added a commit that referenced this pull request Dec 31, 2021
* All the changes needed to make btrfs subvolumes work. It boils down to two points;
the handling of the addressing of subvolumes re. physical partitions, and the small changes at the bootloader level
* We added a new script only_hd for testing purposes. It only handles hadrd drive management
* restoring an escape hatch during subvolume processing
* hipercommented manage_btrfs_subvolumes
* Ready to be able to select and process options in subvolume mounting
* Separte nodatacow processing
* Solving a flake8 complain
* Use of bind names @ get_filesystem_type
* compress mount option bypass
* Preparations for encryption handling
* Compatibility to master version re. encrypted btrfs volumes
* Now we can create subvolumes and mountpoints inside an encrypted btrfs partition
* changes for entries file generation with systemd-bootctl
* flake8 corrections plus some comments

Co-authored-by: Anton Hvornum <anton@hvornum.se>
Torxed added a commit that referenced this pull request Dec 31, 2021
* Btrfs2 (#787)

* All the changes needed to make btrfs subvolumes work. It boils down to two points;
the handling of the addressing of subvolumes re. physical partitions, and the small changes at the bootloader level
* We added a new script only_hd for testing purposes. It only handles hadrd drive management
* restoring an escape hatch during subvolume processing
* hipercommented manage_btrfs_subvolumes
* Ready to be able to select and process options in subvolume mounting
* Separte nodatacow processing
* Solving a flake8 complain
* Use of bind names @ get_filesystem_type
* compress mount option bypass
* Preparations for encryption handling
* Compatibility to master version re. encrypted btrfs volumes
* Now we can create subvolumes and mountpoints inside an encrypted btrfs partition
* changes for entries file generation with systemd-bootctl
* flake8 corrections plus some comments

Co-authored-by: Anton Hvornum <anton@hvornum.se>

* Fixed flake8

Co-authored-by: Werner Llácer <wllacer@gmail.com>
@wllacer
Copy link
Contributor Author

wllacer commented Dec 31, 2021

I'm glad to see it merged. There are still some open issues i'm working now, but they'll come next year

@Torxed
Copy link
Member

Torxed commented Dec 31, 2021

I'm glad to see it merged. There are still some open issues i'm working now, but they'll come next year

Really appreciated! Heh, next year sounds so far away ^^ Happy new year btw!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants