Merge pull request #186 from ancorgs/only_root_snapshots

Expert partitioner only offers snapshots if supported (bsc#944252)

package/yast2-storage.changes

@@ -1,4 +1,13 @@
 -------------------------------------------------------------------
+Thu Feb  5 10:21:35 UTC 2016 - ancor@suse.com
+
+- Fixed expert partioner to only offer the option to enable Btrfs
+  snapshots for the root (/) filesystem (bsc#944252)
+- Fixed expert partitioner to not forget fstab options when dealing
+  with Btrfs partitions (bsc#954691)
+- 3.1.75
+
+--------------------------------------------------------------------
 Thu Feb  4 11:45:44 UTC 2016 - jreidinger@suse.com
 
 - Simplify encrypted home proposal as installer no longer allows

package/yast2-storage.spec

@@ -17,7 +17,7 @@
 
 
 Name:           yast2-storage
-Version:        3.1.74
+Version:        3.1.75
 Release:        0
 
 BuildRoot:      %{_tmppath}/%{name}-%{version}-build

src/include/partitioning/custom_part_dialogs.rb

@@ -1533,11 +1533,11 @@ def SubvolNames(data)
 
 
     # Dialog: Subvolume handling
-    # @parm old map with original partition
-    # @parm new map with changes filled in
-    def SubvolHandling(old, new)
-      old = deep_copy(old)
-      new = deep_copy(new)
+    # @parm old_partition map with original partition
+    # @parm new_partition map with changes filled in
+    def SubvolHandling(old_partition, new_partition)
+      old_partition = deep_copy(old_partition)
+      new_partition = deep_copy(new_partition)
 
       # help text, richtext format
       helptext = _(
@@ -1550,10 +1550,10 @@ def SubvolHandling(old, new)
         )
       end
 
-      old_subvol = Ops.get_list(new, "subvol", [])
-      old_userdata = Ops.get_map(new, "userdata", {})
+      initial_subvol = Ops.get_list(new_partition, "subvol", [])
+      initial_userdata = Ops.get_map(new_partition, "userdata", {})
 
-      items = SubvolNames(new)
+      items = SubvolNames(new_partition)
 
       contents = VBox(
         # label text
@@ -1582,7 +1582,7 @@ def SubvolHandling(old, new)
         )
       )
 
-      if Mode.installation()
+      if Mode.installation() && snapshots_supported?(new_partition)
         contents = Builtins.add(contents, VSpacing(0.5))
         contents = Builtins.add(contents,
           Left(
@@ -1614,7 +1614,7 @@ def SubvolHandling(old, new)
         )
       )
 
-      UI.ChangeWidget(Id(:snapshots), :Value, old_userdata["/"] == "snapshots")
+      UI.ChangeWidget(Id(:snapshots), :Value, initial_userdata["/"] == "snapshots")
 
       UI.ChangeWidget(:help, :HelpText, helptext)
 
@@ -1631,12 +1631,12 @@ def SubvolHandling(old, new)
           Builtins.y2milestone("SubvolHandling remove path:%1", pth)
           Builtins.y2milestone(
             "SubvolHandling remove subvol:%1",
-            Ops.get_list(new, "subvol", [])
+            Ops.get_list(new_partition, "subvol", [])
           )
           Ops.set(
-            new,
+            new_partition,
             "subvol",
-            Builtins.maplist(Ops.get_list(new, "subvol", [])) do |p|
+            Builtins.maplist(Ops.get_list(new_partition, "subvol", [])) do |p|
               if Ops.get_string(p, "name", "") == pth
                 Ops.set(p, "delete", true)
                 p = Builtins.remove(p, "create")
@@ -1646,9 +1646,9 @@ def SubvolHandling(old, new)
           )
           Builtins.y2milestone(
             "SubvolHandling remove subvol:%1",
-            Ops.get_list(new, "subvol", [])
+            Ops.get_list(new_partition, "subvol", [])
           )
-          items = SubvolNames(new)
+          items = SubvolNames(new_partition)
           Builtins.y2milestone("SubvolHandling remove items:%1", items)
           changed = true
           UI.ChangeWidget(Id(:subvol), :Items, items)
@@ -1661,7 +1661,7 @@ def SubvolHandling(old, new)
             pth,
             svtmp
           )
-          Builtins.y2milestone("SubvolHandling names:%1", SubvolNames(new))
+          Builtins.y2milestone("SubvolHandling names:%1", SubvolNames(new_partition))
           if pth == nil || Builtins.size(pth) == 0
             Popup.Message(_("Empty subvolume name not allowed."))
           else
@@ -1676,41 +1676,41 @@ def SubvolHandling(old, new)
               Popup.Message(tmp)
               pth = svtmp + pth  
             end
-            if Builtins.contains(SubvolNames(new), pth)
+            if Builtins.contains(SubvolNames(new_partition), pth)
               Popup.Message(
                 Builtins.sformat(_("Subvolume name %1 already exists."), pth)
               )
             else
               Ops.set(
-                new,
+                new_partition,
                 "subvol",
                 Builtins.add(
-                  Ops.get_list(new, "subvol", []),
+                  Ops.get_list(new_partition, "subvol", []),
                   { "create" => true, "name" => pth }
                 )
               )
               changed = true
             end
           end
-          items = SubvolNames(new)
+          items = SubvolNames(new_partition)
           UI.ChangeWidget(Id(:subvol), :Items, items)
           UI.ChangeWidget(Id(:new_path), :Value, "")
 
         when :ok
           val = UI.QueryWidget(Id(:snapshots), :Value)
           if val
-            old_userdata["/"] = "snapshots"
+            initial_userdata["/"] = "snapshots"
           else
-            old_userdata.delete("/")
+            initial_userdata.delete("/")
           end
-          Ops.set(new, "userdata", old_userdata)
+          Ops.set(new_partition, "userdata", initial_userdata)
 
         when :cancel
           if changed
             if Popup.YesNo(
                 _("Modifications done so far in this dialog will be lost.")
               )
-              Ops.set(new, "subvol", old_subvol)
+              new_partition["subvol"] = initial_subvol
             else
               ret = :again
             end
@@ -1724,11 +1724,18 @@ def SubvolHandling(old, new)
 
       Builtins.y2milestone(
         "SubvolHandling subvol:%1 userdata:%2",
-        Ops.get_list(new, "subvol", []),
-        Ops.get_map(new, "userdata", {})
+        new_partition["subvol"],
+        new_partition["userdata"]
       )
-      deep_copy(new)
+      deep_copy(new_partition)
     end
 
+    # Checks whether Btrfs snapshots are supported for the partition
+    #
+    # @param partition [Hash] map representing the partition
+    # @return [Boolean]
+    def snapshots_supported?(partition)
+      partition["mount"] == "/"
+    end
   end
 end

src/modules/Storage.rb

@@ -1924,18 +1924,13 @@ def IsDiskType(t)
     def HandleBtrfsSimpleVolumes(tg)
       tg = deep_copy(tg)
       if Builtins.haskey(tg, "/dev/btrfs")
-        simple = Builtins.filter(
-          Ops.get_list(tg, ["/dev/btrfs", "partitions"], [])
-        ) do |p|
-          Ops.less_or_equal(Builtins.size(Ops.get_list(p, "devices", [])), 1)
+        btrfs_partitions = Ops.get_list(tg, ["/dev/btrfs", "partitions"], [])
+        simple = Builtins.filter(btrfs_partitions) do |p|
+          p["devices"].nil? || p["devices"].size <= 1
+        end
+        tg["/dev/btrfs"]["partitions"] = Builtins.filter(btrfs_partitions) do |p|
+          p["devices"] &&  p["devices"].size > 1
         end
-        Ops.set(
-          tg,
-          ["/dev/btrfs", "partitions"],
-          Builtins.filter(Ops.get_list(tg, ["/dev/btrfs", "partitions"], [])) do |p|
-            Ops.greater_than(Builtins.size(Ops.get_list(p, "devices", [])), 1)
-          end
-        )
         Builtins.y2milestone("HandleBtrfsSimpleVolumes simple\n%1", format_target_map(simple))
         keys = [
           "subvol",
@@ -1946,10 +1941,11 @@ def HandleBtrfsSimpleVolumes(tg)
           "mount",
           "mountby",
           "used_fs",
+          "fstopt",
           "userdata"
         ]
         Builtins.foreach(simple) do |p|
-          mp = GetPartition(tg, Ops.get_string(p, "device", ""))
+          mp = GetPartition(tg, p["device"])
           Builtins.y2milestone("HandleBtrfsSimpleVolumes before %1", mp)
           Builtins.foreach(keys) do |k|
             if Ops.get(p, k) != nil