From 22d6575ee33a8f181442d91fb57875d8a02e344c Mon Sep 17 00:00:00 2001 From: Ancor Gonzalez Sosa Date: Tue, 14 Aug 2018 19:34:57 +0200 Subject: [PATCH 1/6] Remove a leftover require --- src/lib/y2storage/proposal/autoinst_devices_creator.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/lib/y2storage/proposal/autoinst_devices_creator.rb b/src/lib/y2storage/proposal/autoinst_devices_creator.rb index 9568a6c2c7..716e863137 100644 --- a/src/lib/y2storage/proposal/autoinst_devices_creator.rb +++ b/src/lib/y2storage/proposal/autoinst_devices_creator.rb @@ -22,8 +22,6 @@ # find current contact information at www.suse.com. require "y2storage/proposal/partitions_distribution_calculator" -# TODO: fix distribution calculator to don't require this -require "y2storage/proposal/lvm_helper" require "y2storage/proposal/partition_creator" require "y2storage/proposal/md_creator" require "y2storage/proposal/autoinst_creator_result" From d6c6591621913f07a0520269445988e15f55ad72 Mon Sep 17 00:00:00 2001 From: Ancor Gonzalez Sosa Date: Thu, 16 Aug 2018 18:03:31 +0200 Subject: [PATCH 2/6] Process Xen virtual partitions from AutoYaST profiles --- src/lib/y2storage/planned.rb | 1 + src/lib/y2storage/planned/device.rb | 5 ++ src/lib/y2storage/planned/stray_blk_device.rb | 67 ++++++++++++++ .../proposal/autoinst_devices_creator.rb | 13 +++ .../proposal/autoinst_devices_planner.rb | 39 ++++++++- .../y2storage/proposal/autoinst_drives_map.rb | 58 ++++++++++++- test/y2storage/autoinst_proposal_test.rb | 87 +++++++++++++++++++ 7 files changed, 265 insertions(+), 5 deletions(-) create mode 100644 src/lib/y2storage/planned/stray_blk_device.rb diff --git a/src/lib/y2storage/planned.rb b/src/lib/y2storage/planned.rb index 46e9226ef9..b5e17124cd 100644 --- a/src/lib/y2storage/planned.rb +++ b/src/lib/y2storage/planned.rb @@ -20,6 +20,7 @@ # find current contact information at www.suse.com. require "y2storage/planned/partition" +require "y2storage/planned/stray_blk_device" require "y2storage/planned/lvm_lv" require "y2storage/planned/lvm_vg" require "y2storage/planned/md" diff --git a/src/lib/y2storage/planned/device.rb b/src/lib/y2storage/planned/device.rb index cd52ae660d..6af37bf4d1 100644 --- a/src/lib/y2storage/planned/device.rb +++ b/src/lib/y2storage/planned/device.rb @@ -78,6 +78,11 @@ def ==(other) other.class == self.class && other.internal_state == internal_state end + # Attributes to display in {#to_s} + # + # This method is expected to be redefined in the subclasses + # + # @return [Array] def self.to_string_attrs [:reuse_name, :reuse_sid] end diff --git a/src/lib/y2storage/planned/stray_blk_device.rb b/src/lib/y2storage/planned/stray_blk_device.rb new file mode 100644 index 0000000000..f3f9dafff0 --- /dev/null +++ b/src/lib/y2storage/planned/stray_blk_device.rb @@ -0,0 +1,67 @@ +# encoding: utf-8 + +# Copyright (c) [2015-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 "y2storage/planned/device" +require "y2storage/planned/mixins" +require "y2storage/match_volume_spec" + +module Y2Storage + module Planned + # Specification for a Y2Storage::StrayBlkDevice object to be processed + # during the AutoYaST proposals + # + # @see Device + class StrayBlkDevice < Device + include Planned::CanBeFormatted + include Planned::CanBeMounted + include Planned::CanBeEncrypted + include MatchVolumeSpec + + # Constructor. + def initialize + super + initialize_can_be_formatted + initialize_can_be_mounted + initialize_can_be_encrypted + end + + # @see Device.to_string_attrs + def self.to_string_attrs + [ + :mount_point, :reuse_name, :reuse_sid, :subvolumes + ] + end + + protected + + # Values for volume specification matching + # + # @see MatchVolumeSpec + def volume_match_values + { + mount_point: mount_point, + fs_type: filesystem_type + } + end + end + end +end diff --git a/src/lib/y2storage/proposal/autoinst_devices_creator.rb b/src/lib/y2storage/proposal/autoinst_devices_creator.rb index 716e863137..8fabb654b4 100644 --- a/src/lib/y2storage/proposal/autoinst_devices_creator.rb +++ b/src/lib/y2storage/proposal/autoinst_devices_creator.rb @@ -82,6 +82,9 @@ def populated_devicegraph(planned_devices, disk_names) creator_result = create_partitions(parts_to_create, disk_names) reuse_devices(parts_to_reuse, creator_result.devicegraph) + # Process planned stray block devices (Xen virtual partitions) + process_stray_devs(planned_devices, creator_result.devicegraph) + # Process planned MD arrays planned_mds = planned_devices.select { |d| d.is_a?(Planned::Md) } mds_to_reuse, mds_to_create = planned_mds.partition(&:reuse?) @@ -222,6 +225,16 @@ def flexible_devices(devices) new_device end end + + # Formats and/or mounts the stray block devices (Xen virtual partitions) + # + # @param planned_devices [Array] all planned devices + # @param devicegraph [Devicegraph] devicegraph containing the Xen + # partitions to be processed. It will be modified. + def process_stray_devs(planned_devices, devicegraph) + planned_stray_devs = planned_devices.select { |d| d.is_a?(Planned::StrayBlkDevice) } + planned_stray_devs.each { |d| d.reuse!(devicegraph) } + end end end end diff --git a/src/lib/y2storage/proposal/autoinst_devices_planner.rb b/src/lib/y2storage/proposal/autoinst_devices_planner.rb index 220b9243f8..9185af02b8 100644 --- a/src/lib/y2storage/proposal/autoinst_devices_planner.rb +++ b/src/lib/y2storage/proposal/autoinst_devices_planner.rb @@ -63,7 +63,13 @@ def planned_devices(drives_map) case drive_section.type when :CT_DISK disk = BlkDevice.find_by_name(devicegraph, disk_name) - result.concat(planned_for_disk(disk, drive_section)) + planned_devs = + if disk + planned_for_disk(disk, drive_section) + else + planned_for_stray_devices(drive_section) + end + result.concat(planned_devs) when :CT_LVM result << planned_for_vg(drive_section) when :CT_MD @@ -115,6 +121,37 @@ def planned_for_disk(disk, drive) result end + # Returns an array of planned Xen partitions according to a + # section which groups virtual partitions with a similar name (e.g. a + # "/dev/xvda" section describing "/dev/xvda1" and "/dev/xvda2"). + # + # @param drive [AutoinstProfile::DriveSection] drive section describing + # a set of stray block devices (Xen virtual partitions) + # @return [Array] List of planned devicess + def planned_for_stray_devices(drive) + result = [] + drive.partitions.each_with_index do |section| + name = drive.device + section.partition_nr.to_s + stray = Y2Storage::Planned::StrayBlkDevice.new + device_config(stray, section, drive) + + # Just for symmetry respect partitions, try to infer the filesystem + # type if it's omitted in the profile for devices that are going to be + # re-formatted but not mounted, so there is no reasonable way to infer + # the appropiate filesystem type based on the mount path (bsc#1060637). + if stray.filesystem_type.nil? + device_to_use = devicegraph.stray_blk_devices.find { |d| d.name == name } + stray.filesystem_type = device_to_use.filesystem_type if device_to_use + end + + add_device_reuse(stray, name, section) + + result << stray + end + + result + end + # Returns a planned volume group according to an AutoYaST specification # # @param drive [AutoinstProfile::DriveSection] drive section describing diff --git a/src/lib/y2storage/proposal/autoinst_drives_map.rb b/src/lib/y2storage/proposal/autoinst_drives_map.rb index b9e7724b5f..a0113dba6e 100644 --- a/src/lib/y2storage/proposal/autoinst_drives_map.rb +++ b/src/lib/y2storage/proposal/autoinst_drives_map.rb @@ -137,12 +137,13 @@ def add_disks(disks, devicegraph) fixed_drives.each do |drive| disk = find_disk(devicegraph, drive.device) - if disk.nil? + if disk + @drives[disk.name] = drive + elsif stray_devices_group?(drive, devicegraph) + @drives[drive.device] = drive + else issues_list.add(:no_disk, drive) - next end - - @drives[disk.name] = drive end flexible_drives.each do |drive| @@ -187,6 +188,55 @@ def find_disk(devicegraph, device_name) return nil unless device ([device] + device.ancestors).find { |d| d.is?(:disk_device) } end + + # Whether the given section represents a set of Xen virtual + # partitions + # + # See below for an example of an AutoYaST profile for a system with + # the virtual partitions /dev/xvda1, /dev/xvda2 and /dev/xvdb1. That + # example includes two devices /dev/xvda and /dev/xvdb that really do + # not exist in the system. + # + # So this method checks if the given section contains a set of + # subsections that correspond to existing Xen virtual + # partitions (stray block devices) in the system. + # + # @example AutoYaST profile for Xen virtual partitions + # + # /dev/xvda + # + # 1 + # ...information about /dev/xvda1... + # + # + # 2 + # ...information about /dev/xvda2... + # + # + # + # + # /dev/xvdb + # + # 1 + # ...information about /dev/xvdb1... + # + # + # + # @param drive [AutoinstProfile::DriveSection] AutoYaST drive specification + # @param devicegraph [Devicegraph] Devicegraph + # @return [Boolean] false if there are no partition sections or they do + # not correspond to stray devices + def stray_devices_group?(drive, devicegraph) + return false if drive.partitions.empty? + + devices = devicegraph.stray_blk_devices + drive.partitions.all? do |partition| + next false if partition.partition_nr.nil? + + name = drive.device + partition.partition_nr.to_s + devices.any? { |dev| dev.name == name } + end + end end end end diff --git a/test/y2storage/autoinst_proposal_test.rb b/test/y2storage/autoinst_proposal_test.rb index 2f0002b7d9..54ebcfbfa3 100644 --- a/test/y2storage/autoinst_proposal_test.rb +++ b/test/y2storage/autoinst_proposal_test.rb @@ -620,6 +620,93 @@ end end + describe "with Xen virtual partitions" do + let(:xvda1_sect) do + { + "partition_nr" => 1, "create" => false, + "filesystem" => "btrfs", "format" => true, "mount" => "/" + } + end + + let(:xvda2_sect) do + { + "partition_nr" => 2, "create" => false, "mount_by" => "device", + "filesystem" => "swap", "format" => true, "mount" => "swap" + } + end + + let(:xvdc1_sect) do + { "filesystem" => :xfs, "mount" => "/home", "size" => "max", "create" => true } + end + + context "and no other kind of devices" do + let(:scenario) { "xen-partitions.xml" } + + let(:partitioning) do + [{ "device" => "/dev/xvda", "use" => "all", "partitions" => [xvda1_sect, xvda2_sect] }] + end + + it "does not register any issue" do + proposal.propose + expect(issues_list).to be_empty + end + + it "correctly formats all the virtual partitions" do + proposal.propose + + xvda1 = proposal.devices.find_by_name("/dev/xvda1") + expect(xvda1.filesystem).to have_attributes( + type: Y2Storage::Filesystems::Type::BTRFS, + mount_path: "/" + ) + xvda2 = proposal.devices.find_by_name("/dev/xvda2") + expect(xvda2.filesystem).to have_attributes( + type: Y2Storage::Filesystems::Type::SWAP, + mount_path: "swap" + ) + end + end + + context "and Xen hard disks" do + let(:scenario) { "xen-disks-and-partitions.xml" } + + let(:partitioning) do + [ + { "device" => "/dev/xvda", "use" => "all", "partitions" => [xvda1_sect, xvda2_sect] }, + { "device" => "/dev/xvdc", "use" => "all", "partitions" => [xvdc1_sect] } + ] + end + + it "correctly formats all the virtual and real partitions" do + proposal.propose + + xvda1 = proposal.devices.find_by_name("/dev/xvda1") + expect(xvda1.filesystem).to have_attributes( + type: Y2Storage::Filesystems::Type::BTRFS, + mount_path: "/" + ) + xvda2 = proposal.devices.find_by_name("/dev/xvda2") + expect(xvda2.filesystem).to have_attributes( + type: Y2Storage::Filesystems::Type::SWAP, + mount_path: "swap" + ) + + # NOTE: /dev/xvdc1 is not the only /dev/xvdc partition in the final + # system. AutoYaST also creates a bios_boot partition because it + # always adds partitions needed for booting. In a Xen environment + # with Xen virtual partitions that behavior is probably undesired + # (let's wait for feedback about it). + xvdc = proposal.devices.find_by_name("/dev/xvdc") + expect(xvdc.partitions).to include( + an_object_having_attributes( + filesystem_type: Y2Storage::Filesystems::Type::XFS, + filesystem_mountpoint: "/home" + ) + ) + end + end + end + describe "LVM" do let(:partitioning) do [ From 41f0ddad8f1a823c9d2c8b7f70c5a23009538334 Mon Sep 17 00:00:00 2001 From: Ancor Gonzalez Sosa Date: Thu, 16 Aug 2018 19:03:36 +0200 Subject: [PATCH 3/6] Add test for AutoYaST reusing corner-case --- .../proposal/autoinst_devices_planner_test.rb | 69 +++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/test/y2storage/proposal/autoinst_devices_planner_test.rb b/test/y2storage/proposal/autoinst_devices_planner_test.rb index 89c3559872..d01e981f2d 100644 --- a/test/y2storage/proposal/autoinst_devices_planner_test.rb +++ b/test/y2storage/proposal/autoinst_devices_planner_test.rb @@ -131,6 +131,75 @@ expect(issue).to_not be_nil end end + + context "when formating but not mounting a partition" do + let(:root_spec) do + { "create" => false, "format" => true, "filesystem" => fs, "partition_nr" => 3 } + end + + context "if the file system type is specified" do + let(:fs) { "xfs" } + + it "plans the specified filesystem type" do + devices = planner.planned_devices(drives_map) + planned = devices.find { |d| d.reuse_name == "/dev/sda3" } + expect(planned.reformat?).to eq true + expect(planned.filesystem_type).to eq Y2Storage::Filesystems::Type::XFS + end + end + + context "if the file system type is not specified" do + let(:fs) { nil } + + it "keeps the previous file system type of the partition" do + devices = planner.planned_devices(drives_map) + planned = devices.find { |d| d.reuse_name == "/dev/sda3" } + expect(planned.reformat?).to eq true + expect(planned.filesystem_type).to eq Y2Storage::Filesystems::Type::EXT4 + end + end + end + end + + context "when formatting but not mounting a Xen virtual partitions" do + let(:scenario) { "xen-partitions.xml" } + + let(:partitioning_array) do + [{ "device" => "/dev/xvda", "use" => "all", "partitions" => part_section }] + end + + context "if the file system type is specified" do + let(:part_section) { [{ "partition_nr" => 2, "format" => true, "filesystem" => "ext2" }] } + + it "plans the specified filesystem type" do + devices = planner.planned_devices(drives_map) + planned = devices.find { |d| d.reuse_name == "/dev/xvda2" } + expect(planned.reformat?).to eq true + expect(planned.filesystem_type).to eq Y2Storage::Filesystems::Type::EXT2 + end + end + + context "if the file system type is not specified and there is a previous file system" do + let(:part_section) { [{ "partition_nr" => 2, "format" => true }] } + + it "reuses the previous file system type" do + devices = planner.planned_devices(drives_map) + planned = devices.find { |d| d.reuse_name == "/dev/xvda2" } + expect(planned.reformat?).to eq true + expect(planned.filesystem_type).to eq Y2Storage::Filesystems::Type::XFS + end + end + + context "if the file system type is not specified and there is no previous file system" do + let(:part_section) { [{ "partition_nr" => 1, "format" => true }] } + + it "plans no filesystem type" do + devices = planner.planned_devices(drives_map) + planned = devices.find { |d| d.reuse_name == "/dev/xvda1" } + expect(planned.reformat?).to eq true + expect(planned.filesystem_type).to be_nil + end + end end context "specifying partition type" do From 62f20bb95f3c007ae3c67c537f42be2d8fa4c7d8 Mon Sep 17 00:00:00 2001 From: Ancor Gonzalez Sosa Date: Fri, 17 Aug 2018 13:53:18 +0200 Subject: [PATCH 4/6] Version and changelog --- package/yast2-storage-ng.changes | 7 +++++++ package/yast2-storage-ng.spec | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/package/yast2-storage-ng.changes b/package/yast2-storage-ng.changes index 95aafa9c4d..5ed121b603 100644 --- a/package/yast2-storage-ng.changes +++ b/package/yast2-storage-ng.changes @@ -1,3 +1,10 @@ +------------------------------------------------------------------- +Fri Aug 17 08:47:53 UTC 2018 - ancor@suse.com + +- AutoYaST: recognize Xen virtual partitions in the profile when + importing and installing (bsc#1085134). +- 4.0.206 + ------------------------------------------------------------------- Tue Aug 14 11:43:19 UTC 2018 - igonzalezsosa@suse.com diff --git a/package/yast2-storage-ng.spec b/package/yast2-storage-ng.spec index 02abc9bdaa..fbb4713a84 100644 --- a/package/yast2-storage-ng.spec +++ b/package/yast2-storage-ng.spec @@ -16,7 +16,7 @@ # Name: yast2-storage-ng -Version: 4.0.205 +Version: 4.0.206 Release: 0 BuildRoot: %{_tmppath}/%{name}-%{version}-build From 1c4de39c01bfac0a81a306dd47d83aa69605d206 Mon Sep 17 00:00:00 2001 From: Ancor Gonzalez Sosa Date: Mon, 20 Aug 2018 12:09:13 +0200 Subject: [PATCH 5/6] Minor fix --- src/lib/y2storage/proposal/autoinst_devices_planner.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/y2storage/proposal/autoinst_devices_planner.rb b/src/lib/y2storage/proposal/autoinst_devices_planner.rb index 9185af02b8..8601c6ec88 100644 --- a/src/lib/y2storage/proposal/autoinst_devices_planner.rb +++ b/src/lib/y2storage/proposal/autoinst_devices_planner.rb @@ -130,7 +130,7 @@ def planned_for_disk(disk, drive) # @return [Array] List of planned devicess def planned_for_stray_devices(drive) result = [] - drive.partitions.each_with_index do |section| + drive.partitions.each do |section| name = drive.device + section.partition_nr.to_s stray = Y2Storage::Planned::StrayBlkDevice.new device_config(stray, section, drive) From a445e0221a28e904a7b3f5a305a0f83bdcd4a875 Mon Sep 17 00:00:00 2001 From: Ancor Gonzalez Sosa Date: Mon, 20 Aug 2018 12:10:11 +0200 Subject: [PATCH 6/6] Documentation improvements --- .../y2storage/proposal/autoinst_devices_planner.rb | 5 ++++- src/lib/y2storage/proposal/autoinst_drives_map.rb | 12 ++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/lib/y2storage/proposal/autoinst_devices_planner.rb b/src/lib/y2storage/proposal/autoinst_devices_planner.rb index 8601c6ec88..b7c76a9ad1 100644 --- a/src/lib/y2storage/proposal/autoinst_devices_planner.rb +++ b/src/lib/y2storage/proposal/autoinst_devices_planner.rb @@ -127,10 +127,13 @@ def planned_for_disk(disk, drive) # # @param drive [AutoinstProfile::DriveSection] drive section describing # a set of stray block devices (Xen virtual partitions) - # @return [Array] List of planned devicess + # @return [Array] List of planned devices def planned_for_stray_devices(drive) result = [] drive.partitions.each do |section| + # Since this drive section was included in the drives map, we can be + # sure that all partitions include a valid partition_nr + # (see {AutoinstDrivesMap#stray_devices_group?}). name = drive.device + section.partition_nr.to_s stray = Y2Storage::Planned::StrayBlkDevice.new device_config(stray, section, drive) diff --git a/src/lib/y2storage/proposal/autoinst_drives_map.rb b/src/lib/y2storage/proposal/autoinst_drives_map.rb index a0113dba6e..7630053d3c 100644 --- a/src/lib/y2storage/proposal/autoinst_drives_map.rb +++ b/src/lib/y2storage/proposal/autoinst_drives_map.rb @@ -192,6 +192,18 @@ def find_disk(devicegraph, device_name) # Whether the given section represents a set of Xen virtual # partitions # + # FIXME: this is a very simplistic approach implemented as bugfix for + # bsc#1085134. A section is only considered to represent a set of + # virtual partitions if ALL its partitions contain an explicit + # partition_nr that matches with the name of a stray block device. If any + # of the subsections does not include partition_nr or does not + # match with an existing device, the whole drive is discarded. + # + # NOTE: in the future the AutoYaST profile will hopefully allow a more + # flexible usage of disks. Then Xen virtual partitions could be + # represented as disks in the profile (which matches reality way better), + # and there will be no need to improve this method much further. + # # See below for an example of an AutoYaST profile for a system with # the virtual partitions /dev/xvda1, /dev/xvda2 and /dev/xvdb1. That # example includes two devices /dev/xvda and /dev/xvdb that really do