Merge pull request #712 from ancorgs/xen_autoyast_merge

Merge to master: process Xen virtual partitions from AutoYaST profiles

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.1.7
+
 -------------------------------------------------------------------
 Tue Aug 14 13:58:03 UTC 2018 - igonzalezsosa@suse.com
 

package/yast2-storage-ng.spec

@@ -16,7 +16,7 @@
 #
 
 Name:		yast2-storage-ng
-Version:	4.1.6
+Version:	4.1.7
 Release:	0
 
 BuildRoot:	%{_tmppath}/%{name}-%{version}-build

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"

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<Symbol>]
       def self.to_string_attrs
         [:reuse_name, :reuse_sid]
       end

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

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"
@@ -84,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?)
@@ -224,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<Planned::Device>] 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

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,40 @@ def planned_for_disk(disk, drive)
         result
       end
 
+      # Returns an array of planned Xen partitions according to a <drive>
+      # 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<Planned::StrayBlkDevice>] 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)
+
+          # 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

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,67 @@ def find_disk(devicegraph, device_name)
         return nil unless device
         ([device] + device.ancestors).find { |d| d.is?(:disk_device) }
       end
+
+      # Whether the given <drive> section represents a set of Xen virtual
+      # partitions
+      #
+      # FIXME: this is a very simplistic approach implemented as bugfix for
+      # bsc#1085134. A <drive> 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 <partition> 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
+      # not exist in the system.
+      #
+      # So this method checks if the given <drive> section contains a set of
+      # <partition> subsections that correspond to existing Xen virtual
+      # partitions (stray block devices) in the system.
+      #
+      # @example AutoYaST profile for Xen virtual partitions
+      #   <drive>
+      #     <device>/dev/xvda</device>
+      #     <partition>
+      #       <partition_nr>1</partition_nr>
+      #       ...information about /dev/xvda1...
+      #     </partition>
+      #     <partition>
+      #       <partition_nr>2</partition_nr>
+      #       ...information about /dev/xvda2...
+      #     </partition>
+      #   </drive>
+      #
+      #   <drive>
+      #     <device>/dev/xvdb</device>
+      #     <partition>
+      #       <partition_nr>1</partition_nr>
+      #       ...information about /dev/xvdb1...
+      #     </partition>
+      #   </drive>
+      #
+      # @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

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
         [