Merge 0e4e30a into 4307b87

yast · Feb 28, 2019 · a7b799b · a7b799b
2 parents 4307b87 + 0e4e30a
commit a7b799b
Show file tree

Hide file tree

Showing 4 changed files with 53 additions and 9 deletions.
diff --git a/package/yast2-storage-ng.changes b/package/yast2-storage-ng.changes
@@ -1,3 +1,11 @@
+-------------------------------------------------------------------
+Thu Feb 28 15:33:19 UTC 2019 - David Díaz <dgonzalez@suse.com>
+
+- Do not crash when using an old MD RAID schema and the
+  partition_nr attribute is missing. Instead, do not allow to
+  continue because a missing value issue (bsc#1126059).
+- 4.1.68
+
 -------------------------------------------------------------------
 Wed Feb 27 09:57:10 UTC 2019 - David Díaz <dgonzalez@suse.com>
 

diff --git a/package/yast2-storage-ng.spec b/package/yast2-storage-ng.spec
@@ -16,7 +16,7 @@
 #
 
 Name:		yast2-storage-ng
-Version:	4.1.67
+Version:	4.1.68
 Release:	0
 
 BuildRoot:	%{_tmppath}/%{name}-%{version}-build

diff --git a/src/lib/y2storage/proposal/autoinst_md_planner.rb b/src/lib/y2storage/proposal/autoinst_md_planner.rb
@@ -58,8 +58,9 @@ def planned_devices(drive)
       #   devices (old-style AutoYaST)
       # @return [Array<Planned::Md>] List of planned MD RAID devices
       def non_partitioned_md_old_style(drive)
-        drive.partitions.map do |part_section|
-          md_from_partition_section(drive, part_section)
+        drive.partitions.each_with_object([]) do |part_section, mds|
+          md = md_from_partition_section(drive, part_section)
+          mds << md if md
         end
       end
 
@@ -97,9 +98,11 @@ def partitioned_md(drive)
       # @param drive        [AutoinstProfile::DriveSection] drive section describing the list of MD
       #   RAID devices (old-style AutoYaST)
       # @param part_section [AutoinstProfile::PartitionSection] partition section describing the
-      #   an MD RAID
-      # @return [Array<Planned::Md>] List of planned MD RAID devices
+      #   MD RAID
+      # @return [Planned::Md, nil] the planned MD RAID device; nil if could not be created
       def md_from_partition_section(drive, part_section)
+        return nil unless valid_part_section?(part_section)
+
         md = Planned::Md.new(name: part_section.name_for_md)
         device_config(md, part_section, drive)
         md.lvm_volume_group_name = part_section.lvm_group
@@ -108,6 +111,18 @@ def md_from_partition_section(drive, part_section)
         md
       end
 
+      # Determines whether given partition section is valid and registers issues if not
+      #
+      # @param part_section [AutoinstProfile::PartitionSection]
+      # @return [Boolean] true when the partition section is valid; false otherwise
+      def valid_part_section?(part_section)
+        return true if part_section.partition_nr
+
+        issues_list.add(:missing_value, :partition_nr, part_section)
+
+        false
+      end
+
       # Adds RAID options to a planned RAID
       #
       # @param md [Planned::Md] Planned RAID

diff --git a/test/y2storage/proposal/autoinst_md_planner_test.rb b/test/y2storage/proposal/autoinst_md_planner_test.rb
@@ -172,17 +172,19 @@
         { "raid_type" => "raid5" }
       end
 
+      let(:root_partition_nr) { 1 }
       let(:root_raid_spec) do
         {
-          "mount" => "/", "filesystem" => "ext4", "size" => "max", "partition_nr" => 1,
-          "raid_options" => { "raid_type" => "raid5" }
+          "mount" => "/", "filesystem" => "ext4", "size" => "max",
+          "partition_nr" => root_partition_nr, "raid_options" => { "raid_type" => "raid5" }
         }
       end
 
+      let(:home_partition_nr) { 2 }
       let(:home_raid_spec) do
         {
-          "mount" => "/home", "filesystem" => "xfs", "size" => "max", "partition_nr" => 2,
-          "raid_options" => { "raid_type" => "raid1" }
+          "mount" => "/home", "filesystem" => "xfs", "size" => "max",
+          "partition_nr" => home_partition_nr, "raid_options" => { "raid_type" => "raid1" }
         }
       end
 
@@ -197,6 +199,25 @@
           )
         )
       end
+
+      context "when a partition number is missing" do
+        let(:home_partition_nr) { nil }
+
+        it "does not include the partition in the planned RAID" do
+          mds = planner.planned_devices(drive)
+          expect(mds).to contain_exactly(
+            an_object_having_attributes(
+              "name" => "/dev/md/1", "md_level" => Y2Storage::MdLevel::RAID5
+            )
+          )
+        end
+
+        it "registers an issue" do
+          planner.planned_devices(drive)
+          issue = issues_list.find { |i| i.is_a?(Y2Storage::AutoinstIssues::MissingValue) }
+          expect(issue).to_not be_nil
+        end
+      end
     end
 
     describe "snapshots" do