Merge pull request #235 from yast/set-default-subvolume-name

Set default subvolume name
yast · Nov 3, 2016 · a29db9b · a29db9b
2 parents de6cd79 + f1b7098
commit a29db9b
Show file tree

Hide file tree

Showing 14 changed files with 643 additions and 12 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -5,7 +5,7 @@ before_install:
     # disable rvm, use system Ruby
     - rvm reset
     - wget https://raw.githubusercontent.com/yast/yast-devtools/master/travis-tools/travis_setup.sh
-    - sh ./travis_setup.sh -p "rake yast2-core-dev yast2 yast2-devtools doxygen yast2-testsuite libstorage5-dev libstorage5-ruby ruby-dbus" -g "rspec:3.3.0 yast-rake gettext"
+    - sh ./travis_setup.sh -p "rake yast2-core-dev yast2 yast2-devtools doxygen yast2-testsuite libstorage5-dev libstorage5-ruby ruby-dbus" -g "rspec:3.3.0 yast-rake gettext cheetah abstract_method"
 script:
     - rake check:syntax
     - rake check:pot

diff --git a/package/yast2-storage.changes b/package/yast2-storage.changes
@@ -1,3 +1,10 @@
+-------------------------------------------------------------------
+Mon Oct 31 13:11:04 UTC 2016 - igonzalezsosa@suse.com
+
+- Improve support to redefine/detect the Btrfs default subvolume
+  name (FATE#320342 and FATE#317775) using AutoYaST.
+- 3.1.103.2
+
 -------------------------------------------------------------------
 Thu Oct 27 14:26:49 CEST 2016 - shundhammer@suse.de
 

diff --git a/package/yast2-storage.spec b/package/yast2-storage.spec
@@ -17,7 +17,7 @@
 
 
 Name:           yast2-storage
-Version:        3.1.103.1
+Version:        3.1.103.2
 Release:        0
 
 BuildRoot:      %{_tmppath}/%{name}-%{version}-build

diff --git a/src/modules/FileSystems.rb b/src/modules/FileSystems.rb
@@ -31,9 +31,18 @@
 # $Id$
 require "storage"
 require "yast"
+require "yast2/execute"
 
 module Yast
   class FileSystemsClass < Module
+    include Yast::Logger
+
+    # @return [Array<String>] Supported default subvolume names
+    SUPPORTED_DEFAULT_SUBVOLUME_NAMES = ["", "@"].freeze
+
+    # @return [String] Default subvolume name.
+    attr_reader :default_subvol
+
     def main
 
       textdomain "storage"
@@ -44,6 +53,7 @@ def main
       Yast.import "Encoding"
       Yast.import "Stage"
       Yast.import "StorageInit"
+      Yast.import "ProductFeatures"
 
       @conv_fs = {
         "def_sym" => :unknown,
@@ -74,7 +84,8 @@ def main
       @possible_root_fs = [:ext2, :ext3, :ext4, :btrfs, :reiser, :xfs]
       @swap_m_points = ["swap"]
       @tmp_m_points = ["/tmp", "/var/tmp"]
-      @default_subvol = "UNDEFINED"
+      self.default_subvol = ProductFeatures.GetStringFeature("partitioning", "btrfs_default_subvolume")
+
 
       @suggest_m_points = []
       @suggest_tmp_points = []
@@ -1439,7 +1450,7 @@ def FileSystems
     def InitSlib(value)
       @sint = value
       if @sint != nil
-        @default_subvol = @sint.getDefaultSubvolName()
+        @sint.setDefaultSubvolName(@default_subvol)
         Builtins.y2milestone(
           "InitSlib used default_subvol:\"%1\"",
           @default_subvol
@@ -2030,14 +2041,86 @@ def AddQuotaOpts(part, fst_opts)
       ret
     end
 
+    # Set the default subvolume name
+    #
+    # @param [String] Default subvolume name. Only "" and "@" are supported.
+    # @return [Boolean] True if subvolume was changed; false otherwise.
+    def default_subvol=(name)
+      if SUPPORTED_DEFAULT_SUBVOLUME_NAMES.include?(name)
+        @default_subvol = name
+        @sint.setDefaultSubvolName(name) unless @sint.nil?
+        true
+      else
+        log.warn "Unsupported default subvolume name='#{name}'. Ignoring."
+        false
+      end
+    end
+
+    # Try to find the default subvolume name in the target system
+    #
+    # * Root partition takes precedence
+    # * Not supported: more than 1 Btrfs filesystems, one using
+    #   a '@' default subvolume and the other using ''. In that case,
+    #   default_subvolume is set to product's default.
+    #
+    # @return [String,nil] Default subvolume from the target system
+    def default_subvol_from_target
+      Yast.import "Storage"
+      parts = Storage.GetTargetMap.map { |_k, d| d.fetch("partitions")  }.flatten.compact
+      btrfs_parts = parts.select { |p| p["used_fs"] == :btrfs }
+      default_subvol_names = btrfs_parts.reduce({}) do |memo, part|
+        memo[part["mount"]] = btrfs_subvol_name_for(part["mount"]) unless part["mount"].nil?
+        memo
+      end
+
+      # Root takes precedence
+      return default_subvol_names["/"] if default_subvol_names.has_key?("/")
+
+      # If all has the same default subvolume name
+      found_names = default_subvol_names.values.uniq
+      return found_names.first if found_names.size == 1
+
+      # If there are different values, fallback to product's default
+      default_subvol_from_product
+    end
+
+    # Default subvol name from product
+    #
+    # @return [String] Default subvolume name
+    def default_subvol_from_product
+      ProductFeatures.GetStringFeature("partitioning", "btrfs_default_subvolume")
+    end
+
+    # Read the default subvolume from the filesystem and stores the value
+    #
+    # @return [String,nil] Default subvolume from the target system
+    # @see default_subvol_from_target
+    def read_default_subvol_from_target
+      self.default_subvol = default_subvol_from_target
+    end
+
+    protected
+
+    # Find the default subvolume name
+    #
+    # Only "" and "@" are supported.
+    #
+    # @param mount [String] Mount point.
+    # @return ["@", ""] Default subvolume name for the given mount point.
+    def btrfs_subvol_name_for(mount)
+      ret = Yast::Execute.on_target("btrfs", "subvol", "list", mount, stdout: :capture)
+      ret.split("\n").first =~ /.+ @\z/ ? "@" : ""
+    end
+
     publish :variable => :conv_fs, :type => "map <string, any>"
     publish :variable => :possible_root_fs, :type => "const list <symbol>"
     publish :function => :system_m_points, :type => "list <string> ()"
     publish :function => :crypt_m_points, :type => "list <string> ()"
     publish :variable => :swap_m_points, :type => "const list <string>"
     publish :variable => :tmp_m_points, :type => "const list <string>"
-    publish :variable => :default_subvol, :type => "string"
     publish :variable => :nchars, :type => "string"
+    publish :function => :default_subvol, :type => "string ()"
+    publish :function => :default_subvol=, :type => "string (string)"
     publish :function => :SuggestMPoints, :type => "list <string> ()"
     publish :function => :SuggestTmpfsMPoints, :type => "list <string> ()"
     publish :function => :GetGeneralFstabOptions, :type => "list <map <symbol, any>> ()"

diff --git a/src/modules/Storage.rb b/src/modules/Storage.rb
@@ -339,10 +339,6 @@ def InitLibstorage(readonly)
 
       StorageClients.InstallCallbacks(@sint)
 
-      btrfs_default_subvolume = ProductFeatures.GetStringFeature("partitioning",
-                                                                 "btrfs_default_subvolume")
-      @sint.setDefaultSubvolName(btrfs_default_subvolume) if btrfs_default_subvolume
-
       if Stage.initial
         @sint.setDetectMountedVolumes(false)
         @sint.setRootPrefix(Installation.destdir)
@@ -375,7 +371,7 @@ def FinishLibstorage
 
 
     def default_subvolume_name()
-      return @sint.getDefaultSubvolName()
+      FileSystems.default_subvol
     end
 
 

diff --git a/test/filesystems_test.rb b/test/filesystems_test.rb
@@ -0,0 +1,103 @@
+#!/usr/bin/env rspec
+
+require_relative "spec_helper"
+
+Yast.import "FileSystems"
+Yast.import "Storage"
+
+describe Yast::FileSystems do
+  subject { Yast::FileSystems }
+
+  def btrfs_list_fixture(name)
+    fixture_name = "btrfs_list_#{name}.out"
+    File.read(File.join(FIXTURES_PATH, fixture_name))
+  end
+
+  let(:default_subvol) { "@" }
+  let(:libstorage) do
+    double("libstorage", getDefaultSubvolName: default_subvol, setDefaultSubvolName: default_subvol)
+  end
+
+  before do
+    subject.InitSlib(libstorage)
+  end
+
+  describe "#default_subvol_from_target" do
+    before do
+      allow(Yast::Storage).to receive(:GetTargetMap).and_return(target_map)
+      allow(Yast::ProductFeatures).to receive(:GetStringFeature)
+        .with("partitioning", "btrfs_default_subvolume").and_return(default_subvol)
+    end
+
+    context "when root partition uses the default subvolume name (@)" do
+      let(:target_map) { YAML.load_file(File.join(FIXTURES_PATH, "subvolumes.yml")) }
+
+      before do
+        allow(Yast::Execute).to receive(:on_target).with("btrfs", "subvol", "list", "/", anything)
+          .and_return(btrfs_list_fixture("root"))
+        allow(Yast::Execute).to receive(:on_target).with("btrfs", "subvol", "list", "/srv", anything)
+          .and_return(btrfs_list_fixture("root_no_at"))
+      end
+
+      it "returns the default subvolume name" do
+        expect(subject.default_subvol_from_target).to eq(default_subvol)
+      end
+    end
+
+    context "when root partitions does not use btrfs" do
+      let(:target_map) { YAML.load_file(File.join(FIXTURES_PATH, "subvolumes-no-root.yml")) }
+
+      before do
+        allow(Yast::Execute).to receive(:on_target).with("btrfs", "subvol", "list", "/", anything)
+          .and_return(btrfs_list_fixture("root_no_at"))
+      end
+
+      context "but all btrfs partitions uses the same subvolume name" do
+        before do
+          allow(Yast::Execute).to receive(:on_target).with("btrfs", "subvol", "list", "/srv", anything)
+            .and_return(btrfs_list_fixture("srv_no_at"))
+          allow(Yast::Execute).to receive(:on_target).with("btrfs", "subvol", "list", "/data", anything)
+            .and_return(btrfs_list_fixture("data_no_at"))
+        end
+
+        it "returns the used name ('' in this case)" do
+          expect(subject.default_subvol_from_target).to eq("")
+        end
+      end
+
+      context "and btrfs partitions uses different subvolume names" do
+        before do
+          allow(Yast::Execute).to receive(:on_target).with("btrfs", "subvol", "list", "/srv", anything)
+            .and_return(btrfs_list_fixture("srv"))
+          allow(Yast::Execute).to receive(:on_target).with("btrfs", "subvol", "list", "/data", anything)
+            .and_return(btrfs_list_fixture("data_no_at"))
+        end
+
+        it "returns the distribution default" do
+          expect(subject.default_subvol_from_target).to eq(default_subvol)
+        end
+      end
+    end
+  end
+
+  describe "#read_default_subvol_from_target" do
+    it "sets the default_subvol using the value from target" do
+      subject.default_subvol = ""
+      expect(subject).to receive(:default_subvol_from_target).and_return("@")
+      expect { subject.read_default_subvol_from_target }.to change { subject.default_subvol }
+        .from("").to("@")
+    end
+  end
+
+  describe "#default_subvol=" do
+    it "sets the default_subvol if a valid value is given" do
+      expect(libstorage).to receive(:setDefaultSubvolName).with("@")
+      subject.default_subvol = "@"
+    end
+
+    it "refuses to set default_subvol if an invalid value is given" do
+      expect(libstorage).to_not receive(:setDefaultSubvolName)
+      subject.default_subvol = "UNDEFINED"
+    end
+  end
+end
diff --git a/test/fixtures/btrfs_list_data_no_at.out b/test/fixtures/btrfs_list_data_no_at.out
@@ -0,0 +1,3 @@
+ID 257 gen 16 top level 5 path shared
+ID 258 gen 16 top level 5 path private
+
diff --git a/test/fixtures/btrfs_list_root.out b/test/fixtures/btrfs_list_root.out
@@ -0,0 +1,28 @@
+ID 257 gen 34 top level 5 path @
+ID 258 gen 51 top level 257 path @/.snapshots
+ID 259 gen 88 top level 258 path @/.snapshots/1/snapshot
+ID 260 gen 44 top level 257 path @/boot/grub2/i386-pc
+ID 261 gen 15 top level 257 path @/boot/grub2/x86_64-efi
+ID 262 gen 44 top level 257 path @/home
+ID 263 gen 51 top level 257 path @/opt
+ID 264 gen 50 top level 257 path @/srv
+ID 265 gen 88 top level 257 path @/tmp
+ID 266 gen 64 top level 257 path @/usr/local
+ID 267 gen 38 top level 257 path @/var/crash
+ID 268 gen 22 top level 257 path @/var/lib/mailman
+ID 269 gen 88 top level 257 path @/var/lib/named
+ID 270 gen 24 top level 257 path @/var/lib/pgsql
+ID 271 gen 88 top level 257 path @/var/log
+ID 272 gen 38 top level 257 path @/var/opt
+ID 273 gen 84 top level 257 path @/var/spool
+ID 274 gen 51 top level 257 path @/var/tmp
+ID 275 gen 30 top level 257 path @/mysubvol
+ID 276 gen 47 top level 275 path @/mysubvol/mysubsubvol
+ID 277 gen 32 top level 257 path @/myothersubvol
+ID 278 gen 47 top level 277 path @/myothersubvol/myothersubsubvol
+ID 279 gen 33 top level 257 path @/nocow
+ID 280 gen 35 top level 257 path @/myvol
+ID 282 gen 40 top level 259 path var/lib/machines
+ID 283 gen 48 top level 258 path @/.snapshots/2/snapshot
+ID 284 gen 49 top level 258 path @/.snapshots/3/snapshot
+ID 285 gen 50 top level 258 path @/.snapshots/4/snapshot
diff --git a/test/fixtures/btrfs_list_root_no_at.out b/test/fixtures/btrfs_list_root_no_at.out
@@ -0,0 +1,27 @@
+ID 257 gen 48 top level 5 path .snapshots
+ID 258 gen 92 top level 257 path .snapshots/1/snapshot
+ID 259 gen 41 top level 5 path boot/grub2/i386-pc
+ID 260 gen 12 top level 5 path boot/grub2/x86_64-efi
+ID 261 gen 41 top level 5 path home
+ID 262 gen 49 top level 5 path opt
+ID 263 gen 47 top level 5 path srv
+ID 264 gen 91 top level 5 path tmp
+ID 265 gen 61 top level 5 path usr/local
+ID 266 gen 35 top level 5 path var/crash
+ID 267 gen 19 top level 5 path var/lib/mailman
+ID 268 gen 89 top level 5 path var/lib/named
+ID 269 gen 21 top level 5 path var/lib/pgsql
+ID 270 gen 90 top level 5 path var/log
+ID 271 gen 35 top level 5 path var/opt
+ID 272 gen 91 top level 5 path var/spool
+ID 273 gen 48 top level 5 path var/tmp
+ID 274 gen 27 top level 5 path mysubvol
+ID 275 gen 44 top level 274 path mysubvol/mysubsubvol
+ID 276 gen 29 top level 5 path @/myothersubvol
+ID 277 gen 44 top level 276 path @/myothersubvol/myothersubsubvol
+ID 278 gen 30 top level 5 path nocow
+ID 279 gen 32 top level 5 path myvol
+ID 281 gen 37 top level 258 path var/lib/machines
+ID 282 gen 45 top level 257 path .snapshots/2/snapshot
+ID 283 gen 46 top level 257 path .snapshots/3/snapshot
+ID 284 gen 47 top level 257 path .snapshots/4/snapshot
diff --git a/test/fixtures/btrfs_list_srv.out b/test/fixtures/btrfs_list_srv.out
@@ -0,0 +1,5 @@
+ID 257 gen 16 top level 5 path @
+ID 258 gen 16 top level 257 path www
+ID 259 gen 16 top level 257 path mirror
+ID 260 gen 16 top level 257 path ftp
+ID 261 gen 16 top level 257 path tftpboot
diff --git a/test/fixtures/btrfs_list_srv_no_at.out b/test/fixtures/btrfs_list_srv_no_at.out
@@ -0,0 +1,5 @@
+ID 257 gen 16 top level 5 path www
+ID 258 gen 16 top level 5 path mirror
+ID 259 gen 16 top level 5 path ftp
+ID 260 gen 16 top level 5 path tftpboot
+