Merge pull request #424 from ancorgs/better_proposals

Enhance criteria to choose the best partitions distribution

package/yast2-storage-ng.changes

@@ -1,3 +1,11 @@
+-------------------------------------------------------------------
+Wed Nov 15 15:55:51 UTC 2017 - ancor@suse.com
+
+- When proposing a partitions layout for installation, prefer
+  getting big installations and arranging the partitions by their
+  weights over creating adjacent partitions. Part of fate#318196.
+- 4.0.30
+
 -------------------------------------------------------------------
 Tue Nov 14 15:41:21 UTC 2017 - jlopez@suse.com
 

package/yast2-storage-ng.spec

@@ -16,7 +16,7 @@
 #
 
 Name:		yast2-storage-ng
-Version:        4.0.29
+Version:        4.0.30
 Release:	0
 BuildArch:	noarch
 

src/lib/y2storage/planned/assigned_space.rb

@@ -67,6 +67,13 @@ def partition_type
         end
       end
 
+      # Sum of the weights of all the planned partitions assigned to this space
+      #
+      # @return [Integer]
+      def total_weight
+        partitions.map { |p| p.weight || 0 }.reduce(:+)
+      end
+
       # Checks if the volumes really fit into the assigned space
       #
       # TODO: We do not check for start_offset. Anyways,

src/lib/y2storage/planned/partitions_distribution.rb

@@ -84,7 +84,7 @@ def space_at(disk_space)
 
       # Space wasted by the distribution
       # @return [DiskSize]
-      def gaps_total_disk_size
+      def gaps_total_size
         DiskSize.sum(spaces.map(&:unused))
       end
 
@@ -96,10 +96,16 @@ def gaps_count
 
       # Total space available for the planned partitions
       # @return [DiskSize]
-      def spaces_total_disk_size
+      def spaces_total_size
         DiskSize.sum(spaces.map(&:disk_size))
       end
 
+      # Number of assigned spaces in the distribution
+      # @return [Integer]
+      def spaces_count
+        spaces.count
+      end
+
       # Total number of planned partitions included in the distribution
       # @return [Integer]
       def partitions_count
@@ -111,31 +117,77 @@ def partitions_count
       #
       # @return [Integer] -1, 0, 1 like <=>
       def better_than(other)
-        # The smallest gaps the better
-        res = gaps_total_disk_size <=> other.gaps_total_disk_size
-        return res unless res.zero?
-
-        # The fewer gaps the better
-        res = gaps_count <=> other.gaps_count
-        return res unless res.zero?
-
-        # The fewer physical volumes the better
-        res = partitions_count <=> other.partitions_count
-        return res unless res.zero?
-
-        # The less fragmentation the better
-        res = spaces.count <=> other.spaces.count
-        return res unless res.zero?
-
-        # The biggest installation the better
-        res = other.spaces_total_disk_size <=> spaces_total_disk_size
-        return res unless res.zero?
+        # Sorted list of criteria (starting with the most important) to use in
+        # order to select which distribution is best. Basically names of methods
+        # that return something comparable using #<=>, followed by the direction
+        # of the comparison (whether a bigger value is better or worse).
+        criteria = [
+          # The smaller gaps the better
+          [:gaps_total_size, :smaller],
+
+          # The fewer gaps the better
+          [:gaps_count, :smaller],
+
+          # The fewer physical volumes the better
+          [:partitions_count, :smaller],
+
+          # The bigger installation the better
+          [:spaces_total_size, :bigger],
+
+          # We want the partitions with biggest weight to be placed in the spaces
+          # in which there is more extra space to be distributed
+          [:weight_space_deviation, :smaller],
+
+          # The less fragmentation the better
+          [:spaces_count, :smaller]
+        ]
+
+        criteria.each do |criterion, order|
+          res = public_send(criterion) <=> other.public_send(criterion)
+          res *= -1 if order == :bigger
+          return res unless res.zero?
+        end
 
         # Just to ensure stable sorting between different executions in case
         # of absolute draw in all previous criteria
         comparable_string <=> other.comparable_string
       end
 
+      # A coefficient expresing how far is the distribution from the ideal
+      # situation in which there is a 1:1 correlation between the weights of the
+      # partitions in every assigned space and the extra disk in that space to
+      # be distributed.
+      #
+      # In general, for two distributions that are considered equivalent in
+      # any other aspect, the one in which the partitions with biggest weights
+      # are placed in the spaces with more "growth potential" is considered the
+      # best. This coefficient is zero if the proportion of weights (of every
+      # assigned space compared with the total) is equal to the proportion of
+      # extra spaces (same calculation).
+      #
+      # @see #better_than
+      # @return [Float] a number between 0.0 and 1.0
+      def weight_space_deviation
+        total_extra_size = spaces.map(&:usable_extra_size).reduce(:+)
+        total_weight = spaces.map(&:total_weight).reduce(:+)
+
+        # Edge case #1:
+        # No partition looks interested in growing, so we are fine whatever
+        # the extra sizes are
+        return 0.0 if total_weight.zero?
+        # Edge case #2:
+        # All the spaces are fully packet since the beginning, so no chance for
+        # any partition to grow
+        return 1.0 if total_extra_size.zero?
+
+        diffs = spaces.map do |space|
+          normalized_size = space.usable_extra_size.to_i.to_f / total_extra_size.to_i
+          normalized_weight = space.total_weight.to_f / total_weight
+          (normalized_size - normalized_weight).abs
+        end
+        diffs.reduce(:+)
+      end
+
       def to_s
         spaces_str = spaces.map(&:to_s).join(", ")
         "#<PartitionsDistribution spaces=[#{spaces_str}]>"

