Merge 9492876 into e240489

src/lib/y2storage/planned/device.rb

@@ -109,6 +109,16 @@ def reuse?
         !(reuse_name.nil? || reuse_name.empty?) || !reuse_sid.nil?
       end
 
+      # Determines whether the device is used as part of a VG or a MD
+      #
+      # @todo This method might be implemented in a way which is extended
+      #   by each mixin.
+      #
+      # @return [Boolean]
+      def component?
+        (respond_to?(:lvm_pv?) && lvm_pv?) || (respond_to?(:md_member?) && md_member?)
+      end
+
     protected
 
       def reuse_device!(dev)

src/lib/y2storage/planned/partition.rb

@@ -78,13 +78,6 @@ def initialize(mount_point, filesystem_type = nil)
         @primary = false
       end
 
-      # Determines whether the partition is used as part of a VG or a MD
-      #
-      # @return [Boolean]
-      def component?
-        lvm_pv? || md_member?
-      end
-
       def self.to_string_attrs
         [
           :mount_point, :reuse_name, :reuse_sid, :min_size, :max_size,

src/lib/y2storage/proposal/autoinst_disk_device_planner.rb

@@ -58,7 +58,7 @@ def planned_devices(drive)
       def planned_for_disk(disk, drive)
         master_partition = drive.master_partition
         result = if master_partition
-          planned_for_full_disk(drive, master_partition)
+          planned_for_full_disk(disk, drive, master_partition)
         else
           planned_for_partitions(disk, drive)
         end
@@ -81,12 +81,12 @@ def planned_for_disk(disk, drive)
       #
       # @note The part argument is used when we emulate the sle12 behavior to
       #   have partition 0 mean the full disk.
-      def planned_for_full_disk(drive, part)
+      def planned_for_full_disk(disk, drive, part)
         planned_disk = Y2Storage::Planned::Disk.new
         device_config(planned_disk, part, drive)
         planned_disk.lvm_volume_group_name = part.lvm_group
         planned_disk.raid_name = part.raid_name
-        add_device_reuse(planned_disk, drive.device, part)
+        add_device_reuse(planned_disk, disk, part)
 
         [planned_disk]
       end
@@ -134,7 +134,8 @@ def planned_for_stray_devices(drive)
             stray.filesystem_type = device_to_use.filesystem_type if device_to_use
           end
 
-          add_device_reuse(stray, name, section)
+          device = devicegraph.find_by_name(name)
+          add_device_reuse(stray, device, section)
 
           result << stray
         end

src/lib/y2storage/proposal/autoinst_drive_planner.rb

@@ -166,13 +166,22 @@ def read_only?(mount_point)
         !!spec && spec.btrfs_read_only?
       end
 
-      # @param device  [Planned::Partition,Planned::LvmLV,Planned::Md] Planned device
-      # @param name    [String] Name of the device to reuse
-      # @param section [AutoinstProfile::PartitionSection] AutoYaST specification
-      def add_device_reuse(device, name, section)
-        device.reuse_name = name
-        device.reformat = !!section.format
-        device.resize = !!section.resize if device.respond_to?(:resize=)
+      # @param planned_device [Planned::Partition,Planned::LvmLV,Planned::Md] Planned device
+      # @param device         [Y2Storage::Device] Device to reuse
+      # @param section        [AutoinstProfile::PartitionSection] AutoYaST specification
+      def add_device_reuse(planned_device, device, section)
+        planned_device.reuse_name = device.is_a?(LvmVg) ? device.volume_group_name : device.name
+        planned_device.reformat = !!section.format
+        planned_device.resize = !!section.resize if planned_device.respond_to?(:resize=)
+        check_reusable_filesystem(planned_device, device, section) if device.respond_to?(:filesystem)
+      end
+
+      # @param planned_device [Planned::Partition,Planned::LvmLV,Planned::Md] Planned device
+      # @param device         [Y2Storage::Device] Device to reuse
+      # @param section        [AutoinstProfile::PartitionSection] AutoYaST specification
+      def check_reusable_filesystem(planned_device, device, section)
+        return if planned_device.reformat || device.filesystem || planned_device.component?
+        issues_list.add(:missing_reusable_filesystem, section)
       end
 
       # Parse the 'size' element
@@ -206,7 +215,7 @@ def add_partition_reuse(partition, section)
         partition_to_reuse = find_partition_to_reuse(partition, section)
         return unless partition_to_reuse
         partition.filesystem_type ||= partition_to_reuse.filesystem_type
-        add_device_reuse(partition, partition_to_reuse.name, section)
+        add_device_reuse(partition, partition_to_reuse, section)
         if !partition.reformat && !partition_to_reuse.filesystem && !partition.component?
           issues_list.add(:missing_reusable_filesystem, section)
         end

src/lib/y2storage/proposal/autoinst_vg_planner.rb

@@ -32,18 +32,18 @@ class AutoinstVgPlanner < AutoinstDrivePlanner
       #   the volume group
       # @return [Planned::LvmVg] Planned volume group
       def planned_devices(drive)
-        vg = Y2Storage::Planned::LvmVg.new(volume_group_name: File.basename(drive.device))
+        planned_vg = Y2Storage::Planned::LvmVg.new(volume_group_name: File.basename(drive.device))
 
         pools, regular = drive.partitions.partition(&:pool)
-        (pools + regular).each_with_object(vg.lvs) do |lv_section, lvs|
-          lv = planned_for_lv(drive, vg, lv_section)
-          next if lv.nil? || lv.lv_type == LvType::THIN
-          lvs << lv
+        (pools + regular).each_with_object(planned_vg.lvs) do |lv_section, planned_lvs|
+          planned_lv = planned_for_lv(drive, planned_vg, lv_section)
+          next if planned_lv.nil? || planned_lv.lv_type == LvType::THIN
+          planned_lvs << planned_lv
         end
 
-        vg.thin_pool_lvs.each { |v| add_thin_pool_lv_reuse(v, drive) }
-        add_vg_reuse(vg, drive)
-        [vg]
+        planned_vg.thin_pool_lvs.each { |v| add_thin_pool_lv_reuse(v, drive) }
+        add_vg_reuse(planned_vg, drive)
+        [planned_vg]
       end
 
     private
@@ -52,57 +52,57 @@ def planned_devices(drive)
       #
       # @param drive [AutoinstProfile::DriveSection] drive section describing
       #   the volume group
-      # @param vg [Planned::LvmVg] Planned volume group where the logical volume will
+      # @param planned_vg [Planned::LvmVg] Planned volume group where the logical volume will
       #   be included
       # @param section [AutoinstProfile::PartitionSection] partition section describing
       #   the logical volume
       # @return [Planned::LvmLv,nil] Planned logical volume; nil if it could not be
       #   planned
-      def planned_for_lv(drive, vg, section)
+      def planned_for_lv(drive, planned_vg, section)
         # TODO: fix Planned::LvmLv.initialize
-        lv = Y2Storage::Planned::LvmLv.new(nil, nil)
-        lv.logical_volume_name = section.lv_name
-        lv.lv_type = lv_type_for(section)
-        add_stripes(lv, section)
-        device_config(lv, section, drive)
+        planned_lv = Y2Storage::Planned::LvmLv.new(nil, nil)
+        planned_lv.logical_volume_name = section.lv_name
+        planned_lv.lv_type = lv_type_for(section)
+        add_stripes(planned_lv, section)
+        device_config(planned_lv, section, drive)
         if section.used_pool
-          return nil unless add_to_thin_pool(lv, vg, section)
+          return nil unless add_to_thin_pool(planned_lv, planned_vg, section)
         end
-        add_lv_reuse(lv, vg.volume_group_name, section) if section.create == false
-        assign_size_to_lv(vg, lv, section) ? lv : nil
+        add_lv_reuse(planned_lv, planned_vg, section) if section.create == false
+        assign_size_to_lv(planned_vg, planned_lv, section) ? planned_lv : nil
       end
 
       # Set 'reusing' attributes for a logical volume
       #
       # This method modifies the first argument setting the values related to
       # reusing a logical volume (reuse and format).
       #
-      # @param lv      [Planned::LvmLv] Planned logical volume
-      # @param vg_name [String]         Volume group name to search for the logical volume to reuse
+      # @param planned_lv [Planned::LvmLv] Planned logical volume
+      # @param planned_vg [Planned::LvmVg] Volume group to search for the logical volume to reuse
       # @param section [AutoinstProfile::PartitionSection] AutoYaST specification
-      def add_lv_reuse(lv, vg_name, section)
-        lv_to_reuse = find_lv_to_reuse(vg_name, section)
+      def add_lv_reuse(planned_lv, planned_vg, section)
+        lv_to_reuse, vg = find_lv_to_reuse(planned_vg.volume_group_name, section)
         return unless lv_to_reuse
-        lv.logical_volume_name ||= lv_to_reuse.lv_name
-        lv.filesystem_type ||= lv_to_reuse.filesystem_type
-        add_device_reuse(lv, lv_to_reuse.name, section)
-        add_device_reuse(lv.thin_pool, vg_name, section) if lv.thin_pool
+        planned_lv.logical_volume_name ||= lv_to_reuse.lv_name
+        planned_lv.filesystem_type ||= lv_to_reuse.filesystem_type
+        add_device_reuse(planned_lv, lv_to_reuse, section)
+        add_device_reuse(planned_lv.thin_pool, vg, section) if planned_lv.thin_pool
       end
 
       # Set 'reusing' attributes for a volume group
       #
       # This method modifies the first argument setting the values related to
       # reusing a volume group (reuse and format).
       #
-      # @param vg   [Planned::LvmVg] Planned volume group
-      # @param drive [AutoinstProfile::DriveSection] drive section describing
+      # @param planned_vg [Planned::LvmVg] Planned volume group
+      # @param drive      [AutoinstProfile::DriveSection] drive section describing
       #   the volume group
-      def add_vg_reuse(vg, drive)
-        vg.make_space_policy = drive.keep_unknown_lv ? :keep : :remove
+      def add_vg_reuse(planned_vg, drive)
+        planned_vg.make_space_policy = drive.keep_unknown_lv ? :keep : :remove
 
-        return unless vg.make_space_policy == :keep || vg.all_lvs.any?(&:reuse?)
-        vg_to_reuse = find_vg_to_reuse(vg, drive)
-        vg.reuse_name = vg_to_reuse.vg_name if vg_to_reuse
+        return unless planned_vg.make_space_policy == :keep || planned_vg.all_lvs.any?(&:reuse?)
+        vg_to_reuse = find_vg_to_reuse(planned_vg, drive)
+        planned_vg.reuse_name = vg_to_reuse.vg_name if vg_to_reuse
       end
 
       # Set 'reusing' attributes for a thin pool logical volume
@@ -111,11 +111,11 @@ def add_vg_reuse(vg, drive)
       # a thin logical volume (reuse_name). A thin pool will be planned to be
       # reused if any of its logical volumes will be reused.
       #
-      # @param lv   [Planned::LvmLv] Thin logical volume
-      def add_thin_pool_lv_reuse(lv, _drive)
-        return unless lv.thin_lvs.any?(&:reuse?)
-        lv_to_reuse = devicegraph.lvm_lvs.find { |v| lv.logical_volume_name == v.lv_name }
-        lv.reuse_name = lv_to_reuse.name
+      # @param planned_lv [Planned::LvmLv] Thin logical volume
+      def add_thin_pool_lv_reuse(planned_lv, _drive)
+        return unless planned_lv.thin_lvs.any?(&:reuse?)
+        lv_to_reuse = devicegraph.lvm_lvs.find { |v| planned_lv.logical_volume_name == v.lv_name }
+        planned_lv.reuse_name = lv_to_reuse.name
       end
 
       # @param vg_name     [String]      Volume group name to search for the logical volume to reuse
@@ -135,11 +135,11 @@ def find_lv_to_reuse(vg_name, part_section)
           end
 
         issues_list.add(:missing_reusable_device, part_section) unless device
-        :missing_info == device ? nil : device
+        :missing_info == device ? nil : [device, parent]
       end
 
-      # @param vg_name     [String]      Volume group name to search for the logical volume
-      # @param part_section   [AutoinstProfile::PartitionSection] LV specification from AutoYaST
+      # @param vg_name      [String]      Volume group name to search for the logical volume
+      # @param part_section [AutoinstProfile::PartitionSection] LV specification from AutoYaST
       def find_lv_parent(vg_name, part_section)
         vg = devicegraph.lvm_vgs.find { |v| v.vg_name == vg_name }
         if vg.nil?
@@ -150,11 +150,11 @@ def find_lv_parent(vg_name, part_section)
         part_section.used_pool ? find_thin_pool_lv(vg, part_section) : vg
       end
 
-      # @param vg          [Planned::LvmVg] Planned volume group
+      # @param planned_vg  [Planned::LvmVg] Planned volume group
       # @param drive       [AutoinstProfile::DriveSection] drive section describing
-      def find_vg_to_reuse(vg, drive)
-        return nil unless vg.volume_group_name
-        device = devicegraph.lvm_vgs.find { |v| v.vg_name == vg.volume_group_name }
+      def find_vg_to_reuse(planned_vg, drive)
+        return nil unless planned_vg.volume_group_name
+        device = devicegraph.lvm_vgs.find { |v| v.vg_name == planned_vg.volume_group_name }
         issues_list.add(:missing_reusable_device, drive) unless device
         device
       end
@@ -170,25 +170,25 @@ def find_thin_pool_lv(vg, part_section)
 
       # Assign LV size according to AutoYaST section
       #
-      # @param vg         [Planned::LvmVg] Volume group
-      # @param lv         [Planned::LvmLv] Logical volume
+      # @param planned_vg [Planned::LvmVg] Volume group
+      # @param planned_lv [Planned::LvmLv] Logical volume
       # @param lv_section [AutoinstProfile::PartitionSection] AutoYaST section
       # @return [Boolean] true if the size was parsed and asssigned; false it was not valid
-      def assign_size_to_lv(vg, lv, lv_section)
-        size_info = parse_size(lv_section, vg.extent_size, DiskSize.unlimited)
+      def assign_size_to_lv(planned_vg, planned_lv, lv_section)
+        size_info = parse_size(lv_section, planned_vg.extent_size, DiskSize.unlimited)
 
         if size_info.nil?
           issues_list.add(:invalid_value, lv_section, :size)
           return false
         end
 
         if size_info.percentage
-          lv.percent_size = size_info.percentage
+          planned_lv.percent_size = size_info.percentage
         else
-          lv.min_size = size_info.min
-          lv.max_size = size_info.max
+          planned_lv.min_size = size_info.min
+          planned_lv.max_size = size_info.max
         end
-        lv.weight = 1 if size_info.unlimited?
+        planned_lv.weight = 1 if size_info.unlimited?
 
         true
       end
@@ -209,27 +209,27 @@ def lv_type_for(section)
 
       # Add a logical volume to a thin pool
       #
-      # @param lv [Planned::LvmLv] Planned logical volume
-      # @param vg [Planned::LvmVg] Planned volume group
+      # @param planned_lv [Planned::LvmLv] Planned logical volume
+      # @param planned_vg [Planned::LvmVg] Planned volume group
       # @param section [AutoinstProfile::PartitionSection] AutoYaST specification
       # @return [Boolean] True if it was successfully added; false otherwise.
-      def add_to_thin_pool(lv, vg, section)
-        thin_pool = vg.thin_pool_lvs.find { |v| v.logical_volume_name == section.used_pool }
+      def add_to_thin_pool(planned_lv, planned_vg, section)
+        thin_pool = planned_vg.thin_pool_lvs.find { |v| v.logical_volume_name == section.used_pool }
         if thin_pool.nil?
           issues_list.add(:thin_pool_not_found, section)
           return false
         end
-        thin_pool.add_thin_lv(lv)
+        thin_pool.add_thin_lv(planned_lv)
       end
 
       # Sets stripes related attributes
       #
-      # @param lv      [Planned::LvmLv] Planned logical volume
-      # @param section [AutoinstProfile::PartitionSection] partition section describing
+      # @param planned_lv [Planned::LvmLv] Planned logical volume
+      # @param section    [AutoinstProfile::PartitionSection] partition section describing
       #   the logical volume
-      def add_stripes(lv, section)
-        lv.stripe_size = DiskSize.KiB(section.stripe_size.to_i) if section.stripe_size
-        lv.stripes = section.stripes
+      def add_stripes(planned_lv, section)
+        planned_lv.stripe_size = DiskSize.KiB(section.stripe_size.to_i) if section.stripe_size
+        planned_lv.stripes = section.stripes
       end
     end
   end

test/y2storage/proposal/autoinst_disk_device_planner_test.rb

@@ -522,6 +522,27 @@
           )
         end
       end
