Merge pull request #45 from ancorgs/prep_strategy

Improvements and tests for PReP boot strategy

doc/boot-partition.md

@@ -147,6 +147,8 @@ So we need a filesystem which can be booted by zipl (-> ext2).
 
 > dvaleev: 8 MB PReP
 
+> dvaleev: PReP must be one of the first 4 partitions, ideally the first one [citation needed]
+
 - OPAL/PowerNV/Bare metal
 
 > dvaleev:
@@ -208,8 +210,10 @@ i.e. valid for all architectures (s390, see below)
 ppc
 ---
 KVM/LPAR
-- PReP partition dos type 0x41, flag as bootable
-                 gpt type 9e1a2d38-c612-4316-aa26-8b49521e5a8b
+- PReP partition
+   - dos type 0x41, flag as bootable
+   - gpt type 9e1a2d38-c612-4316-aa26-8b49521e5a8b
+   - must be one of the first 4 partitions (we have no evidence of this)
 
 OPAL/PowerNV/Bare metal
 - no PReP is required

src/lib/storage/boot_requirements_strategies/prep.rb

@@ -34,27 +34,42 @@ module BootRequirementsStrategies
       class PReP < Base
         def needed_partitions
           volumes = super
-          volumes << prep_volume if prep_partition_missing?
+          volumes << prep_volume if prep_partition_required? && prep_partition_missing?
           volumes
         end
 
       protected
 
         def prep_partition_missing?
-          true	# #prep_partitions not implemented yet
-          # disk_analyzer.prep_partitions.empty?
+          partitions = disk_analyzer.prep_partitions[settings.root_device]
+          partitions.nil? || partitions.empty?
+        end
+
+        def prep_partition_required?
+          # no need of PReP partition in OPAL/PowerNV/Bare metal
+          !arch.ppc_power_nv?
         end
 
         def prep_volume
-          # TODO: we need to pass partition type somehow (PReP partition, ID_PPC_PREP, ID_GPT_PREP)
-          # TODO: hoe to really pass 'no mount point' and 'no fs'?
-          vol = PlannedVolume.new("", ::Storage::FsType_UNKNOWN)
-          vol.min_size = DiskSize.MiB(1)
+          vol = PlannedVolume.new(nil)
+          # So far we are always using msdos partition ids
+          vol.partition_id = ::Storage::ID_PPC_PREP
+          vol.min_size = DiskSize.kiB(256)
           vol.max_size = DiskSize.MiB(8)
           vol.desired_size = DiskSize.MiB(1)
+          # Make sure that alignment does not result in a too big partition
+          vol.align = :keep_size
+          vol.bootable = true
           vol.can_live_on_logical_volume = false
+          # TODO: We have been told that PReP must be one of the first 4
+          # partitions, ideally the first one. But we have not found any
+          # rational/evidence. Not implementing that for the time being
           vol
         end
+
+        def arch
+          @arch ||= StorageManager.instance.arch
+        end
       end
     end
   end

src/lib/storage/disk_analyzer.rb

@@ -73,6 +73,7 @@ class DiskAnalyzer
 
       attr_reader :installation_disks, :candidate_disks
       attr_reader :windows_partitions, :linux_partitions, :efi_partitions
+      attr_reader :prep_partitions
       attr_reader :devicegraph
       attr_accessor :disk_check_limit
 
@@ -82,6 +83,8 @@ def initialize
         @installation_disks = [] # device names of installation media
         @candidate_disks    = [] # device names of disks to install on
         @linux_partitions   = [] # device names of existing Linux parititions
+        @efi_partitions     = [] # device names of existing EFI partitions
+        @prep_partitions    = {} # device names of PReP partitions, indexed by disk
         @windows_partitions = [] # only filled if @linux_partitions is empty
 
         # Maximum number of disks to check. This might be important on
@@ -98,7 +101,8 @@ def analyze(devicegraph)
         @installation_disks = find_installation_disks
         @candidate_disks    = find_candidate_disks
         @linux_partitions   = find_linux_partitions
-        @efi_partitions     = find_linux_partitions
+        @efi_partitions     = find_efi_partitions
+        @prep_partitions    = find_prep_partitions
 
         if @linux_partitions.empty?
           @windows_partitions = find_windows_partitions
@@ -163,6 +167,22 @@ def find_efi_partitions
         partitions.map(&:name)
       end
 
+      # Find partitions from any of the candidate disks that can be used as
+      # PReP partition
+      #
+      # The result is a Hash in which each key is the name of a candidate disk
+      # and the value is an Array with the names of all the PReP partitions
+      # present in that disk.
+      #
+      # @return [Hash]
+      def find_prep_partitions
+        disks = devicegraph.disks
+        pairs = candidate_disks.map do |name|
+          [name, disks.with(name: name).partitions.with(id: ::Storage::ID_PPC_PREP).to_a]
+        end
+        Hash[pairs]
+      end
+
       # Array with all disks in the devicegraph
       #
       # @return [Array<::Storage::Disk>]

src/lib/storage/planned_volume.rb

@@ -30,10 +30,48 @@ module Storage
     # its constraints
     #
     class PlannedVolume
-      attr_accessor :mount_point, :filesystem_type, :partition_id, :disk
-      attr_accessor :size, :min_size, :max_size, :desired_size, :weight
-      attr_accessor :can_live_on_logical_volume, :logical_volume_name
+      # @return [String] mount point for this volume. This might be a real mount
+      # point ("/", "/boot", "/home") or a pseudo mount point like "swap".
+      attr_accessor :mount_point
+      # @return [::Storage::FsType] the type of filesystem this volume should
+      # get, like ::Storage::FsType_BTRFS or ::Storage::FsType_SWAP. A value of
+      # nil means the volume will not be formatted.
+      attr_accessor :filesystem_type
+      # @return [::Storage::IdNum] id of the partition in a ms-dos style
+      # partition table. If nil, the final id is expected to be inferred from
+      # the filesystem type.
+      attr_accessor :partition_id
+      # @return [String] device name of the disk in which the volume has to be
+      # located. If nil, the volume can be allocated in any disk.
+      attr_accessor :disk
+      # @return [DiskSize] definitive size of the volume
+      attr_accessor :size
+      # @return [DiskSize] minimum acceptable size in case it's not possible to
+      # ensure the desired one. @see #desired_size
+      attr_accessor :min_size
+      # @return [DiskSize] maximum acceptable size
+      attr_accessor :max_size
+      # @return [DiskSize] preferred size
+      attr_accessor :desired_size
+      # @return [Float] factor used to distribute the extra space between
+      # volumes
+      attr_accessor :weight
+      # @return [Boolean] whether the volume can be placed in LVM
+      attr_accessor :can_live_on_logical_volume
+      # @return [String] name to use if the volume is placed in LVM
+      attr_accessor :logical_volume_name
+      # @return [DiskSize] maximum distance from the start of the disk in which
+      # the partition can start
       attr_accessor :max_start_offset
+      # FIXME: this one is just guessing the final API of alignment
+      # @return [Symbol] modifier to pass to ::Storage::Region#align when
+      # creating the volume. :keep_size to avoid size changes. nil to use
+      # default alignment.
+      attr_accessor :align
+      # @return [Boolean] whether the boot flag should be set. Expected to be
+      # used only with ms-dos style partition tables. GPT has a similar legacy
+      # flag but is not needed in our grub2 setup.
+      attr_accessor :bootable
 
       TO_STRING_ATTRS = [:mount_point, :min_size, :max_size, :desired_size,
                          :disk, :max_start_offset]
@@ -47,15 +85,8 @@ class PlannedVolume
 
       # Constructor.
       #
-      # @param mount_point [string] mount point for this volume. This might be
-      #        a real mount point ("/", "/boot", "/home") or a pseudo mount
-      #        point like "swap".
-      #
-      # @param filesystem_type [::Storage::FsType] the type of filesystem this
-      #        volume should get. Typically one of ::Storage::FsType_BTRFS,
-      #        ::Storage::FsType_EXT4, ::Storage::FsType_XFS,
-      #        ::Storage::FsType_SWAP
-      #
+      # @param mount_point [string] @see #mount_point
+      # @param filesystem_type [::Storage::FsType] @see #filesystem_type
       def initialize(mount_point, filesystem_type = nil)
         @mount_point = mount_point
         @filesystem_type = filesystem_type
@@ -66,12 +97,14 @@ def initialize(mount_point, filesystem_type = nil)
         @max_size     = DiskSize.unlimited
         @desired_size = DiskSize.unlimited
         @max_start_offset = nil
+        @align        = nil
+        @bootable     = nil
         @weight       = 0 # For distributing extra space if desired is unlimited
         @can_live_on_logical_volume = false
         @logical_volume_name = nil
 
-        return unless @mount_point.start_with?("/")
-        return if @mount_point.start_with?("/boot")
+        return unless @mount_point && @mount_point.start_with?("/")
+        return if @mount_point && @mount_point.start_with?("/boot")
 
         @can_live_on_logical_volume = true
         if @mount_point == "/"

src/lib/storage/proposal/partition_creator.rb

@@ -276,6 +276,7 @@ def create_partition(vol, partition_id, free_slot)
           region = new_region_with_size(free_slot, vol.size)
           partition = ptable.create_partition(dev_name, region, partition_type)
           partition.id = partition_id
+          partition.boot = !!vol.bootable
           partition
         end