Include names in the shadowed subvolumes popup (bsc#928641)

package/yast2-storage.changes

@@ -1,3 +1,9 @@
+-------------------------------------------------------------------
+Mon Feb 15 17:23:10 UTC 2016 - ancor@suse.com
+
+- Expert partitioner: improved the shadowed subvolumes warning to
+  include the name of the affected subvolumes (bsc#928641)
+
 -------------------------------------------------------------------
 Fri Feb 12 11:50:59 UTC 2016 - ancor@suse.com
 

src/Makefile.am

@@ -90,8 +90,8 @@ ynclude_DATA = \
 ylibdir = @ylibdir@/storage
 ylib_DATA = \
   lib/storage/target_map_formatter.rb \
-  lib/storage/used_storage_features.rb
-
+  lib/storage/used_storage_features.rb \
+  lib/storage/shadowed_vol_list.rb
 
 scrconf_DATA = \
   scrconf/proc_partitions.scr \

src/include/partitioning/custom_part_check_generated.rb

@@ -23,12 +23,17 @@
 # Package:     yast2-storage
 # Summary:     Expert Partitioner
 # Authors:     Arvin Schnell <aschnell@suse.de>
+
+require "storage/shadowed_vol_list"
+
 module Yast
   module PartitioningCustomPartCheckGeneratedInclude
 
 
     include Yast::Logger
 
+    POPUP_LINE_LENGTH = 70
+    private_constant :POPUP_LINE_LENGTH
 
     def initialize_partitioning_custom_part_check_generated(include_target)
       Yast.import "Arch"
@@ -90,7 +95,7 @@ def check_created_partition_table(targetMap, installation)
       fat_system_boot = false
       raid_type = ""
       rootdlabel = ""
-      root_subvols_shadowed = false
+      shadowed_root_subvols = []
 
       Builtins.foreach(targetMap) do |disk, diskinfo|
         part_info = Ops.get_list(diskinfo, "partitions", [])
@@ -131,24 +136,12 @@ def check_created_partition_table(targetMap, installation)
             end
 
             # search for shadowed subvolumes of root filesystem
-            subvols = part.fetch("subvol", [])
-            subvols.each do |subvol|
-
-              if FileSystems.default_subvol.empty?
-                tmp = "/" + subvol.fetch("name")
-              else
-                tmp = subvol.fetch("name")[FileSystems.default_subvol.size..-1]
-              end
-
-              targetMap.each do |dev, disk|
-                parts = disk.fetch("partitions", [])
-                parts.each do |part|
-                  if part.has_key?("mount")
-                    mp = part["mount"]
-                    if (tmp == mp) || (tmp.start_with?(mp) && tmp[mp.size] == "/")
-                      root_subvols_shadowed = true
-                    end
-                  end
+            targetMap.each do |dev, disk|
+              parts = disk.fetch("partitions", [])
+              parts.each do |other_part|
+                if other_part.has_key?("mount")
+                  shadowed = ShadowedVolList.new(partition: part, mntpoint: other_part["mount"])
+                  shadowed_root_subvols += shadowed.to_a
                 end
               end
             end
@@ -412,13 +405,20 @@ def check_created_partition_table(targetMap, installation)
 
       end
 
-      if root_subvols_shadowed || show_all_popups
+      if !shadowed_root_subvols.empty? || show_all_popups
+        if shadowed_root_subvols.empty?
+          subvol_list = ""
+        else
+          subvols = shadowed_root_subvols.map { |vol| vol["name"] }
+          subvol_list = format_subvols(subvols)
+        end
         message = _(
           "Warning: Some subvolumes of the root filesystem are shadowed by\n" +
           "mount points of other filesystem. This could lead to problems.\n" +
+          "%s" +
           "\n" +
           "Really use this setup?\n"
-        )
+        ) % subvol_list
 
         ok = false if !Popup.YesNo(message)
       end
@@ -549,6 +549,12 @@ def check_created_partition_table(targetMap, installation)
       ok
     end
 
+    def format_subvols(subvols)
+      # Extra space needed for the regexp
+      all = subvols.join(', ') + " "
+      lines = all.scan(/.{1,#{POPUP_LINE_LENGTH}}\s/).map(&:strip)
+      "\n#{lines.join("\n")}\n"
+    end
 
     def check_devices_used(partitions, not_cr)
       partitions = deep_copy(partitions)

src/lib/storage/shadowed_vol_list.rb

@@ -0,0 +1,64 @@
+# Copyright (c) 2016 Novell, Inc.
+#
+# 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.
+#
+# To contact SUSE about this file by physical or electronic mail, you may
+# find current contact information at www.suse.com.
+
+require "yast"
+Yast.import "FileSystems"
+
+module Yast
+  class ShadowedVolList
+    include Enumerable
+    include Yast::Logger
+
+    attr_reader :partition, :mntpoint
+
+    def initialize(partition: nil, mntpoint: nil)
+      raise ArgumentError unless partition && mntpoint
+      @partition = partition
+      @mntpoint = mntpoint
+      @volumes = selected_volumes
+    end
+
+    def each(&block)
+      @volumes.each(&block)
+    end
+
+  protected
+
+    def selected_volumes
+      subvols = partition.fetch("subvol", [])
+      log.debug "Full list of subvolumes: #{subvols}"
+      subvols.select do |subvol|
+        name = full_name(subvol)
+        shadowed?(name)
+      end
+    end
+
+    def shadowed?(name)
+      (name == mntpoint) || (name.start_with?(mntpoint) && name[mntpoint.size] == "/")
+    end
+
+    def full_name(subvol)
+      if FileSystems.default_subvol.empty?
+        "/" + subvol.fetch("name")
+      else
+        subvol.fetch("name")[FileSystems.default_subvol.size..-1]
+      end
+    end
+  end
+end

test/include/partitioning_custom_part_check_generated_include_test.rb

@@ -360,6 +360,11 @@ def build_map(name)
             .and_return(false)
           expect(client.check_created_partition_table(map, installation)).to eq(false)
         end
+
+        it "displays the list of shadowed subvolumes" do
+          expect(Yast::Popup).to receive(:YesNo).with(/@\/boot\/grub2\/i386-pc, @\/boot\/grub2\/x86_64-efi/)
+          client.check_created_partition_table(map, installation)
+        end
       end
 
       context "when no swap is found" do