add support for swap partitions and logical volumes #886
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Rebased on main since the first few commits were merged in #885. |
|
Given how special and weird a swap partition is, I think trying to coerce it into the current model of a filesystem will cause a lot of headaches in the long run. It will be something we'll have to constantly make exceptions for in bits of the code that deal with mountables, for example. The more accurately we model the real world here, the lower the mental load will be needed when working with these parts of the code and the lower the chance for introducing bugs or allowing impossible scenarios. A mountable is a mountable for a reason and if something doesn't fit the definition, it shouldn't implement the interface with exceptions. I don't have the full answer here, but I think a special type for swap partitions will probably make our lives easier. |
I haven't been able to find one, so this commit adds a simple smoke test for NewFSTabStageOptions.
This is the first commit in the series of commits whose goal is to add swap support to disk library. In order to have everything related to swap covered with tests, we need to add support for swap to the fake partition tables used in tests.
If the fs type of a mountable is set to swap, it's a swap partition, and thus we cannot mount it when building a manifest. So let's not create an osbuild mount for it.
Swap has "none" as the mountpoint (target) in fstab. Let's just force it in the Filesystem.GetMountpoint method. Extend the fstab stage test to check it.
The next commit will introduce a generalized version of entityPath.
This is basically the same method as entityPathForMountpoint - a DFS-search through a partition table tree. It takes a start and a function. Every visited node (entity) is passed in such function. If the function returns true, this means that the passed entity is the goal. entityPathForMountpoint is now implemented using the new generalized method.
This commit just shuffles some code around. This is mostly a preparatory commit for the next one. The only functional change is that if no suitable MountpointCreator is found, an error is now returned instead of a panic.
Prior this commit, when the first-found mountpoint creator (a container nearest to the root mountpoint) failed to create a new mountpoint for some reason, the whole partition table customizing failed. This commit changes this behaviour: if a mountpoint creation fails, the code continues to traverse the path in the direction of the partition table root, and tries to find another creator and use that one instead. This is now mostly a pointless change because the only error can be returned from an LVM volume group that fails to allocate a unique name which is basically impossible. However, the following commits will use this feature to correctly allocate swap.
Prior this commit, both a partition table and a volume group contained code to create a new filesystem. Let's deduplicate this.
This commit allows the filesystem customization in a blueprint to add a swap partition. This is done by adding a new field "type". It currently has only two values: unset and swap. The createFilesystem now creates a swap partition if type == swap. Otherwise, it still falls back to hardcoded "xfs". You can clearly see that should not be hard to change. ;) An interesting case is when / is on btrfs. Then, the library will try to create swap as a btrfs subvolume, which is bogus, so it returns an error. However, one of the previous commits changed the logic, so a higher-level container is tried in this case. So swap will be created as a raw partition as usual (or LV, if you have btrfs on LV). The blueprintApplied test helper can now work with swap, and it was also added into one of the test blueprints, so all the new code should be covered.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
now creates a swap partition (or a LV, the usual decision rules between a regular partition an LV apply).
After some pondering, I decided that swap is just
Filesystemwith typeswapand a mountpoint forced/faked tonone. Thus, I added a newtypefield into the filesystem customization. For now, it supports unset and swap only, but extending it should not be hard. ;)However, swap is a weird mountpoint:
In other cases, it shares properties of other mountpoints:
So is
swapactually aFilesystem? It's murky. I'm open to suggestions. The other alternative would be to define a newSwapentity that does not implementMountable, but then we would need to have special cases for swap when generatingmkfsstages andfstab. Also, theMountpointCreatormechanism would have to be separate (SwapCreator?). This sounds more convoluted to me.One can also argue that it's not needed to define a new
typein the filesystem customization,mountpoint == swapwould also do the trick. However, I think that swap is rather a filesystem type (see e.g. fstab) than a mountpoint path.TODO list (nothing feels blocking, though):
Depends on: