Merge 9936d12 into 1e89337

package/yast2-storage-ng.changes

@@ -1,3 +1,11 @@
+-------------------------------------------------------------------
+Tue Apr 18 08:23:44 UTC 2023 - Ancor Gonzalez Sosa <ancor@suse.com>
+
+- AutoYaST: correctly import legacy values for parity_algorithm.
+- Partitioner: when creating an MD RAID, do not ask for the chunk
+  side when it makes no sense. Eg. RAID1 (bsc#1205172).
+- 4.5.21
+
 -------------------------------------------------------------------
 Tue Apr 11 13:38:25 UTC 2023 - Ancor Gonzalez Sosa <ancor@suse.com>
 

package/yast2-storage-ng.spec

@@ -16,7 +16,7 @@
 #
 
 Name:           yast2-storage-ng
-Version:        4.5.20
+Version:        4.5.21
 Release:        0
 Summary:        YaST2 - Storage Configuration
 License:        GPL-2.0-only OR GPL-3.0-only

src/lib/y2partitioner/actions/controllers/md.rb

@@ -37,7 +37,8 @@ class Md < Base
         extend Forwardable
 
         def_delegators :md, :md_level, :md_level=, :md_name,
-          :chunk_size, :chunk_size=, :md_parity, :md_parity=
+          :chunk_size_supported?, :chunk_size, :chunk_size=,
+          :md_parity, :md_parity=
 
         # Constructor
         #
@@ -206,7 +207,7 @@ def wizard_title
         # @see #default_chunk_size
         # @see #default_md_parity
         def apply_default_options
-          md.chunk_size = default_chunk_size
+          md.chunk_size = default_chunk_size if chunk_size_supported?
           md.md_parity = default_md_parity if parity_supported?
         end
 
@@ -299,7 +300,7 @@ def min_chunk_size
             [block_size, Y2Storage::DiskSize.KiB(4)].max # bsc#1200018
           when :raid10
             [block_size, page_size, Y2Storage::DiskSize.KiB(4)].max # bsc#1200018
-          else # including raid1/5/6
+          else # including raid5 and raid6
             [default_chunk_size, Y2Storage::DiskSize.KiB(64)].min
           end
         end
@@ -310,8 +311,6 @@ def max_chunk_size
 
         def default_chunk_size
           case md.md_level.to_sym
-          when :raid1
-            Y2Storage::DiskSize.KiB(4)
           when :raid5, :raid6
             Y2Storage::DiskSize.KiB(128)
           else # including raid0 and raid10

src/lib/y2partitioner/dialogs/md_options.rb

@@ -33,8 +33,8 @@ def initialize(controller)
         textdomain "storage"
 
         @controller = controller
-        @chunk_size_selector = ChunkSize.new(controller)
-        @parity_selector = controller.parity_supported? ? Parity.new(controller) : Empty()
+        @chunk_size_selector = chunk_size? ? ChunkSize.new(controller) : Empty()
+        @parity_selector = parity? ? Parity.new(controller) : Empty()
       end
 
       # @macro seeDialog
@@ -52,10 +52,30 @@ def contents
         )
       end
 
+      def run
+        return :next unless parity? || chunk_size?
+
+        super
+      end
+
       private
 
       attr_reader :controller
 
+      # Whether the widget for selecting the chunk size must be displayed
+      #
+      # @return [Boolean]
+      def chunk_size?
+        controller.chunk_size_supported?
+      end
+
+      # Whether the widget for selecting the RAID parity must be displayed
+      #
+      # @return [Boolean]
+      def parity?
+        controller.parity_supported?
+      end
+
       # Widget to select the chunk size
       class ChunkSize < CWM::ComboBox
         # @param controller [Actions::Controllers::Md]
@@ -73,8 +93,7 @@ def help
           _("<p><b>Chunk Size:</b> " \
             "It is the smallest \"atomic\" mass of data that can be written to the devices. " \
             "A reasonable chunk size for RAID 5 is 128 kB. " \
-            "For RAID 0, 32 kB is a good starting point. " \
-            "For RAID 1, the chunk size does not affect the array very much." \
+            "For RAID 0, 32 kB is a good starting point." \
             "</p>")
         end
 

src/lib/y2storage/autoinst_profile/raid_options_section.rb

@@ -96,6 +96,19 @@ def init_from_raid(md)
         @parity_algorithm = md.md_parity.to_s
         @device_order = md.sorted_devices.map(&:name)
       end
+
+      # RAID parity, according to the value of {#parity_algorithm}
+      #
+      # If possible, finds the corresponding MD parity, no matter if a legacy or modern
+      # representation is used at {#parity_algorithm} (see {MdParity.find_with_legacy}).
+      #
+      # @return [MdParity, nil] nil if {#parity_algorithm} is blank or it's impossible to infer
+      #   the corresponding RAID parity
+      def md_parity
+        return nil if parity_algorithm.nil? || parity_algorithm.empty?
+
+        MdParity.find_with_legacy(parity_algorithm)
+      end
     end
   end
 end

src/lib/y2storage/md.rb

@@ -103,7 +103,10 @@ class Md < Partitionable
     storage_forward :allowed_md_parities, as: "MdParity"
 
     # @!attribute chunk_size
-    #   Chunk size of the MD RAID.
+    #   Chunk size of the MD RAID. Zero if the value is unknown or makes no sense.
+    #
+    #   @see #chunk_size_supported?
+    #
     #   @return [DiskSize]
     storage_forward :chunk_size, as: "DiskSize"
     storage_forward :chunk_size=
@@ -318,6 +321,20 @@ def path_for_mount_by(mount_by)
       end
     end
 
+    # Whether setting a chunk size makes sense (see bsc#1205172).
+    #
+    # For some MD devices, the chunk size is meaningless and the {#chunk_size}
+    # attribute should never be set to a value different from DiskSize.zero.
+    # Moreover, libstorage-ng will ignore {#chunk_size} when creating those
+    # RAIDs during the commit phase.
+    #
+    # @return [Boolean]
+    def chunk_size_supported?
+      # See bsc#1205172. Starting with version 4.2, mdadm returns an error if a
+      # chunk size is specified when creating RAID1 array.
+      !md_level.is?(:raid1)
+    end
+
     protected
 
     # Holders connecting the MD Raid to its component block devices in the

src/lib/y2storage/md_parity.rb

@@ -57,6 +57,35 @@ class MdParity
     }
     private_constant :TRANSLATIONS
 
+    # Matching between identifiers for parity_algorithm in the old libstorage and the current ones
+    LEGACY_TO_CURRENT = {
+      parity_first:   :first,
+      parity_last:    :last,
+      parity_first_6: :first_6,
+      n2:             :near_2,
+      o2:             :offset_2,
+      f2:             :far_2,
+      n3:             :near_3,
+      o3:             :offset_3,
+      f3:             :far_3
+    }.freeze
+    private_constant :LEGACY_TO_CURRENT
+
+    # Parity corresponding to the given identifier
+    #
+    # Before storage-ng (SLE <= 12.x) some MD parities were represented by a slightly different
+    # indentifier in the AutoYaST profiles. This method returns the corresponding value of the
+    # enum, no matter if an old or a modern representation is given.
+    #
+    # @param id [#to_sym] identifier of the parity
+    # @return [MdParity, nil] nil if the given id is not recognized
+    def self.find_with_legacy(id)
+      id = LEGACY_TO_CURRENT[id.to_sym] if LEGACY_TO_CURRENT.key?(id.to_sym)
+      find(id)
+    rescue NameError
+      nil
+    end
+
     # Returns human readable representation of enum which is already translated.
     # @return [String]
     # @raise [RuntimeError] when called on enum value for which translation is not yet defined.

src/lib/y2storage/proposal/autoinst_md_planner.rb

@@ -130,7 +130,7 @@ def add_raid_options(md, raid_options)
 
         md.name = raid_options.raid_name if raid_options.raid_name
         md.chunk_size = chunk_size_from_string(raid_options.chunk_size) if raid_options.chunk_size
-        md.md_parity = MdParity.find(raid_options.parity_algorithm) if raid_options.parity_algorithm
+        md.md_parity = raid_options.md_parity if raid_options.parity_algorithm
         md.devices_order = raid_options.device_order if !raid_options.device_order.empty?
       end
 

test/y2partitioner/actions/controllers/md_test.rb

@@ -735,15 +735,6 @@ def create_raid_partition(dev)
       end
     end
 
-    context "when the Md device is a RAID1" do
-      let(:md_level) { Y2Storage::MdLevel::RAID1 }
-
-      it "returns 4 KiB" do
-        size = Y2Storage::DiskSize.KiB(4)
-        expect(controller.chunk_sizes.min).to eq size
-      end
-    end
-
     context "when the Md device is a RAID5" do
       let(:md_level) { Y2Storage::MdLevel::RAID5 }
 
@@ -855,9 +846,9 @@ def create_raid_partition(dev)
     context "when the Md device is a RAID1" do
       let(:md_level) { Y2Storage::MdLevel::RAID1 }
 
-      it "sets chunk size to 4 KiB" do
+      it "does not set the chunk size" do
         controller.apply_default_options
-        expect(controller.md.chunk_size).to eq(4.KiB)
+        expect(controller.md.chunk_size).to eq(Y2Storage::DiskSize.zero)
       end
 
       it "does not set the parity" do

test/y2storage/autoinst_profile/raid_options_section_test.rb

@@ -98,6 +98,42 @@
     end
   end
 
+  describe "#md_parity" do
+    subject(:raid_options) { described_class.new_from_hashes(spec) }
+
+    context "when parity_algorithm is not specified" do
+      let(:spec) { {} }
+
+      it "returns nil" do
+        expect(raid_options.md_parity).to be_nil
+      end
+    end
+
+    context "when parity_algorithm contains a valid modern value" do
+      let(:spec) { { "parity_algorithm" => "near_3" } }
+
+      it "returns the corresponding MdParity object" do
+        expect(raid_options.md_parity).to eq Y2Storage::MdParity::NEAR_3
+      end
+    end
+
+    context "when parity_algorithm contains a legacy (pre storage-ng) value" do
+      let(:spec) { { "parity_algorithm" => "o2" } }
+
+      it "returns the corresponding MdParity object" do
+        expect(raid_options.md_parity).to eq Y2Storage::MdParity::OFFSET_2
+      end
+    end
+
+    context "when parity_algorithm contains an invalid value" do
+      let(:spec) { { "parity_algorithm" => "invented" } }
+
+      it "returns nil" do
+        expect(raid_options.md_parity).to be_nil
+      end
+    end
+  end
+
   describe ".new_from_storage" do
     let(:numeric?) { false }
     let(:parent) { double("Installation::AutoinstProfile::SectionWithAttributes") }

test/y2storage/autoinst_proposal_md_test.rb

@@ -41,6 +41,9 @@
     let(:scenario) { "bug_1120979" }
 
     let(:format_md) { create }
+    let(:raid_options) do
+      { "raid_type"    => "raid1", "device_order" => ["/dev/vdb1", "/dev/vdc1"] }
+    end
     let(:partitioning) do
       [
         {
@@ -51,10 +54,7 @@
               {
                 "create" => create, "filesystem" => :xfs, "format" => format_md, "mount" => "/home",
                 "mountby" => :uuid, "partition_nr" => partition_nr,
-                "raid_options" => {
-                  "raid_type"    => "raid1",
-                  "device_order" => ["/dev/vdb1", "/dev/vdc1"]
-                }
+                "raid_options" => raid_options
               }
             ]
         },
@@ -328,5 +328,37 @@
         include_examples "all MD create/reuse combinations"
       end
     end
+
+    context "when a parity algorithm is specified " do
+      let(:raid_options) do
+        { "raid_type"    => "raid10", "parity_algorithm" => parity_algorithm }
+      end
+      let(:md_name_in_profile) { "/dev/md/0" }
+      let(:drive_device) { md_name_in_profile }
+      let(:partition_nr) { 0 }
+      let(:create) { true }
+      let(:create_vdb1) { true }
+      let(:create_vdc1) { true }
+
+      context "if parity is given as a modern identifier" do
+        let(:parity_algorithm) { "far_3" }
+
+        it "creates the RAID with the expected parity" do
+          proposal.propose
+          raid = proposal.devices.raids.first
+          expect(raid.md_parity).to eq Y2Storage::MdParity::FAR_3
+        end
+      end
+
+      context "if parity is given as a legacy identifier" do
+        let(:parity_algorithm) { "f3" }
+
+        it "creates the RAID with the expected parity" do
+          proposal.propose
+          raid = proposal.devices.raids.first
+          expect(raid.md_parity).to eq Y2Storage::MdParity::FAR_3
+        end
+      end
+    end
   end
 end