Add storage checks

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

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
 

src/lib/y2partitioner/setup_errors_presenter.rb

@@ -0,0 +1,95 @@
+# 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
+    #
+    # @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 "" 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, nil] nil if there is no boot error
+    def boot_errors_html
+      errors = setup_checker.boot_errors
+      # TRANSLATORS
+      header = _("The system might not be able to boot:\n")
+
+      errors_html(header, errors)
+    end
+
+    # HTML representation for mandatory product errors
+    #
+    # @return [String, nil] nil if there is no product error
+    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<SetupError>] list of errors
+    #
+    # @return [String, nil] nil if there is no error
+    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

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

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"

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

src/lib/y2storage/boot_requirements_checker.rb

@@ -1,5 +1,3 @@
-#!/usr/bin/env ruby
-#
 # encoding: utf-8
 
 # Copyright (c) [2015] SUSE LLC
@@ -56,15 +54,32 @@ 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<Planned::Partition>]
     def needed_partitions(target = :desired)
       strategy.needed_partitions(target)
     rescue BootRequirementsStrategies::Error => error
       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<SetupError>]
+    def errors
+      strategy.errors
+    end
+
   protected
 
     # @return [Devicegraph] starting situation.

src/lib/y2storage/boot_requirements_strategies/analyzer.rb

@@ -1,5 +1,3 @@
-#!/usr/bin/env ruby
-#
 # encoding: utf-8
 
 # Copyright (c) [2015] SUSE LLC
@@ -32,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<Planned::Device>]
+      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.
@@ -92,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
@@ -142,6 +163,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
@@ -169,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
@@ -184,15 +207,21 @@ 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)
 
         devicegraph.disk_devices.find { |d| d.name == root_planned_dev.disk }
       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)