+
+      context "when trying to reuse a filesystem which does not exist" do
+        let(:disk_spec) do
+          { "device" => "/dev/sda", "disklabel" => "none", "partitions" => [root_spec] }
+        end
+        let(:root_spec) do
+          { "create" => false, "format" => false, "mount" => "/" }
+        end
+
+        before do
+          sda = Y2Storage::BlkDevice.find_by_name(fake_devicegraph, "/dev/sda")
+          sda.remove_descendants
+        end
+
+        it "registers an issue" do
+          expect(issues_list).to be_empty
+          planner.planned_devices(drive)
+          issue = issues_list.find { |i| i.is_a?(Y2Storage::AutoinstIssues::MissingReusableFilesystem) }
+          expect(issue).to_not be_nil
+        end
+      end
     end
   end
 end

test/y2storage/proposal/autoinst_vg_planner_test.rb

@@ -190,6 +190,22 @@
             )
           )
         end
+
+        context "when the filesystem does not exist" do
+          before do
+            lv = fake_devicegraph.find_by_name("/dev/vg0/lv1")
+            lv.remove_descendants
+          end
+
+          it "registers an issue" do
+            expect(issues_list).to be_empty
+            planner.planned_devices(drive)
+            issue = issues_list.find do |i|
+              i.is_a?(Y2Storage::AutoinstIssues::MissingReusableFilesystem)
+            end
+            expect(issue).to_not be_nil
+          end
+        end
       end
 
       context "when label is specified" do