Merge pull request #911 from yast/feature/use-new-type-column

Unify the "Type" and "FS Type" columns

package/yast2-storage-ng.changes

@@ -1,3 +1,11 @@
+-------------------------------------------------------------------
+Tue Apr 30 12:30:12 UTC 2019 - David Diaz <dgonzalez@suse.com>
+
+- Partitioner: unify the "Type" and "FS Type" columns.
+- Partitioner: show multidevice filesystems in the system section.
+- Part of jsd#SLE-3877.
+- 4.2.10
+
 -------------------------------------------------------------------
 Tue Apr 30 07:46:10 UTC 2019 - José Iván López González <jlopez@suse.com>
 

package/yast2-storage-ng.spec

@@ -16,7 +16,7 @@
 #
 
 Name:		yast2-storage-ng
-Version:	4.2.9
+Version:	4.2.10
 Release:	0
 
 BuildRoot:	%{_tmppath}/%{name}-%{version}-build

src/lib/y2partitioner/widgets/blk_devices_table.rb

@@ -121,11 +121,6 @@ def type_title
         _("Type")
       end
 
-      def filesystem_type_title
-        # TRANSLATORS: table header, file system type
-        _("FS Type")
-      end
-
       def filesystem_label_title
         # TRANSLATORS: table header, disk or partition label. Can be empty.
         _("Label")
@@ -149,7 +144,11 @@ def end_title
       # Values
 
       def device_value(device)
-        device.name
+        if device.is?(:blk_filesystem)
+          device.type.to_human_string
+        else
+          device.name
+        end
       end
 
       def size_value(device)
@@ -183,14 +182,6 @@ def type_value(device)
         cell(icon, label)
       end
 
-      def filesystem_type_value(device)
-        fs = filesystem(device)
-        return "" if fs.nil?
-
-        type = fs.type
-        type.nil? ? "" : type.to_human
-      end
-
       def filesystem_label_value(device)
         fs = filesystem(device)
         # fs may be nil or a file system not supporting labels, like NFS
@@ -254,75 +245,146 @@ def type_icon(device)
         multipath:     N_("Multipath"),
         nfs:           N_("NFS"),
         bios_raid:     N_("BIOS RAID"),
-        software_raid: N_("MD RAID"),
+        software_raid: N_("RAID"),
         lvm_pv:        N_("PV"),
         lvm_vg:        N_("LVM"),
         lvm_lv:        N_("LV"),
         stray:         N_("Xen"),
         thin_pool:     N_("Thin Pool"),
-        thin:          N_("Thin LV")
+        thin:          N_("Thin LV"),
+        partition:     N_("Partition")
       }
 
-      # Label for the device type (e.g., LVM, MD RAID)
+      # Label for device and filesystem types (e.g., PV of vg1, Ext4 RAID, Part of Btrfs sda1+, etc)
       #
-      # @param device [Y2Storage::BlkDevice]
+      # @param device [Y2Storage::Device]
       # @return [String]
       def type_label(device)
-        return default_type_label(device) unless device.is_a?(Y2Storage::BlkDevice)
+        return device.type.to_human_string if device.is?(:filesystem)
+        return device_label_for(device) if device.is?(:lvm_vg)
 
-        if device.is?(:partition)
-          partition_type_label(device)
-        elsif device.is?(:lvm_lv)
-          lvm_lv_type_label(device)
+        fs = filesystem(device)
+
+        if fs && fs.multidevice?
+          btrfs_multidevice_type_label(fs)
+        elsif fs
+          formatted_device_type_label(device, fs)
         else
-          blk_device_type_label(device)
+          unformatted_device_type_label(device)
         end
       end
 
-      # Default type label for the device
+      # Label for formatted device (e.g., Ext4 LVM, XFS RAID, Swap Partition, etc)
       #
