From 513dc8ba0c94273d590824f1edd4d2b9a87fe32d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Mon, 30 Oct 2017 12:49:31 +0000 Subject: [PATCH] wip --- src/lib/y2storage/autoinst_issues.rb | 2 + .../missing_reusable_device.rb | 52 ++++++ .../autoinst_issues/missing_reuse_info.rb | 52 ++++++ .../autoinst_profile/partitioning_section.rb | 2 +- .../proposal/autoinst_devices_planner.rb | 48 ++++-- .../proposal/autoinst_devices_planner_test.rb | 156 ++++++++++++++---- 6 files changed, 259 insertions(+), 53 deletions(-) create mode 100644 src/lib/y2storage/autoinst_issues/missing_reusable_device.rb create mode 100644 src/lib/y2storage/autoinst_issues/missing_reuse_info.rb diff --git a/src/lib/y2storage/autoinst_issues.rb b/src/lib/y2storage/autoinst_issues.rb index 5a2dbd76c5..d124635f76 100644 --- a/src/lib/y2storage/autoinst_issues.rb +++ b/src/lib/y2storage/autoinst_issues.rb @@ -41,3 +41,5 @@ module AutoinstIssues require "y2storage/autoinst_issues/missing_root" require "y2storage/autoinst_issues/exception" require "y2storage/autoinst_issues/no_disk_space" +require "y2storage/autoinst_issues/missing_reusable_device" +require "y2storage/autoinst_issues/missing_reuse_info" diff --git a/src/lib/y2storage/autoinst_issues/missing_reusable_device.rb b/src/lib/y2storage/autoinst_issues/missing_reusable_device.rb new file mode 100644 index 0000000000..890a62fc41 --- /dev/null +++ b/src/lib/y2storage/autoinst_issues/missing_reusable_device.rb @@ -0,0 +1,52 @@ +# encoding: utf-8 + +# Copyright (c) [2017] SUSE LLC +# +# All Rights Reserved. +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of version 2 of the GNU General Public License as published +# by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, contact SUSE LLC. +# +# To contact SUSE LLC about this file by physical or electronic mail, you may +# find current contact information at www.suse.com. + +require "y2storage/autoinst_issues/issue" + +module Y2Storage + module AutoinstIssues + # The proposal was successful but there is not root partition (/) defined. + # + # This is a fatal error because the installation is not possible. + class MissingReusableDevice < Issue + # @param section [AutoinstProfile::SectionWithAttributes] Section where it was detected + def initialize(section) + @section = section + end + + # Return problem severity + # + # @return [Symbol] :warn + # @see Issue#severity + def severity + :warn + end + + # Return the error message to be displayed + # + # @return [String] Error message + # @see Issue#message + def message + _("Reusable device not found") + end + end + end +end diff --git a/src/lib/y2storage/autoinst_issues/missing_reuse_info.rb b/src/lib/y2storage/autoinst_issues/missing_reuse_info.rb new file mode 100644 index 0000000000..5de6c8d65b --- /dev/null +++ b/src/lib/y2storage/autoinst_issues/missing_reuse_info.rb @@ -0,0 +1,52 @@ +# encoding: utf-8 + +# Copyright (c) [2017] SUSE LLC +# +# All Rights Reserved. +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of version 2 of the GNU General Public License as published +# by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, contact SUSE LLC. +# +# To contact SUSE LLC about this file by physical or electronic mail, you may +# find current contact information at www.suse.com. + +require "y2storage/autoinst_issues/issue" + +module Y2Storage + module AutoinstIssues + # The proposal was successful but there is not root partition (/) defined. + # + # This is a fatal error because the installation is not possible. + class MissingReuseInfo < Issue + # @param section [AutoinstProfile::SectionWithAttributes] Section where it was detected + def initialize(section) + @section = section + end + + # Return problem severity + # + # @return [Symbol] :warn + # @see Issue#severity + def severity + :warn + end + + # Return the error message to be displayed + # + # @return [String] Error message + # @see Issue#message + def message + _("Not enough information to locate a device to reuse") + end + end + end +end diff --git a/src/lib/y2storage/autoinst_profile/partitioning_section.rb b/src/lib/y2storage/autoinst_profile/partitioning_section.rb index 11143ebef0..ba7b48668a 100644 --- a/src/lib/y2storage/autoinst_profile/partitioning_section.rb +++ b/src/lib/y2storage/autoinst_profile/partitioning_section.rb @@ -62,7 +62,7 @@ def parent def self.new_from_hashes(drives_array) result = new result.drives = drives_array.each_with_object([]) do |hash, array| - drive = DriveSection.new_from_hashes(hash, self) + drive = DriveSection.new_from_hashes(hash, result) array << drive if drive end result diff --git a/src/lib/y2storage/proposal/autoinst_devices_planner.rb b/src/lib/y2storage/proposal/autoinst_devices_planner.rb index 898238eea3..9d674ddb7a 100644 --- a/src/lib/y2storage/proposal/autoinst_devices_planner.rb +++ b/src/lib/y2storage/proposal/autoinst_devices_planner.rb @@ -243,11 +243,9 @@ def add_subvolumes_attrs(device, section) # @param section [AutoinstProfile::PartitionSection] AutoYaST specification def add_partition_reuse(partition, section) partition_to_reuse = find_partition_to_reuse(devicegraph, section) + return unless partition_to_reuse partition.filesystem_type ||= partition_to_reuse.filesystem_type add_device_reuse(partition, partition_to_reuse.name, !!section.format) - # TODO: possible errors here - # - missing information about what device to use - # - the specified device was not found end # Set 'reusing' attributes for a logical volume @@ -283,7 +281,7 @@ def add_vg_reuse(vg, drive) vg.make_space_policy = drive.keep_unknown_lv ? :keep : :remove return unless vg.make_space_policy == :keep || vg.lvs.any?(&:reuse?) - vg_to_reuse = find_vg_to_reuse(devicegraph, vg) + vg_to_reuse = find_vg_to_reuse(devicegraph, vg, drive) vg.reuse = vg_to_reuse.vg_name if vg_to_reuse end @@ -291,11 +289,18 @@ def add_vg_reuse(vg, drive) # @param part_section [AutoinstProfile::PartitionSection] Partition specification # from AutoYaST def find_partition_to_reuse(devicegraph, part_section) - if part_section.partition_nr - devicegraph.partitions.find { |i| i.number == part_section.partition_nr } - elsif part_section.label - devicegraph.partitions.find { |i| i.filesystem_label == part_section.label } - end + device = + if part_section.partition_nr + devicegraph.partitions.find { |i| i.number == part_section.partition_nr } + elsif part_section.label + devicegraph.partitions.find { |i| i.filesystem_label == part_section.label } + else + issues_list.add(:missing_reuse_info, part_section) + nil + end + + issues_list.add(:missing_reusable_device, part_section) unless device + device end # @param devicegraph [Devicegraph] Devicegraph to search for the logical volume to reuse @@ -304,18 +309,28 @@ def find_partition_to_reuse(devicegraph, part_section) def find_lv_to_reuse(devicegraph, vg_name, part_section) vg = devicegraph.lvm_vgs.find { |v| v.vg_name == vg_name } return unless vg - if part_section.lv_name - vg.lvm_lvs.find { |v| v.lv_name == part_section.lv_name } - elsif part_section.label - vg.lvm_lvs.find { |v| v.filesystem_label == part_section.label } - end + device = + if part_section.lv_name + vg.lvm_lvs.find { |v| v.lv_name == part_section.lv_name } + elsif part_section.label + vg.lvm_lvs.find { |v| v.filesystem_label == part_section.label } + else + issues_list.add(:missing_reuse_info, part_section) + nil + end + + issues_list.add(:missing_reusable_device, part_section) unless device + device end # @param devicegraph [Devicegraph] Devicegraph to search for the volume group to reuse # @param vg [Planned::LvmVg] Planned volume group - def find_vg_to_reuse(devicegraph, vg) + # @param drive [AutoinstProfile::DriveSection] drive section describing + def find_vg_to_reuse(devicegraph, vg, drive) return nil unless vg.volume_group_name - devicegraph.lvm_vgs.find { |v| v.vg_name == vg.volume_group_name } + device = devicegraph.lvm_vgs.find { |v| v.vg_name == vg.volume_group_name } + issues_list.add(:missing_reusable_device, drive) unless device + device end # @return [DiskSize] Minimal partition size @@ -420,7 +435,6 @@ def root?(devices) return true if devices.any? { |d| d.respond_to?(:mount_point) && d.mount_point == "/" } issues_list.add(:missing_root) end - end end end diff --git a/test/y2storage/proposal/autoinst_devices_planner_test.rb b/test/y2storage/proposal/autoinst_devices_planner_test.rb index 4388963dd1..ffcf534920 100644 --- a/test/y2storage/proposal/autoinst_devices_planner_test.rb +++ b/test/y2storage/proposal/autoinst_devices_planner_test.rb @@ -89,7 +89,7 @@ context "when a partition label is specified" do let(:root_spec) do - { "create" => false, "mount" => "/", "filesystem" => "ext4", "label" => "root" } + { "create" => false, "mount" => "/", "filesystem" => :ext4, "label" => "root" } end it "reuses the partition with that label" do @@ -98,6 +98,42 @@ expect(root.reuse).to eq("/dev/sda3") end end + + context "when the partition to reuse does not exist" do + let(:root_spec) do + { "create" => false, "mount" => "/", "filesystem" => :ext4, "partition_nr" => 99 } + end + + it "adds a new partition" do + devices = planner.planned_devices(drives_map) + root = devices.find { |d| d.mount_point == "/" } + expect(root.reuse).to be_nil + end + + it "registers an issue" do + planner.planned_devices(drives_map) + issue = issues_list.find { |i| i.is_a?(Y2Storage::AutoinstIssues::MissingReusableDevice) } + expect(issue).to_not be_nil + end + end + + context "when no partition number or label is specified" do + let(:root_spec) do + { "create" => false, "mount" => "/", "filesystem" => :ext4 } + end + + it "adds a new partition" do + devices = planner.planned_devices(drives_map) + root = devices.find { |d| d.mount_point == "/" } + expect(root.reuse).to be_nil + end + + it "registers an issue" do + planner.planned_devices(drives_map) + issue = issues_list.find { |i| i.is_a?(Y2Storage::AutoinstIssues::MissingReuseInfo) } + expect(issue).to_not be_nil + end + end end context "specifying size" do @@ -503,50 +539,100 @@ context "reusing logical volumes" do let(:scenario) { "lvm-two-vgs" } - let(:root_spec) do - { - "create" => false, "mount" => "/", "filesystem" => "ext4", "lv_name" => "lv1", - "size" => "20G", "label" => "rootfs" - } - end + context "when volume name is specified" do + let(:root_spec) do + { + "create" => false, "mount" => "/", "filesystem" => "ext4", "lv_name" => "lv1", + "size" => "20G" + } + end - it "sets the reuse attribute of the volume group" do - _pv, vg = planner.planned_devices(drives_map) - expect(vg.reuse).to eq(lvm_group) - expect(vg.make_space_policy).to eq(:remove) + it "sets the reuse attribute of the volume group" do + _pv, vg = planner.planned_devices(drives_map) + expect(vg.reuse).to eq(lvm_group) + expect(vg.make_space_policy).to eq(:remove) + end + + it "sets the reuse attribute of logical volumes" do + _pv, vg = planner.planned_devices(drives_map) + expect(vg.reuse).to eq(lvm_group) + expect(vg.lvs).to contain_exactly( + an_object_having_attributes( + "logical_volume_name" => "lv1", + "reuse" => "/dev/vg0/lv1" + ) + ) + end end - it "sets the reuse attribute of logical volumes" do - _pv, vg = planner.planned_devices(drives_map) - expect(vg.reuse).to eq(lvm_group) - expect(vg.lvs).to contain_exactly( - an_object_having_attributes( - "logical_volume_name" => "lv1", - "reuse" => "/dev/vg0/lv1" + context "when label is specified" do + let(:root_spec) do + { + "create" => false, "mount" => "/", "filesystem" => "ext4", + "size" => "20G", "label" => "rootfs" + } + end + + it "sets the reuse attribute of logical volumes" do + _pv, vg = planner.planned_devices(drives_map) + expect(vg.reuse).to eq(lvm_group) + expect(vg.lvs).to contain_exactly( + an_object_having_attributes( + "logical_volume_name" => "lv2", + "reuse" => "/dev/vg0/lv2" + ) ) - ) + end end - end - context "reusing logical volumes by label" do - let(:scenario) { "lvm-two-vgs" } + context "when the logical volume to be reused does not exist" do + let(:root_spec) do + { + "create" => false, "mount" => "/", "filesystem" => "ext4", "lv_name" => "new_lv", + "size" => "20G" + } + end - let(:root_spec) do - { - "create" => false, "mount" => "/", "filesystem" => "ext4", - "size" => "20G", "label" => "rootfs" - } + it "adds a new logical volume" do + _pv, vg = planner.planned_devices(drives_map) + expect(vg.reuse).to be_nil + expect(vg.lvs).to contain_exactly( + an_object_having_attributes( + "logical_volume_name" => "new_lv", + "reuse" => nil + ) + ) + end + + it "registers an issue" do + planner.planned_devices(drives_map) + issue = issues_list.find { |i| i.is_a?(Y2Storage::AutoinstIssues::MissingReusableDevice) } + expect(issue).to_not be_nil + end end - it "sets the reuse attribute of logical volumes" do - _pv, vg = planner.planned_devices(drives_map) - expect(vg.reuse).to eq(lvm_group) - expect(vg.lvs).to contain_exactly( - an_object_having_attributes( - "logical_volume_name" => "lv2", - "reuse" => "/dev/vg0/lv2" + context "when no volume name or label is specified" do + let(:root_spec) do + { + "create" => false, "mount" => "/", "filesystem" => "ext4", "size" => "20G" + } + end + + it "adds a new logical volume" do + _pv, vg = planner.planned_devices(drives_map) + expect(vg.reuse).to be_nil + expect(vg.lvs).to contain_exactly( + an_object_having_attributes( + "reuse" => nil + ) ) - ) + end + + it "registers an issue" do + planner.planned_devices(drives_map) + issue = issues_list.find { |i| i.is_a?(Y2Storage::AutoinstIssues::MissingReusableDevice) } + expect(issue).to_not be_nil + end end end