Merge ef28a60 into 9687bff

package/yast2-storage-ng.changes

@@ -1,3 +1,12 @@
+-------------------------------------------------------------------
+Wed Oct  2 13:35:42 UTC 2019 - David Diaz <dgonzalez@suse.com>
+
+- Partitioner: fix the type column value for Ext3/4 filesystems
+  with an external journal (bsc#1145841).
+- Do not crash when importing mount points from a multi-device
+  filesystem.
+- 4.2.45
+
 -------------------------------------------------------------------
 Wed Oct  2 08:46:10 UTC 2019 - Ancor Gonzalez Sosa <ancor@suse.com>
 

package/yast2-storage-ng.spec

@@ -16,7 +16,7 @@
 #
 
 Name:           yast2-storage-ng
-Version:        4.2.44
+Version:        4.2.45
 Release:        0
 Summary:        YaST2 - Storage Configuration
 License:        GPL-2.0-only OR GPL-3.0-only

src/lib/y2partitioner/actions/controllers/fstabs.rb

@@ -391,24 +391,37 @@ def setup_blk_filesystem(filesystem)
           filesystem.setup_default_btrfs_subvolumes if filesystem.supports_btrfs_subvolumes?
         end
 
-        # Adds missing devices to the filesystem when the original filesystem was a multi-device Btrfs
+        # Adds missing devices to the filesystem when the original filesystem was multidevice
         #
         # @param filesystem [Y2Storage::Filesystems::BlkFilesystem]
         def add_filesystem_devices(filesystem)
           original_filesystem = original_filesystem(filesystem)
 
-          return unless original_filesystem&.multidevice?
+          return unless add_devices?(filesystem)
 
           devices = original_filesystem.blk_devices.map { |d| current_graph.find_device(d.sid) }.compact
 
           devices.each { |d| add_filesystem_device(filesystem, d) }
         end
 
+        # Whether more devices should be added to a multi-device filesystem
+        #
+        # @param filesystem [Y2Storage::Filesystems::BlkFilesystem]
+        def add_devices?(filesystem)
+          original_filesystem = original_filesystem(filesystem)
+
+          return false unless original_filesystem&.multidevice?
+
+          # FIXME: check the fstab entry UUID instead
+          filesystem.type == original_filesystem&.type
+        end
+
         # Adds a device to the filesystem
         #
         # @param filesystem [Y2Storage::Filesystems::BlkFilesystem]
         # @param device [Y2Storage::BlkDevice]
         def add_filesystem_device(filesystem, device)
+          return unless filesystem.respond_to?(:add_device)
           return if filesystem.blk_devices.map(&:sid).include?(device.sid)
 
           filesystem.add_device(device)

src/lib/y2partitioner/actions/edit_blk_device.rb

@@ -111,7 +111,8 @@ def run?
       # @return [Array<Strings>]
       def errors
         [
-          btrfs_error,
+          multidevice_btrfs_error,
+          multidevice_fs_error,
           used_device_error,
           partitions_error,
           extended_partition_error,
@@ -142,9 +143,10 @@ def used_device_error
       # Error when trying to edit a device that is part of a multi-device Btrfs
       #
       # @return [String, nil] nil if the device is not part of a Btrfs
-      def btrfs_error
+      def multidevice_btrfs_error
         fs = device.filesystem
-        return nil unless fs&.multidevice?
+
+        return nil unless fs&.multidevice? && fs&.type&.is?(:btrfs)
 
         format(
           # TRANSLATORS: %{name} is replaced by a device name (e.g., /dev/sda1).
@@ -161,6 +163,23 @@ def btrfs_error
         )
       end
 
+      # Error when trying to edit a device that is part of a multidevice filesystem
+      #
+      # @return [String, nil] nil if the device is not part of a multidevice filesystem
+      def multidevice_fs_error
+        fs = device.filesystem
+        return nil unless fs&.multidevice?
+
+        format(
+          # TRANSLATORS: %{name} is replaced by a device name (e.g., /dev/sda1).
+          # Since device names can be rather long, make sure the lines
+          # containing %{name} are sorter than the others.
+          _("The device %{name} belongs to a multi-device filesystem.\n" \
+            "It cannot be edited."),
+          name: device.name
+        )
+      end
+
       # Error when trying to edit a device that contains partitions
       #
       # @return [String, nil] nil if the device has no partitions

src/lib/y2partitioner/filesystem_errors.rb

@@ -92,7 +92,7 @@ def installing?
     def small_size_for_snapshots?(filesystem, new_size: nil)
       return false unless filesystem && filesystem_with_snapshots?(filesystem)
 
-      # TODO: check size for multidevice Btrfs
+      # TODO: check size for multi-device filesystem
       return false if filesystem.multidevice?
 
       size = new_size || filesystem.blk_devices.first.size

src/lib/y2partitioner/widgets/blk_devices_table.rb

@@ -148,7 +148,7 @@ def device_value(device)
         if device.multidevice?
           format(
             # TRANSLATORS: fs_type is the filesystem type. I.e., BtrFS
-            #              device_name is the base name of the block device. I.e., sda or sda1+
+            #              device_name is the base name of the block device. I.e., sda or sda1...
             _("%{fs_type} %{device_name}"),
             fs_type:     device.type.to_human_string,
             device_name: device.blk_device_basename
@@ -271,7 +271,7 @@ def type_icon(device)
         partition:     N_("Partition")
       }
 
-      # Label for device and filesystem types (e.g., PV of vg1, Ext4 RAID, Part of Btrfs sda1+, etc)
+      # Label for device and filesystem types (e.g., PV of vg1, Ext4 RAID, Part of Btrfs sda1..., etc)
       #
       # @param device [Y2Storage::Device]
       # @return [String]
@@ -281,15 +281,33 @@ def type_label(device)
 
         fs = filesystem(device)
 
-        if fs&.multidevice?
-          btrfs_multidevice_type_label(fs)
+        if device.journal?
+          journal_type_label(fs)
+        elsif show_multidevice_type_label?(fs)
+          multidevice_type_label(fs)
         elsif fs
           formatted_device_type_label(device, fs)
         else
           unformatted_device_type_label(device)
         end
       end
 
+      # Whether the "Part of *fs.type*" label should be displayed
+      #
+      # The Ext3/4 filesystem could be detected as a multi-device filesystem
+      # when its journal is placed in an external device. However, we do not
+      # want to display "Part of ..." for them because we know that data
+      # partition is over a single device.
+      #
+      # @param filesystem [Y2Storage::Filesystems::Base]
+      # @return [Boolean] true if the filesystem is multi-device BUT not an Ext3/4 one
+      def show_multidevice_type_label?(filesystem)
+        return false unless filesystem
+        return false if filesystem.type.is?(:ext3, :ext4)
+
+        filesystem.multidevice?
+      end
+
       # Label for formatted device (e.g., Ext4 LVM, XFS RAID, Swap Partition, etc)
       #
       # @param device [Y2Storage::BlkDevice]
@@ -350,13 +368,34 @@ def lvm_pv_type_label(device)
         format(_("PV of %s"), vg.basename)
       end
 
-      # Type label when the device belongs to a multidevice Btrfs filesystem
+      # Type label when the device holds a journal
+      #
+      # @param filesystem [Y2Storage::Filesystems::Base]
+      # @return [String]
+      def journal_type_label(filesystem)
+        data_device = filesystem.blk_devices.find { |d| !d.journal? }
+
+        # TRANSLATORS: %{fs_name} is the filesystem name. E.g., Btrfs, Ext4, etc.
+        #              %{data_device_name} is the data device name. E.g., sda1
+        format(
+          _("%{fs_type} Journal (%{data_device_name})"),
+          fs_type:          filesystem.type.to_human_string,
+          data_device_name: data_device.basename
+        )
+      end
+
+      # Type label when the device belongs to a multidevice filesystem
       #
       # @param filesystem [Y2Storage::Filesystems::Base]
       # @return [String]
-      def btrfs_multidevice_type_label(filesystem)
-        # TRANSLATORS: %s is a device base name. E.g., sda1+
-        format(_("Part of Btrfs %s"), filesystem.blk_device_basename)
+      def multidevice_type_label(filesystem)
+        # TRANSLATORS: %{fs_name} is the filesystem name. E.g., Btrfs, Ext4, etc.
+        #              %{blk_device_name} is a device base name. E.g., sda1...
+        format(
+          _("Part of %{fs_name} %{blk_device_name}"),
+          fs_name:         filesystem.type,
+          blk_device_name: filesystem.blk_device_basename
+        )
       end
 
       # Type label when the device is used as caching device in Bcache

src/lib/y2partitioner/widgets/description_section/filesystem.rb

@@ -45,7 +45,7 @@ def title
 
         # @see DescriptionSection::Base#entries
         def entries
-          [:fs_type, :mount_point, :mount_by, :label, :uuid] + btrfs_entries
+          [:fs_type, :mount_point, :mount_by, :label, :uuid] + ext_entries + btrfs_entries
         end
 
         # Extra entries when the filesystem is Btrfs
@@ -57,6 +57,15 @@ def btrfs_entries
           [:btrfs_data_raid_level, :btrfs_metadata_raid_level]
         end
 
+        # Extra entries when the filesystem is Ext
+        #
+        # @return [Array<Symbol>]
+        def ext_entries
+          return [] unless filesystem&.type&.is?(:ext3, :ext4)
+
+          [:journal]
+        end
+
         # Information about the filesystem type
         #
         # @return [String]
@@ -150,6 +159,13 @@ def mount_by
 
           filesystem.mount_point.mount_by.to_human_string
         end
+
+        # Information about journal
+        #
+        # @return [String]
+        def journal_value
+          format(_("Journal Device: %s"), filesystem&.journal_device&.name)
+        end
       end
     end
   end

src/lib/y2partitioner/widgets/help.rb

@@ -4,6 +4,7 @@
 
 module Y2Partitioner
   module Widgets
+    # rubocop:disable Metrics/ModuleLength
     # Helper methods for generating widget helps.
     module Help
       extend Yast::I18n
@@ -123,8 +124,12 @@ def included(_target)
                                       "Currently there are four supported modes: Writethrough, " \
                                       "Writeback, Writearound and None."),
 
+        journal:                   N_("<b>Journal Device</b> shows the device holding the " \
+                                      "external journal."),
+
         btrfs_devices:             N_("<b>Devices</b> shows the kernel name of the devices used by a "\
                                       "Btrfs file system."),
+
         btrfs_metadata_raid_level: N_("<b>Metadata RAID Level</b> shows the RAID level for the Btrfs " \
                                       "metadata."),
 
@@ -159,5 +164,6 @@ def helptext_for(field)
         ret << "</p>"
       end
     end
+    # rubocop:enable Metrics/ModuleLength
   end
 end

src/lib/y2storage/blk_device.rb

@@ -600,6 +600,19 @@ def recoverable_size
       size - resize_info.min_size
     end
 
+    # Whether the block device is being used to hold a journal
+    #
+    # @return [Boolean]
+    def journal?
+      return false unless filesystem
+
+      to_storage_value.out_holders.to_a.any? do |holder|
+        return false unless Storage.filesystem_user?(holder)
+
+        Storage.to_filesystem_user(holder).journal?
+      end
+    end
+
     protected
 
     # Values for volume specification matching

src/lib/y2storage/filesystems/blk_filesystem.rb

@@ -134,13 +134,27 @@ def blk_device_basename
         format(_("(%{basename}\u2026)"), basename: basename)
       end
 
-      # Whether it is a multidevice filesystem
+      # Whether it is a multi-device filesystem
+      #
+      # So far, filesystems detected as multi-device are
+      #
+      #   - Btrfs, when built over several devices
+      #   - Ext3/4, if the journal is placed in an external device
       #
       # @return [Boolean]
       def multidevice?
         blk_devices.size > 1
       end
 
+      # Returns the device used to hold the journal
+      #
+      # Mainly useful for Ext3/4 filesystems with an external journal
+      #
+      # @return [BlkDevice, nil] nil if there is not a device used for the journal
+      def journal_device
+        blk_devices.find(&:journal?)
+      end
+
       # Checks whether the filesystem has the capability of hosting Btrfs subvolumes
       #
       # It only should be true for Btrfs.

src/lib/y2storage/simple_etc_fstab_entry.rb

@@ -74,6 +74,9 @@ def filesystem(devicegraph)
     # @note When the entry refers to an encryption device, it returs the encryption device
     # and not the underlying device.
     #
+    # FIXME: in case the device isn't indicated by UUID or label, ensure the returned device is the
+    # device indicated in the fstab entry
+    #
     # @param devicegraph [Devicegraph]
     # @return [BlkDevice, Filesystems::Nfs, nil]
     def device(devicegraph)
@@ -82,7 +85,7 @@ def device(devicegraph)
       if !filesystem
         find_device(devicegraph)
       elsif filesystem.respond_to?(:blk_devices)
-        filesystem.blk_devices.first
+        filesystem.blk_devices.min_by(&:name)
       else
         filesystem
       end