-      # @see DEVICE_LABELS
+      # @param device [Y2Storage::BlkDevice]
+      # @param fs [Y2Storage::Filesystems::Base]
+      # @return [String]
+      def formatted_device_type_label(device, fs)
+        # TRANSLATORS: %{fs_type} is the filesystem type. I.e., FAT, Ext4, etc
+        #              %{device_label} is the device label. I.e., Partition, Disk, etc
+        format(
+          _("%{fs_type} %{device_label}"),
+          fs_type:      fs_type_for(device, fs),
+          device_label: device_label_for(device)
+        )
+      end
+
+      # Filesystem representation for given device and filesystem
       #
       # @param device [Y2Storage::BlkDevice]
+      # @param fs [Y2Storage::Filesystems::Base]
       # @return [String]
-      def default_type_label(device)
-        type = DEVICE_LABELS.keys.find { |k| device.is?(k) }
-        return "" if type.nil?
+      def fs_type_for(device, fs)
+        if device.is?(:partition) && device.efi_system?
+          device.id.to_human_string
+        else
+          fs.type.to_human_string
+        end
+      end
 
-        _(DEVICE_LABELS[type])
+      # Label for unformatted device (e.g., LVM, RAID, Partition, etc)
+      #
+      # @param device [Y2Storage::BlkDevice]
+      # @return [String]
+      def unformatted_device_type_label(device)
+        if device.lvm_pv
+          lvm_pv_type_label(device.lvm_pv)
+        elsif device.md
+          part_of_label(device.md)
+        elsif device.bcache
+          part_of_label(device.bcache)
+        elsif device.in_bcache_cset
+          bcache_cset_label
+        else
+          default_type_label(device)
+        end
       end
 
-      # Type label when the device is a partition
+      # Type label when the device is a LVM physical volume
       #
-      # @param device [Y2Storage::Partition]
+      # @param device [Y2Storage::LvmPv]
       # @return [String]
-      def partition_type_label(device)
-        device.id.to_human_string
+      def lvm_pv_type_label(device)
+        vg = device.lvm_vg
+
+        return _("Orphan PV") if vg.nil?
+
+        # TRANSLATORS: %s is the volume group name. E.g., "vg0"
+        format(_("PV of %s"), vg.basename)
       end
 
-      # Type label when the device is a LVM logical volume
+      # Type label when the device belongs to a multidevice Btrfs filesystem
       #
-      # @note Different label is shown depending on the logical volume type
-      #   (i.e., normal, thin pool or thin volume).
+      # @param fs [Y2Storage::Filesystems::Base]
+      # @return [String]
+      def btrfs_multidevice_type_label(fs)
+        # TRANSLATORS: %s is a device base name. E.g., sda1+
+        format(_("Part of Btrfs %s"), fs.blk_device_basename)
+      end
+
+      # Type label when the device is used as caching device in Bcache
       #
-      # @param device [Y2Storage::LvmLv]
       # @return [String]
-      def lvm_lv_type_label(device)
-        return blk_device_type_label(device) if device.lv_type.is?(:normal)
+      def bcache_cset_label
+        # TRANSLATORS: an special type of device
+        _("Bcache cache")
+      end
 
-        type = device.lv_type.to_sym
-        _(DEVICE_LABELS[type]) || ""
+      # Type label when the device is part of another one, like Bcache or RAID
+      #
+      # @param ancestor_device [Y2Storage::BlkDevice]
+      # @return [String]
+      def part_of_label(ancestor_device)
+        format(_("Part of %s"), ancestor_device.basename)
       end
 
-      # Type label when the device is not a partition
+      # Default type label for the device
+      #
+      # @see DEVICE_LABELS
       #
       # @param device [Y2Storage::BlkDevice]
       # @return [String]
-      def blk_device_type_label(device)
+      def default_type_label(device)
         data = [device.vendor, device.model].compact
-        # bcache has as vendor Disk, but we want to write bcache as type here
-        data = "" if device.is?(:bcache)
-        data.empty? ? default_type_label(device) : data.join("-")
+
+        return data.join("-") unless data.empty?
+        return device.id.to_human_string if device.respond_to?(:id)
+
+        device_label_for(device)
+      end
+
+      # Default device label based on its type
+      #
+      # @see DEVICE_LABELS
+      #
+      # @param device [Device]
+      # @return [String]
+      def device_label_for(device)
+        type = DEVICE_LABELS.keys.find { |k| device.is?(k) }
+
+        return "" if type.nil?
+
+        _(DEVICE_LABELS[type])
       end
     end
   end

