yast / yast-storage-ng

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

Changes to allow automatic generation of LVM PVs at the AgamaProposal #1392

Closed ancorgs closed 1 week ago

ancorgs commented 3 weeks ago

We want to offer new possibilities with the so-called AgamaProposal (that will be used to implement all the features described at this document.

For that we need a more powerful SpaceMaker that can automatically calculate physical volumes for an arbitrary number of LVM volume groups extended over different sets of disks.

This pull request generalizes the existing SpaceMaker to gain that capability and to be less coupled with the traditional YaST GuidedProposal.

Original API

class Y2Storage::Proposal::SpaceMaker
  # @param settings [ProposalSettings] Note the class
  def initialize(disk_analyzer, settings); end

  # @param planned_partitions [Array<Planned::Partition>]
  # @param lvm_helper [Proposal::LvmHelper]
  def provide_space(original_graph, planned_partitions, lvm_helper); end

   # @param planned_partitions [Array<Planned::Partition>]
   def prepare_devicegraph(original_graph, planned_partitions = []); end
end

New API.

Does not use LvmHelper or ProposalSettings. Accepts different sets of partitions and/or volume groups. Many arguments can be omitted on the calls. The caller is more explicit about what to do instead of relying on SpaceMaker to take decisions (like which disks to clean) based on the proposal settings.

class Y2Storage::Proposal::SpaceMaker
  # @param settings [ProposalSpaceSettings] Note the class
  def initialize(disk_analyzer, settings); end

  # @param partitions [Array<Planned::Partition>]
  # @param volume_groups [Array<Planned::LvmVg>]
  # @param default_disks [Array<String>]
  def provide_space(original_graph, partitions: [], volume_groups: [], default_disks: [])
  end

  # @param disks [Array<String>]
   def prepare_devicegraph(original_graph, disks); end
end

The behavior is unchanged except one corner case related to how it calculates the space needed to subtract from existing partitions in order to make space. The new behavior looks more correct. Anyways, that corner case should not affect YaST.

Bonus

This also modifies the mentioned calculation of how much a partition must be resized.

The previous algorithm was quite aggressive when LVM is involved. A perfect size would be computationally too expensive to find, but now we do two attempts, one with the original logic and another one with a logic that can still succeed but is less aggressive when there are several spaces in the disk.

Notes

As usual, this is structured in separate logical commits to easy review.

coveralls commented 3 weeks ago

Coverage Status

coverage: 97.814% (+0.004%) from 97.81% when pulling 2aa1c768c48ad77818f8a039053dd3ff6a4c900a on ancorgs:less_lvm_helper into d9842210785a0c4988911db34400358711d748f2 on yast:master.

github-actions[bot] commented 1 week ago

:white_check_mark: Autosubmission job #11251174448 successfully finished :white_check_mark: Created submit request #1206481