yast / yast-storage-ng

Rewrite of https://github.com/yast/yast-storage
http://yast.github.io/
GNU General Public License v2.0
13 stars 19 forks source link

Improve the :bigger_resize SpaceMaker strategy #1382

Closed ancorgs closed 3 months ago

ancorgs commented 3 months ago

IMPORTANT: This should be merged in sync with https://github.com/openSUSE/agama/pull/1164, which includes the needed adaptations in the Agama side to keep everything consistent.

Problem

The :bigger_resize strategy was added to give Agama a more fine-grained control of the actions to be performed by the SpaceMaker.

But we found several inconsistencies in the way we originally defined :bigger_resize.

Before this change, the SpaceMaker strategy expected a list like this.

First problem

The behavior described above turned out to be quite confusing since actions on non-partitioned disks are about the content (filesystems, PVs, etc.) and actions on partitions are about the devices themselves, offering no option for things like keeping the device but allowing to destroy the content.

Second problem

The first problem refers to disks that are part of the candidate devices or are selected as root disk. But the situation of disks or RAIDs that were chosen to be formatted (ie. reused by a volume) also needed clarification.

Additional unrelated problem

While testing this I found that installing on a disk that previously contained a RAID could result in a proposal that didn't include the partitions needed for booting. After some debugging, turns out the culprit was some code introduced many years ago in order to support non-standard RAID1 setups (https://github.com/yast/yast-storage-ng/pull/788).

Solution

Solution to the first problem

We agreed that having explicit actions for non-partitioned disks has very little value. If a non-partitioned device is selected to be formatted or to be a candidate disk or root disk, it’s content will be deleted if needed no matter whether there is such action. In other words, actions refer to the devices themselves, not to their content. As such, actions only make sense for partitions and maybe for other devices that can be destroyed (like logical volumes if we implement reusing of LVM VGs in the future).

Solution to the second problem

We came to the conclusion that selecting a disk to be directly formatted (or any other partitionable device like a RAID) already implies an statement about deleting all its content (partitions or whatever is there). There is no need to have space actions for those cases (they would be an open door for conflicts). Space actions only make sense for candidate disks and for the root disk. They are not needed for reused devices or its descendants.

Solution to the additional problem

If the root device is explicitly set (something that always happens in the Agama proposals), that value is now honored, ignoring any possible RAID consideration.

Testing

coveralls commented 3 months ago

Coverage Status

coverage: 97.802% (+0.001%) from 97.801% when pulling 88340d630e3e22d4e418f7156ca75787f6d9293a on ancorgs:better_bigger_resize into d185af588f30e677ff02f0657b5be44471e6140a on yast:master.

yast-bot commented 3 months ago

:heavy_check_mark: Internal Jenkins job #1155 successfully finished :heavy_check_mark: Created OBS submit request #1170162