From d7188258e724ed422eecce488c1512c965e12e38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Fri, 12 Jan 2018 18:56:41 +0000 Subject: [PATCH 01/21] Add support for matching volumes --- src/lib/y2storage/blk_device.rb | 16 ++ src/lib/y2storage/match_volume_spec.rb | 105 +++++++++++++ src/lib/y2storage/partition.rb | 7 + src/lib/y2storage/planned/lvm_lv.rb | 16 ++ src/lib/y2storage/planned/md.rb | 16 ++ src/lib/y2storage/planned/partition.rb | 15 +- test/y2storage/match_volume_spec_test.rb | 191 +++++++++++++++++++++++ test/y2storage/partition_test.rb | 39 +++++ test/y2storage/planned/lvm_lv_test.rb | 49 ++++++ test/y2storage/planned/md_test.rb | 47 ++++++ test/y2storage/planned/partition_test.rb | 75 +++++++++ 11 files changed, 575 insertions(+), 1 deletion(-) create mode 100644 src/lib/y2storage/match_volume_spec.rb create mode 100644 test/y2storage/match_volume_spec_test.rb create mode 100755 test/y2storage/planned/partition_test.rb diff --git a/src/lib/y2storage/blk_device.rb b/src/lib/y2storage/blk_device.rb index 8794082145..9e8e7905be 100644 --- a/src/lib/y2storage/blk_device.rb +++ b/src/lib/y2storage/blk_device.rb @@ -23,6 +23,7 @@ require "y2storage/device" require "y2storage/hwinfo_reader" require "y2storage/comparable_by_name" +require "y2storage/match_volume_spec" module Y2Storage # Base class for most devices having a device name, udev path and udev ids. @@ -33,6 +34,7 @@ class BlkDevice < Device downcast_to: ["Partitionable", "Partition", "Encryption", "LvmLv"] include ComparableByName + include MatchVolumeSpec # @!method self.all(devicegraph) # @param devicegraph [Devicegraph] @@ -409,5 +411,19 @@ def recoverable_size return DiskSize.zero unless resize_info.resize_ok? size - resize_info.min_size end + + protected + + # Values for volume specification matching + # + # @see MatchVolumeSpec + def volume_match_values + { + mount_point: filesystem_mountpoint, + size: size, + fs_type: filesystem_type, + partition_id: nil + } + end end end diff --git a/src/lib/y2storage/match_volume_spec.rb b/src/lib/y2storage/match_volume_spec.rb new file mode 100644 index 0000000000..dfbb39322f --- /dev/null +++ b/src/lib/y2storage/match_volume_spec.rb @@ -0,0 +1,105 @@ +# 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. + +module Y2Storage + # Mixin to match with a volume specification + module MatchVolumeSpec + # Whether matches with the given volume specification + # + # @note Matching is performed by mount point, size, filesystem type and partition id. + # The exclude param can be used to avoid some of those matches. + # + # @param volume [VolumeSpecification] + # @param exclude [Symbol] :mount_point, :size, :fs_type, :partition_id + # + # @return [Boolean] whether matches the volume; false otherwise. + def match_volume?(volume, exclude: []) + exclude = [exclude].flatten + + match = true + match &&= match_mount_point?(volume) unless exclude.include?(:mount_point) + match &&= match_size?(volume) unless exclude.include?(:size) + match &&= match_fs_type?(volume) unless exclude.include?(:fs_type) + match &&= match_partition_id?(volume) unless exclude.include?(:partition_id) + match + end + + protected + + # This can be redefined with the values to take into account during matching + # + # @note Only symbols :mount_point, :size, :fs_type and :partition_id are used + # for matching. + # + # @example + # def volume_match_values + # { size: device.min_size, partition_id: id, fs_type: filesystem.type } + # end + # + # @return [Hash] + def volume_match_values + {} + end + + # Whether the mount point value matches the volume mount point + # + # @param volume [VolumeSpecification] + # @return [Boolean] + def match_mount_point?(volume) + volume_match_values[:mount_point] == volume.mount_point + end + + # Whether the size value matches the volume min size + # + # @note The size matches when the give size value is equal or bigger + # than the volume min size. + # + # @param volume [VolumeSpecification] + # @return [Boolean] + def match_size?(volume) + volume_match_values[:size] >= volume.min_size + end + + # Whether the fileystem type matches the volume filesystem type + # + # @note This is always considered as true when the volume specification does not + # have any filesystem type. + # + # @param volume [VolumeSpecification] + # @return [Boolean] + def match_fs_type?(volume) + return true if volume.fs_types.empty? + volume.fs_types.include?(volume_match_values[:fs_type]) + end + + # Whether the partition id matches the volume partition id + # + # @note This is always considered as true when the volume specification does not + # have any partition id. + # + # @param volume [VolumeSpecification] + # @return [Boolean] + def match_partition_id?(volume) + return true if volume.partition_id.nil? + volume_match_values[:partition_id] == volume.partition_id + end + end +end diff --git a/src/lib/y2storage/partition.rb b/src/lib/y2storage/partition.rb index 7c82beddad..c55046f42d 100644 --- a/src/lib/y2storage/partition.rb +++ b/src/lib/y2storage/partition.rb @@ -226,6 +226,13 @@ def adapted_id=(partition_id) protected + # Values for volume specification matching + # + # @see MatchVolumeSpec + def volume_match_values + super.merge(partition_id: id) + end + def types_for_is super << :partition end diff --git a/src/lib/y2storage/planned/lvm_lv.rb b/src/lib/y2storage/planned/lvm_lv.rb index 6e44477571..4df55e90c0 100644 --- a/src/lib/y2storage/planned/lvm_lv.rb +++ b/src/lib/y2storage/planned/lvm_lv.rb @@ -22,6 +22,7 @@ require "yast" require "y2storage/planned/device" require "y2storage/planned/mixins" +require "y2storage/match_volume_spec" module Y2Storage module Planned @@ -35,6 +36,7 @@ class LvmLv < Device include Planned::CanBeResized include Planned::CanBeMounted include Planned::CanBeEncrypted + include MatchVolumeSpec # @return [String] name to use for Y2Storage::LvmLv#lv_name attr_accessor :logical_volume_name @@ -107,6 +109,20 @@ def size_in(volume_group) def self.to_string_attrs [:mount_point, :reuse, :min_size, :max_size, :logical_volume_name, :subvolumes] end + + protected + + # Values for volume specification matching + # + # @see MatchVolumeSpec + def volume_match_values + { + mount_point: mount_point, + size: min_size, + fs_type: filesystem_type, + partition_id: nil + } + end end end end diff --git a/src/lib/y2storage/planned/md.rb b/src/lib/y2storage/planned/md.rb index fd11848b71..ec49544e80 100644 --- a/src/lib/y2storage/planned/md.rb +++ b/src/lib/y2storage/planned/md.rb @@ -24,6 +24,7 @@ require "yast" require "y2storage/planned/device" require "y2storage/planned/mixins" +require "y2storage/match_volume_spec" module Y2Storage module Planned @@ -36,6 +37,7 @@ class Md < Device include Planned::CanBeMounted include Planned::CanBeEncrypted include Planned::CanBePv + include MatchVolumeSpec # @return [name] device name of the MD RAID attr_accessor :name @@ -94,6 +96,20 @@ def add_devices(md_device, devices) def self.to_string_attrs [:mount_point, :reuse, :name, :lvm_volume_group_name, :subvolumes] end + + protected + + # Values for volume specification matching + # + # @see MatchVolumeSpec + def volume_match_values + { + mount_point: mount_point, + size: DiskSize.zero, + fs_type: filesystem_type, + partition_id: nil + } + end end end end diff --git a/src/lib/y2storage/planned/partition.rb b/src/lib/y2storage/planned/partition.rb index 4aaabe1803..0fdfa5130e 100644 --- a/src/lib/y2storage/planned/partition.rb +++ b/src/lib/y2storage/planned/partition.rb @@ -22,6 +22,7 @@ require "yast" require "y2storage/planned/device" require "y2storage/planned/mixins" +require "y2storage/match_volume_spec" module Y2Storage module Planned @@ -36,6 +37,7 @@ class Partition < Device include Planned::CanBeMounted include Planned::CanBeEncrypted include Planned::CanBePv + include MatchVolumeSpec # @return [PartitionId] id of the partition. If nil, the final id is # expected to be inferred from the filesystem type. @@ -85,9 +87,20 @@ def self.to_string_attrs protected + # Values for volume specification matching + # + # @see MatchVolumeSpec + def volume_match_values + { + mount_point: mount_point, + size: min_size, + fs_type: filesystem_type, + partition_id: partition_id + } + end + def reuse_device!(device) super - device.boot = true if bootable && device.respond_to?(:boot=) end end diff --git a/test/y2storage/match_volume_spec_test.rb b/test/y2storage/match_volume_spec_test.rb new file mode 100644 index 0000000000..f9d4741eba --- /dev/null +++ b/test/y2storage/match_volume_spec_test.rb @@ -0,0 +1,191 @@ +#!/usr/bin/env rspec +# encoding: utf-8 + +# Copyright (c) [2018] 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_relative "../spec_helper" +require "y2storage" + +describe Y2Storage::MatchVolumeSpec do + using Y2Storage::Refinements::SizeCasts + + # Dummy class to test the mixin + class Matcher + include Y2Storage::MatchVolumeSpec + + def initialize(mount_point, partition_id, fs_type, size) + @mount_point = mount_point + @partition_id = partition_id + @fs_type = fs_type + @size = size + end + + private + + def volume_match_values + { + mount_point: @mount_point, + partition_id: @partition_id, + fs_type: @fs_type, + size: @size + } + end + end + + describe "#match_volume?" do + subject(:matcher) { Matcher.new(mount_point, partition_id, fs_type, size) } + + let(:volume) { Y2Storage::VolumeSpecification.new({}) } + + before do + volume.mount_point = volume_mount_point + volume.partition_id = volume_partition_id + volume.fs_types = volume_fs_types + volume.min_size = volume_min_size + end + + let(:volume_mount_point) { "swap" } + let(:volume_partition_id) { Y2Storage::PartitionId::SWAP } + let(:volume_fs_types) { [Y2Storage::Filesystems::Type::SWAP, Y2Storage::Filesystems::Type::VFAT] } + let(:volume_min_size) { Y2Storage::DiskSize.GiB(1) } + + context "when it has the same values than the volume" do + let(:mount_point) { volume_mount_point } + let(:partition_id) { volume_partition_id } + let(:fs_type) { volume_fs_types.first } + let(:size) { volume_min_size } + + it "returns true" do + expect(matcher.match_volume?(volume)).to eq(true) + end + + context "but it has different mount point" do + let(:mount_point) { "/boot" } + + it "returns false" do + expect(matcher.match_volume?(volume)).to eq(false) + end + + context "and mount point is excluded for matching" do + it "returns true" do + expect(matcher.match_volume?(volume, exclude: :mount_point)).to eq(true) + end + end + end + + context "and the size is bigger than volume min size" do + let(:size) { volume_min_size + 1.GiB } + + it "returns true" do + expect(matcher.match_volume?(volume)).to eq(true) + end + end + + context "but the size is less than volume min size" do + let(:size) { volume_min_size - 10.MiB } + + it "returns false" do + expect(matcher.match_volume?(volume)).to eq(false) + end + + context "and size is excluded for matching" do + it "returns true" do + expect(matcher.match_volume?(volume, exclude: :size)).to eq(true) + end + end + end + + context "and fs type is included in the possible fs for the volume" do + let(:fs_type) { Y2Storage::Filesystems::Type::SWAP } + + it "returns true" do + expect(matcher.match_volume?(volume)).to eq(true) + end + end + + context "but fs type is not included in the possible fs for the volume" do + let(:fs_type) { Y2Storage::Filesystems::Type::EXT2 } + + it "returns false" do + expect(matcher.match_volume?(volume)).to eq(false) + end + + context "and fs type is excluded for matching" do + it "returns true" do + expect(matcher.match_volume?(volume, exclude: :fs_type)).to eq(true) + end + end + end + + context "and the volume does not require any specific fs" do + let(:volume_fs_types) { [] } + + let(:fs_type) { Y2Storage::Filesystems::Type::EXT2 } + + it "returns true" do + expect(matcher.match_volume?(volume)).to eq(true) + end + end + + context "but it has different partition id" do + let(:partition_id) { Y2Storage::PartitionId::ESP } + + it "returns false" do + expect(matcher.match_volume?(volume)).to eq(false) + end + + context "and partition id is excluded for matching" do + it "returns true" do + expect(matcher.match_volume?(volume, exclude: :partition_id)).to eq(true) + end + end + end + + context "and the volume does not require any specific partition id" do + let(:volume_partition_id) { nil } + + let(:partition_id) { Y2Storage::PartitionId::ESP } + + it "returns true" do + expect(matcher.match_volume?(volume)).to eq(true) + end + end + end + + context "when it has different values than the volume" do + let(:mount_point) { "/boot" } + let(:partition_id) { Y2Storage::PartitionId::ESP } + let(:fs_type) { [Y2Storage::Filesystems::Type::EXT2] } + let(:size) { Y2Storage::DiskSize.MiB(100) } + + it "returns false" do + expect(matcher.match_volume?(volume)).to eq(false) + end + + context "but all values are excluded for matching" do + let(:exclude) { [:mount_point, :partition_id, :fs_type, :size] } + + it "returns true" do + expect(matcher.match_volume?(volume, exclude: exclude)).to eq(true) + end + end + end + end +end diff --git a/test/y2storage/partition_test.rb b/test/y2storage/partition_test.rb index d0c74d05b7..189b1820a8 100644 --- a/test/y2storage/partition_test.rb +++ b/test/y2storage/partition_test.rb @@ -439,4 +439,43 @@ end end end + + # Only basic cases are tested here. More exhaustive tests can be found in tests + # for Y2Storage::MatchVolumeSpec + describe "#match_volume?" do + let(:scenario) { "windows-linux-free-pc" } + + subject(:partition) { Y2Storage::Partition.find_by_name(fake_devicegraph, "/dev/sda2") } + + let(:volume) { Y2Storage::VolumeSpecification.new({}) } + + before do + volume.mount_point = volume_mount_point + volume.partition_id = volume_partition_id + volume.fs_types = volume_fs_types + volume.min_size = volume_min_size + end + + context "when the partition has the same values than the volume" do + let(:volume_mount_point) { "swap" } + let(:volume_partition_id) { Y2Storage::PartitionId::SWAP } + let(:volume_fs_types) { [Y2Storage::Filesystems::Type::SWAP] } + let(:volume_min_size) { Y2Storage::DiskSize.GiB(2) } + + it "returns true" do + expect(partition.match_volume?(volume)).to eq(true) + end + end + + context "when the partition has different values than the volume" do + let(:volume_mount_point) { "/boot" } + let(:volume_partition_id) { Y2Storage::PartitionId::ESP } + let(:volume_fs_types) { [Y2Storage::Filesystems::Type::VFAT] } + let(:volume_min_size) { Y2Storage::DiskSize.GiB(3) } + + it "returns false" do + expect(partition.match_volume?(volume)).to eq(false) + end + end + end end diff --git a/test/y2storage/planned/lvm_lv_test.rb b/test/y2storage/planned/lvm_lv_test.rb index cccc432d30..b205dca75a 100755 --- a/test/y2storage/planned/lvm_lv_test.rb +++ b/test/y2storage/planned/lvm_lv_test.rb @@ -84,4 +84,53 @@ end end end + + # Only basic cases are tested here. More exhaustive tests can be found in tests + # for Y2Storage::MatchVolumeSpec + describe "match_volume?" do + let(:volume) { Y2Storage::VolumeSpecification.new({}) } + + before do + lvm_lv.min_size = min_size + lvm_lv.filesystem_type = filesystem_type + + volume.mount_point = volume_mount_point + volume.partition_id = volume_partition_id + volume.fs_types = volume_fs_types + volume.min_size = volume_min_size + end + + let(:volume_mount_point) { "/boot" } + let(:volume_partition_id) { nil } + let(:volume_fs_types) { [Y2Storage::Filesystems::Type::EXT2] } + let(:volume_min_size) { 1.GiB } + + context "when the planned lv has the same values" do + let(:mount_point) { volume_mount_point } + let(:filesystem_type) { volume_fs_types.first } + let(:min_size) { volume_min_size } + + it "returns true" do + expect(lvm_lv.match_volume?(volume)).to eq(true) + end + + context "but the volume requires a specific partition id" do + let(:volume_partition_id) { Y2Storage::PartitionId::ESP } + + it "returns false" do + expect(lvm_lv.match_volume?(volume)).to eq(false) + end + end + end + + context "when the planned lv does not have the same values" do + let(:mount_point) { "/boot/efi" } + let(:filesystem_type) { Y2Storage::Filesystems::Type::VFAT } + let(:min_size) { 2.GiB } + + it "returns false" do + expect(lvm_lv.match_volume?(volume)).to eq(false) + end + end + end end diff --git a/test/y2storage/planned/md_test.rb b/test/y2storage/planned/md_test.rb index 5cf1173489..f88e686567 100755 --- a/test/y2storage/planned/md_test.rb +++ b/test/y2storage/planned/md_test.rb @@ -104,4 +104,51 @@ end end end + + # Only basic cases are tested here. More exhaustive tests can be found in tests + # for Y2Storage::MatchVolumeSpec + describe "match_volume?" do + let(:volume) { Y2Storage::VolumeSpecification.new({}) } + + before do + planned_md.mount_point = mount_point + planned_md.filesystem_type = filesystem_type + + volume.mount_point = volume_mount_point + volume.partition_id = volume_partition_id + volume.fs_types = volume_fs_types + volume.min_size = volume_min_size + end + + let(:volume_mount_point) { "/boot" } + let(:volume_partition_id) { nil } + let(:volume_fs_types) { [Y2Storage::Filesystems::Type::EXT2] } + let(:volume_min_size) { Y2Storage::DiskSize.zero } + + context "when the planned MD has the same values" do + let(:mount_point) { volume_mount_point } + let(:filesystem_type) { volume_fs_types.first } + + it "returns true" do + expect(planned_md.match_volume?(volume)).to eq(true) + end + + context "but the volume requires a specific partition id" do + let(:volume_partition_id) { Y2Storage::PartitionId::ESP } + + it "returns false" do + expect(planned_md.match_volume?(volume)).to eq(false) + end + end + end + + context "when the planned MD does not have the same values" do + let(:mount_point) { "/boot/efi" } + let(:filesystem_type) { Y2Storage::Filesystems::Type::VFAT } + + it "returns false" do + expect(planned_md.match_volume?(volume)).to eq(false) + end + end + end end diff --git a/test/y2storage/planned/partition_test.rb b/test/y2storage/planned/partition_test.rb new file mode 100755 index 0000000000..409bb33e42 --- /dev/null +++ b/test/y2storage/planned/partition_test.rb @@ -0,0 +1,75 @@ +#!/usr/bin/env rspec +# +# encoding: utf-8 + +# Copyright (c) [2018] 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_relative "../spec_helper" +require "y2storage/planned" + +describe Y2Storage::Planned::Partition do + using Y2Storage::Refinements::SizeCasts + + subject(:partition) { described_class.new(mount_point) } + + # Only basic cases are tested here. More exhaustive tests can be found in tests + # for Y2Storage::MatchVolumeSpec + describe "match_volume?" do + let(:volume) { Y2Storage::VolumeSpecification.new({}) } + + before do + partition.partition_id = partition_id + partition.filesystem_type = filesystem_type + partition.min_size = min_size + + volume.mount_point = volume_mount_point + volume.partition_id = volume_partition_id + volume.fs_types = volume_fs_types + volume.min_size = volume_min_size + end + + let(:volume_mount_point) { "/boot" } + let(:volume_partition_id) { Y2Storage::PartitionId::ESP } + let(:volume_fs_types) { [Y2Storage::Filesystems::Type::EXT2] } + let(:volume_min_size) { 1.GiB } + + context "when the planned partition has the same values" do + let(:mount_point) { volume_mount_point } + let(:partition_id) { volume_partition_id } + let(:filesystem_type) { volume_fs_types.first } + let(:min_size) { volume_min_size } + + it "returns true" do + expect(partition.match_volume?(volume)).to eq(true) + end + end + + context "when the planned partition does not have the same values" do + let(:mount_point) { "/boot/efi" } + let(:partition_id) { Y2Storage::PartitionId::LINUX } + let(:filesystem_type) { Y2Storage::Filesystems::Type::VFAT } + let(:min_size) { 2.GiB } + + it "returns false" do + expect(partition.match_volume?(volume)).to eq(false) + end + end + end +end From 4ff3880f77cc4d5100a0111a6da9802a01f741d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Fri, 12 Jan 2018 19:08:18 +0000 Subject: [PATCH 02/21] Add support for disk devices --- src/lib/y2storage/dasd.rb | 2 + src/lib/y2storage/devicegraph.rb | 11 +--- src/lib/y2storage/disk.rb | 2 + src/lib/y2storage/disk_device.rb | 22 ++++++++ src/lib/y2storage/dm_raid.rb | 2 + src/lib/y2storage/md_member.rb | 4 ++ src/lib/y2storage/multipath.rb | 2 + src/lib/y2storage/partitionable.rb | 2 - test/y2storage/devicegraph_test.rb | 40 +------------- test/y2storage/disk_test.rb | 87 ++++++++++++++++++++++++++++++ 10 files changed, 124 insertions(+), 50 deletions(-) diff --git a/src/lib/y2storage/dasd.rb b/src/lib/y2storage/dasd.rb index 0b43ab6210..7aea1571c7 100644 --- a/src/lib/y2storage/dasd.rb +++ b/src/lib/y2storage/dasd.rb @@ -21,6 +21,7 @@ require "y2storage/storage_class_wrapper" require "y2storage/partitionable" +require "y2storage/disk_device" module Y2Storage # A DASD (direct-access storage device), typically used in mainframes @@ -28,6 +29,7 @@ module Y2Storage # This is a wrapper for Storage::Dasd class Dasd < Partitionable wrap_class Storage::Dasd + include DiskDevice # @!method rotational? # @return [Boolean] whether this is a rotational device diff --git a/src/lib/y2storage/devicegraph.rb b/src/lib/y2storage/devicegraph.rb index f550aa31e2..3135030e89 100644 --- a/src/lib/y2storage/devicegraph.rb +++ b/src/lib/y2storage/devicegraph.rb @@ -226,16 +226,7 @@ def software_raids # # @return [Array] def disk_devices - # NOTE: to avoid sorting something that is going to be sorted again, we - # could call Disk.all instead of #disks, Multipath.all instead - # of #multipaths and so on. But the current implementation is more - # readable and the impact is probably unnoticeable. - - multi_disk_devs = multipaths + bios_raids - parent_devs = multi_disk_devs.map(&:parents).flatten - # Use #reject because Array#- is not trustworthy with SWIG - devices = (multi_disk_devs + dasds + disks).reject { |d| parent_devs.include?(d) } - devices.sort { |a, b| a.compare_by_name(b) } + BlkDevice.sorted_by_name(self).select { |d| d.is?(:disk_device) } end # All partitions in the devicegraph, sorted by name diff --git a/src/lib/y2storage/disk.rb b/src/lib/y2storage/disk.rb index c376014de4..83eb040fb5 100644 --- a/src/lib/y2storage/disk.rb +++ b/src/lib/y2storage/disk.rb @@ -23,6 +23,7 @@ require "y2storage/partitionable" require "y2storage/free_disk_space" require "y2storage/data_transport" +require "y2storage/disk_device" module Y2Storage # A physical disk device @@ -30,6 +31,7 @@ module Y2Storage # This is a wrapper for Storage::Disk class Disk < Partitionable wrap_class Storage::Disk + include DiskDevice # @!method rotational? # @return [Boolean] whether this is a rotational device diff --git a/src/lib/y2storage/disk_device.rb b/src/lib/y2storage/disk_device.rb index 6d579a320e..7974dfae0a 100644 --- a/src/lib/y2storage/disk_device.rb +++ b/src/lib/y2storage/disk_device.rb @@ -41,5 +41,27 @@ def usb? def in_network? false end + + # Checks whether the device is a multipath wire + # + # @return [Boolean] + def multipath_wire? + descendants.any? { |d| d.is?(:multipath) } + end + + # Checks whether the device is a disk belonging to a BIOS RAID + # + # @return [Boolean] + def bios_raid_disk? + descendants.any? { |d| d.is?(:bios_raid) } + end + + protected + + def types_for_is + types = super + types << :disk_device unless multipath_wire? || bios_raid_disk? + types + end end end diff --git a/src/lib/y2storage/dm_raid.rb b/src/lib/y2storage/dm_raid.rb index 4a502b990a..a34241063d 100644 --- a/src/lib/y2storage/dm_raid.rb +++ b/src/lib/y2storage/dm_raid.rb @@ -21,6 +21,7 @@ require "y2storage/storage_class_wrapper" require "y2storage/partitionable" +require "y2storage/disk_device" require "y2storage/multi_disk_device" module Y2Storage @@ -29,6 +30,7 @@ module Y2Storage # This is a wrapper for Storage::DmRaid class DmRaid < Partitionable wrap_class Storage::DmRaid + include DiskDevice include MultiDiskDevice # @!method rotational? diff --git a/src/lib/y2storage/md_member.rb b/src/lib/y2storage/md_member.rb index e93f471b8d..66392df4ad 100644 --- a/src/lib/y2storage/md_member.rb +++ b/src/lib/y2storage/md_member.rb @@ -21,6 +21,8 @@ require "y2storage/storage_class_wrapper" require "y2storage/md" +require "y2storage/disk_device" +require "y2storage/multi_disk_device" module Y2Storage # A BIOS MD RAID @@ -34,6 +36,8 @@ module Y2Storage # MD Members can be used as generic MD RAIDs. class MdMember < Md wrap_class Storage::MdMember + include DiskDevice + include MultiDiskDevice # @!method self.create(devicegraph, name) # @param devicegraph [Devicegraph] diff --git a/src/lib/y2storage/multipath.rb b/src/lib/y2storage/multipath.rb index e1498b08a9..5003590577 100644 --- a/src/lib/y2storage/multipath.rb +++ b/src/lib/y2storage/multipath.rb @@ -21,6 +21,7 @@ require "y2storage/storage_class_wrapper" require "y2storage/partitionable" +require "y2storage/disk_device" require "y2storage/multi_disk_device" module Y2Storage @@ -29,6 +30,7 @@ module Y2Storage # This is a wrapper for Storage::Multipath class Multipath < Partitionable wrap_class Storage::Multipath + include DiskDevice include MultiDiskDevice # @!method rotational? diff --git a/src/lib/y2storage/partitionable.rb b/src/lib/y2storage/partitionable.rb index d147d79517..58f59a280b 100644 --- a/src/lib/y2storage/partitionable.rb +++ b/src/lib/y2storage/partitionable.rb @@ -22,7 +22,6 @@ require "y2storage/storage_class_wrapper" require "y2storage/blk_device" require "y2storage/partition_tables" -require "y2storage/disk_device" module Y2Storage # Base class for all the devices that can contain a partition table, like @@ -31,7 +30,6 @@ module Y2Storage # This is a wrapper for Storage::Partitionable class Partitionable < BlkDevice wrap_class Storage::Partitionable, downcast_to: ["Disk", "Dasd", "DmRaid", "Md", "Multipath"] - include DiskDevice # @!attribute range # Maximum number of partitions that the kernel can handle for the device. diff --git a/test/y2storage/devicegraph_test.rb b/test/y2storage/devicegraph_test.rb index 2e11739831..cd943000a6 100644 --- a/test/y2storage/devicegraph_test.rb +++ b/test/y2storage/devicegraph_test.rb @@ -440,25 +440,6 @@ def with_sda2_deleted(initial_graph) "/dev/sdc", "/dev/sdd", "/dev/sdf", "/dev/sdh", "/dev/sdaa" ] end - - context "even if Disk.all and Dasd.all return unsorted arrays" do - before do - allow(Y2Storage::Disk).to receive(:all) do |devicegraph| - # Let's shuffle things a bit - shuffle(Y2Storage::Partitionable.all(devicegraph).select { |i| i.is?(:disk) }) - end - dasda = Y2Storage::Dasd.find_by_name(fake_devicegraph, "/dev/dasda") - dasdb = Y2Storage::Dasd.find_by_name(fake_devicegraph, "/dev/dasdb") - allow(Y2Storage::Dasd).to receive(:all).and_return [dasdb, dasda] - end - - it "returns an array sorted by name" do - expect(graph.disk_devices.map(&:name)).to eq [ - "/dev/dasda", "/dev/dasdb", "/dev/nvme0n1", "/dev/sda", "/dev/sdb", - "/dev/sdc", "/dev/sdd", "/dev/sdf", "/dev/sdh", "/dev/sdaa" - ] - end - end end context "if there are multipath devices" do @@ -471,7 +452,7 @@ def with_sda2_deleted(initial_graph) expect(devices).to all(satisfy { |dev| less_than_next?(dev, devices) }) end - it "includes all the multipath devices" do + it "includes all multipath devices" do expect(graph.disk_devices.map(&:name)).to include( "/dev/mapper/36005076305ffc73a00000000000013b4", "/dev/mapper/36005076305ffc73a00000000000013b5" @@ -487,23 +468,6 @@ def with_sda2_deleted(initial_graph) "/dev/sda", "/dev/sdb", "/dev/sdc", "/dev/sdd" ) end - - context "even if Disk.all and Multipath.all return unsorted arrays" do - # Let's shuffle things a bit - before do - allow(Y2Storage::Disk).to receive(:all) do |devicegraph| - shuffle(Y2Storage::Partitionable.all(devicegraph).select { |i| i.is?(:disk) }) - end - allow(Y2Storage::Multipath).to receive(:all) do |devicegraph| - shuffle(Y2Storage::Partitionable.all(devicegraph).select { |i| i.is?(:multipath) }) - end - end - - it "returns an array sorted by name" do - devices = graph.disk_devices - expect(devices).to all(satisfy { |dev| less_than_next?(dev, devices) }) - end - end end context "if there are BIOS RAIDs" do @@ -520,7 +484,7 @@ def with_sda2_deleted(initial_graph) expect(devices).to all(satisfy { |dev| less_than_next?(dev, devices) }) end - it "includes all the BIOS RAIDs" do + it "includes all BIOS RAIDs" do expect(graph.disk_devices.map(&:name)).to include( "/dev/mapper/isw_ddgdcbibhd_test1", "/dev/mapper/isw_ddgdcbibhd_test2", "/dev/md0" ) diff --git a/test/y2storage/disk_test.rb b/test/y2storage/disk_test.rb index 1c2e60f1f4..7fed10a166 100644 --- a/test/y2storage/disk_test.rb +++ b/test/y2storage/disk_test.rb @@ -256,6 +256,60 @@ end end + describe "#multipath_wire?" do + context "when the disk is a multipath wire" do + let(:scenario) { "multipath-formatted.xml" } + + let(:disk_name) { "/dev/sda" } + + it "returns true" do + expect(disk.multipath_wire?).to eq(true) + end + end + + context "when the disk is not a multipath wire" do + let(:scenario) { "mixed_disks" } + + let(:disk_name) { "/dev/sda" } + + it "returns false" do + expect(disk.multipath_wire?).to eq(false) + end + end + end + + describe "#bios_raid_disk?" do + context "when the disk belongs to a BIOS RAID" do + let(:scenario) { "empty-dm_raids.xml" } + + let(:disk_name) { "/dev/sdb" } + + it "returns true" do + expect(disk.bios_raid_disk?).to eq(true) + end + end + + context "when the disk belongs to a Software RAID" do + let(:scenario) { "md_raid.xml" } + + let(:disk_name) { "/dev/sda" } + + it "returns false" do + expect(disk.bios_raid_disk?).to eq(false) + end + end + + context "when the disk does not belong to a RAID" do + let(:scenario) { "mixed_disks" } + + let(:disk_name) { "/dev/sda" } + + it "returns false" do + expect(disk.bios_raid_disk?).to eq(false) + end + end + end + describe "#is?" do let(:disk_name) { "/dev/sda" } @@ -280,6 +334,39 @@ it "returns false for a list of names not containing :disk" do expect(disk.is?(:filesystem, :partition)).to eq false end + + context "when the disk is a multipath wire" do + let(:scenario) { "multipath-formatted.xml" } + + let(:disk_name) { "/dev/sda" } + + it "returns false for values whose symbol is :disk_device" do + expect(disk.is?(:disk_device)).to eq false + expect(disk.is?("disk_device")).to eq false + end + end + + context "when the disk belongs to a BIOS RAID" do + let(:scenario) { "empty-dm_raids.xml" } + + let(:disk_name) { "/dev/sdb" } + + it "returns false for values whose symbol is :disk_device" do + expect(disk.is?(:disk_device)).to eq false + expect(disk.is?("disk_device")).to eq false + end + end + + context "when the disk is not a multipath wire and it does not belong to a RAID" do + let(:scenario) { "mixed_disks" } + + let(:disk_name) { "/dev/sda" } + + it "returns true for values whose symbol is :disk_device" do + expect(disk.is?(:disk_device)).to eq true + expect(disk.is?("disk_device")).to eq true + end + end end describe "#usb?" do From c124544004ff3973e35b565345cfad3c0a9f3a46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Fri, 12 Jan 2018 19:13:15 +0000 Subject: [PATCH 03/21] Add support for raids --- src/lib/y2storage/devicegraph.rb | 12 ++++-- src/lib/y2storage/dm_raid.rb | 6 ++- src/lib/y2storage/md.rb | 6 ++- src/lib/y2storage/md_member.rb | 6 ++- test/y2storage/devicegraph_test.rb | 61 +++++++++++++++++++++++------ test/y2storage/dm_raid_test.rb | 15 +++++++ test/y2storage/md_container_test.rb | 26 ++++++++++++ test/y2storage/md_member_test.rb | 26 ++++++++++++ test/y2storage/md_test.rb | 26 ++++++++++++ 9 files changed, 167 insertions(+), 17 deletions(-) diff --git a/src/lib/y2storage/devicegraph.rb b/src/lib/y2storage/devicegraph.rb index 3135030e89..bb17d9d68c 100644 --- a/src/lib/y2storage/devicegraph.rb +++ b/src/lib/y2storage/devicegraph.rb @@ -194,14 +194,20 @@ def md_member_raids MdMember.sorted_by_name(self) end + # All RAIDs in the devicegraph, sorted by name + # + # @return [Array] + def raids + BlkDevice.sorted_by_name(self).select { |d| d.is?(:raid) } + end + # All BIOS RAIDs in the devicegraph, sorted by name # # @note BIOS RAIDs are the set composed by MD BIOS RAIDs and DM RAIDs. # # @return [Array] def bios_raids - devices = dm_raids + md_member_raids - devices.sort { |a, b| a.compare_by_name(b) } + BlkDevice.sorted_by_name(self).select { |d| d.is?(:bios_raid) } end # All Software RAIDs in the devicegraph, sorted by name @@ -210,7 +216,7 @@ def bios_raids # # @return [Array] def software_raids - md_raids.select(&:software_defined?) + BlkDevice.sorted_by_name(self).select { |d| d.is?(:software_raid) } end # All the devices that are usually treated like disks by YaST, sorted by diff --git a/src/lib/y2storage/dm_raid.rb b/src/lib/y2storage/dm_raid.rb index a34241063d..1e258e454e 100644 --- a/src/lib/y2storage/dm_raid.rb +++ b/src/lib/y2storage/dm_raid.rb @@ -72,7 +72,11 @@ def inspect protected def types_for_is - super << :dm_raid + types = super + types << :dm_raid + types << :raid + types << :bios_raid + types end end end diff --git a/src/lib/y2storage/md.rb b/src/lib/y2storage/md.rb index 2b2f1f650c..ae4f5f5c83 100644 --- a/src/lib/y2storage/md.rb +++ b/src/lib/y2storage/md.rb @@ -288,7 +288,11 @@ def md_users end def types_for_is - super << :md + types = super + types << :md + types << :raid if software_defined? + types << :software_raid if software_defined? + types end end end diff --git a/src/lib/y2storage/md_member.rb b/src/lib/y2storage/md_member.rb index 66392df4ad..9238cbc8cc 100644 --- a/src/lib/y2storage/md_member.rb +++ b/src/lib/y2storage/md_member.rb @@ -70,7 +70,11 @@ def software_defined? protected def types_for_is - super << :md_member + types = super + types << :md_member + types << :raid + types << :bios_raid + types end end end diff --git a/test/y2storage/devicegraph_test.rb b/test/y2storage/devicegraph_test.rb index cd943000a6..8a3bbf0f2c 100644 --- a/test/y2storage/devicegraph_test.rb +++ b/test/y2storage/devicegraph_test.rb @@ -191,6 +191,48 @@ def with_sda2_deleted(initial_graph) end end + describe "#raids" do + before do + fake_scenario("mixed_disks") + end + + subject(:devicegraph) { fake_devicegraph } + + context "when there are RAIDs" do + before do + Y2Storage::DmRaid.create(devicegraph, "/dev/mapper/imsm0") + Y2Storage::DmRaid.create(devicegraph, "/dev/mapper/imsm1") + + Y2Storage::MdMember.create(devicegraph, "/dev/md0") + Y2Storage::MdMember.create(devicegraph, "/dev/md/1") + Y2Storage::MdMember.create(devicegraph, "/dev/md2") + + Y2Storage::Md.create(devicegraph, "/dev/md3") + Y2Storage::Md.create(devicegraph, "/dev/md/4") + Y2Storage::Md.create(devicegraph, "/dev/md5") + end + + it "includes all RAIDs sorted by name" do + expect(devicegraph.raids.map(&:name)).to eq [ + "/dev/mapper/imsm0", + "/dev/mapper/imsm1", + "/dev/md/1", + "/dev/md/4", + "/dev/md0", + "/dev/md2", + "/dev/md3", + "/dev/md5" + ] + end + end + + context "when there are no RAIDs" do + it "does not include any device" do + expect(devicegraph.raids).to be_empty + end + end + end + describe "#bios_raids" do before do fake_scenario("mixed_disks") @@ -200,14 +242,12 @@ def with_sda2_deleted(initial_graph) context "when there are BIOS RAIDs" do before do - dm0 = Y2Storage::DmRaid.create(devicegraph, "/dev/mapper/imsm0") - dm1 = Y2Storage::DmRaid.create(devicegraph, "/dev/mapper/imsm1") - allow(Y2Storage::Md).to receive(:all).and_return [dm1, dm0] + Y2Storage::DmRaid.create(devicegraph, "/dev/mapper/imsm0") + Y2Storage::DmRaid.create(devicegraph, "/dev/mapper/imsm1") - md0 = Y2Storage::MdMember.create(devicegraph, "/dev/md0") - md1 = Y2Storage::MdMember.create(devicegraph, "/dev/md/1") - md2 = Y2Storage::MdMember.create(devicegraph, "/dev/md2") - allow(Y2Storage::Md).to receive(:all).and_return [md2, md0, md1] + Y2Storage::MdMember.create(devicegraph, "/dev/md0") + Y2Storage::MdMember.create(devicegraph, "/dev/md/1") + Y2Storage::MdMember.create(devicegraph, "/dev/md2") end it "includes all DM RAIDs and BIOS MD RAIDs sorted by name" do @@ -241,10 +281,9 @@ def with_sda2_deleted(initial_graph) context "when there are Software RAIDs" do before do - md0 = Y2Storage::Md.create(devicegraph, "/dev/md0") - md1 = Y2Storage::Md.create(devicegraph, "/dev/md/1") - md2 = Y2Storage::Md.create(devicegraph, "/dev/md2") - allow(Y2Storage::Md).to receive(:all).and_return [md2, md0, md1] + Y2Storage::Md.create(devicegraph, "/dev/md0") + Y2Storage::Md.create(devicegraph, "/dev/md/1") + Y2Storage::Md.create(devicegraph, "/dev/md2") end it "includes all Software RAIDs sorted by name" do diff --git a/test/y2storage/dm_raid_test.rb b/test/y2storage/dm_raid_test.rb index ffba33db61..fe6f51178d 100644 --- a/test/y2storage/dm_raid_test.rb +++ b/test/y2storage/dm_raid_test.rb @@ -66,6 +66,21 @@ expect(dm_raid.is?("DM_RAID")).to eq false end + it "returns true for values whose symbol is :raid" do + expect(subject.is?(:raid)).to eq true + expect(subject.is?("raid")).to eq true + end + + it "returns true for values whose symbol is :bios_raid" do + expect(subject.is?(:bios_raid)).to eq true + expect(subject.is?("bios_raid")).to eq true + end + + it "returns false for values whose symbol is :software_raid" do + expect(subject.is?(:software_raid)).to eq false + expect(subject.is?("software_raid")).to eq false + end + it "returns false for different device names like :disk or :partition" do expect(dm_raid.is?(:disk)).to eq false expect(dm_raid.is?(:partition)).to eq false diff --git a/test/y2storage/md_container_test.rb b/test/y2storage/md_container_test.rb index 62823f0e68..d057b3c58e 100644 --- a/test/y2storage/md_container_test.rb +++ b/test/y2storage/md_container_test.rb @@ -50,4 +50,30 @@ expect(subject.software_defined?).to eq(false) end end + + describe "#is?" do + it "returns true for values whose symbol is :md_container" do + expect(subject.is?(:md_container)).to eq true + expect(subject.is?("md_container")).to eq true + end + + it "returns false for values whose symbol is :raid" do + expect(subject.is?(:raid)).to eq false + expect(subject.is?("raid")).to eq false + end + + it "returns false for values whose symbol is :bios_raid" do + expect(subject.is?(:bios_raid)).to eq false + expect(subject.is?("bios_raid")).to eq false + end + + it "returns false for values whose symbol is :software_raid" do + expect(subject.is?(:software_raid)).to eq false + expect(subject.is?("software_raid")).to eq false + end + + it "returns false for different device names like :disk" do + expect(subject.is?(:disk)).to eq false + end + end end diff --git a/test/y2storage/md_member_test.rb b/test/y2storage/md_member_test.rb index ff5d115cb1..5b02f70643 100644 --- a/test/y2storage/md_member_test.rb +++ b/test/y2storage/md_member_test.rb @@ -56,4 +56,30 @@ expect(subject.usb?).to eq(false) end end + + describe "#is?" do + it "returns true for values whose symbol is :md_member" do + expect(subject.is?(:md_member)).to eq true + expect(subject.is?("md_member")).to eq true + end + + it "returns true for values whose symbol is :raid" do + expect(subject.is?(:raid)).to eq true + expect(subject.is?("raid")).to eq true + end + + it "returns true for values whose symbol is :bios_raid" do + expect(subject.is?(:bios_raid)).to eq true + expect(subject.is?("bios_raid")).to eq true + end + + it "returns false for values whose symbol is :software_raid" do + expect(subject.is?(:software_raid)).to eq false + expect(subject.is?("software_raid")).to eq false + end + + it "returns false for different device names like :disk" do + expect(subject.is?(:disk)).to eq false + end + end end diff --git a/test/y2storage/md_test.rb b/test/y2storage/md_test.rb index 2a8cb60175..3cd671177a 100644 --- a/test/y2storage/md_test.rb +++ b/test/y2storage/md_test.rb @@ -242,4 +242,30 @@ end end end + + describe "#is?" do + it "returns true for values whose symbol is :md" do + expect(subject.is?(:md)).to eq true + expect(subject.is?("md")).to eq true + end + + it "returns true for values whose symbol is :raid" do + expect(subject.is?(:raid)).to eq true + expect(subject.is?("raid")).to eq true + end + + it "returns true for values whose symbol is :software_raid" do + expect(subject.is?(:software_raid)).to eq true + expect(subject.is?("software_raid")).to eq true + end + + it "returns false for values whose symbol is :bios_raid" do + expect(subject.is?(:bios_raid)).to eq false + expect(subject.is?("bios_raid")).to eq false + end + + it "returns false for different device names like :disk" do + expect(subject.is?(:disk)).to eq false + end + end end From 1e121abd0a6bbe5f90ed15943e6074af6d2504db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Fri, 12 Jan 2018 19:18:17 +0000 Subject: [PATCH 04/21] Fix analyzer --- .../boot_requirements_strategies/analyzer.rb | 14 ++++- .../analyzer_test.rb | 63 +++++++++++++++++++ 2 files changed, 74 insertions(+), 3 deletions(-) diff --git a/src/lib/y2storage/boot_requirements_strategies/analyzer.rb b/src/lib/y2storage/boot_requirements_strategies/analyzer.rb index 69270bf17a..18ff56403c 100644 --- a/src/lib/y2storage/boot_requirements_strategies/analyzer.rb +++ b/src/lib/y2storage/boot_requirements_strategies/analyzer.rb @@ -1,5 +1,3 @@ -#!/usr/bin/env ruby -# # encoding: utf-8 # Copyright (c) [2015] SUSE LLC @@ -142,6 +140,10 @@ def boot_ptable_type?(type) # @param path [String] mount point to check for # @return [Boolean] def free_mountpoint?(path) + # FIXME: This method takes into account all mount points, even for filesystems over a + # logical volume, software raid or a directly formatted disk. That check could produce + # false possitives due to the presence of a mount point is not enough + # (e.g., /boot/efi over a logical volume is not valid for booting). cleanpath = Pathname.new(path).cleanpath return false if planned_devices.any? do |dev| dev.mount_point && Pathname.new(dev.mount_point).cleanpath == cleanpath @@ -184,6 +186,9 @@ def boot_ptable_type # TODO: handle planned LV (not needed so far) def boot_disk_from_planned_dev + # FIXME: This method is only able to find the boot disk when the planned + # root is over a partition. This could not work properly in autoyast when + # root is planned over logical volumes or software raids. return nil unless root_planned_dev return nil unless root_planned_dev.respond_to?(:disk) @@ -191,8 +196,11 @@ def boot_disk_from_planned_dev end def boot_disk_from_devicegraph + # FIXME: In case root filesystem is over a multidevice (vg, software raid), + # the first disk is considered the boot disk. This could not work properly + # for some scenarios. return nil unless root_filesystem - root_filesystem.ancestors.find { |d| d.is?(:disk) } + root_filesystem.ancestors.find { |d| d.is?(:disk_device) } end def planned_partitions_with_id(id) diff --git a/test/y2storage/boot_requirements_strategies/analyzer_test.rb b/test/y2storage/boot_requirements_strategies/analyzer_test.rb index 4efb05247f..4b47e4cf39 100644 --- a/test/y2storage/boot_requirements_strategies/analyzer_test.rb +++ b/test/y2storage/boot_requirements_strategies/analyzer_test.rb @@ -46,6 +46,69 @@ end end + context "if a partition over a Dasd device is configured as '/' in the devicegraph" do + let(:scenario) { "dasd_50GiB" } + + before do + partition = Y2Storage::Partition.find_by_name(fake_devicegraph, "/dev/sda1") + partition.filesystem.mount_point = "/" + end + + it "returns a Dasd object" do + expect(analyzer.boot_disk).to be_a Y2Storage::Dasd + end + + it "returns the dasd device containing the '/' partition" do + expect(analyzer.boot_disk.name).to eq "/dev/sda" + end + end + + context "if a partition over a Multipath device is configured as '/' in the devicegraph" do + let(:scenario) { "empty-dasd-and-multipath.xml" } + + let(:multipath_name) { "/dev/mapper/36005076305ffc73a00000000000013b4" } + + before do + device = Y2Storage::BlkDevice.find_by_name(fake_devicegraph, multipath_name) + part = device.partition_table.create_partition("/dev/#{multipath_name}-1", + Y2Storage::Region.create(2048, 1048576, 512), + Y2Storage::PartitionType::PRIMARY) + fs = part.create_filesystem(Y2Storage::Filesystems::Type::EXT4) + fs.mount_point = "/" + end + + it "returns a Multipath object" do + expect(analyzer.boot_disk).to be_a Y2Storage::Multipath + end + + it "returns the Multipath device containing the '/' partition" do + expect(analyzer.boot_disk.name).to eq multipath_name + end + end + + context "if a partition over a BIOS RAID is configured as '/' in the devicegraph" do + let(:scenario) { "empty-dm_raids.xml" } + + let(:raid_name) { "/dev/mapper/isw_ddgdcbibhd_test1" } + + before do + device = Y2Storage::BlkDevice.find_by_name(fake_devicegraph, raid_name) + part = device.partition_table.create_partition("/dev/#{raid_name}-1", + Y2Storage::Region.create(2048, 1048576, 512), + Y2Storage::PartitionType::PRIMARY) + fs = part.create_filesystem(Y2Storage::Filesystems::Type::EXT4) + fs.mount_point = "/" + end + + it "returns a BIOS RAID" do + expect(analyzer.boot_disk.is?(:bios_raid)).to eq(true) + end + + it "returns the BIOS RAID containing the '/' partition" do + expect(analyzer.boot_disk.name).to eq raid_name + end + end + context "if a LVM LV is configured as '/' in the devicegraph" do let(:scenario) { "complex-lvm-encrypt" } From 7c559096f4cf808bdcab96c253afbcde4eceb91a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Fri, 12 Jan 2018 19:18:56 +0000 Subject: [PATCH 05/21] Add SetupError class --- src/lib/y2storage/setup_error.rb | 243 +++++++++++++++++++++++++++++ test/y2storage/setup_error_test.rb | 189 ++++++++++++++++++++++ 2 files changed, 432 insertions(+) create mode 100644 src/lib/y2storage/setup_error.rb create mode 100644 test/y2storage/setup_error_test.rb diff --git a/src/lib/y2storage/setup_error.rb b/src/lib/y2storage/setup_error.rb new file mode 100644 index 0000000000..417b50abd2 --- /dev/null +++ b/src/lib/y2storage/setup_error.rb @@ -0,0 +1,243 @@ +# encoding: utf-8 + +# Copyright (c) [2018] 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 "yast" +require "yast/i18n" +require "y2storage" + +module Y2Storage + # Class to represent storage setup error + class SetupError + include Yast::I18n + + # @return [VolumeSpecification] + attr_reader :missing_volume + + # Constructor + # + # @param message [String, nil] error message + # @param missing_volume [VolumeSpecification, nil] missing volume that causes the error + def initialize(message: nil, missing_volume: nil) + textdomain "storage" + @message = message + @missing_volume = missing_volume + end + + # Error message + # + # @note If no message was indicated, it can be generated from the missing volume. + # + # @return [String, nil] + def message + message = @message + message ||= message_for_missing_volume if missing_volume + message + end + + private + + # Error text for a volume + # + # @param volume [VolumeSpecification] + # @return [String] + def message_for_missing_volume + if mount_point_info + message_with_mount_point + else + message_without_mount_point + end + end + + # Error text when the volume has mount point + # + # @param volume [VolumeSpecification] + # @return [String] + def message_with_mount_point + if partition_id_info && fs_types_info + message_with_mount_point_and_partition_id_and_fs + elsif partition_id_info + message_with_mount_point_and_partition_id + elsif fs_types_info + message_with_mount_point_and_fs + else + message_with_mount_point_default + end + end + + # Error text when the volume does not have mount point + # + # @param volume [VolumeSpecification] + # @return [String] + def message_without_mount_point + if partition_id_info && fs_types_info + message_with_partition_id_and_fs + elsif partition_id_info + message_with_partition_id + elsif fs_types_info + message_with_fs + else + message_without_mount_point_default + end + end + + # @param volume [VolumeSpecification] + # @return [String] + def message_with_mount_point_and_partition_id_and_fs + # TRANSLATORS: error message, where %{mount_point} is repalaced by a mount point + # (e.g., /lib/docker), %{size} by a disk size (e.g., 5 GiB), %{partition_id} by a + # partition id (e.g., Linux) and %{fs_types} by a list of filesystem types separated + # by comma (e.g., ext2, ext3, ext4). + format( + "Missing device for %{mount_point} with size equal or bigger than %{size}, " \ + "partition id %{partition_id} and filesystem %{fs_types}", + mount_point: mount_point_info, + size: size_info, + partition_id: partition_id_info, + fs_types: fs_types_info + ) + end + + # @param volume [VolumeSpecification] + # @return [String] + def message_with_mount_point_and_partition_id + # TRANSLATORS: error message, where %{mount_point} is repalaced by a mount point + # (e.g., /lib/docker), %{size} by a disk size (e.g., 5 GiB) and %{partition_id}. + format( + "Missing device for %{mount_point} with size equal or bigger than %{size} " \ + "and partition id %{partition_id}", + mount_point: mount_point_info, + size: size_info, + partition_id: partition_id_info + ) + end + + # @param volume [VolumeSpecification] + # @return [String] + def message_with_mount_point_and_fs + # TRANSLATORS: error message, where %{mount_point} is repalaced by a mount point + # (e.g., /lib/docker), %{size} by a disk size (e.g., 5 GiB) and %{fs_types} by a + # list of filesystem types separated by comma (e.g., ext2, ext3, ext4). + format( + "Missing device for %{mount_point} with size equal or bigger than %{size} " \ + "and filesystem %{fs_types}", + mount_point: mount_point_info, + size: size_info, + fs_types: fs_types_info + ) + end + + # @param volume [VolumeSpecification] + # @return [String] + def message_with_mount_point_default + # TRANSLATORS: error message, where %{mount_point} is repalaced by a mount point + # (e.g., /lib/docker) and %{size} by a disk size (e.g., 5 GiB). + format( + "Missing device for %{mount_point} with size equal or bigger than %{size} ", + mount_point: mount_point_info, + size: size_info + ) + end + + # @param volume [VolumeSpecification] + # @return [String] + def message_with_partition_id_and_fs + # TRANSLATORS: error message, where %{size} is replaced by a disk size (e.g., 5 GiB), + # %{partition_id} by a partition id (e.g., Linux) and %{fs_types} by a list of filesystem + # types separated by comma (e.g., ext2, ext3, ext4). + format( + "Missing device with size equal or bigger than %{size}, " \ + "partition id %{partition_id} and filesystem %{fs_types}", + size: size_info, + partition_id: partition_id_info, + fs_types: fs_types_info + ) + end + + # @param volume [VolumeSpecification] + # @return [String] + def message_with_partition_id + # TRANSLATORS: error message, where %{size} is replaced by a disk size (e.g., 5 GiB) and + # %{partition_id} by a partition id (e.g., Linux). + format( + "Missing device with size equal or bigger than %{size} " \ + "and partition id %{partition_id}", + size: size_info, + partition_id: partition_id_info + ) + end + + # @param volume [VolumeSpecification] + # @return [String] + def message_with_fs + # TRANSLATORS: error message, where %{size} is replaced by a disk size (e.g., 5 GiB) and + # %{fs_types} by a list of filesystem types separated by comma (e.g., ext2, ext3, ext4). + format( + "Missing device with size equal or bigger than %{size} " \ + "and filesystem %{fs_types}", + size: size_info, + fs_types: fs_types_info + ) + end + + # @param volume [VolumeSpecification] + # @return [String] + def message_without_mount_point_default + # TRANSLATORS: error message, where %{size} is replaced by a disk size (e.g., 5 GiB). + format( + "Missing device with size equal or bigger than %{size}", + size: size_info + ) + end + + # Volume mount point to show in the error message + # + # @param volume [VolumeSpecification] + # @return [String, nil] + def mount_point_info + missing_volume.mount_point + end + + # Volume partition id to show in the error message + # + # @param volume [VolumeSpecification] + # @return [Integer, nil] + def partition_id_info + missing_volume.partition_id + end + + # Possible volume fileystem types to show in the error message + # + # @param volume [VolumeSpecification] + # @return [String, nil] + def fs_types_info + return nil if missing_volume.fs_types.empty? + missing_volume.fs_types.map(&:to_s).join(", ") + end + + # Volume size to show in the error message + # + # @param volume [VolumeSpecification] + # @return [DiskSize, nil] + def size_info + missing_volume.min_size + end + end +end diff --git a/test/y2storage/setup_error_test.rb b/test/y2storage/setup_error_test.rb new file mode 100644 index 0000000000..300b2bd05d --- /dev/null +++ b/test/y2storage/setup_error_test.rb @@ -0,0 +1,189 @@ +#!/usr/bin/env rspec +# encoding: utf-8 + +# Copyright (c) [2018] 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_relative "spec_helper" +require "y2storage" +require "y2storage/setup_error" + +describe Y2Storage::SetupError do + using Y2Storage::Refinements::SizeCasts + + subject { described_class.new(message: error_message, missing_volume: missing_volume) } + + let(:error_message) { nil } + + let(:missing_volume) { nil } + + let(:volume) do + volume = Y2Storage::VolumeSpecification.new({}) + volume.mount_point = mount_point + volume.min_size = min_size + volume.fs_types = fs_types + volume.partition_id = partition_id + volume + end + + let(:mount_point) { nil } + let(:min_size) { Y2Storage::DiskSize.zero } + let(:fs_types) { [] } + let(:partition_id) { nil } + + describe "#message" do + context "when an error message is given" do + let(:error_message) { "an error message" } + + context "and a missing volume is not given" do + let(:missing_volume) { nil } + + it "returns the given message" do + expect(subject.message).to eq(error_message) + end + end + + context "and a missing volume is given" do + let(:missing_volume) { volume } + + it "returns the given message" do + expect(subject.message).to eq(error_message) + end + end + end + + context "when an error message is not given" do + let(:error_message) { nil } + + context "and a missing volume is not given" do + it "returns nil" do + expect(subject.message).to be_nil + end + end + + context "and a missing volume is given" do + let(:missing_volume) { volume } + + context "when the volume has a mount point" do + let(:mount_point) { "/mnt/foo" } + + it "generates an error message with mount point data" do + expect(subject.message).to match(/device for \/.* /) + end + + context "and the volume has a partition id and fs type" do + let(:partition_id) { Y2Storage::PartitionId::LINUX } + + let(:fs_types) { [Y2Storage::Filesystems::Type::EXT3] } + + it "generates an error message with partition id and fs type data" do + expect(subject.message).to match(/partition id .*/) + expect(subject.message).to match(/filesystem .*/) + end + end + + context "and the volume has a partition id but does not have fs type" do + let(:partition_id) { Y2Storage::PartitionId::LINUX } + + let(:fs_types) { [] } + + it "generates an error message with partition id but not fs type data" do + expect(subject.message).to match(/partition id .*/) + expect(subject.message).to_not match(/filesystem .*/) + end + end + + context "and the volume does not have a partition id but has a fs type" do + let(:partition_id) { nil } + + let(:fs_types) { [Y2Storage::Filesystems::Type::EXT3] } + + it "generates an error message without partition id but with fs type data" do + expect(subject.message).to_not match(/partition id .*/) + expect(subject.message).to match(/filesystem .*/) + end + end + + context "and the volume does not have neither partition id nor fs type" do + let(:partition_id) { nil } + + let(:fs_types) { [] } + + it "generates a message without partition id neither fs type data" do + expect(subject.message).to_not match(/partition id .*/) + expect(subject.message).to_not match(/filesystem .*/) + end + end + end + + context "when the volume does not have a mount point" do + let(:mount_point) { nil } + + it "generates an error messagge without mount point data" do + expect(subject.message).to_not match(/\/.* /) + end + + context "and the volume has a partition id and fs type" do + let(:partition_id) { Y2Storage::PartitionId::LINUX } + + let(:fs_types) { [Y2Storage::Filesystems::Type::EXT3] } + + it "generates an error message with partition id and fs type data" do + expect(subject.message).to match(/partition id .*/) + expect(subject.message).to match(/filesystem .*/) + end + end + + context "and the volume has a partition id but does not have fs type" do + let(:partition_id) { Y2Storage::PartitionId::LINUX } + + let(:fs_types) { [] } + + it "generates an error message with partition id but not fs type data" do + expect(subject.message).to match(/partition id .*/) + expect(subject.message).to_not match(/filesystem .*/) + end + end + + context "and the volume does not have a partition id but has a fs type" do + let(:partition_id) { nil } + + let(:fs_types) { [Y2Storage::Filesystems::Type::EXT3] } + + it "generates an error message without partition id but with fs type data" do + expect(subject.message).to_not match(/partition id .*/) + expect(subject.message).to match(/filesystem .*/) + end + end + + context "and the volume does not have neither partition id nor fs type" do + let(:partition_id) { nil } + + let(:fs_types) { [] } + + it "generates a message without partition id neither fs type data" do + expect(subject.message).to_not match(/partition id .*/) + expect(subject.message).to_not match(/filesystem .*/) + end + end + end + end + end + end +end From 4de0d8774e84827a6e6d646f2a08a6454d0c6c7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Fri, 12 Jan 2018 19:25:00 +0000 Subject: [PATCH 06/21] Add support for boot errors --- .../y2storage/boot_requirements_checker.rb | 23 +- .../boot_requirements_strategies/analyzer.rb | 25 +- .../boot_requirements_strategies/base.rb | 96 +++- .../boot_requirements_strategies/legacy.rb | 139 ++++- .../boot_requirements_strategies/prep.rb | 66 ++- .../boot_requirements_strategies/uefi.rb | 63 ++- .../boot_requirements_strategies/zipl.rb | 52 +- src/lib/y2storage/volume_specification.rb | 25 +- test/support/boot_requirements_context.rb | 11 +- test/support/boot_requirements_uefi.rb | 16 +- test/support/storage_helpers.rb | 5 + .../boot_requirements_checker_aarch64_test.rb | 2 + .../boot_requirements_checker_ppc_test.rb | 10 +- .../boot_requirements_checker_x86_test.rb | 13 +- .../boot_requirements_duplicate_test.rb | 50 +- .../boot_requirements_errors_test.rb | 511 ++++++++++++++++++ .../analyzer_test.rb | 108 ++++ 17 files changed, 1099 insertions(+), 116 deletions(-) create mode 100644 test/y2storage/boot_requirements_errors_test.rb diff --git a/src/lib/y2storage/boot_requirements_checker.rb b/src/lib/y2storage/boot_requirements_checker.rb index 2da29b7c96..8504a7a6e3 100644 --- a/src/lib/y2storage/boot_requirements_checker.rb +++ b/src/lib/y2storage/boot_requirements_checker.rb @@ -1,5 +1,3 @@ -#!/usr/bin/env ruby -# # encoding: utf-8 # Copyright (c) [2015] SUSE LLC @@ -56,8 +54,8 @@ def initialize(devicegraph, planned_devices: [], boot_disk_name: nil) # enough to make the system bootable # # @param target [Symbol] :desired means the sizes of the partitions should - # be the ideal ones, :min for generating the smallest functional - # partitions + # be the ideal ones, :min for generating the smallest functional partitions + # # @return [Array] def needed_partitions(target = :desired) strategy.needed_partitions(target) @@ -65,6 +63,23 @@ def needed_partitions(target = :desired) raise Error, error.message end + # Whether the current setup contains all necessary devices for booting + # + # @return [Boolean] + def valid? + errors.empty? + end + + # All boot errors detected in the setup, for example, when a /boot/efi partition + # is missing in a UEFI system + # + # @see SetupError + # + # @return [Array] + def errors + strategy.errors + end + protected # @return [Devicegraph] starting situation. diff --git a/src/lib/y2storage/boot_requirements_strategies/analyzer.rb b/src/lib/y2storage/boot_requirements_strategies/analyzer.rb index 18ff56403c..d4c33861b3 100644 --- a/src/lib/y2storage/boot_requirements_strategies/analyzer.rb +++ b/src/lib/y2storage/boot_requirements_strategies/analyzer.rb @@ -30,6 +30,13 @@ module BootRequirementsStrategies # information (regarding calculation of boot requirements) about the # expected final system. class Analyzer + # Devices that are already planned to be added to the starting devicegraph. + # @return [Array] + attr_reader :planned_devices + + # @return [Filesystems::Base, nil] nil if there is not filesystem for root + attr_reader :root_filesystem + # Constructor # # @param devicegraph [Devicegraph] starting situation. @@ -90,6 +97,22 @@ def root_in_lvm? end end + # Whether the root (/) filesystem is over a Software RAID + # + # @return [Boolean] true if the root filesystem is going to be in a + # Software RAID. False if the root filesystem is unknown (not in the + # planned devices or in the devicegraph) or is not placed over a Software + # RAID. + def root_in_software_raid? + if root_planned_dev + root_planned_dev.is_a?(Planned::Md) + elsif root_filesystem + root_filesystem.ancestors.any? { |dev| dev.is?(:software_raid) } + else + false + end + end + # Whether the root (/) filesystem is going to be in an encrypted device # # @return [Boolean] true if the root filesystem is going to be in an @@ -171,10 +194,8 @@ def planned_grub_partitions protected attr_reader :devicegraph - attr_reader :planned_devices attr_reader :boot_disk_name attr_reader :root_planned_dev - attr_reader :root_filesystem def boot_ptable_type return nil unless boot_disk diff --git a/src/lib/y2storage/boot_requirements_strategies/base.rb b/src/lib/y2storage/boot_requirements_strategies/base.rb index b9449500f9..a6fe12ccb8 100644 --- a/src/lib/y2storage/boot_requirements_strategies/base.rb +++ b/src/lib/y2storage/boot_requirements_strategies/base.rb @@ -1,5 +1,3 @@ -#!/usr/bin/env ruby -# # encoding: utf-8 # Copyright (c) [2015] SUSE LLC @@ -22,11 +20,14 @@ # find current contact information at www.suse.com. require "yast" +require "yast/i18n" require "y2storage/disk_size" require "y2storage/filesystems/type" require "y2storage/planned" require "y2storage/boot_requirements_strategies/analyzer" require "y2storage/exceptions" +require "y2storage/volume_specification" +require "y2storage/setup_error" module Y2Storage module BootRequirementsStrategies @@ -37,10 +38,12 @@ class Error < Y2Storage::Error # requirements class Base include Yast::Logger + include Yast::I18n extend Forwardable def_delegators :@analyzer, - :boot_disk, :root_in_lvm?, :encrypted_root?, :btrfs_root?, :boot_ptable_type?, :free_mountpoint? + :root_filesystem, :boot_disk, :root_in_lvm?, :root_in_software_raid?, + :encrypted_root?, :btrfs_root?, :boot_ptable_type?, :free_mountpoint? # Constructor # @@ -48,20 +51,50 @@ class Base # @see [BootRequirementsChecker#planned_devices] # @see [BootRequirementsChecker#boot_disk_name] def initialize(devicegraph, planned_devices, boot_disk_name) + textdomain "storage" + @devicegraph = devicegraph @analyzer = Analyzer.new(devicegraph, planned_devices, boot_disk_name) end + # Partitions that should be created to boot the system + # + # @param target [Symbol] :desired, :min + # + # @return [Array] def needed_partitions(target) - return [] unless boot_partition_needed? && boot_partition_missing? - [boot_partition(target)] + planned_partitions = [] + planned_partitions << boot_partition(target) if boot_partition_needed? && boot_partition_missing? + planned_partitions + end + + # All boot errors detected in the setup, for example, when a /boot/efi partition + # is missing in a UEFI system + # + # @note This method should be overloaded for derived classes. + # + # @see SetupError + # + # @return [Array] + def errors + [] end protected + # @return [Devicegraph] attr_reader :devicegraph + + # @return [BootRequirementsStrategies::Analyzer] attr_reader :analyzer + # Whether there is not root + # + # @return [Boolean] true if there is no root; false otherwise. + def root_filesystem_missing? + root_filesystem.nil? + end + def boot_partition_needed? false end @@ -70,12 +103,55 @@ def boot_partition_missing? free_mountpoint?("/boot") end + # @return [VolumeSpecification] + def boot_volume + return @boot_volume unless @boot_volume.nil? + + @boot_volume = VolumeSpecification.new({}) + @boot_volume.mount_point = "/boot" + @boot_volume.fs_types = Filesystems::Type.root_filesystems + @boot_volume.fs_type = Filesystems::Type::EXT4 + @boot_volume.min_size = DiskSize.MiB(100) + @boot_volume.desired_size = DiskSize.MiB(200) + @boot_volume.max_size = DiskSize.MiB(500) + @boot_volume + end + + # @return [Planned::Partition] def boot_partition(target) - vol = Planned::Partition.new("/boot", Filesystems::Type::EXT4) - vol.disk = boot_disk.name - vol.min_size = target == :min ? DiskSize.MiB(100) : DiskSize.MiB(200) - vol.max_size = DiskSize.MiB(500) - vol + planned_partition = create_planned_partition(boot_volume, target) + planned_partition.disk = boot_disk.name + planned_partition + end + + # Create a planned partition from a volume specification + # + # @param volume [VolumeSpecification] + # @param target [Symbol] :desired, :min + # + # @return [VolumeSpecification] + def create_planned_partition(volume, target) + planned_partition = Planned::Partition.new(volume.mount_point, volume.fs_type) + planned_partition.min_size = target == :min ? volume.min_size : volume.desired_size + planned_partition.max_size = volume.max_size + planned_partition.partition_id = volume.partition_id + planned_partition + end + + # Whether there is no partition that matches the volume + # + # @return [Boolean] true if there is no partition; false otherwise. + def missing_partition_for?(volume) + Partition.all(devicegraph).none? { |p| p.match_volume?(volume) } + end + + # Specific error when the boot disk cannot be detected + # + # @return [SetupError] + def unknown_boot_disk_error + # TRANSLATORS: error message + error_message = _("Unknwon boot disk") + SetupError.new(message: error_message) end end end diff --git a/src/lib/y2storage/boot_requirements_strategies/legacy.rb b/src/lib/y2storage/boot_requirements_strategies/legacy.rb index aef9deedde..5252a14a34 100644 --- a/src/lib/y2storage/boot_requirements_strategies/legacy.rb +++ b/src/lib/y2storage/boot_requirements_strategies/legacy.rb @@ -1,5 +1,3 @@ -#!/usr/bin/env ruby -# # encoding: utf-8 # Copyright (c) [2015] SUSE LLC @@ -26,22 +24,55 @@ module Y2Storage module BootRequirementsStrategies - # Strategy to calculate the boot requirements in a legacy system (x86 - # without EFI) + # Strategy to calculate the boot requirements in a legacy system (x86 without EFI) class Legacy < Base GRUB_SIZE = DiskSize.KiB(256) GRUBENV_SIZE = DiskSize.KiB(1) + # @see Base#needed_partitions def needed_partitions(target) - volumes = super - volumes << grub_partition(target) if grub_partition_needed? && grub_partition_missing? - raise Error if grub_in_mbr? && mbr_gap && mbr_gap < GRUB_SIZE + raise Error if grub_in_mbr? && mbr_gap && !valid_mbr_gap? + planned_partitions = super + planned_partitions << grub_partition(target) if grub_partition_needed? && grub_partition_missing? + planned_partitions + end - volumes + # Boot errors in the current setup + # + # @return [Array] + def errors + errors = super + + if root_filesystem_missing? + errors << unknown_boot_disk_error + elsif boot_partition_table_missing? + errors << unknown_boot_partition_table_error + elsif grub_partition_needed? + errors += errors_on_gpt + else + errors += errors_on_msdos + end + + errors end protected + # Whether the boot disk has not partition table + # + # @return [Boolean] true if boot disk does not have partition table; + # false otherwise. + def boot_partition_table_missing? + boot_disk.partition_table.nil? + end + + # Whether the MBR gap is big enough + # + # @return [Boolean] true if the MBR gap is enough; false otherwise. + def valid_mbr_gap? + mbr_gap && mbr_gap >= GRUB_SIZE + end + def grub_partition_needed? boot_ptable_type?(:gpt) end @@ -49,10 +80,8 @@ def grub_partition_needed? def grub_partition_missing? # We don't check if the planned partition is in the boot disk, # whoever created it is in control of the details - return false if analyzer.planned_grub_partitions.any? - - partitions = boot_disk.grub_partitions - partitions.nil? || partitions.empty? + current_devices = analyzer.planned_devices + boot_disk.partitions + current_devices.none? { |d| d.match_volume?(grub_volume) } end def grub_in_mbr? @@ -60,34 +89,100 @@ def grub_in_mbr? end def plain_btrfs? - btrfs_without_lvm? && btrfs_without_encryption? + btrfs_without_lvm? && btrfs_without_software_raid? && btrfs_without_encryption? end def btrfs_without_lvm? btrfs_root? && !root_in_lvm? end + # Whether the root filesystem is a BTRFS over a software raid + # + # @return [Boolean] true if it is a BTRFS over a software raid; false otherwise. + def btrfs_without_software_raid? + btrfs_root? && !root_in_software_raid? + end + def btrfs_without_encryption? btrfs_root? && !encrypted_root? end def boot_partition_needed? - grub_in_mbr? && mbr_gap && mbr_gap < GRUB_SIZE + GRUBENV_SIZE + grub_in_mbr? && valid_mbr_gap? && mbr_gap < GRUB_SIZE + GRUBENV_SIZE end def mbr_gap boot_disk.mbr_gap end + # @return [VolumeSpecification] + def grub_volume + return @grub_volume unless @grub_volume.nil? + + @grub_volume = VolumeSpecification.new({}) + @grub_volume.min_size = DiskSize.KiB(256) + @grub_volume.desired_size = DiskSize.MiB(1) + @grub_volume.max_size = DiskSize.MiB(8) + # Only required on GPT + @grub_volume.partition_id = PartitionId::BIOS_BOOT + @grub_volume + end + + # @return [Planned::Partition] def grub_partition(target) - vol = Planned::Partition.new(nil) - # only required on GPT - vol.partition_id = PartitionId::BIOS_BOOT - vol.min_size = target == :min ? DiskSize.KiB(256) : DiskSize.MiB(1) - vol.max_size = DiskSize.MiB(8) - vol.align = :keep_size - vol.bootable = false - vol + planned_partition = create_planned_partition(grub_volume, target) + planned_partition.align = :keep_size + planned_partition.bootable = false + planned_partition + end + + # Boot errors when partition table is gpt + # + # @return [Array] + def errors_on_gpt + errors = [] + + if missing_partition_for?(grub_volume) + errors << SetupError.new(missing_volume: grub_volume) + end + + errors + end + + # Boor errors when partition table is msdos + # + # @return [Array] + def errors_on_msdos + errors = [] + + if !valid_mbr_gap? + errors << mbr_gap_error if !plain_btrfs? + elsif boot_partition_needed? && missing_partition_for?(boot_volume) + errors << SetupError.new(missing_volume: boot_volume) + end + + errors + end + + # Specific error when the boot disk has not partition table + # + # @return [SetupError] + def unknown_boot_partition_table_error + # TRANSLATORS: error message + error_message = _( + "Boot requirements cannot be determined because " \ + "boot disk has not partition table" + ) + SetupError.new(message: error_message) + end + + # Specific error when the MBR gap is small + # + # @return [SetupError] + def mbr_gap_error + # TRANSLATORS: error message + error_message = _("MBR gap size is not enough to correctly install bootloader") + SetupError.new(message: error_message) end end end diff --git a/src/lib/y2storage/boot_requirements_strategies/prep.rb b/src/lib/y2storage/boot_requirements_strategies/prep.rb index 33dc0cd7ac..20152028b1 100644 --- a/src/lib/y2storage/boot_requirements_strategies/prep.rb +++ b/src/lib/y2storage/boot_requirements_strategies/prep.rb @@ -1,5 +1,3 @@ -#!/usr/bin/env ruby -# # encoding: utf-8 # Copyright (c) [2015] SUSE LLC @@ -28,16 +26,38 @@ module Y2Storage module BootRequirementsStrategies # Strategy to calculate boot requirements in systems using PReP class PReP < Base + # @see Base#needed_partitions def needed_partitions(target) - volumes = super - volumes << prep_partition(target) if prep_partition_needed? && prep_partition_missing? - volumes + planned_partitions = super + planned_partitions << prep_partition(target) if prep_partition_needed? && prep_partition_missing? + planned_partitions + end + + # Boot errors in the current setup + # + # @return [Array] + def errors + errors = super + + if root_filesystem_missing? + errors << unknown_boot_disk_error + else + if prep_partition_needed? && missing_partition_for?(prep_volume) + errors << SetupError.new(missing_volume: prep_volume) + end + + if boot_partition_needed? && missing_partition_for?(boot_volume) + errors << SetupError.new(missing_volume: boot_volume) + end + end + + errors end protected def boot_partition_needed? - root_in_lvm? || encrypted_root? + root_in_lvm? || root_in_software_raid? || encrypted_root? end def prep_partition_needed? @@ -48,25 +68,33 @@ def prep_partition_needed? def prep_partition_missing? # We don't check if the planned PReP partition is in the boot disk, # whoever created it is in control of the details - return false if analyzer.planned_prep_partitions.any? - - partitions = boot_disk.prep_partitions - partitions.nil? || partitions.empty? + current_devices = analyzer.planned_devices + boot_disk.partitions + current_devices.none? { |d| d.match_volume?(prep_volume) } end - def prep_partition(target) - vol = Planned::Partition.new(nil) + # @return [VolumeSpecification] + def prep_volume + return @prep_volume unless @prep_volume.nil? + + @prep_volume = VolumeSpecification.new({}) # So far we are always using msdos partition ids - vol.partition_id = PartitionId::PREP - vol.min_size = target == :min ? DiskSize.KiB(256) : DiskSize.MiB(1) - vol.max_size = DiskSize.MiB(8) - # Make sure that alignment does not result in a too big partition - vol.align = :keep_size - vol.bootable = true + @prep_volume.partition_id = PartitionId::PREP + @prep_volume.min_size = DiskSize.KiB(256) + @prep_volume.desired_size = DiskSize.MiB(1) + @prep_volume.max_size = DiskSize.MiB(8) # TODO: We have been told that PReP must be one of the first 4 # partitions, ideally the first one. But we have not found any # rationale/evidence. Not implementing that for the time being - vol + @prep_volume + end + + # @return [Planned::Partition] + def prep_partition(target) + planned_partition = create_planned_partition(prep_volume, target) + # Make sure that alignment does not result in a too big partition + planned_partition.align = :keep_size + planned_partition.bootable = true + planned_partition end def arch diff --git a/src/lib/y2storage/boot_requirements_strategies/uefi.rb b/src/lib/y2storage/boot_requirements_strategies/uefi.rb index f3af3743be..f8a999fb4b 100644 --- a/src/lib/y2storage/boot_requirements_strategies/uefi.rb +++ b/src/lib/y2storage/boot_requirements_strategies/uefi.rb @@ -1,5 +1,3 @@ -#!/usr/bin/env ruby -# # encoding: utf-8 # Copyright (c) [2015] SUSE LLC @@ -29,10 +27,24 @@ module Y2Storage module BootRequirementsStrategies # Strategy to calculate boot requirements in UEFI systems class UEFI < Base + # @see Base#needed_partitions def needed_partitions(target) - volumes = super - volumes << efi_partition(target) if efi_missing? - volumes + planned_partitions = super + planned_partitions << efi_partition(target) if efi_missing? + planned_partitions + end + + # Boot errors in the current setup + # + # @return [Array] + def errors + errors = super + + if missing_partition_for?(efi_volume) + errors << SetupError.new(missing_volume: efi_volume) + end + + errors end protected @@ -45,32 +57,47 @@ def efi_missing? free_mountpoint?("/boot/efi") end + # @return [VolumeSpecification] + def efi_volume + return @efi_volume unless @efi_volume.nil? + + @efi_volume = VolumeSpecification.new({}) + @efi_volume.mount_point = "/boot/efi" + @efi_volume.fs_types = [Filesystems::Type::VFAT] + @efi_volume.fs_type = Filesystems::Type::VFAT + @efi_volume.partition_id = PartitionId::ESP + @efi_volume.min_size = MIN_SIZE + @efi_volume.desired_size = DESIRED_SIZE + @efi_volume.max_size = MAX_SIZE + @efi_volume + end + + # @return [Planned::Partition] def efi_partition(target) - vol = Planned::Partition.new("/boot/efi", Filesystems::Type::VFAT) + planned_partition = create_planned_partition(efi_volume, target) if reusable_efi - vol.reuse = reusable_efi.name + planned_partition.reuse = reusable_efi.name else - vol.partition_id = PartitionId::ESP - vol.min_size = target == :min ? MIN_SIZE : DESIRED_SIZE - vol.max_size = MAX_SIZE - vol.max_start_offset = DiskSize.TiB(2) + planned_partition.max_start_offset = DiskSize.TiB(2) end - vol + planned_partition end def reusable_efi - efi = biggest_efi_in_boot_device || biggest_efi - efi = nil if !efi.nil? && efi.size < MIN_SIZE - @reusable_efi = efi + @reusable_efi = biggest_efi_in_boot_device || biggest_efi end def biggest_efi_in_boot_device - biggest_partition(boot_disk.efi_partitions) + biggest_partition(suitable_efi_partitions(boot_disk)) end def biggest_efi - efi_parts = devicegraph.disk_devices.map(&:efi_partitions).flatten - biggest_partition(efi_parts) + efi_partitions = devicegraph.disk_devices.map { |d| suitable_efi_partitions(d) }.flatten + biggest_partition(efi_partitions) + end + + def suitable_efi_partitions(device) + device.partitions.select { |p| p.match_volume?(efi_volume, exclude: :mount_point) } end def biggest_partition(partitions) diff --git a/src/lib/y2storage/boot_requirements_strategies/zipl.rb b/src/lib/y2storage/boot_requirements_strategies/zipl.rb index 3be625b8f2..d42a612651 100644 --- a/src/lib/y2storage/boot_requirements_strategies/zipl.rb +++ b/src/lib/y2storage/boot_requirements_strategies/zipl.rb @@ -1,5 +1,3 @@ -#!/usr/bin/env ruby -# # encoding: utf-8 # Copyright (c) [2015] SUSE LLC @@ -27,11 +25,29 @@ module Y2Storage module BootRequirementsStrategies # Strategy to calculate boot requirements in systems using ZIPL class ZIPL < Base + # @see Base#needed_partitions def needed_partitions(target) raise Error, "Impossible to boot system from the chosen disk" unless supported_boot_disk? zipl_partition_missing? ? [zipl_partition(target)] : [] end + # Boot errors in the current setup + # + # @return [Array] + def errors + errors = super + + if root_filesystem_missing? + errors << unknown_boot_disk_error + elsif !supported_boot_disk? + errors << unsupported_boot_disk_error + elsif missing_partition_for?(zipl_volume) + errors << SetupError.new(missing_volume: zipl_volume) + end + + errors + end + protected def supported_boot_disk? @@ -48,12 +64,34 @@ def zipl_partition_missing? free_mountpoint?("/boot/zipl") end + # @return [VolumeSpecification] + def zipl_volume + return @zipl_volume unless @zipl_volume.nil? + + @zipl_volume = VolumeSpecification.new({}) + @zipl_volume.mount_point = "/boot/zipl" + @zipl_volume.fs_types = [Filesystems::Type::EXT2] + @zipl_volume.fs_type = Filesystems::Type::EXT2 + @zipl_volume.min_size = DiskSize.MiB(100) + @zipl_volume.desired_size = DiskSize.MiB(200) + @zipl_volume.max_size = DiskSize.GiB(1) + @zipl_volume + end + + # @return [Planned::Partition] def zipl_partition(target) - vol = Planned::Partition.new("/boot/zipl", Filesystems::Type::EXT2) - vol.disk = boot_disk.name - vol.min_size = target == :min ? DiskSize.MiB(100) : DiskSize.MiB(200) - vol.max_size = DiskSize.GiB(1) - vol + planned_partition = create_planned_partition(zipl_volume, target) + planned_partition.disk = boot_disk.name + planned_partition + end + + # Specific error when the boot disk is not valid for booting + # + # @return [SetupError] + def unsupported_boot_disk_error + # TRANSLATORS: error message + error_message = _("Current boot disk cannot be used for booting") + SetupError.new(message: error_message) end end end diff --git a/src/lib/y2storage/volume_specification.rb b/src/lib/y2storage/volume_specification.rb index 5d213e29a0..f0748f908e 100644 --- a/src/lib/y2storage/volume_specification.rb +++ b/src/lib/y2storage/volume_specification.rb @@ -28,6 +28,9 @@ module Y2Storage class VolumeSpecification include PartitioningFeatures + # @return [PartitionId] when the volume needs to be a partition with a specific id + attr_accessor :partition_id + # @return [String] directory where the volume will be mounted in the system attr_accessor :mount_point @@ -37,11 +40,11 @@ class VolumeSpecification # @return [Boolean] whether the user can change the proposed setting in the UI attr_accessor :proposed_configurable - # @return [List] acceptable filesystem types - attr_accessor :fs_types - # @return [Filesystems::Type] default file system type to format the volume - attr_accessor :fs_type + attr_reader :fs_type + + # @return [List] acceptable filesystem types + attr_reader :fs_types # @return [DiskSize] initial size to use in the first proposal attempt attr_accessor :desired_size @@ -130,6 +133,13 @@ def fs_type=(type) @fs_type = validated_fs_type(type) end + # @param types [Array, String] an array of filesystem types or a + # list of comma-separated ones + def fs_types=(types) + types = types.strip.split(/\s*,\s*/) if types.is_a?(String) + @fs_types = types.map { |t| validated_fs_type(t) } + end + # Whether the user can configure some aspect of the volume # # Returns false if there is no chance for the volume to be proposed or if @@ -224,13 +234,6 @@ def load_features(volume_features) apply_fallbacks end - # @param types [Array, String] an array of filesystem types or a - # list of comma-separated ones - def fs_types=(types) - types = types.strip.split(/\s*,\s*/) if types.is_a?(String) - @fs_types = types.map { |t| validated_fs_type(t) } - end - def validated_fs_type(type) raise(ArgumentError, "Filesystem cannot be nil") unless type return type if type.is_a?(Filesystems::Type) diff --git a/test/support/boot_requirements_context.rb b/test/support/boot_requirements_context.rb index c528d18789..cd91873a48 100644 --- a/test/support/boot_requirements_context.rb +++ b/test/support/boot_requirements_context.rb @@ -33,23 +33,30 @@ def find_vol(mount_point, volumes) let(:storage_arch) { instance_double("::Storage::Arch") } let(:devicegraph) { double("Y2Storage::Devicegraph") } - let(:dev_sda) { double("Y2Storage::Disk", name: "/dev/sda") } + let(:dev_sda) { double("Y2Storage::Disk", name: "/dev/sda", partition_table: boot_partition_table) } let(:dev_sdb) { double("Y2Storage::Disk", name: "/dev/sdb") } let(:boot_disk) { dev_sda } + let(:boot_partition_table) { instance_double(Y2Storage::PartitionTables::Base) } + let(:root_filesystem) { instance_double(Y2Storage::Filesystems::Base) } + let(:analyzer) do double( "Y2Storage::BootRequirementsStrategies::Analyzer", boot_disk: boot_disk, + root_filesystem: root_filesystem, root_in_lvm?: use_lvm, + root_in_software_raid?: use_raid, encrypted_root?: use_encryption, btrfs_root?: use_btrfs, planned_prep_partitions: planned_prep_partitions, - planned_grub_partitions: planned_grub_partitions + planned_grub_partitions: planned_grub_partitions, + planned_devices: planned_grub_partitions + planned_prep_partitions ) end let(:use_lvm) { false } + let(:use_raid) { false } let(:use_encryption) { false } let(:use_btrfs) { true } let(:boot_ptable_type) { :msdos } diff --git a/test/support/boot_requirements_uefi.rb b/test/support/boot_requirements_uefi.rb index f34c1cc053..8a112f6885 100644 --- a/test/support/boot_requirements_uefi.rb +++ b/test/support/boot_requirements_uefi.rb @@ -35,10 +35,16 @@ end context "if there is already an EFI partition" do - let(:efi_partitions) { [partition_double("/dev/sda1", size)] } + let(:efi_partitions) { [efi_partition] } - context "and it does not have enough size" do - let(:size) { 32.MiB } + let(:efi_partition) { partition_double("/dev/sda1") } + + before do + allow(efi_partition).to receive(:match_volume?).and_return(match) + end + + context "and it is not a suitable EFI partition (not enough size, invalid filesystem)" do + let(:match) { false } it "requires only a new /boot/efi partition" do expect(checker.needed_partitions).to contain_exactly( @@ -47,8 +53,8 @@ end end - context "and it has enough size" do - let(:size) { 33.MiB } + context "and it is a suitable EFI partition (enough size, valid filesystem)" do + let(:match) { true } it "only requires to use the existing EFI partition" do expect(checker.needed_partitions).to contain_exactly( diff --git a/test/support/storage_helpers.rb b/test/support/storage_helpers.rb index c971929fdc..52b0a57724 100644 --- a/test/support/storage_helpers.rb +++ b/test/support/storage_helpers.rb @@ -87,6 +87,11 @@ def planned_lv(attrs = {}) add_planned_attributes(lv, attrs) end + def planned_md(attrs = {}) + md = Y2Storage::Planned::Md.new + add_planned_attributes(md, attrs) + end + def add_planned_attributes(device, attrs) attrs = attrs.dup diff --git a/test/y2storage/boot_requirements_checker_aarch64_test.rb b/test/y2storage/boot_requirements_checker_aarch64_test.rb index 3815aedf60..0b6b0aeb09 100644 --- a/test/y2storage/boot_requirements_checker_aarch64_test.rb +++ b/test/y2storage/boot_requirements_checker_aarch64_test.rb @@ -46,7 +46,9 @@ allow(storage_arch).to receive(:efiboot?).and_return(efiboot) allow(dev_sda).to receive(:mbr_gap).and_return mbr_gap_size allow(dev_sda).to receive(:efi_partitions).and_return efi_partitions + allow(dev_sda).to receive(:partitions).and_return(efi_partitions) allow(dev_sdb).to receive(:efi_partitions).and_return other_efi_partitions + allow(dev_sdb).to receive(:partitions).and_return(other_efi_partitions) end include_context "plain UEFI" diff --git a/test/y2storage/boot_requirements_checker_ppc_test.rb b/test/y2storage/boot_requirements_checker_ppc_test.rb index 1612a6f985..5eef245a37 100644 --- a/test/y2storage/boot_requirements_checker_ppc_test.rb +++ b/test/y2storage/boot_requirements_checker_ppc_test.rb @@ -40,8 +40,12 @@ allow(storage_arch).to receive(:efiboot?).and_return(false) allow(dev_sda).to receive(:grub_partitions).and_return [] allow(dev_sda).to receive(:prep_partitions).and_return prep_partitions + allow(dev_sda).to receive(:partitions).and_return(prep_partitions) + allow(prep_partition).to receive(:match_volume?).and_return(true) end + let(:prep_partition) { partition_double("/dev/sda1") } + context "in a non-PowerNV system (KVM/LPAR)" do let(:power_nv) { false } @@ -59,7 +63,7 @@ end context "if there is already a PReP partition in the disk" do - let(:prep_partitions) { [partition_double("/dev/sda1")] } + let(:prep_partitions) { [prep_partition] } it "does not require any particular volume" do expect(checker.needed_partitions).to be_empty @@ -82,7 +86,7 @@ end context "if there is already a PReP partition in the disk" do - let(:prep_partitions) { [partition_double("/dev/sda1")] } + let(:prep_partitions) { [prep_partition] } it "requires only a /boot partition" do expect(checker.needed_partitions).to contain_exactly( @@ -108,7 +112,7 @@ end context "if there is already a PReP partition in the disk" do - let(:prep_partitions) { [partition_double("/dev/sda1")] } + let(:prep_partitions) { [prep_partition] } it "requires only a /boot partition" do expect(checker.needed_partitions).to contain_exactly( diff --git a/test/y2storage/boot_requirements_checker_x86_test.rb b/test/y2storage/boot_requirements_checker_x86_test.rb index 085db1dcc7..f68ffc5323 100644 --- a/test/y2storage/boot_requirements_checker_x86_test.rb +++ b/test/y2storage/boot_requirements_checker_x86_test.rb @@ -48,9 +48,14 @@ allow(dev_sda).to receive(:mbr_gap).and_return mbr_gap_size allow(dev_sda).to receive(:grub_partitions).and_return grub_partitions allow(dev_sda).to receive(:efi_partitions).and_return efi_partitions + allow(dev_sda).to receive(:partitions).and_return(grub_partitions + efi_partitions) allow(dev_sdb).to receive(:efi_partitions).and_return other_efi_partitions + allow(dev_sdb).to receive(:partitions).and_return(other_efi_partitions) + allow(grub_partition).to receive(:match_volume?).and_return(true) end + let(:grub_partition) { partition_double("/dev/sda1") } + context "using UEFI" do let(:efiboot) { true } @@ -77,7 +82,7 @@ end context "if there is already a GRUB partition" do - let(:grub_partitions) { [partition_double("/dev/sda2")] } + let(:grub_partitions) { [grub_partition] } it "does not require any particular volume" do expect(checker.needed_partitions).to be_empty @@ -99,7 +104,7 @@ end context "if there is already a GRUB partition" do - let(:grub_partitions) { [partition_double("/dev/sda2")] } + let(:grub_partitions) { [grub_partition] } it "does not require any particular volume" do expect(checker.needed_partitions).to be_empty @@ -122,7 +127,7 @@ end context "if there is already a GRUB partition" do - let(:grub_partitions) { [partition_double("/dev/sda2")] } + let(:grub_partitions) { [grub_partition] } it "does not require any particular volume" do expect(checker.needed_partitions).to be_empty @@ -252,7 +257,7 @@ # Default values to ensure a GRUB partition let(:boot_ptable_type) { :gpt } let(:efiboot) { false } - let(:grub_partitions) { {} } + let(:grub_partitions) { [] } include_examples "proposed GRUB partition" end diff --git a/test/y2storage/boot_requirements_duplicate_test.rb b/test/y2storage/boot_requirements_duplicate_test.rb index 2145376b6c..cd3ae921b9 100644 --- a/test/y2storage/boot_requirements_duplicate_test.rb +++ b/test/y2storage/boot_requirements_duplicate_test.rb @@ -46,7 +46,9 @@ allow(dev_sda).to receive(:mbr_gap).and_return mbr_gap_size allow(dev_sda).to receive(:grub_partitions).and_return grub_partitions allow(dev_sda).to receive(:efi_partitions).and_return efi_partitions - allow(dev_sdb).to receive(:efi_partitions).and_return other_efi_partitions + allow(dev_sda).to receive(:partitions).and_return(grub_partitions + efi_partitions) + allow(dev_sdb).to receive(:efi_partitions).and_return(other_efi_partitions) + allow(dev_sdb).to receive(:partitions).and_return(other_efi_partitions) end context "when /boot/efi is needed" do @@ -78,7 +80,13 @@ let(:missing_efi) { true } context "if there is suitable EFI partition in the devicegraph" do - let(:efi_partitions) { [partition_double("/dev/sda1", 50.MiB)] } + let(:efi_partitions) { [efi_partition] } + + let(:efi_partition) { partition_double("/dev/sda1") } + + before do + allow(efi_partition).to receive(:match_volume?).and_return(true) + end it "proposes to use the existing EFI partition" do expect(checker.needed_partitions).to contain_exactly( @@ -148,11 +156,17 @@ before do allow(storage_arch).to receive(:ppc_power_nv?).and_return false - allow(dev_sda).to receive(:prep_partitions).and_return prep_partitions + allow(dev_sda).to receive(:partitions).and_return prep_partitions end - context "and some PReP is already in the list of planned partitions" do - let(:planned_prep_partitions) { [planned_partition] } + context "and a suitable PReP is already in the list of planned partitions" do + let(:planned_prep_partitions) { [planned_prep_partition] } + + let(:planned_prep_partition) { [planned_partition] } + + before do + allow(planned_prep_partition).to receive(:match_volume?).and_return(true) + end it "does not propose another PReP" do expect(checker.needed_partitions).to be_empty @@ -172,8 +186,14 @@ end end - context "but there is already a PReP partition in the disk" do - let(:prep_partitions) { [partition_double("/dev/sda1")] } + context "but there is already a suitable PReP partition in the disk" do + let(:prep_partitions) { [prep_partition] } + + let(:prep_partition) { partition_double("/dev/sda1") } + + before do + allow(prep_partition).to receive(:match_volume?).and_return(true) + end it "does not propose another PReP" do expect(checker.needed_partitions).to be_empty @@ -232,7 +252,13 @@ let(:boot_ptable_type) { :gpt } context "and some GRUB is already in the list of planned partitions" do - let(:planned_grub_partitions) { [planned_partition] } + let(:planned_grub_partitions) { [planned_grub_partition] } + + before do + allow(planned_grub_partition).to receive(:match_volume?).and_return(true) + end + + let(:planned_grub_partition) { planned_partition } it "does not propose another GRUB partition" do expect(checker.needed_partitions).to be_empty @@ -253,7 +279,13 @@ end context "but there is already a GRUB partition in the disk" do - let(:grub_partitions) { [partition_double("/dev/sda1")] } + let(:grub_partitions) { [grub_partition] } + + let(:grub_partition) { partition_double("/dev/sda1") } + + before do + allow(grub_partition).to receive(:match_volume?).with(anything).and_return(true) + end it "does not propose another GRUB partition" do expect(checker.needed_partitions).to be_empty diff --git a/test/y2storage/boot_requirements_errors_test.rb b/test/y2storage/boot_requirements_errors_test.rb new file mode 100644 index 0000000000..734af087a4 --- /dev/null +++ b/test/y2storage/boot_requirements_errors_test.rb @@ -0,0 +1,511 @@ +#!/usr/bin/env rspec +# encoding: utf-8 + +# Copyright (c) [2018] 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_relative "spec_helper" +require_relative "#{TEST_PATH}/support/boot_requirements_context" +require "y2storage" + +describe Y2Storage::BootRequirementsChecker do + using Y2Storage::Refinements::SizeCasts + + include_context "boot requirements" + + describe "#valid?" do + let(:architecture) { :x86 } + + before do + allow(checker).to receive(:errors).and_return(errors) + end + + context "when there are setup errors" do + let(:errors) { [instance_double(Y2Storage::SetupError)] } + + it "returns false" do + expect(checker.valid?).to eq(false) + end + end + + context "when there are no setup errors" do + let(:errors) { [] } + + it "returns true" do + expect(checker.valid?).to eq(true) + end + end + end + + describe "#errors" do + RSpec.shared_examples "missing boot partition" do + it "contains an error for missing boot partition" do + expect(checker.errors.size).to eq(1) + expect(checker.errors).to all(be_a(Y2Storage::SetupError)) + + missing_volume = checker.errors.first.missing_volume + expect(missing_volume).to eq(boot_volume) + end + end + + RSpec.shared_examples "missing prep partition" do + it "contains an error for missing PReP partition" do + expect(checker.errors.size).to eq(1) + expect(checker.errors).to all(be_a(Y2Storage::SetupError)) + + missing_volume = checker.errors.first.missing_volume + expect(missing_volume).to eq(prep_volume) + end + end + + RSpec.shared_examples "missing zipl partition" do + it "contains an error for missing ZIPL partition" do + expect(checker.errors.size).to eq(1) + expect(checker.errors).to all(be_a(Y2Storage::SetupError)) + + missing_volume = checker.errors.first.missing_volume + expect(missing_volume).to eq(zipl_volume) + end + end + + RSpec.shared_examples "unknown boot disk" do + it "contains an error for unknown boot disk" do + expect(checker.errors.size).to eq(1) + expect(checker.errors).to all(be_a(Y2Storage::SetupError)) + + message = checker.errors.first.message + expect(message).to match(/boot disk/) + end + end + + RSpec.shared_examples "unsupported boot disk" do + it "contains an error for unsupported boot disk" do + expect(checker.errors.size).to eq(1) + expect(checker.errors).to all(be_a(Y2Storage::SetupError)) + + message = checker.errors.first.message + expect(message).to match(/cannot be used/) + end + end + + RSpec.shared_examples "boot partition" do + context "and there is no /boot partition in the system" do + let(:partitions) { [grub_partition] } + include_examples "missing boot partition" + end + + context "and there is a /boot partition in the system" do + let(:partitions) { [boot_partition] } + + it "does not contain errors" do + expect(checker.errors).to be_empty + end + end + end + + RSpec.shared_examples "efi partition" do + context "when there is no /boot/efi partition in the system" do + let(:partitions) { [boot_partition, grub_partition] } + + it "contains an error for the efi partition" do + expect(checker.errors.size).to eq(1) + expect(checker.errors).to all(be_a(Y2Storage::SetupError)) + + missing_volume = checker.errors.first.missing_volume + expect(missing_volume).to eq(efi_volume) + end + end + + context "when there is a /boot/efi partition in the system" do + let(:partitions) { [boot_partition, efi_partition] } + + it "does not contain errors" do + expect(checker.errors).to be_empty + end + end + end + + RSpec.shared_examples "zipl partition" do + context "and there is no /boot/zipl partition in the system" do + let(:partitions) { [grub_partition] } + include_examples "missing zipl partition" + end + + context "and there is a /boot/zipl partition in the system" do + let(:partitions) { [zipl_partition] } + + it "does not contain errors" do + expect(checker.errors).to be_empty + end + end + end + + RSpec.shared_examples "MBR gap" do + context "if the MBR gap has additional space for grubenv" do + let(:mbr_gap_size) { 260.KiB } + + it "does not contain errors" do + expect(checker.errors).to be_empty + end + end + + context "if the MBR gap has no additional space" do + let(:mbr_gap_size) { 256.KiB } + include_examples "boot partition" + end + end + + RSpec.shared_examples "PPC non-PowerNV" do + context "and there is no a PReP partition in the system" do + let(:partitions) { [boot_partition] } + include_examples "missing prep partition" + end + + context "and there is no a /boot partition in the system" do + let(:partitions) { [prep_partition] } + include_examples "missing boot partition" + end + + context "and there is a PReP and a /boot partitions in the system" do + let(:partitions) { [boot_partition, prep_partition] } + + it "does not contain errors" do + expect(checker.errors).to be_empty + end + end + end + + before do + allow(storage_arch).to receive(:efiboot?).and_return(efiboot) + + allow_any_instance_of(Y2Storage::BootRequirementsStrategies::Base) + .to receive(:boot_volume).and_return(boot_volume) + + allow_any_instance_of(Y2Storage::BootRequirementsStrategies::UEFI) + .to receive(:efi_volume).and_return(efi_volume) + + allow_any_instance_of(Y2Storage::BootRequirementsStrategies::Legacy) + .to receive(:grub_volume).and_return(grub_volume) + + allow_any_instance_of(Y2Storage::BootRequirementsStrategies::PReP) + .to receive(:prep_volume).and_return(prep_volume) + + allow_any_instance_of(Y2Storage::BootRequirementsStrategies::ZIPL) + .to receive(:zipl_volume).and_return(zipl_volume) + + allow(Y2Storage::Partition).to receive(:all).and_return partitions + + allow(boot_partition).to receive(:match_volume?).with(anything).and_return(false) + allow(boot_partition).to receive(:match_volume?).with(boot_volume).and_return(true) + + allow(efi_partition).to receive(:match_volume?).with(anything).and_return(false) + allow(efi_partition).to receive(:match_volume?).with(efi_volume).and_return(true) + + allow(grub_partition).to receive(:match_volume?).with(anything).and_return(false) + allow(grub_partition).to receive(:match_volume?).with(grub_volume).and_return(true) + + allow(prep_partition).to receive(:match_volume?).with(anything).and_return(false) + allow(prep_partition).to receive(:match_volume?).with(prep_volume).and_return(true) + + allow(zipl_partition).to receive(:match_volume?).with(anything).and_return(false) + allow(zipl_partition).to receive(:match_volume?).with(zipl_volume).and_return(true) + end + + let(:boot_volume) { instance_double(Y2Storage::VolumeSpecification) } + + let(:efi_volume) { instance_double(Y2Storage::VolumeSpecification) } + + let(:grub_volume) { instance_double(Y2Storage::VolumeSpecification) } + + let(:prep_volume) { instance_double(Y2Storage::VolumeSpecification) } + + let(:zipl_volume) { instance_double(Y2Storage::VolumeSpecification) } + + let(:partitions) { [] } + + let(:boot_partition) { partition_double("/dev/sda1") } + + let(:efi_partition) { partition_double("/dev/sda1") } + + let(:grub_partition) { partition_double("/dev/sda1") } + + let(:prep_partition) { partition_double("/dev/sda1") } + + let(:zipl_partition) { partition_double("/dev/sda1") } + + context "in a x86 system" do + let(:architecture) { :x86 } + + context "using UEFI" do + let(:efiboot) { true } + include_examples "efi partition" + end + + context "not using UEFI (legacy PC)" do + let(:efiboot) { false } + + context "when there is no root" do + let(:root_filesystem) { nil } + include_examples "unknown boot disk" + end + + context "when boot device has no partition table" do + let(:boot_partition_table) { nil } + + it "contains an error for unknown partition table" do + expect(checker.errors.size).to eq(1) + expect(checker.errors).to all(be_a(Y2Storage::SetupError)) + + message = checker.errors.first.message + expect(message).to match(/partition table/) + end + end + + context "when boot device has a GPT partition table" do + let(:boot_ptable_type) { :gpt } + + context "and there is no a grub partition in the system" do + let(:partitions) { [boot_partition] } + + it "contains an error for missing grub partition" do + expect(checker.errors.size).to eq(1) + expect(checker.errors).to all(be_a(Y2Storage::SetupError)) + + missing_volume = checker.errors.first.missing_volume + expect(missing_volume).to eq(grub_volume) + end + end + + context "and there is a grub partition in the system" do + let(:partitions) { [boot_partition, grub_partition] } + + it "does not contain errors" do + expect(checker.errors).to be_empty + end + end + end + + context "with a MS-DOS partition table" do + let(:boot_ptable_type) { :msdos } + + before do + allow(dev_sda).to receive(:mbr_gap).and_return mbr_gap_size + end + + let(:mbr_gap_size) { Y2Storage::DiskSize.zero } + + context "with a too small MBR gap" do + let(:mbr_gap_size) { 16.KiB } + + context "in a plain btrfs setup" do + let(:use_lvm) { false } + let(:use_raid) { false } + let(:use_encryption) { false } + let(:use_btrfs) { true } + + it "does not contain errors" do + expect(checker.errors).to be_empty + end + end + + context "in a not plain btrfs setup" do + let(:use_lvm) { true } + let(:use_btrfs) { true } + + it "contains an error for small MBR gap" do + expect(checker.errors.size).to eq(1) + expect(checker.errors).to all(be_a(Y2Storage::SetupError)) + + message = checker.errors.first.message + expect(message).to match(/gap size is not enough/) + end + end + end + + context "if the MBR gap is big enough to embed Grub" do + let(:mbr_gap_size) { 256.KiB } + + context "in a partitions-based setup" do + let(:use_lvm) { false } + + it "does not contain errors" do + expect(checker.errors).to be_empty + end + end + + context "in a LVM-based setup" do + let(:use_lvm) { true } + include_examples "MBR gap" + end + + context "in a Software RAID setup" do + let(:use_raid) { true } + include_examples "MBR gap" + end + + context "in an encrypted setup" do + let(:use_lvm) { false } + let(:use_encryption) { true } + include_examples "MBR gap" + end + end + end + end + end + + context "in an aarch64 system" do + let(:architecture) { :aarch64 } + # it's always UEFI + let(:efiboot) { true } + include_examples "efi partition" + end + + context "in a PPC64 system" do + let(:architecture) { :ppc } + let(:efiboot) { false } + let(:prep_id) { Y2Storage::PartitionId::PREP } + + before do + allow(storage_arch).to receive(:ppc_power_nv?).and_return(power_nv) + end + + let(:power_nv) { false } + + context "when there is no root" do + let(:root_filesystem) { nil } + include_examples "unknown boot disk" + end + + context "in a non-PowerNV system (KVM/LPAR)" do + let(:power_nv) { false } + + context "with a partitions-based proposal" do + let(:use_lvm) { false } + + context "and there is no a PReP partition in the system" do + let(:partitions) { [boot_partition] } + include_examples "missing prep partition" + end + + context "and there is a PReP partition in the system" do + let(:partitions) { [boot_partition, prep_partition] } + + it "does not contain errors" do + expect(checker.errors).to be_empty + end + end + end + + context "with a LVM-based proposal" do + let(:use_lvm) { true } + include_examples "PPC non-PowerNV" + end + + context "with a Software RAID proposal" do + let(:use_raid) { true } + include_examples "PPC non-PowerNV" + end + + context "with an encrypted proposal" do + let(:use_lvm) { false } + let(:use_encryption) { true } + include_examples "PPC non-PowerNV" + end + end + + context "in bare metal (PowerNV)" do + let(:power_nv) { true } + + context "with a partitions-based proposal" do + let(:use_lvm) { false } + + it "does not contain errors" do + expect(checker.errors).to be_empty + end + end + + context "with a LVM-based proposal" do + let(:use_lvm) { true } + include_examples "boot partition" + end + + context "with a Software RAID proposal" do + let(:use_lvm) { true } + include_examples "boot partition" + end + + context "with an encrypted proposal" do + let(:use_lvm) { false } + let(:use_encryption) { true } + include_examples "boot partition" + end + end + end + + context "in a S/390 system" do + let(:architecture) { :s390 } + let(:efiboot) { false } + let(:use_lvm) { false } + + before do + allow(dev_sda).to receive(:is?).with(:dasd).and_return(dasd) + allow(dev_sda).to receive(:type).and_return(type) + allow(dev_sda).to receive(:format).and_return(format) + end + + let(:dasd) { false } + let(:type) { Y2Storage::DasdType::UNKNOWN } + let(:format) { Y2Storage::DasdFormat::NONE } + + context "when there is no root" do + let(:root_filesystem) { nil } + include_examples "unknown boot disk" + end + + context "using a zfcp disk as boot disk" do + let(:dasd) { false } + let(:type) { Y2Storage::DasdType::UNKNOWN } + let(:format) { Y2Storage::DasdFormat::NONE } + + include_examples "zipl partition" + end + + context "using a FBA DASD disk as boot disk" do + let(:dasd) { true } + let(:type) { Y2Storage::DasdType::FBA } + include_examples "unsupported boot disk" + end + + context "using a (E)CKD DASD disk as boot disk" do + let(:dasd) { true } + let(:type) { Y2Storage::DasdType::ECKD } + + context "if the disk is formatted as LDL" do + let(:format) { Y2Storage::DasdFormat::LDL } + include_examples "unsupported boot disk" + end + + context "if the disk is formatted as CDL" do + let(:format) { Y2Storage::DasdFormat::CDL } + include_examples "zipl partition" + end + end + end + end +end diff --git a/test/y2storage/boot_requirements_strategies/analyzer_test.rb b/test/y2storage/boot_requirements_strategies/analyzer_test.rb index 4b47e4cf39..3403149c81 100644 --- a/test/y2storage/boot_requirements_strategies/analyzer_test.rb +++ b/test/y2storage/boot_requirements_strategies/analyzer_test.rb @@ -141,6 +141,25 @@ expect(planned_devs.map(&:mount_point)).to eq ["/boot", "/"] expect(devicegraph.actiongraph(from: initial_graph)).to be_empty end + + context "when there is a root" do + let(:scenario) { "mixed_disks" } + + it "stores the root filesystem" do + analyzer = described_class.new(devicegraph, planned_devs, boot_name) + expect(analyzer.root_filesystem).to be_a(Y2Storage::Filesystems::Base) + expect(analyzer.root_filesystem.mount_point).to eq("/") + end + end + + context "when there is no root" do + let(:scenario) { "empty_hard_disk_50GiB" } + + it "does not store a filesystem" do + analyzer = described_class.new(devicegraph, planned_devs, boot_name) + expect(analyzer.root_filesystem).to be_nil + end + end end describe "#boot_disk" do @@ -240,6 +259,95 @@ end end + describe "#root_in_software_raid?" do + subject(:analyzer) { described_class.new(devicegraph, planned_devs, boot_name) } + + context "if '/' is a planned software raid" do + let(:planned_root) { planned_md(mount_point: "/") } + + it "returns true" do + expect(analyzer.root_in_software_raid?).to eq true + end + end + + context "if '/' is a planned partition" do + let(:planned_root) { planned_partition(mount_point: "/") } + + it "returns false" do + expect(analyzer.root_in_software_raid?).to eq false + end + end + + context "if '/' is a planned logical volume" do + let(:planned_root) { planned_lv(mount_point: "/") } + + it "returns false" do + expect(analyzer.root_in_software_raid?).to eq false + end + end + + context "if '/' is a software raid from the devicegraph" do + let(:planned_devs) { [] } + let(:scenario) { "md2-devicegraph.xml" } + + before do + md = Y2Storage::Md.find_by_name(fake_devicegraph, "/dev/md0") + md.remove_descendants + fs = md.create_filesystem(Y2Storage::Filesystems::Type::EXT4) + fs.mount_point = "/" + end + + it "returns true" do + expect(analyzer.root_in_software_raid?).to eq true + end + end + + context "if '/' is a partition over a software raid from the devicegraph" do + let(:planned_devs) { [] } + let(:scenario) { "md2-devicegraph.xml" } + + before do + md = Y2Storage::Md.find_by_name(fake_devicegraph, "/dev/md0") + part = md.partition_table.create_partition("/dev/md0-1", + Y2Storage::Region.create(2048, 1048576, 512), + Y2Storage::PartitionType::PRIMARY) + fs = part.create_filesystem(Y2Storage::Filesystems::Type::EXT4) + fs.mount_point = "/" + end + + it "returns true" do + expect(analyzer.root_in_software_raid?).to eq true + end + end + + context "if '/' is a partition over a disk from the devicegraph" do + let(:planned_devs) { [] } + let(:scenario) { "mixed_disks" } + + it "returns false" do + expect(analyzer.root_in_software_raid?).to eq false + end + end + + context "if '/' is a logical volume from the devicegraph" do + let(:planned_devs) { [] } + let(:scenario) { "complex-lvm-encrypt" } + + it "returns false" do + expect(analyzer.root_in_software_raid?).to eq false + end + end + + context "if no device or planned device is configured as '/'" do + let(:planned_devs) { [] } + let(:scenario) { "double-windows-pc" } + + it "returns false" do + expect(analyzer.root_in_software_raid?).to eq false + end + end + end + describe "#encrypted_root?" do subject(:analyzer) { described_class.new(devicegraph, planned_devs, boot_name) } From 712a787327e2cdf38391629081824cba16900ec8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Fri, 12 Jan 2018 19:26:11 +0000 Subject: [PATCH 07/21] Add class SetupChecker --- src/lib/y2storage.rb | 1 + src/lib/y2storage/setup_checker.rb | 138 ++++++++++++++ test/y2storage/setup_checker_test.rb | 261 +++++++++++++++++++++++++++ 3 files changed, 400 insertions(+) create mode 100644 src/lib/y2storage/setup_checker.rb create mode 100644 test/y2storage/setup_checker_test.rb diff --git a/src/lib/y2storage.rb b/src/lib/y2storage.rb index 13c78f08d9..48209d6eab 100644 --- a/src/lib/y2storage.rb +++ b/src/lib/y2storage.rb @@ -72,3 +72,4 @@ require "y2storage/inst_dialog_mixin" require "y2storage/storage_manager" require "y2storage/yaml_writer" +require "y2storage/setup_checker" diff --git a/src/lib/y2storage/setup_checker.rb b/src/lib/y2storage/setup_checker.rb new file mode 100644 index 0000000000..03aa188fd5 --- /dev/null +++ b/src/lib/y2storage/setup_checker.rb @@ -0,0 +1,138 @@ +# encoding: utf-8 + +# Copyright (c) [2018] 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 "yast" +require "y2storage" +require "y2storage/boot_requirements_checker" +require "y2storage/proposal_settings" + +# This 'import' is necessary to load the control file (/etc/YaST/control.xml) +# when running in an installed system. During installation, this module +# is imported by WorkflowManager. +Yast.import "ProductControl" + +module Y2Storage + # Class to check whether a setup (devicegraph) fulfills the storage requirements + # + # The storage requirements are the set of necessary volumes to properly boot and + # run the system. Boot checkings are actually performed by class BootRequirementsChecker, + # and needed volumes to run the system are read from control file (control.xml). + # + # @example + # checker = SetupChecker.new(devicegraph) + # checker.valid? #=> true + class SetupChecker + # @return [Devicegraph] + attr_reader :devicegraph + + # Constructor + # + # @param devicegraph [Devicegraph] setup to check + def initialize(devicegraph) + @devicegraph = devicegraph + end + + # Whether the current setup fulfills the storage requirements + # + # @return [Boolean] + def valid? + errors.empty? + end + + # Whether the system would be bootable using current setup + # + # @return[Boolean] true if the system is bootable; false otherwise. + def bootable? + boot_errors.empty? + end + + # All storage errors detected in the setup, for example, when a /boot/efi partition + # is missing in a UEFI system, or a valid partition for root is missing. + # + # @see SetupError + # + # @return [Array] + def errors + boot_errors + product_errors + end + + # All boot errors detected in the setup + # + # @return [Array] + # + # @see BootRequirementsChecker#errors + def boot_errors + @boot_errors ||= BootRequirementsChecker.new(devicegraph).errors + end + + # All product errors detected in the setup + # + # This checks that all mandatory volumes specified in control file are present + # in the system. + # + # @return [Array] + def product_errors + @product_errors ||= missing_product_volumes.map { |v| SetupError.new(missing_volume: v) } + end + + private + + # Mandatory product volumes that are not present in the current setup + # + # @see #product_volumes + # + # @return [Array] + def missing_product_volumes + product_volumes.select { |v| missing?(v) } + end + + # Mandatory product volumes for a valid storage setup + # + # @note This volumes are obtained from the control file (control.xml) and only + # mandatory volumes are taken into account. + # + # @see ProposalSettings#volumes + # + # @return [Array] + def product_volumes + ProposalSettings.new_for_current_product.volumes.select { |v| mandatory?(v) } + end + + # Whether a volume is mandatory, that is, the volume is proposed to be created + # and cannot be deactivated. + # + # @param volume [VolumeSpecification] + # @return [Boolean] true if the volume is mandatory; false otherwise. + def mandatory?(volume) + volume.proposed? && !volume.proposed_configurable? + end + + # Whether a volume is missing in the current setup + # + # @see BlkDevice#match_volume? + # + # @param volume [VolumeSpecification] + # @return [Boolean] true if the volume is missing; false otherwise. + def missing?(volume) + BlkDevice.all(devicegraph).none? { |d| d.match_volume?(volume) } + end + end +end diff --git a/test/y2storage/setup_checker_test.rb b/test/y2storage/setup_checker_test.rb new file mode 100644 index 0000000000..4d54ec5c76 --- /dev/null +++ b/test/y2storage/setup_checker_test.rb @@ -0,0 +1,261 @@ +#!/usr/bin/env rspec +# encoding: utf-8 + +# Copyright (c) [2018] 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_relative "spec_helper" +require "y2storage" + +describe Y2Storage::SetupChecker do + using Y2Storage::Refinements::SizeCasts + + before do + fake_scenario(scenario) + end + + let(:scenario) { "empty_hard_disk_gpt_50GiB" } + + let(:disk) { Y2Storage::Disk.find_by_name(fake_devicegraph, "/dev/sda") } + + def create_root + root = disk.partition_table.create_partition("/dev/sda2", + Y2Storage::Region.create(1050624, 33554432, 512), + Y2Storage::PartitionType::PRIMARY) + root.size = 15.GiB + fs = root.create_filesystem(Y2Storage::Filesystems::Type::EXT4) + fs.mount_point = "/" + end + + subject { described_class.new(fake_devicegraph) } + + before do + allow(Y2Storage::BootRequirementsChecker).to receive(:new).and_return(boot_checker) + allow(boot_checker).to receive(:errors).and_return(boot_errors) + + allow(Y2Storage::ProposalSettings).to receive(:new_for_current_product).and_return(settings) + allow(settings).to receive(:volumes).and_return(product_volumes) + end + + let(:boot_checker) { instance_double(Y2Storage::BootRequirementsChecker) } + + let(:settings) { instance_double(Y2Storage::ProposalSettings) } + + let(:boot_errors) { [] } + + let(:product_volumes) { [] } + + let(:boot_error) { instance_double(Y2Storage::SetupError) } + + let(:root_volume) do + volume = Y2Storage::VolumeSpecification.new({}) + volume.mount_point = "/" + volume.min_size = 10.GiB + volume.fs_types = Y2Storage::Filesystems::Type.root_filesystems + volume.proposed = true + volume.proposed_configurable = false + volume + end + + let(:swap_volume) do + volume = Y2Storage::VolumeSpecification.new({}) + volume.mount_point = "swap" + volume.min_size = 4.GiB + volume.partition_id = Y2Storage::PartitionId::SWAP + volume.fs_types = [Y2Storage::Filesystems::Type::SWAP] + volume.proposed = true + volume.proposed_configurable = false + volume + end + + let(:home_volume) do + volume = Y2Storage::VolumeSpecification.new({}) + volume.mount_point = "/home" + volume.min_size = 10.GiB + volume.partition_id = Y2Storage::PartitionId::LINUX + volume.fs_types = Y2Storage::Filesystems::Type.home_filesystems + volume.proposed = true + volume.proposed_configurable = true + volume + end + + describe "#valid?" do + context "when some mandatory product volume is not present in the system" do + let(:product_volumes) { [root_volume, home_volume] } + let(:boot_errors) { [] } + + it "returns false" do + expect(subject.valid?).to eq(false) + end + end + + context "when there is some boot error" do + let(:product_volumes) { [] } + let(:boot_errors) { [boot_error] } + + it "returns false" do + expect(subject.valid?).to eq(false) + end + end + + context "when all mandatory product volumes are present in the system and there is no boot error" do + let(:product_volumes) { [root_volume, home_volume] } + let(:boot_errors) { [] } + + before do + create_root + end + + it "returns true" do + expect(subject.valid?).to eq(true) + end + end + end + + describe "#bootable?" do + context "when there is some boot error" do + let(:boot_errors) { [boot_error] } + + it "returns false" do + expect(subject.bootable?).to eq(false) + end + end + + context "when there is no boot error" do + let(:boot_errors) { [] } + + it "returns true" do + expect(subject.bootable?).to eq(true) + end + end + end + + describe "#errors" do + let(:boot_error1) { instance_double(Y2Storage::SetupError) } + let(:boot_error2) { instance_double(Y2Storage::SetupError) } + let(:boot_errors) { [boot_error1, boot_error2] } + + let(:product_volumes) { [root_volume, swap_volume, home_volume] } + + it "includes all boot errors" do + expect(subject.errors).to include(boot_error1, boot_error2) + end + + it "does not include an error for optional product volumes" do + expect(subject.errors).to_not include(an_object_having_attributes(missing_volume: home_volume)) + end + + it "includes an error for each mandatory product volume not present in the system" do + expect(subject.errors).to include(an_object_having_attributes(missing_volume: root_volume)) + expect(subject.errors).to include(an_object_having_attributes(missing_volume: swap_volume)) + end + + context "when a mandatory product volume is present in the system" do + before do + create_root + end + + it "does not include an error for that volume" do + expect(subject.errors).to_not include(an_object_having_attributes(missing_volume: root_volume)) + end + end + + context "when there is no boot error and all mandatory product volumes are present in the system" do + let(:boot_errors) { [] } + let(:product_volumes) { [root_volume, home_volume] } + + before do + create_root + end + + it "returns an empty list" do + expect(subject.errors).to be_empty + end + end + end + + describe "#boot_errors" do + let(:boot_error1) { instance_double(Y2Storage::SetupError) } + let(:boot_error2) { instance_double(Y2Storage::SetupError) } + let(:boot_errors) { [boot_error1, boot_error2] } + + let(:product_volumes) { [root_volume, swap_volume, home_volume] } + + it "includes all boot errors" do + expect(subject.boot_errors).to contain_exactly(boot_error1, boot_error2) + end + + context "when there is no boot error" do + let(:boot_errors) { [] } + + it "returns an empty list" do + expect(subject.boot_errors).to be_empty + end + end + end + + describe "#product_errors" do + let(:boot_error1) { instance_double(Y2Storage::SetupError) } + let(:boot_error2) { instance_double(Y2Storage::SetupError) } + let(:boot_errors) { [boot_error1, boot_error2] } + + let(:product_volumes) { [root_volume, swap_volume, home_volume] } + + it "returns a list of setup errors" do + expect(subject.product_errors).to all(be_a(Y2Storage::SetupError)) + end + + it "does not include boot errors" do + expect(subject.product_errors).to_not include(boot_error1, boot_error2) + end + + it "does not include an error for optional product volumes" do + expect(subject.product_errors) + .to_not include(an_object_having_attributes(missing_volume: home_volume)) + end + + it "includes an error for each mandatory product volume not present in the system" do + expect(subject.product_errors).to include(an_object_having_attributes(missing_volume: root_volume)) + expect(subject.product_errors).to include(an_object_having_attributes(missing_volume: swap_volume)) + end + + context "when a mandatory product volume is present in the system" do + before do + create_root + end + + it "does not include an error for that volume" do + expect(subject.product_errors) + .to_not include(an_object_having_attributes(missing_volume: root_volume)) + end + end + + context "when all mandatory product volumes are present in the system" do + let(:product_volumes) { [root_volume, home_volume] } + + before do + create_root + end + + it "returns an empty list" do + expect(subject.product_errors).to be_empty + end + end + end +end From c8c277713e307cf6e9dd38cf27a3c75a07b844ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Fri, 12 Jan 2018 19:26:49 +0000 Subject: [PATCH 08/21] Add class SetupErrorsPresenter --- .../y2partitioner/setup_errors_presenter.rb | 93 +++++++++++++++ .../setup_errors_presenter_test.rb | 108 ++++++++++++++++++ 2 files changed, 201 insertions(+) create mode 100644 src/lib/y2partitioner/setup_errors_presenter.rb create mode 100644 test/y2partitioner/setup_errors_presenter_test.rb diff --git a/src/lib/y2partitioner/setup_errors_presenter.rb b/src/lib/y2partitioner/setup_errors_presenter.rb new file mode 100644 index 0000000000..6464a133ba --- /dev/null +++ b/src/lib/y2partitioner/setup_errors_presenter.rb @@ -0,0 +1,93 @@ +# 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 "yast" +require "yast/i18n" +require "y2storage" + +Yast.import "HTML" + +module Y2Partitioner + # Class to represent storage setup errors + class SetupErrorsPresenter + include Yast::I18n + + # Constructor + # + # @param setup_checker [SetupChecker] + def initialize(setup_checker) + textdomain "storage" + @setup_checker = setup_checker + end + + # HTML represetation of the storage setup errors + # + # @return [String, nil] nil if there are no errors + def to_html + errors = [boot_errors_html, product_errors_html].compact + return nil if errors.empty? + + errors.join(Yast::HTML.Newline) + end + + private + + # @return [SetupChecker] checker for the current setup + attr_reader :setup_checker + + # HTML representation for boot errors + # + # @return [String] + def boot_errors_html + errors = setup_checker.boot_errors + # TRANSLATORS + header = _("The system could not load because the following errors were found:\n") + + errors_html(header, errors) + end + + # HTML representation for mandatory product errors + # + # @return [String] + def product_errors_html + errors = setup_checker.product_errors + # TRANSLATORS + header = _("The system could not work properly because the following errors were found:\n") + + errors_html(header, errors) + end + + # HTML representation for a set of errors + # + # @note The representation is composed by a header message and the list error messages. + # + # @param header [String] header text + # @param errors [Array] list of errors + # + # @return [String] + def errors_html(header, errors) + return nil if errors.empty? + + error_messages = errors.map(&:message) + header + Yast::HTML.Newline + Yast::HTML.List(error_messages) + end + end +end diff --git a/test/y2partitioner/setup_errors_presenter_test.rb b/test/y2partitioner/setup_errors_presenter_test.rb new file mode 100644 index 0000000000..11a5722f88 --- /dev/null +++ b/test/y2partitioner/setup_errors_presenter_test.rb @@ -0,0 +1,108 @@ +#!/usr/bin/env rspec +# encoding: utf-8 + +# Copyright (c) [2018] 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_relative "test_helper" +require "y2storage" +require "y2partitioner/setup_errors_presenter" + +describe Y2Partitioner::SetupErrorsPresenter do + using Y2Storage::Refinements::SizeCasts + + let(:setup_checker) { instance_double(Y2Storage::SetupChecker) } + + before do + Y2Storage::StorageManager.create_test_instance + allow(setup_checker).to receive(:boot_errors).and_return(boot_errors) + allow(setup_checker).to receive(:product_errors).and_return(product_errors) + end + + subject { described_class.new(setup_checker) } + + let(:boot_errors) { [] } + + let(:product_errors) { [] } + + describe "#to_html" do + context "when there is no error" do + let(:boot_errors) { [] } + let(:product_errors) { [] } + + it "returns nil" do + expect(subject.to_html).to be_nil + end + end + + context "when there are errors" do + let(:boot_error1) { instance_double(Y2Storage::SetupError, message: "boot error 1") } + let(:boot_error2) { instance_double(Y2Storage::SetupError, message: "boot error 2") } + let(:product_error1) { instance_double(Y2Storage::SetupError, message: "product error 1") } + let(:product_error2) { instance_double(Y2Storage::SetupError, message: "product error 2") } + let(:product_error3) { instance_double(Y2Storage::SetupError, message: "product error 3") } + + let(:boot_errors) { [boot_error1, boot_error2] } + let(:product_errors) { [product_error1, product_error2, product_error3] } + + it "contains a message for each error" do + expect(subject.to_html.scan(/
  • /).size).to eq(5) + end + + context "and there are boot errors" do + let(:boot_errors) { [boot_error1] } + let(:product_errors) { [] } + + it "contains a general error message for boot errors" do + expect(subject.to_html).to match(/could not load/) + end + + it "does not contain a general error message for product errors" do + expect(subject.to_html).to_not match(/could not work/) + end + end + + context "and there are product errors" do + let(:boot_errors) { [] } + let(:product_errors) { [product_error1] } + + it "contains a general error message for product errors" do + expect(subject.to_html).to match(/could not work/) + end + + it "does not contain a general error message for boot errors" do + expect(subject.to_html).to_not match(/could not load/) + end + end + + context "and there are boot and product errors" do + let(:boot_errors) { [boot_error1] } + let(:product_errors) { [product_error1] } + + it "contains a general error message for boot errors" do + expect(subject.to_html).to match(/could not load/) + end + + it "contains a general error message for product errors" do + expect(subject.to_html).to match(/could not work/) + end + end + end + end +end From 4a6f5826d974b08962e0bd85fa23f94be1c9ed51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Fri, 12 Jan 2018 19:27:39 +0000 Subject: [PATCH 09/21] Validate partitioner setup --- src/lib/y2partitioner/widgets/overview.rb | 25 ++++++++ test/y2partitioner/widgets/overview_test.rb | 69 ++++++++++++++++++++- 2 files changed, 93 insertions(+), 1 deletion(-) diff --git a/src/lib/y2partitioner/widgets/overview.rb b/src/lib/y2partitioner/widgets/overview.rb index 7847c0dd90..d0e739b71e 100644 --- a/src/lib/y2partitioner/widgets/overview.rb +++ b/src/lib/y2partitioner/widgets/overview.rb @@ -19,12 +19,16 @@ # To contact SUSE LLC about this file by physical or electronic mail, you may # find current contact information at www.suse.com. +require "yast" +require "yast2/popup" require "cwm/widget" require "cwm/tree" require "y2partitioner/icons" require "y2partitioner/device_graphs" require "y2partitioner/ui_state" require "y2partitioner/widgets/pages" +require "y2partitioner/setup_errors_presenter" +require "y2storage/setup_checker" module Y2Partitioner module Widgets @@ -102,6 +106,27 @@ def device_page(device) @pages.find { |p| p.respond_to?(:device) && p.device.sid == device.sid } end + # @macro seeAbstractWidget + # Checks whether the current setup is valid, that is, it contains necessary + # devices for booting (e.g., /boot/efi) and for the system runs properly (e.g., /). + # + # @see Y2Storage::SetupChecker + # + # @return [Boolean] + def validate + setup_checker = Y2Storage::SetupChecker.new(device_graph) + return true if setup_checker.valid? + + errors = SetupErrorsPresenter.new(setup_checker).to_html + # FIXME: improve Yast2::Popup to allow some text before the buttons + errors += _("Do you want to continue?") + + result = Yast2::Popup.show(errors, + headline: :error, richtext: true, buttons: :yes_no, focus: :no) + + result == :yes + end + private attr_reader :hostname diff --git a/test/y2partitioner/widgets/overview_test.rb b/test/y2partitioner/widgets/overview_test.rb index c9ade5c143..c7b06661b5 100644 --- a/test/y2partitioner/widgets/overview_test.rb +++ b/test/y2partitioner/widgets/overview_test.rb @@ -29,13 +29,22 @@ describe Y2Partitioner::Widgets::OverviewTreePager do before do devicegraph_stub(scenario) - subject.init + Yast::ProductFeatures.Import(control_file_content) + + allow(Yast2::Popup).to receive(:show).and_return(:yes) + end + + let(:control_file_content) do + file_path = File.join(DATA_PATH, "control_files", control_file) + Yast::XML.XMLToYCPFile(file_path) end subject { described_class.new("hostname") } let(:scenario) { "lvm-two-vgs.yml" } + let(:control_file) { "caasp.xml" } + include_examples "CWM::Pager" describe "#device_page" do @@ -172,4 +181,62 @@ end end end + + describe "#validate" do + before do + allow(Y2Storage::SetupChecker).to receive(:new).and_return(checker) + allow(checker).to receive(:valid?).and_return(valid_setup) + + allow(Y2Partitioner::SetupErrorsPresenter).to receive(:new).and_return(presenter) + allow(presenter).to receive(:to_html).and_return("html representation") + + allow(Yast2::Popup).to receive(:show).and_return(user_input) + end + + let(:checker) { instance_double(Y2Storage::SetupChecker) } + + let(:presenter) { instance_double(Y2Partitioner::SetupErrorsPresenter) } + + let(:valid_setup) { nil } + + let(:user_input) { nil } + + context "when the current setup is not valid" do + let(:valid_setup) { false } + + it "shows an error popup" do + expect(Yast2::Popup).to receive(:show) + subject.validate + end + + context "and the user accepts to continue" do + let(:user_input) { :yes } + + it "returns true" do + expect(subject.validate).to eq(true) + end + end + + context "and the user declines to continue" do + let(:user_input) { :no } + + it "returns false" do + expect(subject.validate).to eq(false) + end + end + end + + context "when the current setup is valid" do + let(:valid_setup) { true } + + it "does not show an error popup" do + expect(Yast2::Popup).to_not receive(:show) + subject.validate + end + + it "returns true" do + expect(subject.validate).to eq(true) + end + end + end end From 87b8938bc67304e47b5ff71de1cd7bfcdb518c19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Mon, 15 Jan 2018 08:21:32 +0000 Subject: [PATCH 10/21] Fix doc --- src/lib/y2storage/setup_error.rb | 21 +++------------------ 1 file changed, 3 insertions(+), 18 deletions(-) diff --git a/src/lib/y2storage/setup_error.rb b/src/lib/y2storage/setup_error.rb index 417b50abd2..f2299a9af4 100644 --- a/src/lib/y2storage/setup_error.rb +++ b/src/lib/y2storage/setup_error.rb @@ -54,9 +54,8 @@ def message private - # Error text for a volume + # Error text for the missing volume # - # @param volume [VolumeSpecification] # @return [String] def message_for_missing_volume if mount_point_info @@ -66,9 +65,8 @@ def message_for_missing_volume end end - # Error text when the volume has mount point + # Error text when the missing volume has mount point # - # @param volume [VolumeSpecification] # @return [String] def message_with_mount_point if partition_id_info && fs_types_info @@ -82,9 +80,8 @@ def message_with_mount_point end end - # Error text when the volume does not have mount point + # Error text when the missing volume does not have mount point # - # @param volume [VolumeSpecification] # @return [String] def message_without_mount_point if partition_id_info && fs_types_info @@ -98,7 +95,6 @@ def message_without_mount_point end end - # @param volume [VolumeSpecification] # @return [String] def message_with_mount_point_and_partition_id_and_fs # TRANSLATORS: error message, where %{mount_point} is repalaced by a mount point @@ -115,7 +111,6 @@ def message_with_mount_point_and_partition_id_and_fs ) end - # @param volume [VolumeSpecification] # @return [String] def message_with_mount_point_and_partition_id # TRANSLATORS: error message, where %{mount_point} is repalaced by a mount point @@ -129,7 +124,6 @@ def message_with_mount_point_and_partition_id ) end - # @param volume [VolumeSpecification] # @return [String] def message_with_mount_point_and_fs # TRANSLATORS: error message, where %{mount_point} is repalaced by a mount point @@ -144,7 +138,6 @@ def message_with_mount_point_and_fs ) end - # @param volume [VolumeSpecification] # @return [String] def message_with_mount_point_default # TRANSLATORS: error message, where %{mount_point} is repalaced by a mount point @@ -156,7 +149,6 @@ def message_with_mount_point_default ) end - # @param volume [VolumeSpecification] # @return [String] def message_with_partition_id_and_fs # TRANSLATORS: error message, where %{size} is replaced by a disk size (e.g., 5 GiB), @@ -171,7 +163,6 @@ def message_with_partition_id_and_fs ) end - # @param volume [VolumeSpecification] # @return [String] def message_with_partition_id # TRANSLATORS: error message, where %{size} is replaced by a disk size (e.g., 5 GiB) and @@ -184,7 +175,6 @@ def message_with_partition_id ) end - # @param volume [VolumeSpecification] # @return [String] def message_with_fs # TRANSLATORS: error message, where %{size} is replaced by a disk size (e.g., 5 GiB) and @@ -197,7 +187,6 @@ def message_with_fs ) end - # @param volume [VolumeSpecification] # @return [String] def message_without_mount_point_default # TRANSLATORS: error message, where %{size} is replaced by a disk size (e.g., 5 GiB). @@ -209,7 +198,6 @@ def message_without_mount_point_default # Volume mount point to show in the error message # - # @param volume [VolumeSpecification] # @return [String, nil] def mount_point_info missing_volume.mount_point @@ -217,7 +205,6 @@ def mount_point_info # Volume partition id to show in the error message # - # @param volume [VolumeSpecification] # @return [Integer, nil] def partition_id_info missing_volume.partition_id @@ -225,7 +212,6 @@ def partition_id_info # Possible volume fileystem types to show in the error message # - # @param volume [VolumeSpecification] # @return [String, nil] def fs_types_info return nil if missing_volume.fs_types.empty? @@ -234,7 +220,6 @@ def fs_types_info # Volume size to show in the error message # - # @param volume [VolumeSpecification] # @return [DiskSize, nil] def size_info missing_volume.min_size From 1b9222d500d650422d8460746ed41848a347a22d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Mon, 15 Jan 2018 08:24:20 +0000 Subject: [PATCH 11/21] Update version and changelog --- package/yast2-storage-ng.changes | 9 +++++++++ package/yast2-storage-ng.spec | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/package/yast2-storage-ng.changes b/package/yast2-storage-ng.changes index 6a3079e0fa..86b54a5082 100644 --- a/package/yast2-storage-ng.changes +++ b/package/yast2-storage-ng.changes @@ -1,3 +1,11 @@ +------------------------------------------------------------------- +Mon Jan 15 14:22:32 UTC 2018 - jlopez@suse.com + +- Added sanity checks for partitioning setup. +- Partitioner: setup issues are shown to the user before continue. + Mandatory product volumes are required according to control file. +- 4.0.70 + ------------------------------------------------------------------- Mon Jan 15 12:51:01 UTC 2018 - ancor@suse.com @@ -6,6 +14,7 @@ Mon Jan 15 12:51:01 UTC 2018 - ancor@suse.com alignment types, in addition to the optimal one. - Added a (temporary) workaround to a possible bug in libstorage-ng regarding alignment. +- 4.0.69 ------------------------------------------------------------------- Tue Jan 9 15:36:15 UTC 2018 - lslezak@suse.cz diff --git a/package/yast2-storage-ng.spec b/package/yast2-storage-ng.spec index c742f2313b..3a66d62c9b 100644 --- a/package/yast2-storage-ng.spec +++ b/package/yast2-storage-ng.spec @@ -16,7 +16,7 @@ # Name: yast2-storage-ng -Version: 4.0.68 +Version: 4.0.70 Release: 0 BuildArch: noarch From fb197cd466bcac0d2d9c7792f2c53a99df935926 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Mon, 15 Jan 2018 09:55:25 +0000 Subject: [PATCH 12/21] Add fixme --- src/lib/y2storage/boot_requirements_strategies/legacy.rb | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/lib/y2storage/boot_requirements_strategies/legacy.rb b/src/lib/y2storage/boot_requirements_strategies/legacy.rb index 5252a14a34..c258ff6412 100644 --- a/src/lib/y2storage/boot_requirements_strategies/legacy.rb +++ b/src/lib/y2storage/boot_requirements_strategies/legacy.rb @@ -73,6 +73,14 @@ def valid_mbr_gap? mbr_gap && mbr_gap >= GRUB_SIZE end + # FIXME: Bootloader could work properly without BIOS BOOT when the + # partition supports embedding or it is possible to boot from the + # partition. For example, for EXT filesystem it is possible to boot + # from the partition, and grub2 can be embedded into the partition + # when BTRFS is used. For LVM or RAID it is not possible to neither + # embed nor boot from the partition. + # + # (gpt && (lvm || raid || encrypted)) || (gpt && !ext && !btrfs) def grub_partition_needed? boot_ptable_type?(:gpt) end From 26e34652b9c91259abd5c3a82bcd5c4a4e070e26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Mon, 15 Jan 2018 15:51:32 +0000 Subject: [PATCH 13/21] Remove align from planned partitions --- src/lib/y2storage/boot_requirements_strategies/legacy.rb | 1 - src/lib/y2storage/boot_requirements_strategies/prep.rb | 2 -- src/lib/y2storage/planned/partition.rb | 6 ------ test/support/proposed_partitions_examples.rb | 4 ---- 4 files changed, 13 deletions(-) diff --git a/src/lib/y2storage/boot_requirements_strategies/legacy.rb b/src/lib/y2storage/boot_requirements_strategies/legacy.rb index c258ff6412..c3c51d6318 100644 --- a/src/lib/y2storage/boot_requirements_strategies/legacy.rb +++ b/src/lib/y2storage/boot_requirements_strategies/legacy.rb @@ -139,7 +139,6 @@ def grub_volume # @return [Planned::Partition] def grub_partition(target) planned_partition = create_planned_partition(grub_volume, target) - planned_partition.align = :keep_size planned_partition.bootable = false planned_partition end diff --git a/src/lib/y2storage/boot_requirements_strategies/prep.rb b/src/lib/y2storage/boot_requirements_strategies/prep.rb index 20152028b1..b1120bb022 100644 --- a/src/lib/y2storage/boot_requirements_strategies/prep.rb +++ b/src/lib/y2storage/boot_requirements_strategies/prep.rb @@ -91,8 +91,6 @@ def prep_volume # @return [Planned::Partition] def prep_partition(target) planned_partition = create_planned_partition(prep_volume, target) - # Make sure that alignment does not result in a too big partition - planned_partition.align = :keep_size planned_partition.bootable = true planned_partition end diff --git a/src/lib/y2storage/planned/partition.rb b/src/lib/y2storage/planned/partition.rb index 0fdfa5130e..45854acdba 100644 --- a/src/lib/y2storage/planned/partition.rb +++ b/src/lib/y2storage/planned/partition.rb @@ -51,12 +51,6 @@ class Partition < Device # the partition can start attr_accessor :max_start_offset - # @return [Symbol] modifier to pass to ::Storage::Region#align when - # creating the volume. :keep_size to avoid size changes. nil to use - # default alignment. - # FIXME: this one is just guessing the final API of alignment - attr_accessor :align - # @return [Boolean] whether the boot flag should be set. Expected to be # used only with ms-dos style partition tables. GPT has a similar legacy # flag but is not needed in our grub2 setup. diff --git a/test/support/proposed_partitions_examples.rb b/test/support/proposed_partitions_examples.rb index e380bbf337..bfc994d896 100644 --- a/test/support/proposed_partitions_examples.rb +++ b/test/support/proposed_partitions_examples.rb @@ -69,7 +69,6 @@ it "requires it to be between 1 and 8MiB, despite the alignment" do expect(grub_part.min).to eq 1.MiB expect(grub_part.max).to eq 8.MiB - expect(grub_part.align).to eq :keep_size end end @@ -79,7 +78,6 @@ it "requires it to be between 256KiB and 8MiB, despite the alignment" do expect(grub_part.min).to eq 256.KiB expect(grub_part.max).to eq 8.MiB - expect(grub_part.align).to eq :keep_size end end end @@ -138,7 +136,6 @@ it "requires it to be between 1MiB and 8MiB, despite the alignment" do expect(prep_part.min).to eq 1.MiB expect(prep_part.max).to eq 8.MiB - expect(prep_part.align).to eq :keep_size end end @@ -148,7 +145,6 @@ it "requires it to be between 256KiB and 8MiB, despite the alignment" do expect(prep_part.min).to eq 256.KiB expect(prep_part.max).to eq 8.MiB - expect(prep_part.align).to eq :keep_size end end end From 690ff06ad07efa6d26b88007dd57b763a43db5af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Mon, 15 Jan 2018 15:53:03 +0000 Subject: [PATCH 14/21] Method #to_html always returns a string --- src/lib/y2partitioner/setup_errors_presenter.rb | 14 ++++++++------ test/y2partitioner/setup_errors_presenter_test.rb | 4 ++-- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/lib/y2partitioner/setup_errors_presenter.rb b/src/lib/y2partitioner/setup_errors_presenter.rb index 6464a133ba..c4a47c087e 100644 --- a/src/lib/y2partitioner/setup_errors_presenter.rb +++ b/src/lib/y2partitioner/setup_errors_presenter.rb @@ -40,10 +40,12 @@ def initialize(setup_checker) # HTML represetation of the storage setup errors # - # @return [String, nil] nil if there are no errors + # @note An empty string is returned when there is no error. + # + # @return [String] def to_html errors = [boot_errors_html, product_errors_html].compact - return nil if errors.empty? + return "" if errors.empty? errors.join(Yast::HTML.Newline) end @@ -55,18 +57,18 @@ def to_html # HTML representation for boot errors # - # @return [String] + # @return [String, nil] nil if there is no boot error def boot_errors_html errors = setup_checker.boot_errors # TRANSLATORS - header = _("The system could not load because the following errors were found:\n") + header = _("The system might not be able to boot:\n") errors_html(header, errors) end # HTML representation for mandatory product errors # - # @return [String] + # @return [String, nil] nil if there is no product error def product_errors_html errors = setup_checker.product_errors # TRANSLATORS @@ -82,7 +84,7 @@ def product_errors_html # @param header [String] header text # @param errors [Array] list of errors # - # @return [String] + # @return [String, nil] nil if there is no error def errors_html(header, errors) return nil if errors.empty? diff --git a/test/y2partitioner/setup_errors_presenter_test.rb b/test/y2partitioner/setup_errors_presenter_test.rb index 11a5722f88..1a43e39458 100644 --- a/test/y2partitioner/setup_errors_presenter_test.rb +++ b/test/y2partitioner/setup_errors_presenter_test.rb @@ -46,8 +46,8 @@ let(:boot_errors) { [] } let(:product_errors) { [] } - it "returns nil" do - expect(subject.to_html).to be_nil + it "returns an empty string" do + expect(subject.to_html).to be_empty end end From 53f5af409aeb4ed14e8da9fad802299bb39097c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Mon, 15 Jan 2018 16:08:38 +0000 Subject: [PATCH 15/21] Several fixes --- .../boot_requirements_strategies/legacy.rb | 2 +- src/lib/y2storage/match_volume_spec.rb | 4 +++- src/lib/y2storage/md.rb | 5 +++++ src/lib/y2storage/planned/md.rb | 2 +- src/lib/y2storage/setup_checker.rb | 3 ++- src/lib/y2storage/setup_error.rb | 9 ++++----- test/y2storage/match_volume_spec_test.rb | 14 ++++++++++++++ test/y2storage/planned/md_test.rb | 18 +++++++++++------- 8 files changed, 41 insertions(+), 16 deletions(-) diff --git a/src/lib/y2storage/boot_requirements_strategies/legacy.rb b/src/lib/y2storage/boot_requirements_strategies/legacy.rb index c3c51d6318..7ec59cc4cc 100644 --- a/src/lib/y2storage/boot_requirements_strategies/legacy.rb +++ b/src/lib/y2storage/boot_requirements_strategies/legacy.rb @@ -47,7 +47,7 @@ def errors errors << unknown_boot_disk_error elsif boot_partition_table_missing? errors << unknown_boot_partition_table_error - elsif grub_partition_needed? + elsif boot_ptable_type?(:gpt) errors += errors_on_gpt else errors += errors_on_msdos diff --git a/src/lib/y2storage/match_volume_spec.rb b/src/lib/y2storage/match_volume_spec.rb index dfbb39322f..8d2c507381 100644 --- a/src/lib/y2storage/match_volume_spec.rb +++ b/src/lib/y2storage/match_volume_spec.rb @@ -70,11 +70,13 @@ def match_mount_point?(volume) # Whether the size value matches the volume min size # # @note The size matches when the give size value is equal or bigger - # than the volume min size. + # than the volume min size. It always return false if a size value + # is not given. # # @param volume [VolumeSpecification] # @return [Boolean] def match_size?(volume) + return false if volume_match_values[:size].nil? volume_match_values[:size] >= volume.min_size end diff --git a/src/lib/y2storage/md.rb b/src/lib/y2storage/md.rb index ae4f5f5c83..1c173a2d69 100644 --- a/src/lib/y2storage/md.rb +++ b/src/lib/y2storage/md.rb @@ -287,6 +287,11 @@ def md_users to_storage_value.in_holders.to_a.map { |h| Storage.to_md_user(h) } end + # All Software RAIDs are Md devices, but MdMember derived class represents + # BIOS RAIDs. Class MdContainer is also derived from Md, but objects of this + # class are not be considered neither Software nor BIOS RAIDs. + # To properly exclude MdContainer as :raid device, here only Software RAIDs + # are considered to be raid type. def types_for_is types = super types << :md diff --git a/src/lib/y2storage/planned/md.rb b/src/lib/y2storage/planned/md.rb index ec49544e80..8afb821072 100644 --- a/src/lib/y2storage/planned/md.rb +++ b/src/lib/y2storage/planned/md.rb @@ -105,7 +105,7 @@ def self.to_string_attrs def volume_match_values { mount_point: mount_point, - size: DiskSize.zero, + size: nil, fs_type: filesystem_type, partition_id: nil } diff --git a/src/lib/y2storage/setup_checker.rb b/src/lib/y2storage/setup_checker.rb index 03aa188fd5..47dcd49e08 100644 --- a/src/lib/y2storage/setup_checker.rb +++ b/src/lib/y2storage/setup_checker.rb @@ -20,7 +20,8 @@ # find current contact information at www.suse.com. require "yast" -require "y2storage" +require "y2storage/blk_device" +require "y2storage/setup_error" require "y2storage/boot_requirements_checker" require "y2storage/proposal_settings" diff --git a/src/lib/y2storage/setup_error.rb b/src/lib/y2storage/setup_error.rb index f2299a9af4..563eddd875 100644 --- a/src/lib/y2storage/setup_error.rb +++ b/src/lib/y2storage/setup_error.rb @@ -21,7 +21,6 @@ require "yast" require "yast/i18n" -require "y2storage" module Y2Storage # Class to represent storage setup error @@ -97,7 +96,7 @@ def message_without_mount_point # @return [String] def message_with_mount_point_and_partition_id_and_fs - # TRANSLATORS: error message, where %{mount_point} is repalaced by a mount point + # TRANSLATORS: error message, where %{mount_point} is replaced by a mount point # (e.g., /lib/docker), %{size} by a disk size (e.g., 5 GiB), %{partition_id} by a # partition id (e.g., Linux) and %{fs_types} by a list of filesystem types separated # by comma (e.g., ext2, ext3, ext4). @@ -113,7 +112,7 @@ def message_with_mount_point_and_partition_id_and_fs # @return [String] def message_with_mount_point_and_partition_id - # TRANSLATORS: error message, where %{mount_point} is repalaced by a mount point + # TRANSLATORS: error message, where %{mount_point} is replaced by a mount point # (e.g., /lib/docker), %{size} by a disk size (e.g., 5 GiB) and %{partition_id}. format( "Missing device for %{mount_point} with size equal or bigger than %{size} " \ @@ -126,7 +125,7 @@ def message_with_mount_point_and_partition_id # @return [String] def message_with_mount_point_and_fs - # TRANSLATORS: error message, where %{mount_point} is repalaced by a mount point + # TRANSLATORS: error message, where %{mount_point} is replaced by a mount point # (e.g., /lib/docker), %{size} by a disk size (e.g., 5 GiB) and %{fs_types} by a # list of filesystem types separated by comma (e.g., ext2, ext3, ext4). format( @@ -140,7 +139,7 @@ def message_with_mount_point_and_fs # @return [String] def message_with_mount_point_default - # TRANSLATORS: error message, where %{mount_point} is repalaced by a mount point + # TRANSLATORS: error message, where %{mount_point} is replaced by a mount point # (e.g., /lib/docker) and %{size} by a disk size (e.g., 5 GiB). format( "Missing device for %{mount_point} with size equal or bigger than %{size} ", diff --git a/test/y2storage/match_volume_spec_test.rb b/test/y2storage/match_volume_spec_test.rb index f9d4741eba..433f2791ef 100644 --- a/test/y2storage/match_volume_spec_test.rb +++ b/test/y2storage/match_volume_spec_test.rb @@ -112,6 +112,20 @@ def volume_match_values end end + context "but the size is nil" do + let(:size) { nil } + + it "returns false" do + expect(matcher.match_volume?(volume)).to eq(false) + end + + context "and size is excluded for matching" do + it "returns true" do + expect(matcher.match_volume?(volume, exclude: :size)).to eq(true) + end + end + end + context "and fs type is included in the possible fs for the volume" do let(:fs_type) { Y2Storage::Filesystems::Type::SWAP } diff --git a/test/y2storage/planned/md_test.rb b/test/y2storage/planned/md_test.rb index f88e686567..d2aeb7c83f 100755 --- a/test/y2storage/planned/md_test.rb +++ b/test/y2storage/planned/md_test.rb @@ -129,15 +129,19 @@ let(:mount_point) { volume_mount_point } let(:filesystem_type) { volume_fs_types.first } - it "returns true" do - expect(planned_md.match_volume?(volume)).to eq(true) - end + context "and the size is excluded for matching" do + let(:exclude) { :size } + + it "returns true" do + expect(planned_md.match_volume?(volume, exclude: exclude)).to eq(true) + end - context "but the volume requires a specific partition id" do - let(:volume_partition_id) { Y2Storage::PartitionId::ESP } + context "but the volume requires a specific partition id" do + let(:volume_partition_id) { Y2Storage::PartitionId::ESP } - it "returns false" do - expect(planned_md.match_volume?(volume)).to eq(false) + it "returns false" do + expect(planned_md.match_volume?(volume, exclude: exclude)).to eq(false) + end end end end From 6425d8942ded07352c666801dfb344f190bffee3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Mon, 15 Jan 2018 16:14:02 +0000 Subject: [PATCH 16/21] Allow more filesystems for zipl --- .../boot_requirements_strategies/zipl.rb | 4 ++-- src/lib/y2storage/filesystems/type.rb | 15 ++++++++++++++- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/lib/y2storage/boot_requirements_strategies/zipl.rb b/src/lib/y2storage/boot_requirements_strategies/zipl.rb index d42a612651..2aee342955 100644 --- a/src/lib/y2storage/boot_requirements_strategies/zipl.rb +++ b/src/lib/y2storage/boot_requirements_strategies/zipl.rb @@ -70,8 +70,8 @@ def zipl_volume @zipl_volume = VolumeSpecification.new({}) @zipl_volume.mount_point = "/boot/zipl" - @zipl_volume.fs_types = [Filesystems::Type::EXT2] - @zipl_volume.fs_type = Filesystems::Type::EXT2 + @zipl_volume.fs_types = Filesystems::Type.zipl_filesystems + @zipl_volume.fs_type = Filesystems::Type.zipl_filesystems.first @zipl_volume.min_size = DiskSize.MiB(100) @zipl_volume.desired_size = DiskSize.MiB(200) @zipl_volume.max_size = DiskSize.GiB(1) diff --git a/src/lib/y2storage/filesystems/type.rb b/src/lib/y2storage/filesystems/type.rb index 153bbf44bf..a68282abfb 100644 --- a/src/lib/y2storage/filesystems/type.rb +++ b/src/lib/y2storage/filesystems/type.rb @@ -120,9 +120,11 @@ class Type LEGACY_HOME_FILESYSTEMS = [:reiserfs] + ZIPL_FILESYSTEMS = [:ext2, :ext3, :ext4, :xfs] + private_constant :PROPERTIES, :ROOT_FILESYSTEMS, :HOME_FILESYSTEMS, :COMMON_FSTAB_OPTIONS, :EXT_FSTAB_OPTIONS, :LEGACY_ROOT_FILESYSTEMS, - :LEGACY_HOME_FILESYSTEMS + :LEGACY_HOME_FILESYSTEMS, :ZIPL_FILESYSTEMS # Allowed filesystems for root # @@ -152,6 +154,17 @@ def self.legacy_home_filesystems LEGACY_HOME_FILESYSTEMS.map { |f| find(f) } end + # Allowed filesystems for zipl boot partition + # + # @note See page 13 in following link + # https://share.confex.com/share/123/webprogram/Handout/\ + # Session15694/SHARE_Bootloader_Ihno_PittsPPT_0.09.pdf + # + # @return [Array] + def self.zipl_filesystems + ZIPL_FILESYSTEMS.map { |f| find(f) } + end + # Check if filesystem is usable as root (mountpoint "/") filesystem. # # return [Boolean] From c7133b744fbb8cc4edfe2a16393f5838407c8383 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Mon, 15 Jan 2018 16:14:28 +0000 Subject: [PATCH 17/21] Improve wording --- src/lib/y2storage/boot_requirements_strategies/base.rb | 2 +- src/lib/y2storage/boot_requirements_strategies/zipl.rb | 5 ++++- test/y2partitioner/setup_errors_presenter_test.rb | 2 +- test/y2storage/boot_requirements_errors_test.rb | 4 ++-- 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/lib/y2storage/boot_requirements_strategies/base.rb b/src/lib/y2storage/boot_requirements_strategies/base.rb index a6fe12ccb8..06b8ae0fb4 100644 --- a/src/lib/y2storage/boot_requirements_strategies/base.rb +++ b/src/lib/y2storage/boot_requirements_strategies/base.rb @@ -150,7 +150,7 @@ def missing_partition_for?(volume) # @return [SetupError] def unknown_boot_disk_error # TRANSLATORS: error message - error_message = _("Unknwon boot disk") + error_message = _("Boot requirements cannot be determined because there is no '/' mount point") SetupError.new(message: error_message) end end diff --git a/src/lib/y2storage/boot_requirements_strategies/zipl.rb b/src/lib/y2storage/boot_requirements_strategies/zipl.rb index 2aee342955..ce902ae928 100644 --- a/src/lib/y2storage/boot_requirements_strategies/zipl.rb +++ b/src/lib/y2storage/boot_requirements_strategies/zipl.rb @@ -90,7 +90,10 @@ def zipl_partition(target) # @return [SetupError] def unsupported_boot_disk_error # TRANSLATORS: error message - error_message = _("Current boot disk cannot be used for booting") + error_message = _( + "Looks like the system is going to be installed on a FBA " \ + "or LDL device. Booting from such device is not supported" + ) SetupError.new(message: error_message) end end diff --git a/test/y2partitioner/setup_errors_presenter_test.rb b/test/y2partitioner/setup_errors_presenter_test.rb index 1a43e39458..86e57cc1cd 100644 --- a/test/y2partitioner/setup_errors_presenter_test.rb +++ b/test/y2partitioner/setup_errors_presenter_test.rb @@ -70,7 +70,7 @@ let(:product_errors) { [] } it "contains a general error message for boot errors" do - expect(subject.to_html).to match(/could not load/) + expect(subject.to_html).to match(/not be able to boot/) end it "does not contain a general error message for product errors" do diff --git a/test/y2storage/boot_requirements_errors_test.rb b/test/y2storage/boot_requirements_errors_test.rb index 734af087a4..f89e2aeb1e 100644 --- a/test/y2storage/boot_requirements_errors_test.rb +++ b/test/y2storage/boot_requirements_errors_test.rb @@ -90,7 +90,7 @@ expect(checker.errors).to all(be_a(Y2Storage::SetupError)) message = checker.errors.first.message - expect(message).to match(/boot disk/) + expect(message).to match(/there is no '\/'/) end end @@ -100,7 +100,7 @@ expect(checker.errors).to all(be_a(Y2Storage::SetupError)) message = checker.errors.first.message - expect(message).to match(/cannot be used/) + expect(message).to match(/is not supported/) end end From 645f734f06f2fc3902f048447b2135a519871c15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Mon, 15 Jan 2018 16:14:50 +0000 Subject: [PATCH 18/21] Rename filesystem id to partition id --- src/lib/y2partitioner/widgets/format_and_mount.rb | 2 +- src/lib/y2partitioner/widgets/partition_description.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/y2partitioner/widgets/format_and_mount.rb b/src/lib/y2partitioner/widgets/format_and_mount.rb index c45697ec3f..bbfa1df830 100644 --- a/src/lib/y2partitioner/widgets/format_and_mount.rb +++ b/src/lib/y2partitioner/widgets/format_and_mount.rb @@ -736,7 +736,7 @@ def refresh end def label - _("File system &ID:") + _("Partition &ID:") end def items diff --git a/src/lib/y2partitioner/widgets/partition_description.rb b/src/lib/y2partitioner/widgets/partition_description.rb index 0bcfa705ef..1ca30cd40e 100644 --- a/src/lib/y2partitioner/widgets/partition_description.rb +++ b/src/lib/y2partitioner/widgets/partition_description.rb @@ -60,7 +60,7 @@ def device_attributes_list device_udev_by_path.join(Yast::HTML.Newline), device_udev_by_id.join(Yast::HTML.Newline), # TRANSLATORS: acronym for Filesystem Identifier - format(_("FS ID: %s"), "TODO") + format(_("Partition ID: %s"), "TODO") ] end end From f14910284bc2f5b768ef7dd7d628a2b59d86a257 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Mon, 15 Jan 2018 16:32:24 +0000 Subject: [PATCH 19/21] Silly fixes --- src/lib/y2storage/match_volume_spec.rb | 2 +- test/y2partitioner/setup_errors_presenter_test.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lib/y2storage/match_volume_spec.rb b/src/lib/y2storage/match_volume_spec.rb index 8d2c507381..9268a8a0f7 100644 --- a/src/lib/y2storage/match_volume_spec.rb +++ b/src/lib/y2storage/match_volume_spec.rb @@ -70,7 +70,7 @@ def match_mount_point?(volume) # Whether the size value matches the volume min size # # @note The size matches when the give size value is equal or bigger - # than the volume min size. It always return false if a size value + # than the volume min size. It always returns false if a size value # is not given. # # @param volume [VolumeSpecification] diff --git a/test/y2partitioner/setup_errors_presenter_test.rb b/test/y2partitioner/setup_errors_presenter_test.rb index 86e57cc1cd..50a49ad59a 100644 --- a/test/y2partitioner/setup_errors_presenter_test.rb +++ b/test/y2partitioner/setup_errors_presenter_test.rb @@ -87,7 +87,7 @@ end it "does not contain a general error message for boot errors" do - expect(subject.to_html).to_not match(/could not load/) + expect(subject.to_html).to_not match(/not be able to boot/) end end @@ -96,7 +96,7 @@ let(:product_errors) { [product_error1] } it "contains a general error message for boot errors" do - expect(subject.to_html).to match(/could not load/) + expect(subject.to_html).to match(/not be able to boot/) end it "contains a general error message for product errors" do From d716a7ea21c945da059b65cc4ae5cd1c31b49ff5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Mon, 15 Jan 2018 17:45:57 +0000 Subject: [PATCH 20/21] Improve changelog and fix version number --- package/yast2-storage-ng.changes | 5 +++-- package/yast2-storage-ng.spec | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/package/yast2-storage-ng.changes b/package/yast2-storage-ng.changes index 86b54a5082..2396a66adb 100644 --- a/package/yast2-storage-ng.changes +++ b/package/yast2-storage-ng.changes @@ -4,7 +4,9 @@ Mon Jan 15 14:22:32 UTC 2018 - jlopez@suse.com - Added sanity checks for partitioning setup. - Partitioner: setup issues are shown to the user before continue. Mandatory product volumes are required according to control file. -- 4.0.70 +- Part of fate#31896 and fix for bsc#1059160, bsc#1055747 and + bsc#1063957. +- 4.0.69 ------------------------------------------------------------------- Mon Jan 15 12:51:01 UTC 2018 - ancor@suse.com @@ -14,7 +16,6 @@ Mon Jan 15 12:51:01 UTC 2018 - ancor@suse.com alignment types, in addition to the optimal one. - Added a (temporary) workaround to a possible bug in libstorage-ng regarding alignment. -- 4.0.69 ------------------------------------------------------------------- Tue Jan 9 15:36:15 UTC 2018 - lslezak@suse.cz diff --git a/package/yast2-storage-ng.spec b/package/yast2-storage-ng.spec index 3a66d62c9b..fd72f80599 100644 --- a/package/yast2-storage-ng.spec +++ b/package/yast2-storage-ng.spec @@ -16,7 +16,7 @@ # Name: yast2-storage-ng -Version: 4.0.70 +Version: 4.0.69 Release: 0 BuildArch: noarch From 6f23a5e115367ea57a9dc3e8fe1d8cc815171c03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Mon, 15 Jan 2018 17:46:55 +0000 Subject: [PATCH 21/21] Several fixes --- src/lib/y2partitioner/widgets/partition_description.rb | 2 +- src/lib/y2storage/boot_requirements_strategies/zipl.rb | 2 +- src/lib/y2storage/filesystems/type.rb | 5 ++++- src/lib/y2storage/md.rb | 10 +++++----- test/support/proposed_partitions_examples.rb | 8 ++++---- 5 files changed, 15 insertions(+), 12 deletions(-) diff --git a/src/lib/y2partitioner/widgets/partition_description.rb b/src/lib/y2partitioner/widgets/partition_description.rb index 1ca30cd40e..1850d369bd 100644 --- a/src/lib/y2partitioner/widgets/partition_description.rb +++ b/src/lib/y2partitioner/widgets/partition_description.rb @@ -59,7 +59,7 @@ def device_attributes_list device_encrypted, device_udev_by_path.join(Yast::HTML.Newline), device_udev_by_id.join(Yast::HTML.Newline), - # TRANSLATORS: acronym for Filesystem Identifier + # TRANSLATORS: Partition Identifier, where %s is replaced by a value like Linux, swap, etc. format(_("Partition ID: %s"), "TODO") ] end diff --git a/src/lib/y2storage/boot_requirements_strategies/zipl.rb b/src/lib/y2storage/boot_requirements_strategies/zipl.rb index ce902ae928..8d9bbb20f4 100644 --- a/src/lib/y2storage/boot_requirements_strategies/zipl.rb +++ b/src/lib/y2storage/boot_requirements_strategies/zipl.rb @@ -92,7 +92,7 @@ def unsupported_boot_disk_error # TRANSLATORS: error message error_message = _( "Looks like the system is going to be installed on a FBA " \ - "or LDL device. Booting from such device is not supported" + "or LDL device. Booting from such device is not supported." ) SetupError.new(message: error_message) end diff --git a/src/lib/y2storage/filesystems/type.rb b/src/lib/y2storage/filesystems/type.rb index a68282abfb..b5c0d3d862 100644 --- a/src/lib/y2storage/filesystems/type.rb +++ b/src/lib/y2storage/filesystems/type.rb @@ -156,8 +156,11 @@ def self.legacy_home_filesystems # Allowed filesystems for zipl boot partition # + # EXT2 is the preferred type used by default when the proposal + # proposes a new zipl partition. + # # @note See page 13 in following link - # https://share.confex.com/share/123/webprogram/Handout/\ + # https://share.confex.com/share/123/webprogram/Handout/\ # Session15694/SHARE_Bootloader_Ihno_PittsPPT_0.09.pdf # # @return [Array] diff --git a/src/lib/y2storage/md.rb b/src/lib/y2storage/md.rb index 1c173a2d69..91d0d91e2b 100644 --- a/src/lib/y2storage/md.rb +++ b/src/lib/y2storage/md.rb @@ -287,14 +287,14 @@ def md_users to_storage_value.in_holders.to_a.map { |h| Storage.to_md_user(h) } end - # All Software RAIDs are Md devices, but MdMember derived class represents - # BIOS RAIDs. Class MdContainer is also derived from Md, but objects of this - # class are not be considered neither Software nor BIOS RAIDs. - # To properly exclude MdContainer as :raid device, here only Software RAIDs - # are considered to be raid type. def types_for_is types = super types << :md + # All Software RAIDs are Md devices, but MdMember derived class represents + # BIOS RAIDs. Class MdContainer is also derived from Md, but objects of this + # class are not be considered neither Software nor BIOS RAIDs. + # To properly exclude MdContainer as :raid device, here only Software RAIDs + # are considered to be raid type. types << :raid if software_defined? types << :software_raid if software_defined? types diff --git a/test/support/proposed_partitions_examples.rb b/test/support/proposed_partitions_examples.rb index bfc994d896..376ee17df6 100644 --- a/test/support/proposed_partitions_examples.rb +++ b/test/support/proposed_partitions_examples.rb @@ -66,7 +66,7 @@ context "when aiming for the recommended size" do let(:target) { :desired } - it "requires it to be between 1 and 8MiB, despite the alignment" do + it "requires it to be between 1 and 8MiB" do expect(grub_part.min).to eq 1.MiB expect(grub_part.max).to eq 8.MiB end @@ -75,7 +75,7 @@ context "when aiming for the minimal size" do let(:target) { :min } - it "requires it to be between 256KiB and 8MiB, despite the alignment" do + it "requires it to be between 256KiB and 8MiB" do expect(grub_part.min).to eq 256.KiB expect(grub_part.max).to eq 8.MiB end @@ -133,7 +133,7 @@ context "when aiming for the recommended size" do let(:target) { :desired } - it "requires it to be between 1MiB and 8MiB, despite the alignment" do + it "requires it to be between 1MiB and 8MiB" do expect(prep_part.min).to eq 1.MiB expect(prep_part.max).to eq 8.MiB end @@ -142,7 +142,7 @@ context "when aiming for the minimal size" do let(:target) { :min } - it "requires it to be between 256KiB and 8MiB, despite the alignment" do + it "requires it to be between 256KiB and 8MiB" do expect(prep_part.min).to eq 256.KiB expect(prep_part.max).to eq 8.MiB end