src/lib/y2partitioner/widgets/configurable_blk_devices_table.rb

@@ -160,7 +160,6 @@ def help
         :format,
         :encrypted,
         :type,
-        :filesystem_type,
         :filesystem_label,
         :mount_point,
         :start,

src/lib/y2partitioner/widgets/fstab_selector.rb

@@ -244,7 +244,7 @@ def initialize(fstab)
         attr_reader :fstab
 
         def columns
-          [:device, :size, :type, :filesystem_type, :filesystem_label, :mount_point]
+          [:device, :size, :type, :filesystem_label, :mount_point]
         end
 
         # For each row, the device is a fstab entry

src/lib/y2partitioner/widgets/pages/system.rb

@@ -133,7 +133,7 @@ def import_mount_points_button
         #
         # @return [Array<Y2Storage::Device>]
         def devices
-          disk_devices + software_raids + lvm_vgs + nfs_devices + bcaches
+          disk_devices + software_raids + lvm_vgs + nfs_devices + bcaches + multidevice_filesystems
         end
 
         # @return [Array<Y2Storage::Device>]
@@ -182,6 +182,11 @@ def bcaches
           end
         end
 
+        # @return [Array<Y2Storage::Filesystems::Base>]
+        def multidevice_filesystems
+          device_graph.blk_filesystems.select(&:multidevice?)
+        end
+
         def device_graph
           DeviceGraphs.instance.current
         end

src/lib/y2storage/partition.rb

@@ -275,6 +275,13 @@ def suitable_for_windows?
       type.is?(:primary) && id.is?(:windows_system)
     end
 
+    # Whether it is a (valid) EFI System partition
+    #
+    # @return [Boolean]
+    def efi_system?
+      id.is?(:esp) && formatted_as?(:vfat)
+    end
+
   protected
 
     # Values for volume specification matching

test/data/devicegraphs/efi_not_mounted.yml

@@ -17,5 +17,3 @@
         file_system:  ext4
         label:        root
         mount_point:  "/"
-
-

test/y2partitioner/widgets/pages/system_test.rb

@@ -221,6 +221,17 @@ def row_names(table)
       end
     end
 
+    context "when there are multidevice filesystems" do
+      let(:scenario) { "btrfs2-devicegraph.xml" }
+      let(:multidevice_filesystems) { current_graph.btrfs_filesystems.select(&:multidevice?) }
+
+      it "contains all multidevice filesystems" do
+        expected_items = multidevice_filesystems.map(&:type).map(&:to_human_string)
+
+        expect(items).to include(*expected_items)
+      end
+    end
+
     describe "caching" do
       let(:scenario) { "empty_hard_disk_15GiB" }
       let(:pager) { Y2Partitioner::Widgets::OverviewTreePager.new("hostname") }

test/y2storage/partition_test.rb

@@ -583,4 +583,39 @@
       end
     end
   end
+
+  describe "#efi_system?" do
+    let(:scenario) { "efi_not_mounted" }
+
+    subject(:partition) { fake_devicegraph.find_by_name(device_name) }
+
+    context "when does have an EFI System id" do
+      let(:device_name) { "/dev/sda1" }
+
+      context "and it is formatted as VFAT" do
+        it "returns true" do
+          expect(subject.efi_system?).to eq(true)
+        end
+      end
+
+      context "and it is not formatted as VFAT" do
+        before do
+          subject.delete_filesystem
+          subject.create_filesystem(Y2Storage::Filesystems::Type::EXT4)
+        end
+
+        it "returns false" do
+          expect(subject.efi_system?).to eq(false)
+        end
+      end
+    end
+
+    context "when does not have an EFI System id" do
+      let(:device_name) { "/dev/sda2" }
+
+      it "returns false" do
+        expect(subject.efi_system?).to eq(false)
+      end
+    end
+  end
 end