diff --git a/src/lib/y2storage/planned/device.rb b/src/lib/y2storage/planned/device.rb index 6af37bf4d1..b26eadf232 100644 --- a/src/lib/y2storage/planned/device.rb +++ b/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) diff --git a/src/lib/y2storage/planned/partition.rb b/src/lib/y2storage/planned/partition.rb index 66079b1ec3..348432db87 100644 --- a/src/lib/y2storage/planned/partition.rb +++ b/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, diff --git a/src/lib/y2storage/proposal/autoinst_disk_device_planner.rb b/src/lib/y2storage/proposal/autoinst_disk_device_planner.rb index 3db32801e4..dff638c972 100644 --- a/src/lib/y2storage/proposal/autoinst_disk_device_planner.rb +++ b/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 diff --git a/src/lib/y2storage/proposal/autoinst_drive_planner.rb b/src/lib/y2storage/proposal/autoinst_drive_planner.rb index 8aa556838b..feee89419b 100644 --- a/src/lib/y2storage/proposal/autoinst_drive_planner.rb +++ b/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 diff --git a/src/lib/y2storage/proposal/autoinst_vg_planner.rb b/src/lib/y2storage/proposal/autoinst_vg_planner.rb index a5bf969ce7..ad3c4c0c22 100644 --- a/src/lib/y2storage/proposal/autoinst_vg_planner.rb +++ b/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,24 +52,24 @@ 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 @@ -77,16 +77,16 @@ def planned_for_lv(drive, vg, section) # 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 @@ -94,15 +94,15 @@ def add_lv_reuse(lv, vg_name, section) # 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,12 +170,12 @@ 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) @@ -183,12 +183,12 @@ def assign_size_to_lv(vg, lv, lv_section) 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 diff --git a/test/y2storage/proposal/autoinst_disk_device_planner_test.rb b/test/y2storage/proposal/autoinst_disk_device_planner_test.rb index 56986707e3..c53b06987d 100644 --- a/test/y2storage/proposal/autoinst_disk_device_planner_test.rb +++ b/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 diff --git a/test/y2storage/proposal/autoinst_vg_planner_test.rb b/test/y2storage/proposal/autoinst_vg_planner_test.rb index 0a2536cd2f..f074ba3628 100755 --- a/test/y2storage/proposal/autoinst_vg_planner_test.rb +++ b/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