Merge pull request #115 from joseivanlopez/fix/encrypted_partitions

Fix upgrade with encrypted partitions

package/yast2-update.changes

@@ -1,3 +1,13 @@
+-------------------------------------------------------------------
+Mon Nov  5 17:10:54 UTC 2018 - jlopez@suse.com
+
+- Avoid to crash when crypttab contains entries where the LUKS is
+  given by UUID= format.
+- Avoid to crash when an encryption device is not probed with the
+  name indicated in a fstab entry.
+- Related to bsc#1094963.
+- 4.1.5
+
 -------------------------------------------------------------------
 Tue Oct 16 16:37:07 CEST 2018 - schubi@suse.de
 

package/yast2-update.spec

@@ -17,7 +17,7 @@
 
 
 Name:           yast2-update
-Version:        4.1.4
+Version:        4.1.5
 Release:        0
 
 BuildRoot:      %{_tmppath}/%{name}-%{version}-build
@@ -41,10 +41,10 @@ BuildRequires:	yast2-installation-control
 # Needed for tests
 BuildRequires:  rubygem(rspec)
 
-# Encryption.use_crypttab_names
-BuildRequires:	yast2-storage-ng >= 4.0.186
-# Encryption.use_crypttab_names
-Requires:	yast2-storage-ng >= 4.0.186
+# Encryption.save_crypttab_names
+BuildRequires:	yast2-storage-ng >= 4.1.31
+# Encryption.save_crypttab_names
+Requires:	yast2-storage-ng >= 4.1.31
 # FSSnapshotStore
 Requires:	yast2 >= 3.1.126
 Requires:	yast2-installation

src/modules/RootPart.rb

@@ -1121,7 +1121,18 @@ def MountFSTab(fstab, message)
 
             mount_err = ""
             while mount_err != nil
-              mount_err = FsckAndMount(fspath, spec, mount_type, mntops)
+              # An encryption device might be probed with a name that does not match with the name
+              # indicated in the fstab file. For example, when the fstab entry is:
+              #
+              #   /dev/mapper/cr_home   /home   ext4  defaults  0   0
+              #
+              # and that encryption device was probed as /dev/mapper/cr-auto-1.
+              #
+              # In that case, to mount /dev/mapper/cr_home would fail because there is not a device
+              # in the inst-sys with such name. To avoid possible failures when mounting the fstab
+              # device, the safest device name is used instead, that is, UUID= format or its uuid
+              # udev name, see {#safest_device_name}.
+              mount_err = FsckAndMount(fspath, safest_device_name(spec), mount_type, mntops)
               if mount_err != nil
                 Builtins.y2error(
                   "mounting %1 (type %2) on %3 failed",
@@ -1531,9 +1542,23 @@ def MountPartitions(root_device_current)
         fstab = fstab_ref.value
         crtab = crtab_ref.value
 
-        # Update encryption devices with the names indicated in the crypttab file (bsc#1094963)
+        # Encryption names indicated in the crypttab file are stored in its correspondig encryption
+        # device to make possible to find a device by using the name specified in a fstab file,
+        # (bsc#1094963).
+        #
+        # For example, when fstab has:
+        #
+        #   /dev/disk/by-id/dm-name-cr_home / auto 0 0
+        #
+        # and the fstab device is searched by that name:
+        #
+        #   devicegraph.find_by_any_name("/dev/disk/by-id/dm-name-cr_home")
+        #
+        # The proper encryption device could be found if there is a encrypttion device where
+        #
+        #   encryption.crypttab_name  #=> "cr_home"
         crypttab_path = File.join(Installation.destdir, "/etc/crypttab")
-        Y2Storage::Encryption.use_crypttab_names(probed, crypttab_path)
+        Y2Storage::Encryption.save_crypttab_names(probed, crypttab_path)
 
         Update.GetProductName
 
@@ -2246,6 +2271,39 @@ def fs_by_udev_lookup(devicegraph, name)
       dev.filesystem
     end
 
+    # Safest device name to perform the mount action
+    #
+    # It will be the udev uuid name (e.g., /dev/disk/by-uuid/111-222-333) when the device
+    # spec has not UUID= format.
+    #
+    # @see udev_uuid
+    #
+    # @example
+    #
+    #   safest_device_name("UUID=111-222-333")    #=> "UUID=111-222-333"
+    #   safest_device_name("/dev/mapper/cr_home") #=> "/dev/disk/by-uuid/111-222-333"
+    #
+    # @param device_spec [String] e.g., "UUID=111-222-333", "/dev/sda2", "/dev/mapper/cr_home"
+    # @return [String] safest device name, e.g., "/dev/disk/by-uuid/111-222-333"
+    def safest_device_name(device_spec)
+      return device_spec if device_spec.start_with?("UUID=")
+
+      udev_uuid(device_spec) || device_spec
+    end
+
+    # Finds a device and returns its udev uuid name
+    #
+    # @param device_spec [String] e.g., "UUID=111-222-333", "/dev/sda2", "/dev/mapper/cr_home"
+    # @return [String, nil] uuid name (e.g., "/dev/disk/by-uuid/111-222-333") or nil if the
+    #   device is not found.
+    def udev_uuid(device_spec)
+      filesystem = fs_by_devicename(probed, device_spec)
+      return nil if filesystem.nil?
+
+      device = filesystem.blk_devices.first
+      device.udev_full_uuid
+    end
+
     # Whether the given fstab spec corresponds to a device mounted by its kernel
     # device name.
     #

test/root_part_test.rb

@@ -236,6 +236,25 @@
   end
 
   describe "#MountFSTab" do
+    before do
+      stub_storage(scenario)
+      # Mock the system lookup executed as last resort when the devicegraph
+      # doesn't contain the searched information
+      allow(Y2Storage::BlkDevice).to receive(:find_by_any_name)
+    end
+
+    let(:scenario) { "two-disks-two-btrfs.xml" }
+
+    let(:fstab) do
+      [
+        {
+          "file"=>"/home", "mntops"=>"defaults", "vfstype"=>"ext4", "spec"=> device_spec
+        }
+      ]
+    end
+
+    let(:device_spec) { nil }
+
     it "mounts /dev, /proc and /sys" do
       allow(subject).to receive(:AddMountedPartition)
 
@@ -247,6 +266,41 @@
       fstab = []
       subject.MountFSTab(fstab, "")
     end
+
+    context "when the device spec has UUID= format" do
+      let(:device_spec) { "UUID=111-222-333" }
+
+      it "tries to mount by using UUID= spec" do
+        expect(subject).to receive(:FsckAndMount)
+          .with("/home", "UUID=111-222-333", anything, anything)
+
+        subject.MountFSTab(fstab, "")
+      end
+    end
+
+    context "when the device spec does not have UUID= format" do
+      context "and a device with such spec is not found" do
+        let(:device_spec) { "/dev/sdc1" }
+
+        it "tries to mount by using the given device spec" do
+          expect(subject).to receive(:FsckAndMount)
+            .with("/home", "/dev/sdc1", anything, anything)
+
+          subject.MountFSTab(fstab, "")
+        end
+      end
+
+      context "and a device with such spec is found" do
+        let(:device_spec) { "/dev/sda2" }
+
+        it "tries to mount by using its udev uuid name" do
+          expect(subject).to receive(:FsckAndMount)
+            .with("/home", "/dev/disk/by-uuid/d6e5c710-3067-48de-8363-433e54a9d0b5", anything, anything)
+
+          subject.MountFSTab(fstab, "")
+        end
+      end
+    end
   end
 
   describe "#inject_intsys_files" do