test/data/control_files/kvm-role-like.xml

@@ -0,0 +1,61 @@
+<?xml version="1.0"?>
+<productDefines xmlns="http://www.suse.com/1.0/yast2ns" xmlns:config="http://www.suse.com/1.0/configns">
+
+  <partitioning>
+    <proposal>
+      <windows_delete_mode config:type="symbol">all</windows_delete_mode>
+      <linux_delete_mode config:type="symbol">all</linux_delete_mode>
+      <other_delete_mode config:type="symbol">ondemand</other_delete_mode>
+    </proposal>
+
+    <volumes config:type="list">
+      <volume>
+        <mount_point>/</mount_point>
+        <fs_type>xfs</fs_type>
+        <desired_size config:type="disksize">10 GiB</desired_size>
+        <min_size config:type="disksize">5 GiB</min_size>
+        <max_size config:type="disksize">30 GiB</max_size>
+        <weight config:type="integer">20</weight>
+
+        <snapshots config:type="boolean">false</snapshots>
+        <snapshots_configurable config:type="boolean">true</snapshots_configurable>
+        <snapshots_percentage config:type="integer">300</snapshots_percentage>
+
+        <!-- Disable snapshots for / if disabling /var/lib/libvirt is not enough
+             to fit in the disk -->
+        <disable_order config:type="integer">2</disable_order>
+      </volume>
+
+      <!-- The libvirt partition/lv -->
+      <volume>
+        <mount_point>/var/lib/libvirt</mount_point>
+        <fs_type>xfs</fs_type>
+        <fs_types>xfs,ext3,ext4</fs_types>
+
+        <proposed config:type="boolean">true</proposed>
+        <proposed_configurable config:type="boolean">true</proposed_configurable>
+        <!-- Disable it in first place if we don't fit in the disk -->
+        <disable_order config:type="integer">1</disable_order>
+
+        <desired_size config:type="disksize">15 GiB</desired_size>
+        <min_size config:type="disksize">5 GiB</min_size>
+        <max_size config:type="disksize">unlimited</max_size>
+        <weight config:type="integer">70</weight>
+        <!-- If this volume is disabled, we want "/" to become greedy (unlimited
+             max), since it will contain /var/lib/libvirt -->
+        <fallback_for_max_size>/</fallback_for_max_size>
+      </volume>
+
+       <!-- swap partition -->
+       <volume>
+         <mount_point>swap</mount_point>
+         <fs_type>swap</fs_type>
+
+         <desired_size config:type="disksize">1 GiB</desired_size>
+         <min_size config:type="disksize">512 MiB</min_size>
+         <max_size config:type="disksize">2 GiB</max_size>
+         <weight config:type="integer">10</weight>
+       </volume>
+     </volumes>
+   </partitioning>
+ </productDefines>

test/data/devicegraphs/kvm_role_scenario.yml

@@ -0,0 +1,27 @@
+---
+- disk:
+    name: "/dev/sda"
+    size: 26 GiB
+    partition_table: gpt
+    partitions:
+    - partition:
+        size: 6622 MiB (6.47 GiB)
+        name: "/dev/sda1"
+        id: linux
+        file_system: ext4
+    - partition:
+        size: 10376 MiB (10.13 GiB)
+        name: "/dev/sda2"
+        id: linux
+        file_system: xfs
+    - partition:
+        size: 1262 MiB (1.23 GiB)
+        name: "/dev/sda4"
+        id: swap
+        file_system: swap
+    - partition:
+        size: 1 MiB
+        name: "/dev/sda3"
+        id: bios_boot
+    - free:
+        size: 8362 MiB (8.17 GiB)

test/data/devicegraphs/output/kvm_role_scenario.yml

@@ -0,0 +1,30 @@
+---
+- disk:
+    name: "/dev/sda"
+    size: 26 GiB
+    partition_table: gpt
+    partitions:
+    - partition:
+        size: 16212 MiB (15.83 GiB)
+        name: "/dev/sda2"
+        id: linux
+        file_system: xfs
+        mount_point: "/var/lib/libvirt"
+    - partition:
+        size: 2 GiB
+        name: "/dev/sda4"
+        id: swap
+        file_system: swap
+        mount_point: swap
+    - partition:
+        size: 1 MiB
+        name: "/dev/sda3"
+        id: bios_boot
+    - partition:
+        size: 8562671.5 KiB (8.17 GiB)
+        name: "/dev/sda1"
+        id: linux
+        file_system: xfs
+        mount_point: "/"
+    - free:
+        size: 16.5 KiB

test/data/devicegraphs/spaces_5_5_10.yml

@@ -0,0 +1,40 @@
+---
+- disk:
+    name: /dev/sda
+    size: 400 GiB
+    partition_table:  gpt
+    partitions:
+
+    - partition:
+        size:         100 GiB
+        name:         /dev/sda1
+        id:           windows_basic_data
+        file_system:  ntfs
+        label:        data1
+
+    - free:
+        size:         5 GiB
+
+    - partition:
+        size:         94 GiB
+        name:         /dev/sda2
+        file_system:  ntfs
+        label:        data2
+
+    - free:
+        size:         5 GiB
+
+    - partition:
+        size:         92 GiB
+        name:         /dev/sda3
+        file_system:  ntfs
+        label:        data3
+
+    - free:
+        size:         10 GiB
+
+    - partition:
+        size:         unlimited
+        name:         /dev/sda4
+        file_system:  ntfs
+        label:        data4