-
Notifications
You must be signed in to change notification settings - Fork 49
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
add support for swap partitions and logical volumes #886
base: main
Are you sure you want to change the base?
Conversation
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.
This PR is stale because it has been open 30 days with no activity. Remove "Stale" label or comment or this will be closed in 7 days. |
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
Filesystem
with typeswap
and a mountpoint forced/faked tonone
. Thus, I added a newtype
field 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
swap
actually aFilesystem
? It's murky. I'm open to suggestions. The other alternative would be to define a newSwap
entity that does not implementMountable
, but then we would need to have special cases for swap when generatingmkfs
stages andfstab
. Also, theMountpointCreator
mechanism 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
type
in the filesystem customization,mountpoint == swap
would 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: