From ce32dc603a78d0adda4bfa7fc5a6b1d776746063 Mon Sep 17 00:00:00 2001 From: Arvin Schnell Date: Mon, 18 Feb 2019 14:55:34 +0100 Subject: [PATCH 01/25] handle device_order for MD RAIDs during installation --- package/yast2-storage-ng.changes | 7 +++++ package/yast2-storage-ng.spec | 2 +- src/lib/y2storage/planned/md.rb | 4 +-- .../y2storage/proposal/autoinst_md_planner.rb | 1 + test/y2storage/autoinst_proposal_md_test.rb | 14 ++++++--- test/y2storage/planned/md_test.rb | 29 +++++++------------ 6 files changed, 30 insertions(+), 27 deletions(-) diff --git a/package/yast2-storage-ng.changes b/package/yast2-storage-ng.changes index 384e0076ef..67e807e493 100644 --- a/package/yast2-storage-ng.changes +++ b/package/yast2-storage-ng.changes @@ -1,3 +1,10 @@ +------------------------------------------------------------------- +Mon Feb 18 14:54:15 CET 2019 - aschnell@suse.com + +- AutoYaST: handle device_order for MD RAIDs during installation + (bsc#1083542) +- 4.1.60 + ------------------------------------------------------------------- Thu Feb 14 17:03:05 UTC 2019 - Stefan Hundhammer diff --git a/package/yast2-storage-ng.spec b/package/yast2-storage-ng.spec index 8c289bc7f0..0c856381a7 100644 --- a/package/yast2-storage-ng.spec +++ b/package/yast2-storage-ng.spec @@ -16,7 +16,7 @@ # Name: yast2-storage-ng -Version: 4.1.59 +Version: 4.1.60 Release: 0 BuildRoot: %{_tmppath}/%{name}-%{version}-build diff --git a/src/lib/y2storage/planned/md.rb b/src/lib/y2storage/planned/md.rb index 4dc5d57a77..a54a990d81 100644 --- a/src/lib/y2storage/planned/md.rb +++ b/src/lib/y2storage/planned/md.rb @@ -96,9 +96,7 @@ def add_devices(md_device, devices) included + missing end - sorted.each do |device| - md_device.add_device(device) - end + md_device.sorted_devices = sorted end # Whether the given name matches the name of the planned MD diff --git a/src/lib/y2storage/proposal/autoinst_md_planner.rb b/src/lib/y2storage/proposal/autoinst_md_planner.rb index f142ea0ce8..8b5786d4a2 100644 --- a/src/lib/y2storage/proposal/autoinst_md_planner.rb +++ b/src/lib/y2storage/proposal/autoinst_md_planner.rb @@ -119,6 +119,7 @@ def add_raid_options(md, raid_options) md.name = raid_options.raid_name if raid_options.raid_name md.chunk_size = chunk_size_from_string(raid_options.chunk_size) if raid_options.chunk_size md.md_parity = MdParity.find(raid_options.parity_algorithm) if raid_options.parity_algorithm + md.devices_order = raid_options.device_order if !raid_options.device_order.empty? end # Sets 'reusing' attributes for a MD RAID diff --git a/test/y2storage/autoinst_proposal_md_test.rb b/test/y2storage/autoinst_proposal_md_test.rb index a22858a4dc..acc2c90c8e 100644 --- a/test/y2storage/autoinst_proposal_md_test.rb +++ b/test/y2storage/autoinst_proposal_md_test.rb @@ -50,7 +50,10 @@ { "create" => create, "filesystem" => :xfs, "format" => create, "mount" => "/home", "mountby" => :uuid, "partition_nr" => partition_nr, - "raid_options" => { "raid_type" => "raid1" } + "raid_options" => { + "raid_type" => "raid1", + "device_order" => ["/dev/vdb1", "/dev/vdc1"] + } } ] }, @@ -140,11 +143,14 @@ raid_sid = fake_devicegraph.raids.first.sid proposal.propose - raids = proposal.devices.raids + raids = proposal.devices.raids expect(raids.size).to eq 1 - expect(raids.first.sid).to_not eq raid_sid - expect(raids.first.devices.map(&:name)).to contain_exactly("/dev/vdb1", "/dev/vdc1") + + raid = raids.first + expect(raid.sid).to_not eq raid_sid + expect(raid.md_level).to eq Y2Storage::MdLevel::RAID1 + expect(raid.sorted_devices.map(&:name)).to eq ["/dev/vdb1", "/dev/vdc1"] end include_examples "format MD with no issues" diff --git a/test/y2storage/planned/md_test.rb b/test/y2storage/planned/md_test.rb index d2aeb7c83f..5b53c50012 100755 --- a/test/y2storage/planned/md_test.rb +++ b/test/y2storage/planned/md_test.rb @@ -36,16 +36,14 @@ let(:real_md) { double("Y2Storage::Md") } it "calls Y2Storage::Md#add_device for all the devices" do - expect(real_md).to receive(:add_device).exactly(4).times + expect(real_md).to receive(:sorted_devices=).once + planned_md.add_devices(real_md, devices) end context "if #devices_order is not set" do it "adds the devices in name's alphabetical order" do - expect(real_md).to receive(:add_device).with(sda1).ordered - expect(real_md).to receive(:add_device).with(sda2).ordered - expect(real_md).to receive(:add_device).with(sdb1).ordered - expect(real_md).to receive(:add_device).with(sdb2).ordered + expect(real_md).to receive(:sorted_devices=).with([sda1, sda2, sdb1, sdb2]).once planned_md.add_devices(real_md, devices) end @@ -57,10 +55,7 @@ end it "adds the devices in the specified order" do - expect(real_md).to receive(:add_device).with(sdb2).ordered - expect(real_md).to receive(:add_device).with(sda1).ordered - expect(real_md).to receive(:add_device).with(sda2).ordered - expect(real_md).to receive(:add_device).with(sdb1).ordered + expect(real_md).to receive(:sorted_devices=).with([sdb2, sda1, sda2, sdb1]).once planned_md.add_devices(real_md, devices) end @@ -71,16 +66,14 @@ end it "adds the devices in the specified order" do - expect(real_md).to receive(:add_device).with(sdb2).ordered - expect(real_md).to receive(:add_device).with(sda1).ordered - expect(real_md).to receive(:add_device).with(sda2).ordered - expect(real_md).to receive(:add_device).with(sdb1).ordered + expect(real_md).to receive(:sorted_devices=).with([sdb2, sda1, sda2, sdb1]).once planned_md.add_devices(real_md, devices) end it "does not try to add additional devices" do - expect(real_md).to receive(:add_device).exactly(4).times + expect(real_md).to receive(:sorted_devices=).once + planned_md.add_devices(real_md, devices) end end @@ -89,15 +82,13 @@ before { planned_md.devices_order = ["/dev/sdb2", "/dev/sda2"] } it "adds all the devices" do - expect(real_md).to receive(:add_device).exactly(4).times + expect(real_md).to receive(:sorted_devices=).once + planned_md.add_devices(real_md, devices) end it "adds first the sorted devices and then the rest (alphabetically)" do - expect(real_md).to receive(:add_device).with(sdb2).ordered - expect(real_md).to receive(:add_device).with(sda2).ordered - expect(real_md).to receive(:add_device).with(sda1).ordered - expect(real_md).to receive(:add_device).with(sdb1).ordered + expect(real_md).to receive(:sorted_devices=).with([sdb2, sda2, sda1, sdb1]).once planned_md.add_devices(real_md, devices) end From a0da574cc593a4b369ba92be975e589be0977dcc Mon Sep 17 00:00:00 2001 From: Stefan Hundhammer Date: Mon, 18 Feb 2019 17:37:22 +0100 Subject: [PATCH 02/25] Help texts for guided setup --- .../dialogs/guided_setup/select_disks.rb | 20 ++++++++++ .../guided_setup/select_filesystem/base.rb | 12 ++++++ .../dialogs/guided_setup/select_root_disk.rb | 37 ++++++++++++++++++ .../dialogs/guided_setup/select_scheme.rb | 38 +++++++++++++++++++ 4 files changed, 107 insertions(+) diff --git a/src/lib/y2storage/dialogs/guided_setup/select_disks.rb b/src/lib/y2storage/dialogs/guided_setup/select_disks.rb index 1b6d41e1e2..3f32864c66 100644 --- a/src/lib/y2storage/dialogs/guided_setup/select_disks.rb +++ b/src/lib/y2storage/dialogs/guided_setup/select_disks.rb @@ -75,6 +75,26 @@ def update_settings! settings.candidate_devices = selected_disks.map(&:name) end + def help_text + # TRANSLATORS: Help text for guided storage setup + _( + "

" \ + "Select the disks to install on." \ + "

" \ + "In later dialogs, you can select which of those disks " \ + "will be used for the root filesystem and what to do with " \ + "any existing Linux, Windows or other partitions (keep, " \ + "remove, sometimes resize)." \ + "

" \ + "How many of the disks you select here will actually be used " \ + "depends on the exact scenario you choose (LVM, with or " \ + "without separate home partition etc.)." \ + "

" \ + "Disks that are not selected here remain completely untouched." \ + "

" + ) + end + private def valid? diff --git a/src/lib/y2storage/dialogs/guided_setup/select_filesystem/base.rb b/src/lib/y2storage/dialogs/guided_setup/select_filesystem/base.rb index 0127767f2e..47d794a52b 100644 --- a/src/lib/y2storage/dialogs/guided_setup/select_filesystem/base.rb +++ b/src/lib/y2storage/dialogs/guided_setup/select_filesystem/base.rb @@ -38,6 +38,18 @@ def initialize(*params) def dialog_title _("Filesystem Options") end + + def help_text + settings.lvm ? help_text_for_volumes : help_text_for_partitions + end + + def help_text_for_volumes + _("Select the filesystem type for each of the volumes.") + end + + def help_text_for_partitions + _("Select the filesystem type for each of the partitions.") + end end end end diff --git a/src/lib/y2storage/dialogs/guided_setup/select_root_disk.rb b/src/lib/y2storage/dialogs/guided_setup/select_root_disk.rb index f8e9a4b50f..4f8cc89526 100644 --- a/src/lib/y2storage/dialogs/guided_setup/select_root_disk.rb +++ b/src/lib/y2storage/dialogs/guided_setup/select_root_disk.rb @@ -161,6 +161,43 @@ def update_settings! update_windows_settings if activate_windows_actions? end + def help_text + # TRANSLATORS: Help text for root disk selection + msg = _( + "

" \ + "Select the disk where to create the root filesystem. " \ + "

" \ + "This is also the disk where any boot-related partitions " \ + "will be created as necessary: /boot, ESP (EFI System " \ + "Partition), BIOS-Grub. " \ + "That means that this disk should be usable by the machine's " \ + "BIOS / firmware." \ + "

" \ + "In this dialog you can also choose what to do with existing partitions:" \ + "

" \ + "

    " \ + "
  • Do not modify (keep them as they are)
    • " \ + "
  • Remove if needed
    • " \ + "
  • Remove even if not needed (always remove)
    • " \ + "
" + ) + + # TRANSLATORS: Help text for root disk selection, continued + msg << _( + "
    " \ + "
  • Resize if needed (Windows partitions only)
    • " \ + "
  • Resize or remove if needed (Windows partitions only)
    • " \ + "
" \ + "

" \ + "That last option means to try to resize the Windows partition(s) to " \ + "make enough disk space available for Linux, but if that is not " \ + "enough, completely delete the Windows partition." \ + "

" + ) if activate_windows_actions? + + msg + end + private def update_windows_settings diff --git a/src/lib/y2storage/dialogs/guided_setup/select_scheme.rb b/src/lib/y2storage/dialogs/guided_setup/select_scheme.rb index fa4d5a71a5..8bf00ba8e1 100644 --- a/src/lib/y2storage/dialogs/guided_setup/select_scheme.rb +++ b/src/lib/y2storage/dialogs/guided_setup/select_scheme.rb @@ -107,6 +107,44 @@ def update_settings! settings.encryption_password = password end + # rubocop:disable Metrics/MethodLength + def help_text + # TRANSLATORS: Help text for the partitioning scheme (LVM / encryption) + _( + "

" \ + "Select the parititioning scheme:" \ + "

" \ + "

    " \ + "
  • Plain partitions (no LVM), the simple traditional way
    • " \ + "
  • LVM (Logical Volume Management): " \ + "

    " \ + "This is a more flexible way of managing disk space. " \ + "

    " \ + "You can spread single filesystems over multiple disks and add " \ + "(or, to some extent, remove) disks later as necessary. " \ + "

    " \ + "You define PVs (Physical Volumes) from partitions or whole disks " \ + "and combine them into VGs (Volume Groups) that serve as storage " \ + "pools. You can create LVs (logical volumes) to create filesystems " \ + "(Btrfs, Ext2/3/4, XFS) on." \ + "

    " \ + "In this Guided Setup, all this is done for you for the " \ + "standard filesystem layout if you check Enable LVM." \ + "

    • " \ + "
" \ + "

" \ + "Enable Disk Encryption (with or without LVM) adds a LUKS " \ + " disk encryption layer to the partitioning setup. " \ + "Notice that you will have to enter the correct password each time " \ + "you boot the system. " \ + "

" \ + "If you lose the password, there is no way to recover it, " \ + "so make sure not to lose it!" \ + "

" + ) + # rubocop:enable Metrics/MethodLength + end + private def valid? From 77e25cfdd3a1dc4910c8a80039d143ddac83c103 Mon Sep 17 00:00:00 2001 From: Stefan Hundhammer Date: Tue, 19 Feb 2019 14:40:28 +0100 Subject: [PATCH 03/25] Version bump and change log --- package/yast2-storage-ng.changes | 6 ++++++ package/yast2-storage-ng.spec | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/package/yast2-storage-ng.changes b/package/yast2-storage-ng.changes index 67e807e493..1e3a083fcc 100644 --- a/package/yast2-storage-ng.changes +++ b/package/yast2-storage-ng.changes @@ -1,3 +1,9 @@ +------------------------------------------------------------------- +Tue Feb 19 13:39:49 UTC 2019 - Stefan Hundhammer + +- Added help texts for guided setup (bsc#1121801) +- 4.1.61 + ------------------------------------------------------------------- Mon Feb 18 14:54:15 CET 2019 - aschnell@suse.com diff --git a/package/yast2-storage-ng.spec b/package/yast2-storage-ng.spec index 0c856381a7..586822cb7b 100644 --- a/package/yast2-storage-ng.spec +++ b/package/yast2-storage-ng.spec @@ -16,7 +16,7 @@ # Name: yast2-storage-ng -Version: 4.1.60 +Version: 4.1.61 Release: 0 BuildRoot: %{_tmppath}/%{name}-%{version}-build From a511878598ffc83d4a047be049974b7e34cea76c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Fri, 15 Feb 2019 16:10:59 +0000 Subject: [PATCH 04/25] Adapt Bcache wrapper --- src/lib/y2storage/bcache.rb | 6 ++++-- test/y2storage/bcache_test.rb | 40 ++++++++++++++++++++++++++++++----- 2 files changed, 39 insertions(+), 7 deletions(-) diff --git a/src/lib/y2storage/bcache.rb b/src/lib/y2storage/bcache.rb index 14f3c3c702..784f320ce3 100644 --- a/src/lib/y2storage/bcache.rb +++ b/src/lib/y2storage/bcache.rb @@ -53,7 +53,7 @@ class Bcache < Partitionable # @return [BlkDevice, nil] nil for Flash-only Bcache storage_forward :backing_device, as: "BlkDevice" - # @!method attach_bcache_cset(set) + # @!method add_bcache_cset(set) # This method does not make sense for Flash-only Bcache devices. # # @raise [storage::Exception] if attaching failed @@ -61,7 +61,9 @@ class Bcache < Partitionable # the Bcache device already has a caching set. # # @param set [BcacheCset] set to attach - storage_forward :attach_bcache_cset + storage_forward :add_bcache_cset + + storage_forward :remove_bcache_cset # @!attribute cache_mode # Mode in which cache operates. diff --git a/test/y2storage/bcache_test.rb b/test/y2storage/bcache_test.rb index b01d77b3a2..da72931ea1 100644 --- a/test/y2storage/bcache_test.rb +++ b/test/y2storage/bcache_test.rb @@ -119,7 +119,7 @@ end end - describe "#attach_bcache_cset" do + describe "#add_bcache_cset" do before do described_class.create(fake_devicegraph, bcache_name) end @@ -128,19 +128,49 @@ let(:cset) { fake_devicegraph.bcache_csets.first } - it "attach a caching set to bcache device" do + it "adds a caching set to bcache device" do expect(subject.bcache_cset).to be_nil - subject.attach_bcache_cset(cset) + subject.add_bcache_cset(cset) expect(subject.bcache_cset).to eq(cset) end context "when the bcache already has an associated caching set" do + let(:bcache_name) { "/dev/bcache0" } + + it "raises an exception" do + expect { subject.add_bcache_cset(cset) }.to raise_error(Storage::LogicException) + end + end + + context "when the bcache is flash-only" do let(:bcache_name) { "/dev/bcache1" } it "raises an exception" do - expect { subject.attach_bcache_cset(cset) }.to raise_error(Storage::LogicException) + expect { subject.add_bcache_cset(cset) }.to raise_error(Storage::LogicException) + end + end + end + + describe "#remove_bcache_cset" do + let(:bcache_name) { "/dev/bcache0" } + + it "removes the caching set" do + expect(subject.bcache_cset).to_not be_nil + + subject.remove_bcache_cset + + expect(subject.bcache_cset).to be_nil + end + + context "when the bcache has no caching set" do + before do + subject.remove_bcache_cset + end + + it "raises an exception" do + expect { subject.remove_bcache_cset }.to raise_error(Storage::LogicException) end end @@ -148,7 +178,7 @@ let(:bcache_name) { "/dev/bcache1" } it "raises an exception" do - expect { subject.attach_bcache_cset(cset) }.to raise_error(Storage::LogicException) + expect { subject.remove_bcache_cset }.to raise_error(Storage::LogicException) end end end From 0d45274ffd4fe6b1e019ba282e35132c6ddaa248 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Fri, 15 Feb 2019 16:11:58 +0000 Subject: [PATCH 05/25] Add devicegraph method for removing caching set --- src/lib/y2storage/devicegraph.rb | 11 ++++++++++- test/data/devicegraphs/bcache2.xml | 4 ++-- test/y2storage/devicegraph_test.rb | 19 +++++++++++++++++++ 3 files changed, 31 insertions(+), 3 deletions(-) diff --git a/src/lib/y2storage/devicegraph.rb b/src/lib/y2storage/devicegraph.rb index cab1ebb9a5..2fb341d6fe 100644 --- a/src/lib/y2storage/devicegraph.rb +++ b/src/lib/y2storage/devicegraph.rb @@ -1,6 +1,6 @@ # encoding: utf-8 -# Copyright (c) [2017] SUSE LLC +# Copyright (c) [2017-2019] SUSE LLC # # All Rights Reserved. # @@ -381,6 +381,15 @@ def remove_bcache(bcache) remove_with_dependants(bcache_cset) if bcache_cset && bcache_cset.bcaches.empty? end + def remove_bcache_cset(bcache_cset) + if !(bcache_cset && bcache_cset.is?(:bcache_cset)) + raise(ArgumentError, "Incorrect device #{bcache_cset.inspect}") + end + + # Bcaches using this caching set are not removed + remove_device(bcache_cset) + end + # Removes a Md raid and all its descendants # # It also removes other devices that may have become useless, like the diff --git a/test/data/devicegraphs/bcache2.xml b/test/data/devicegraphs/bcache2.xml index 5a74eb2be9..4af3cd6de2 100644 --- a/test/data/devicegraphs/bcache2.xml +++ b/test/data/devicegraphs/bcache2.xml @@ -283,11 +283,11 @@ 133128257536 192.168.56.1 - /home/ivan/projects + /home/user/projects 69 - /home/ivan/projects + /home/user/projects device nfs true diff --git a/test/y2storage/devicegraph_test.rb b/test/y2storage/devicegraph_test.rb index 7b1a29b7d0..eac82388f9 100644 --- a/test/y2storage/devicegraph_test.rb +++ b/test/y2storage/devicegraph_test.rb @@ -728,6 +728,25 @@ def with_sda2_deleted(initial_graph) end end + describe "#remove_bcache_cset" do + subject(:devicegraph) { Y2Storage::StorageManager.instance.staging } + + before do + fake_scenario("bcache1.xml") + end + + it "removes the given caching set device" do + bcache_cset = Y2Storage::BcacheCset.all(devicegraph).first + expect(bcache_cset).to_not be_nil + + sid = bcache_cset.sid + + devicegraph.remove_bcache_cset(bcache_cset) + + expect(devicegraph.find_device(sid)).to be_nil + end + end + describe "#remove_md" do subject(:devicegraph) { Y2Storage::StorageManager.instance.staging } From 30f4075aee65121c9dd2ce3f4c083eb6a05d032d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Fri, 15 Feb 2019 16:17:11 +0000 Subject: [PATCH 06/25] Add controller for bcache actions --- .../actions/controllers/bcache.rb | 303 ++++++++ .../actions/controllers/bcache_test.rb | 673 ++++++++++++++++++ 2 files changed, 976 insertions(+) create mode 100644 src/lib/y2partitioner/actions/controllers/bcache.rb create mode 100644 test/y2partitioner/actions/controllers/bcache_test.rb diff --git a/src/lib/y2partitioner/actions/controllers/bcache.rb b/src/lib/y2partitioner/actions/controllers/bcache.rb new file mode 100644 index 0000000000..18d9b753b3 --- /dev/null +++ b/src/lib/y2partitioner/actions/controllers/bcache.rb @@ -0,0 +1,303 @@ +# encoding: utf-8 + +# Copyright (c) [2019] SUSE LLC +# +# All Rights Reserved. +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of version 2 of the GNU General Public License as published +# by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, contact SUSE LLC. +# +# To contact SUSE LLC about this file by physical or electronic mail, you may +# find current contact information at www.suse.com. + +require "yast" +require "y2storage/bcache" +require "y2partitioner/device_graphs" +require "y2partitioner/ui_state" +require "y2partitioner/blk_device_restorer" + +module Y2Partitioner + module Actions + module Controllers + # This class is used by different Bcache actions (see, {Actions::AddBcache}, + # {Actions::EditBcache} and {Actions::DeleteBcache}). + class Bcache + # @return [Y2Storage::Bcache] + attr_reader :bcache + + # Constructor + # + # @param bcache [Y2Storage::Bcache, nil] nil if a new bcache is being created. + def initialize(bcache = nil) + @bcache = bcache + + UIState.instance.select_row(bcache) if bcache + end + + # Suitable devices to be used as backing device + # + # When the bcache is being edited, only its own backing device is returned. + # + # @return [Array] + def suitable_backing_devices + return [bcache.backing_device] if bcache + + usable_blk_devices + end + + # Suitable devices to be used for caching + # + # @return [Array] + def suitable_caching_devices + bcache_csets + usable_blk_devices + end + + # Creates a bcache device + # + # @param backing_device [Y2Storage::BlkDevice] + # @param caching_device [Y2Storage::BlkDevice, Y2Storage::BcacheCset, nil] + # @param options [Hash] + def create_bcache(backing_device, caching_device, options) + raise "A Bcache cannot be created without a Backing device." unless backing_device + + BlkDeviceRestorer.new(backing_device).update_checkpoint + + @bcache = backing_device.create_bcache(Y2Storage::Bcache.find_free_name(current_graph)) + + apply_options(options) + attach(caching_device) if caching_device + end + + # Updates the bcache device + # + # @param caching_device [Y2Storage::BlkDevice, Y2Storage::BcacheCset, nil] + # @param options [Hash] + def update_bcache(caching_device, options) + apply_options(options) + + return if caching_device == bcache.bcache_cset + + detach if bcache.bcache_cset + + attach(caching_device) if caching_device + end + + # Deletes the bcache device + def delete_bcache + backing_device = bcache.backing_device + caching_device = nil + + if bcache.bcache_cset && bcache.bcache_cset.bcaches.size == 1 + caching_device = bcache.bcache_cset.blk_devices.first + end + + current_graph.remove_bcache(bcache) + + # Tries to restore the previous status of the caching and backing devices + # (e.g., its filesystem is restored back). + BlkDeviceRestorer.new(caching_device).restore_from_checkpoint if caching_device + BlkDeviceRestorer.new(backing_device).restore_from_checkpoint + end + + # Whether the bcache already exists on disk + # + # @return [Boolean] + def committed_bcache? + !committed_bcache.nil? + end + + # Whether the bcache on disk already had a caching set + # + # @return [Boolean] + def committed_bcache_cset? + !committed_bcache_cset.nil? + end + + # Whether the bcache on disk already had a caching set, and this caching set was not + # used by another bcache. + # + # @return [Boolean] + def single_committed_bcache_cset? + return false unless committed_bcache_cset? + + committed_bcache_cset.bcaches.size == 1 + end + + private + + # Block devices that can be used as backing or caching device + # + # @return [Array] + def usable_blk_devices + current_graph.blk_devices.select { |d| usable_blk_device?(d) } + end + + # Whether the device can be used as backing or caching device + # + # @param device [Y2Storage::BlkDevice] + # @return [Boolean] + def usable_blk_device?(device) + !device_used?(device) && + !device_formatted_and_mounted?(device) && + !device_has_partitions?(device) && + !device_extended_partition?(device) && + !device_belong_to_bcache?(device) + end + + # Whether the device is already in use (e.g., as LVM PV or raid disk) + # + # @param device [Y2Storage::BlkDevice] + # @return [Boolean] + def device_used?(device) + device.component_of.any? + end + + # Whether the device is formatted and mounted + # + # @param device [Y2Storage::BlkDevice] + # @return [Boolean] + def device_formatted_and_mounted?(device) + device.filesystem && device.filesystem.mount_point + end + + # Whether the device has partitions + # + # @param device [Y2Storage::BlkDevice] + # @return [Boolean] + def device_has_partitions?(device) + device.respond_to?(:partitions) && device.partitions.any? + end + + # Whether the device is an extended partition + # + # @param device [Y2Storage::BlkDevice] + # @return [Boolean] + def device_extended_partition?(device) + device.is?(:partition) && device.type.is?(:extended) + end + + # Whether the device is part of a bcache device + # + # @param device [Y2Storage::BlkDevice] + # @return [Boolean] + def device_belong_to_bcache?(device) + # do not allow nested bcaches, see doc/bcache.md + ([device] + device.ancestors).any? { |a| a.is?(:bcache, :bcache_cset) } + end + + # Currently existing caching sets + # + # @return [Array] + def bcache_csets + current_graph.bcache_csets + end + + # Applies options to the bcache device + # + # Right now, only the cache mode can be permanently modified. + # + # @param options [Hash] + def apply_options(options) + options.each_pair do |key, value| + bcache.public_send(:"#{key}=", value) + end + end + + # Attaches the selected caching to the bcache + # + # @param caching_device [Y2Storage::BcacheCset, Y2Storage::BlkDevice] + def attach(caching_device) + bcache_cset = create_bcache_cset(caching_device) + + bcache.add_bcache_cset(bcache_cset) + end + + # Detaches the caching set from the bcache device + def detach + bcache_cset = bcache.bcache_cset + + bcache.remove_bcache_cset + + remove_bcache_cset(bcache_cset) if remove_bcache_cset?(bcache_cset) + end + + # Creates a new caching set if the given caching device is not a caching set yet + # + # @param caching_device [Y2Storage::BlkDevice, Y2Storage::BcacheCset] + def create_bcache_cset(caching_device) + return caching_device if caching_device.is?(:bcache_cset) + + # The descendants of the caching device should be restored in case that + # this caching set is finally removed and not used at all. + BlkDeviceRestorer.new(caching_device).update_checkpoint + + caching_device.remove_descendants + caching_device.create_bcache_cset + end + + # Whether the caching set can be removed + # + # @param bcache_cset [Y2Storage::BcacheCset] + def remove_bcache_cset?(bcache_cset) + bcache_cset.bcaches.none? + end + + # Removes the caching set + # + # Previous state of the caching device is retored. + # + # @param bcache_cset [Y2Storage::BcacheCset] + def remove_bcache_cset(bcache_cset) + caching_device = bcache_cset.blk_devices.first + + current_graph.remove_bcache_cset(bcache_cset) + + BlkDeviceRestorer.new(caching_device).restore_from_checkpoint + end + + # Bcache existing on disk + # + # @return [Y2Storage::Bcache, nil] nil if the bcache is being created or + # does not exist on the disk yet. + def committed_bcache + return nil unless bcache + + system_graph.find_device(bcache.sid) + end + + # Caching set of the bcache existing on disk + # + # @return [Y2Storage::BcacheCset, nil] nil if the bcache does not exist + # on disk or it has no caching set. + def committed_bcache_cset + return nil unless committed_bcache + + committed_bcache.bcache_cset + end + + # Current devicegraph in which the action operates on + # + # @return [Y2Storage::Devicegraph] + def current_graph + DeviceGraphs.instance.current + end + + # Devicegraph representing the system status + # + # @return [Y2Storage::Devicegraph] + def system_graph + DeviceGraphs.instance.system + end + end + end + end +end diff --git a/test/y2partitioner/actions/controllers/bcache_test.rb b/test/y2partitioner/actions/controllers/bcache_test.rb new file mode 100644 index 0000000000..0d10e94e0f --- /dev/null +++ b/test/y2partitioner/actions/controllers/bcache_test.rb @@ -0,0 +1,673 @@ +#!/usr/bin/env rspec +# encoding: utf-8 + +# Copyright (c) [2019] SUSE LLC +# +# All Rights Reserved. +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of version 2 of the GNU General Public License as published +# by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, contact SUSE LLC. +# +# To contact SUSE LLC about this file by physical or electronic mail, you may +# find current contact information at www.suse.com. + +require_relative "../../test_helper" + +require "y2partitioner/device_graphs" +require "y2partitioner/actions/controllers/bcache" + +describe Y2Partitioner::Actions::Controllers::Bcache do + before do + devicegraph_stub(scenario) + end + + subject(:controller) { described_class.new(device) } + + let(:device) { nil } + + let(:current_graph) { Y2Partitioner::DeviceGraphs.instance.current } + + def dev(name) + result = Y2Storage::BlkDevice.find_by_name(current_graph, name) + result ||= Y2Storage::LvmVg.all(current_graph).find { |i| i.name == name } + result + end + + shared_examples "usable devices" do + def name_of_devices + devices = controller.send(testing_method) + devices = devices.select { |d| d.respond_to?(:name) } + + devices.map(&:name) + end + + let(:scenario) { "complex-lvm-encrypt.yml" } + + it "returns an array of block devices" do + expect(controller.send(testing_method)).to be_an(Array) + expect(controller.send(testing_method)).to all be_a(Y2Storage::BlkDevice) + end + + it "includes partitions with an unmounted filesystem" do + expect(name_of_devices).to include("/dev/sda2", "/dev/sde3") + end + + it "excludes partitions with a mount point" do + expect(name_of_devices).to include("/dev/sda2", "/dev/sde3") + + sda2 = dev("/dev/sda2") + sda2.filesystem.mount_path = "/var" + sde3 = dev("/dev/sde3") + sde3.filesystem.mount_path = "swap" + + expect(name_of_devices).to_not include("/dev/sda2", "/dev/sde3") + end + + it "excludes partitions that are part of an LVM" do + expect(name_of_devices).to_not include("/dev/sde1", "/dev/sde2") + sda3 = dev("/dev/sda3") + expect(controller.send(testing_method)).to include sda3 + + vg0 = dev("/dev/vg0") + vg0.add_lvm_pv(sda3) + expect(controller.send(testing_method)).to_not include sda3 + end + + it "excludes partitions that are part of a MD Raid" do + sda3 = dev("/dev/sda3") + expect(controller.send(testing_method)).to include sda3 + + new_md = Y2Storage::Md.create(current_graph, "/dev/md0") + new_md.add_device(sda3) + expect(controller.send(testing_method)).to_not include sda3 + end + + it "includes disks with no partition tables" do + expect(name_of_devices).to include "/dev/sdb" + end + + it "includes disks with empty partition tables" do + sdb = dev("/dev/sdb") + sdb.create_partition_table(Y2Storage::PartitionTables::Type::GPT) + + expect(name_of_devices).to include "/dev/sdb" + end + + it "excludes disks with partitions" do + expect(name_of_devices).to_not include "/dev/sdf" + end + + it "excludes disks with a mount point" do + expect(name_of_devices).to include "/dev/sdb" + + sdb = dev("/dev/sdb") + sdb.create_filesystem(Y2Storage::Filesystems::Type::EXT3) + sdb.filesystem.mount_path = "/var" + + expect(name_of_devices).to_not include "/dev/sdb" + end + + it "excludes disks that are part of an LVM" do + expect(name_of_devices).to_not include "/dev/sdg" + end + + it "excludes disks that are part of a MD Raid" do + sdb = dev("/dev/sdb") + expect(controller.send(testing_method)).to include sdb + + new_md = Y2Storage::Md.create(current_graph, "/dev/md0") + new_md.add_device(sdb) + expect(controller.send(testing_method)).to_not include sdb + end + + context "when there are extended partitions" do + let(:scenario) { "lvm-two-vgs.yml" } + + it "excludes extended partitions" do + expect(name_of_devices).to_not include("/dev/sda3") + end + end + + context "when there are DM RAIDs" do + let(:scenario) { "empty-dm_raids.xml" } + + it "excludes disks that are part of a DM RAID" do + expect(name_of_devices).to_not include("/dev/sdb", "/dev/sdc") + end + end + + context "when there are bcaches" do + let(:scenario) { "bcache2.xml" } + + it "excludes Bcaches" do + expect(name_of_devices).to_not include("/dev/bcache0", "/dev/bcache1", "/dev/bcache2") + end + + it "excludes partitions from a Bcache" do + expect(name_of_devices).to_not include("/dev/bcache1p1", "/dev/bcache1p2") + end + + it "excludes devices used as backing device" do + expect(name_of_devices).to_not include("/dev/sdb2") + end + + it "excludes devices used as caching device" do + expect(name_of_devices).to_not include("/dev/sdb1") + end + end + end + + describe "#suitable_backing_devices" do + context "when the bcache device is being created" do + let(:device) { nil } + + let(:testing_method) { :suitable_backing_devices } + + include_examples "usable devices" + + context "when there are caching sets" do + let(:scenario) { "bcache2.xml" } + + it "excludes all caching set devices" do + bcache_csets = Y2Storage::BcacheCset.all(current_graph) + + expect(controller.suitable_backing_devices).to_not include(bcache_csets) + end + end + end + + context "when the bcache device is being edited" do + let(:scenario) { "bcache2.xml" } + + let(:device) { current_graph.find_by_name("/dev/bcache0") } + + it "returns an array of block devices" do + expect(controller.suitable_backing_devices).to be_an(Array) + expect(controller.suitable_backing_devices).to all be_a(Y2Storage::BlkDevice) + end + + it "only includes its backing device" do + expect(controller.suitable_backing_devices.map(&:name)).to contain_exactly("/dev/sdb2") + end + end + end + + describe "#suitable_caching_devices" do + let(:testing_method) { :suitable_caching_devices } + + include_examples "usable devices" + + context "when there are caching sets" do + let(:scenario) { "bcache1.xml" } + + before do + vda1 = current_graph.find_by_name("/dev/vda1") + vda1.create_bcache_cset + end + + it "includes all caching set devices" do + bcache_csets = Y2Storage::BcacheCset.all(current_graph) + + expect(bcache_csets.size).to eq(2) + + expect(controller.suitable_caching_devices).to include(*bcache_csets) + end + end + end + + describe "#create_bcache" do + let(:scenario) { "bcache2.xml" } + + let(:backing_device) { current_graph.find_by_name("/dev/sda1") } + + let(:caching_device) { nil } + + let(:options) { { cache_mode: Y2Storage::CacheMode::WRITEBACK } } + + it "creates a new bcache over the given backing device" do + expect(current_graph.find_by_name("/dev/bcache3")).to be_nil + + subject.create_bcache(backing_device, caching_device, options) + + bcache = current_graph.find_by_name("/dev/bcache3") + + expect(bcache).to_not be_nil + expect(bcache.backing_device).to eq(backing_device) + end + + it "creates a new bcache with the given cache mode" do + expect(current_graph.find_by_name("/dev/bcache3")).to be_nil + + subject.create_bcache(backing_device, caching_device, options) + + bcache = current_graph.find_by_name("/dev/bcache3") + + expect(bcache).to_not be_nil + expect(bcache.cache_mode).to eq(options[:cache_mode]) + end + + context "when non caching device is given" do + let(:caching_device) { nil } + + it "creates a new bcache without an associated caching set" do + expect(current_graph.find_by_name("/dev/bcache3")).to be_nil + + subject.create_bcache(backing_device, caching_device, options) + + bcache = current_graph.find_by_name("/dev/bcache3") + + expect(bcache).to_not be_nil + expect(bcache.bcache_cset).to be_nil + end + end + + context "when a blk device is given for caching" do + let(:caching_device) { current_graph.find_by_name("/dev/sda2") } + + it "creates a new bcache with the given device for caching" do + expect(current_graph.find_by_name("/dev/bcache3")).to be_nil + + subject.create_bcache(backing_device, caching_device, options) + + bcache = current_graph.find_by_name("/dev/bcache3") + + expect(bcache).to_not be_nil + expect(bcache.bcache_cset.blk_devices.first).to eq(caching_device) + end + end + + context "when a existing caching set is given for caching" do + let(:caching_device) { current_graph.bcache_csets.first } + + it "creates a new bcache with the given caching set for caching" do + expect(current_graph.find_by_name("/dev/bcache3")).to be_nil + + subject.create_bcache(backing_device, caching_device, options) + + bcache = current_graph.find_by_name("/dev/bcache3") + + expect(bcache).to_not be_nil + expect(bcache.bcache_cset).to eq(caching_device) + end + end + end + + shared_examples "update previous caching" do + def call_testing_method + subject.send(*testing_method) + end + + context "and the previous caching set was being used only by this bcache" do + let(:previous_caching_device) { current_graph.find_by_name("/dev/sda1") } + + it "removes the previous caching set" do + expect(previous_caching_device.in_bcache_cset).to_not be_nil + + call_testing_method + + expect(previous_caching_device.in_bcache_cset).to be_nil + end + + it "restores the previous status of the caching device" do + expect(previous_caching_device.filesystem).to be_nil + + call_testing_method + + expect(previous_caching_device.filesystem).to_not be_nil + end + end + + context "and the previous caching set was being used by several bcache devices" do + let(:previous_caching_device) { Y2Storage::BcacheCset.all(current_graph).first } + + it "does not remove the previous caching set" do + call_testing_method + + expect(previous_caching_device).to_not be_nil + end + end + end + + describe "#update_bcache" do + let(:scenario) { "bcache2.xml" } + + before do + sda2 = current_graph.find_by_name("/dev/sda2") + described_class.new.create_bcache(sda2, previous_caching_device, {}) + end + + let(:device) { current_graph.find_by_name("/dev/bcache3") } + + let(:previous_caching_device) { nil } + + let(:caching_device) { nil } + + let(:options) { { cache_mode: Y2Storage::CacheMode::WRITEBACK } } + + shared_examples "detach actions" do + it "detaches its previous caching set" do + previous_sid = device.bcache_cset.sid + + subject.update_bcache(caching_device, options) + + detached = device.bcache_cset.nil? || device.bcache_cset.sid != previous_sid + + expect(detached).to eq(true) + end + + let(:testing_method) { [:update_bcache, caching_device, options] } + + include_examples "update previous caching" + end + + it "sets the given cache mode" do + expect(device.cache_mode).to_not eq(options[:cache_mode]) + + subject.update_bcache(caching_device, options) + + expect(device.cache_mode).to eq(options[:cache_mode]) + end + + context "when a caching device is given" do + let(:caching_device) { Y2Storage::BcacheCset.all(current_graph).first } + + context "and the bcache has no previous caching set" do + let(:previous_caching_device) { nil } + + it "attaches the given caching device" do + expect(device.bcache_cset).to be_nil + + subject.update_bcache(caching_device, options) + + expect(device.bcache_cset).to eq(caching_device) + end + end + + context "and the bcache is already associated to the given caching set" do + let(:previous_caching_device) { caching_device } + + it "does not change the caching set" do + expect(device.bcache_cset).to eq(previous_caching_device) + + subject.update_bcache(caching_device, options) + + expect(device.bcache_cset).to eq(previous_caching_device) + end + end + + context "and the bcache is already associated to another caching set" do + let(:previous_caching_device) { current_graph.find_by_name("/dev/sda1") } + + it "attaches the given caching device" do + expect(device.bcache_cset).to eq(previous_caching_device.in_bcache_cset) + + subject.update_bcache(caching_device, options) + + expect(device.bcache_cset).to eq(caching_device) + end + + include_examples "detach actions" + end + end + + context "when a caching device is not given" do + let(:caching_device) { nil } + + context "and the bcache has already a previous caching set" do + let(:previous_caching_device) { Y2Storage::BcacheCset.all(current_graph).first } + + include_examples "detach actions" + end + end + end + + describe "#delete_bcache" do + let(:scenario) { "bcache2.xml" } + + before do + described_class.new.create_bcache(backing_device, previous_caching_device, {}) + end + + let(:device) { current_graph.find_by_name("/dev/bcache3") } + + let(:backing_device) { current_graph.find_by_name("/dev/sda3") } + + let(:previous_caching_device) { nil } + + it "deletes the bcache device" do + expect(current_graph.find_by_name("/dev/bcache3")).to_not be_nil + + subject.delete_bcache + + expect(current_graph.find_by_name("/dev/bcache3")).to be_nil + end + + it "restores the previous status of the backing device" do + backing_device = device.backing_device + + expect(backing_device.filesystem).to be_nil + + subject.delete_bcache + + expect(backing_device.filesystem).to_not be_nil + end + + context "when the bcache has an associated caching set" do + let(:testing_method) { [:delete_bcache] } + + include_examples "update previous caching" + end + end + + describe "#committed_bcache?" do + let(:scenario) { "bcache2.xml" } + + context "when the bcache exists on disk" do + let(:device) { current_graph.find_by_name("/dev/bcache0") } + + it "returns true" do + expect(subject.committed_bcache?).to eq(true) + end + end + + context "when the bcache does not exist on disk" do + before do + sdb1 = current_graph.find_by_name("/dev/sdb1") + sdb1.create_bcache("/dev/bcache99") + end + + let(:device) { current_graph.find_by_name("/dev/bcache99") } + + it "returns false" do + expect(subject.committed_bcache?).to eq(false) + end + end + end + + describe "#committed_bcache_cset?" do + let(:scenario) { "bcache1.xml" } + + let(:system_graph) { Y2Partitioner::DeviceGraphs.instance.system } + + let(:system_device) { system_graph.find_by_name(device.name) } + + context "when the bcache exists on disk" do + let(:device) { current_graph.find_by_name("/dev/bcache0") } + + context "and currently it has a caching set" do + before do + expect(device.bcache_cset).to_not be_nil + end + + context "but it does not have a caching set on disk" do + before do + system_device.remove_bcache_cset + end + + it "returns false" do + expect(subject.committed_bcache_cset?).to eq(false) + end + end + + context "and it has a caching set on disk" do + before do + expect(system_device.bcache_cset).to_not be_nil + end + + it "returns true" do + expect(subject.committed_bcache_cset?).to eq(true) + end + end + end + + context "and currently it does not have a caching set" do + before do + device.remove_bcache_cset + end + + context "and it does not have a caching set on disk" do + before do + system_device.remove_bcache_cset + end + + it "returns false" do + expect(subject.committed_bcache_cset?).to eq(false) + end + end + + context "but it has a caching set on disk" do + before do + expect(system_device.bcache_cset).to_not be_nil + end + + it "returns true" do + expect(subject.committed_bcache_cset?).to eq(true) + end + end + end + end + + context "when the bcache does not exist on disk" do + before do + sdb1 = current_graph.find_by_name("/dev/vda1") + sdb1.create_bcache("/dev/bcache99") + end + + let(:device) { current_graph.find_by_name("/dev/bcache99") } + + it "returns false" do + expect(subject.committed_bcache_cset?).to eq(false) + end + end + end + + describe "#single_committed_bcache_cset" do + let(:scenario) { "bcache1.xml" } + + let(:system_graph) { Y2Partitioner::DeviceGraphs.instance.system } + + let(:system_device) { system_graph.find_by_name(device.name) } + + def bcache_cset_only_for(bcache) + bcache.bcache_cset.bcaches.each do |dev| + dev.remove_bcache_cset if dev != bcache + end + end + + shared_examples "caching set usage" do + context "and its caching set is used only by this bcache on disk" do + before do + bcache_cset_only_for(system_device) + end + + it "returns true" do + expect(subject.single_committed_bcache_cset?).to eq(true) + end + end + + context "and its caching set is used by several bcaches on disk" do + before do + expect(system_device.bcache_cset.bcaches.size).to be > 1 + end + + it "returns false" do + expect(subject.single_committed_bcache_cset?).to eq(false) + end + end + end + + context "when the bcache exists on disk" do + let(:device) { current_graph.find_by_name("/dev/bcache0") } + + context "and currently it has a caching set" do + before do + expect(device.bcache_cset).to_not be_nil + end + + context "but it does not have a caching set on disk" do + before do + system_device.remove_bcache_cset + end + + it "returns false" do + expect(subject.single_committed_bcache_cset?).to eq(false) + end + end + + context "and it has a caching set on disk" do + before do + expect(system_device.bcache_cset).to_not be_nil + end + + include_examples "caching set usage" + end + end + + context "and currently it does not have a caching set" do + before do + device.remove_bcache_cset + end + + context "and it does not have a caching set on disk" do + before do + system_device.remove_bcache_cset + end + + it "returns false" do + expect(subject.single_committed_bcache_cset?).to eq(false) + end + end + + context "but it has a caching set on disk" do + before do + expect(system_device.bcache_cset).to_not be_nil + end + + include_examples "caching set usage" + end + end + end + + context "when the bcache does not exist on disk" do + before do + sdb1 = current_graph.find_by_name("/dev/vda1") + sdb1.create_bcache("/dev/bcache99") + end + + let(:device) { current_graph.find_by_name("/dev/bcache99") } + + it "returns false" do + expect(subject.single_committed_bcache_cset?).to eq(false) + end + end + end +end From 40e01f82408ad02c2712842ee92be571a96fa2d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Fri, 15 Feb 2019 16:17:36 +0000 Subject: [PATCH 07/25] Add base class for partitioner actions --- src/lib/y2partitioner/actions/base.rb | 89 +++++++++++++++++++++++++++ 1 file changed, 89 insertions(+) create mode 100644 src/lib/y2partitioner/actions/base.rb diff --git a/src/lib/y2partitioner/actions/base.rb b/src/lib/y2partitioner/actions/base.rb new file mode 100644 index 0000000000..4b2d339c63 --- /dev/null +++ b/src/lib/y2partitioner/actions/base.rb @@ -0,0 +1,89 @@ +# encoding: utf-8 + +# Copyright (c) [2019] SUSE LLC +# +# All Rights Reserved. +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of version 2 of the GNU General Public License as published +# by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, contact SUSE LLC. +# +# To contact SUSE LLC about this file by physical or electronic mail, you may +# find current contact information at www.suse.com. + +require "yast" +require "yast/i18n" +require "yast2/popup" + +require "abstract_method" + +module Y2Partitioner + module Actions + # Base class for actions that can be performed by the Expert Partitioner + # + # This base class is mainly intended to one-step actions. For more complex actions, + # see {TransactionWizard}. + class Base + include Yast::I18n + + def initialize + textdomain "storage" + end + + # Runs a dialog for adding a bcache and also modifies the device graph if the user + # confirms the dialog. + # + # @return [Symbol] :back, :finish + def run + return :back unless run? + + perform_action + + :finish + end + + private + + # Performs the action, see {#run} + # + # This method should be defined by derived classes. + abstract_method :perform_action + + # Checks whether the action can be performed + # + # @return [Boolean] + def run? + validate + end + + # Validations before performing the action + # + # @note The action can be performed if there are no errors (see #errors). + # Only the first error is shown. + # + # @return [Boolean] + def validate + current_errors = errors + return true if current_errors.empty? + + Yast2::Popup.show(current_errors.first, headline: :error) + false + end + + # List of errors that avoid to perform the action + # + # @return [Array] translated error messages + def errors + [] + end + end + end +end From 514e2430aa475400e9492dca263abe64330d46e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Fri, 15 Feb 2019 16:19:49 +0000 Subject: [PATCH 08/25] Adapt action for adding a bcache --- src/lib/y2partitioner/actions/add_bcache.rb | 136 ++++-------------- src/lib/y2partitioner/dialogs/bcache.rb | 22 +-- test/y2partitioner/actions/add_bcache_test.rb | 2 + test/y2partitioner/dialogs/bcache_test.rb | 19 ++- 4 files changed, 56 insertions(+), 123 deletions(-) diff --git a/src/lib/y2partitioner/actions/add_bcache.rb b/src/lib/y2partitioner/actions/add_bcache.rb index e4cbb9d6ec..3a37c750d4 100644 --- a/src/lib/y2partitioner/actions/add_bcache.rb +++ b/src/lib/y2partitioner/actions/add_bcache.rb @@ -20,150 +20,64 @@ # find current contact information at www.suse.com. require "yast" -require "yast/i18n" -require "yast2/popup" -require "y2storage/bcache" +require "y2partitioner/actions/base" +require "y2partitioner/actions/controllers/bcache" require "y2partitioner/dialogs/bcache" -require "y2partitioner/device_graphs" module Y2Partitioner module Actions - # Action for adding a bcache device - class AddBcache - include Yast::I18n - + # Action for adding a bcache device, see {Actions::Base} + class AddBcache < Base def initialize - textdomain "storage" - end - - # Runs a dialog for adding a bcache and also modifies the device graph if the user - # confirms the dialog. - # - # @return [Symbol] :back, :finish - def run - return :back unless validate + super - dialog = Dialogs::Bcache.new(suitable_backing_devices, suitable_caching_devices) - - create_device(dialog) if dialog.run == :next + textdomain "storage" - :finish + @controller = Controllers::Bcache.new end private - # Validations before performing the action - # - # @note The action can be performed is there are no errors (see #errors). - # Only the first error is shown. - # - # @return [Boolean] - def validate - current_errors = errors - return true if current_errors.empty? - - Yast2::Popup.show(current_errors.first, headline: :error) - false - end + # @return [Controllers::Bcache] + attr_reader :controller # List of errors that avoid to create a Bcache # + # @see Actions::Base#errors + # # @return [Array] def errors - [no_backing_devices_error].compact + (super + [no_backing_devices_error]).compact end # Error when there is no suitable backing devices for creating a Bcache # - # @return [String, nil] nil if there are devices. + # @return [String, nil] nil if there are suitable backing devices. def no_backing_devices_error - return nil if suitable_backing_devices.any? + return nil if suitable_backing_devices? # TRANSLATORS: Error message. _("There are not enough suitable unused devices to create a Bcache.") end - # Creates a bcache device according to the user input - # - # @param dialog [Dialogs::Bcache] - def create_device(dialog) - backing = dialog.backing_device - - raise "Invalid result #{dialog.inspect}. Backing not found." unless backing - - bcache = backing.create_bcache(Y2Storage::Bcache.find_free_name(device_graph)) - - apply_options(bcache, dialog.options) - - attach(bcache, dialog.caching_device) if dialog.caching_device - end - - # Applies options to the bcache device - # - # Right now, the dialog only allows to indicate the cache mode. - # - # @param bcache [Y2Storage::Bcache] - # @param options [Hash] - def apply_options(bcache, options) - options.each_pair do |key, value| - bcache.public_send(:"#{key}=", value) - end - end - - # Attaches the selected caching to the bcache - # - # @param bcache [Y2Storage::Bcache] - # @param caching [Y2Storage::BcacheCset, Y2Storage::BlkDevice, nil] - def attach(bcache, caching) - return if caching.nil? - - if !caching.is?(:bcache_cset) - caching.remove_descendants - caching = caching.create_bcache_cset - end - - bcache.attach_bcache_cset(caching) - end - - # Device graph in which the action operates on + # Opens a dialog to create a Bcache # - # @return [Y2Storage::Devicegraph] - def device_graph - DeviceGraphs.instance.current - end - - # Suitable devices to be used as backing device + # The Bcache is created only if the dialog is accepted. # - # @return [Array] - def suitable_backing_devices - usable_blk_devices - end + # @see Actions::Base#perform_action + def perform_action + dialog = Dialogs::Bcache.new(controller) - # Suitable devices to be used for caching - # - # @return [Array] - def suitable_caching_devices - existing_caches + usable_blk_devices - end + return unless dialog.run == :next - # Block devices that can be used as backing or caching device - # - # @return [Array] - def usable_blk_devices - device_graph.blk_devices.select do |dev| - dev.component_of.empty? && - (dev.filesystem.nil? || dev.filesystem.mount_point.nil?) && - (!dev.respond_to?(:partitions) || dev.partitions.empty?) && - # do not allow nested bcaches, see doc/bcache.md - ([dev] + dev.ancestors).none? { |a| a.is?(:bcache, :bcache_cset) } - end + controller.create_bcache(dialog.backing_device, dialog.caching_device, dialog.options) end - # Currently existing caching sets + # Whether there is suitable backing devices # - # @return [Array] - def existing_caches - device_graph.bcache_csets + # @return [Boolean] + def suitable_backing_devices? + controller.suitable_backing_devices.any? end end end diff --git a/src/lib/y2partitioner/dialogs/bcache.rb b/src/lib/y2partitioner/dialogs/bcache.rb index d2cdb9b0c1..f953891d00 100644 --- a/src/lib/y2partitioner/dialogs/bcache.rb +++ b/src/lib/y2partitioner/dialogs/bcache.rb @@ -34,16 +34,14 @@ module Dialogs class Bcache < Base # Constructor # - # @param suitable_backing [Array] devices that can be used for backing. - # @param suitable_caching [Array] - # devices that can be used for caching. - # @param device [Y2Storage::Bcache] existing bcache device or nil if it is a new one. - def initialize(suitable_backing, suitable_caching, device = nil) + # @param controller [Actions::Controllers::Bcache] + def initialize(controller) textdomain "storage" - @caching = CachingDeviceSelector.new(device, suitable_caching) - @backing = BackingDeviceSelector.new(device, suitable_backing, @caching) - @cache_mode = CacheModeSelector.new(device) + @caching = CachingDeviceSelector.new(controller.bcache, controller.suitable_caching_devices) + @backing = BackingDeviceSelector.new(controller.bcache, + controller.suitable_backing_devices, @caching) + @cache_mode = CacheModeSelector.new(controller.bcache) end # @macro seeDialog @@ -189,6 +187,12 @@ def initialize(bcache, devices, caching) @caching = caching end + def init + super + + disable if bcache + end + # @macro seeAbstractWidget def label _("Backing Device") @@ -290,7 +294,7 @@ def item_for_non_device # When the bcache exists, its caching set should be the default device. # Otherwise, the first available device is the default one. def default_device - return bcache.bcache_cset if bcache && bcache.bcache_cset + return bcache.bcache_cset if bcache super end diff --git a/test/y2partitioner/actions/add_bcache_test.rb b/test/y2partitioner/actions/add_bcache_test.rb index 5c97841063..be709aae8b 100644 --- a/test/y2partitioner/actions/add_bcache_test.rb +++ b/test/y2partitioner/actions/add_bcache_test.rb @@ -165,6 +165,8 @@ end context "when the dialog is discarded" do + let(:dialog_result) { :back } + it "does not create a new bcache" do subject.run diff --git a/test/y2partitioner/dialogs/bcache_test.rb b/test/y2partitioner/dialogs/bcache_test.rb index 08dc348149..583fdd28d0 100644 --- a/test/y2partitioner/dialogs/bcache_test.rb +++ b/test/y2partitioner/dialogs/bcache_test.rb @@ -1,7 +1,7 @@ #!/usr/bin/env rspec # encoding: utf-8 -# Copyright (c) [2018] SUSE LLC +# Copyright (c) [2018-2019] SUSE LLC # # All Rights Reserved. # @@ -26,15 +26,28 @@ require "cwm/rspec" require "y2storage" require "y2partitioner/dialogs/bcache" +require "y2partitioner/actions/controllers/bcache" describe Y2Partitioner::Dialogs::Bcache do - before { devicegraph_stub("bcache1.xml") } + before do + devicegraph_stub("bcache1.xml") + + allow(Y2Partitioner::Actions::Controllers::Bcache).to receive(:new).and_return(controller) + + allow(controller).to receive(:bcache).and_return(bcache) + allow(controller).to receive(:suitable_caching_devices).and_return(suitable_caching) + allow(controller).to receive(:suitable_backing_devices).and_return(suitable_backing) + end let(:architecture) { :x86_64 } # bcache is only supported on x86_64 + + let(:controller) { instance_double(Y2Partitioner::Actions::Controllers::Bcache) } + + let(:bcache) { nil } let(:suitable_backing) { fake_devicegraph.blk_devices } let(:suitable_caching) { fake_devicegraph.blk_devices + fake_devicegraph.bcache_csets } - subject { described_class.new(suitable_backing, suitable_caching) } + subject { described_class.new(controller) } include_examples "CWM::Dialog" From 2a3dca2b117d3e1cc93a5fd4d02707fa30cb86ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Fri, 15 Feb 2019 16:21:52 +0000 Subject: [PATCH 09/25] Adapt action for deleting a bcache --- .../y2partitioner/actions/delete_bcache.rb | 84 +++++--- .../y2partitioner/actions/delete_device.rb | 66 +++---- .../actions/delete_bcache_test.rb | 182 ++++++++++++------ 3 files changed, 205 insertions(+), 127 deletions(-) diff --git a/src/lib/y2partitioner/actions/delete_bcache.rb b/src/lib/y2partitioner/actions/delete_bcache.rb index a7e46ab4a5..6d3c42a272 100644 --- a/src/lib/y2partitioner/actions/delete_bcache.rb +++ b/src/lib/y2partitioner/actions/delete_bcache.rb @@ -1,6 +1,6 @@ # encoding: utf-8 -# Copyright (c) [2018] SUSE LLC +# Copyright (c) [2018-2019] SUSE LLC # # All Rights Reserved. # @@ -21,48 +21,66 @@ require "yast" require "y2partitioner/actions/delete_device" -require "y2partitioner/device_graphs" +require "y2partitioner/actions/controllers/bcache" module Y2Partitioner module Actions - # Action for deleting a Bcache - # - # @see DeleteDevice + # Action for deleting a Bcache, see {Actions::DeleteBcache} class DeleteBcache < DeleteDevice - def initialize(*args) + # Constructor + # + # @param bcache [Y2Storage::Bcache] + def initialize(bcache) super + textdomain "storage" + + @bcache_controller = Controllers::Bcache.new(bcache) end private - # @see DeleteDevice#errors + # @return [Controllers::Bcache] + attr_reader :bcache_controller + + # @see Actions::Base#errors def errors - errors = super + [shared_cset] + (super + [flash_only_error, unsafe_detach_error]).compact + end + + # Error when the bcache is Flash-only + # + # @return [String, nil] nil if the bcache is not Flash-only. + def flash_only_error + return nil unless flash_only_bcache? - errors.compact + # TRANSLATORS: error message, %{name} is replaced by a bcache name (e.g., /dev/bcache0) + format( + _("The device %{name} is a Flash-only Bcache. Deleting this kind of devices\n" \ + "is not supported yet."), + name: bcache_controller.bcache.name + ) end - # Error when the bcache shares cset with any other bcache. + # Error when the caching set cannot be detached safely # # @see doc/bcache.md - # @return [String, nil] nil if there is no error - def shared_cset - return nil unless device.bcache_cset - system = Y2Partitioner::DeviceGraphs.instance.system - return nil if single_bcache_cset? || !device.exists_in_devicegraph?(system) + # + # @return [String, nil] nil if detach is safe + def unsafe_detach_error + return nil if safe_detach_bcache? - # TRANSLATORS: Error when trying to modify an existing bcache that shares caches. + # TRANSLATORS: Error message when detach is not a safe action _( - "The bcache shares its cache set with other devices.\nThis can result in unreachable space " \ - "if done without detaching.\nDetaching can take a very long time in some situations." + "The bcache cannot be deleted because it shares its cache set with other devices.\n" \ + "Detaching is required to avoid possible unreachable space, but detaching action\n" \ + "could take a very long time in some situations." ) end - # Deletes the indicated Bcache (see {DeleteDevice#device}) + # Deletes the indicated Bcache (see {Actions::DeleteDevice#device}) def delete - log.info "deleting bcache #{device}" - device_graph.remove_bcache(device) + bcache_controller.delete_bcache end # @see DeleteDevice @@ -75,13 +93,12 @@ def recursive_confirm_text_below bcache_cset_note + super end - # Note explaining that also bcache cset will be deleted + # Note explaining that the caching set will be deleted # - # @return [String] empty string if the bcache cset is not going - # to be deleted + # @return [String] empty string if the caching set is not going to be deleted. def bcache_cset_note # no note if there is no bcache cset associated or if cset is shared by more devices - return "" unless single_bcache_cset? + return "" unless bcache_controller.single_committed_bcache_cset? _( "The selected Bcache is the only one using its caching set.\n" \ @@ -89,9 +106,20 @@ def bcache_cset_note ) end - # Checks if there is only single bcache cset used by this bcache, so it will be deleted - def single_bcache_cset? - device.bcache_cset && device.bcache_cset.bcaches.size == 1 + # Whether the bcache is Flash-only + # + # @return [Boolean] + def flash_only_bcache? + bcache_controller.bcache.flash_only? + end + + # Whether the caching set can be safely detached + # + # @return [Boolean] + def safe_detach_bcache? + !bcache_controller.committed_bcache? || + !bcache_controller.committed_bcache_cset? || + bcache_controller.single_committed_bcache_cset? end end end diff --git a/src/lib/y2partitioner/actions/delete_device.rb b/src/lib/y2partitioner/actions/delete_device.rb index fbc51c70ed..5c906ad950 100644 --- a/src/lib/y2partitioner/actions/delete_device.rb +++ b/src/lib/y2partitioner/actions/delete_device.rb @@ -1,6 +1,6 @@ # encoding: utf-8 -# Copyright (c) [2017] SUSE LLC +# Copyright (c) [2017-2019] SUSE LLC # # All Rights Reserved. # @@ -20,11 +20,11 @@ # find current contact information at www.suse.com. require "yast" -require "yast/i18n" require "yast2/popup" require "y2partitioner/device_graphs" require "y2partitioner/confirm_recursive_delete" require "y2partitioner/immediate_unmount" +require "y2partitioner/actions/base" require "y2partitioner/actions/controllers/blk_device" require "y2storage/filesystems/btrfs" require "abstract_method" @@ -32,76 +32,56 @@ module Y2Partitioner module Actions # Base class for the action to delete a device - class DeleteDevice + class DeleteDevice < Base include Yast::Logger - include Yast::I18n include Yast::UIShortcuts include ConfirmRecursiveDelete include ImmediateUnmount # Constructor + # # @param device [Y2Storage::Device] def initialize(device) + super() + textdomain "storage" @device = device end - # Checks whether delete action can be performed and if so, a confirmation popup is shown. - # It only asks for unmounting the device it is currently mounted in the system. - # - # @note Delete action and refresh for shadowing of BtrFS subvolumes - # are only performed when user confirms. - # - # @see Y2Storage::Filesystems::Btrfs.refresh_subvolumes_shadowing - # - # @return [Symbol, nil] - def run - return :back unless validate && try_unmount && confirm - delete - Y2Storage::Filesystems::Btrfs.refresh_subvolumes_shadowing(device_graph) - - :finish - end - private # @return [Y2Storage::Device] device to delete attr_reader :device - # Current devicegraph - # - # @return [Y2Storage::Devicegraph] - def device_graph - DeviceGraphs.instance.current - end - # Deletes the indicated device # - # @note Derived classes should implement this method. + # Derived classes should implement this method. abstract_method :delete - # Validations before performing the delete action + # Checks whether delete action can be performed and if so, a confirmation popup is shown. + # It only asks for unmounting the device it is currently mounted in the system. # - # @note The action can be performed is there are no errors (see #errors). - # Only the first error is shown. + # @see Actions::Base#run? # # @return [Boolean] - def validate - current_errors = errors - return true if current_errors.empty? - - Yast2::Popup.show(current_errors.first, headline: :error) - false + def run? + super && try_unmount && confirm end - # List of errors that avoid to delete the device + # Deletes the device and refreshes the shadowing BtrFS subvolumes # - # @note Derived classes should overload this method. + # @see Y2Storage::Filesystems::Btrfs.refresh_subvolumes_shadowing + def perform_action + delete + Y2Storage::Filesystems::Btrfs.refresh_subvolumes_shadowing(device_graph) + end + + # Current devicegraph # - # @return [Array] - def errors - [] + # @return [Y2Storage::Devicegraph] + def device_graph + DeviceGraphs.instance.current end # Confirmation before performing the delete action diff --git a/test/y2partitioner/actions/delete_bcache_test.rb b/test/y2partitioner/actions/delete_bcache_test.rb index 57a991b15e..fd6436c098 100644 --- a/test/y2partitioner/actions/delete_bcache_test.rb +++ b/test/y2partitioner/actions/delete_bcache_test.rb @@ -1,7 +1,7 @@ #!/usr/bin/env rspec # encoding: utf-8 -# Copyright (c) [2018] SUSE LLC +# Copyright (c) [2018-2019] SUSE LLC # # All Rights Reserved. # @@ -26,6 +26,12 @@ require "y2partitioner/actions/delete_bcache" describe Y2Partitioner::Actions::DeleteBcache do + def bcache_cset_only_for(bcache) + bcache.bcache_cset.bcaches.each do |dev| + dev.remove_bcache_cset if dev != bcache + end + end + before do devicegraph_stub(scenario) end @@ -34,12 +40,8 @@ let(:architecture) { :x86_64 } # bcache is only supported on x86_64 - let(:scenario) { "bcache1.xml" } - let(:device) { Y2Storage::BlkDevice.find_by_name(device_graph, device_name) } - let(:device_name) { "/dev/bcache1" } - let(:device_graph) { Y2Partitioner::DeviceGraphs.instance.current } describe "#run" do @@ -49,88 +51,156 @@ let(:accept) { nil } - context "when deleting probed bcache which share cache with other bcache" do - let(:device_name) { "/dev/bcache1" } - - it "shows error popup" do - expect(Yast2::Popup).to receive(:show).with(anything, headline: :error) - subject.run - end - + shared_examples "not delete bcache" do it "does not delete the bcache" do subject.run + expect(Y2Storage::BlkDevice.find_by_name(device_graph, device_name)).to_not be_nil end it "returns :back" do - expect(subject.run).to eq :back + expect(subject.run).to eq(:back) end end - context "when deleting a bcache without associated devices" do - let(:device_name) { "/dev/bcache1" } + shared_examples "confirm and delete bcache" do + context "when the bcache is not being used (e.g., partitioned)" do + before do + device.remove_descendants + end - it "shows a confirm message" do - device_graph.remove_bcache(Y2Storage::BlkDevice.find_by_name(device_graph, "/dev/bcache2")) - device_graph.remove_bcache(Y2Storage::BlkDevice.find_by_name(device_graph, "/dev/bcache0")) - expect(Yast2::Popup).to receive(:show) - subject.run + it "shows a simple confirm message" do + expect(Yast2::Popup).to receive(:show).with(/Really delete/, anything) + + subject.run + end end - it "adds note that also cset will be deleted if the bcache is only user of it" do - device_graph.remove_bcache(Y2Storage::BlkDevice.find_by_name(device_graph, "/dev/bcache2")) - device_graph.remove_bcache(Y2Storage::BlkDevice.find_by_name(device_graph, "/dev/bcache0")) + context "when the bcache is being used (e.g., partitioned)" do + before do + device.remove_descendants - expect(Yast2::Popup).to receive(:show).with(/only one using its caching set/, anything) - subject.run + device.create_partition_table(Y2Storage::PartitionTables::Type::GPT) + slot = device.partition_table.unused_partition_slots.first + device.partition_table.create_partition(slot.name, + slot.region, Y2Storage::PartitionType::PRIMARY) + end + + it "shows a specific confirm message for recursive delete" do + expect(subject).to receive(:confirm_recursive_delete).and_call_original + + subject.run + end + end + + context "when the confirm message is not accepted" do + let(:accept) { :no } + + include_examples "not delete bcache" + end + + context "when the confirm message is accepted" do + let(:accept) { :yes } + + it "deletes the bcache" do + subject.run + + expect(device_graph.find_by_name(device_name)).to be_nil + end + + it "returns :finish" do + expect(subject.run).to eq(:finish) + end end end - context "when deleting a bcache with associated devices" do - let(:device_name) { "/dev/bcache2" } + context "when the device is a Flash-only bcache" do + let(:scenario) { "bcache2.xml" } - it "shows a specific confirm message for recursive delete" do - device_graph.remove_bcache(Y2Storage::BlkDevice.find_by_name(device_graph, "/dev/bcache1")) - device_graph.remove_bcache(Y2Storage::BlkDevice.find_by_name(device_graph, "/dev/bcache0")) - expect(subject).to receive(:confirm_recursive_delete) - .and_call_original + let(:device_name) { "/dev/bcache1" } + + it "shows an error message" do + expect(Yast2::Popup).to receive(:show).with(/is a Flash-only/, headline: :error) subject.run end + + include_examples "not delete bcache" end - context "when the confirm message is not accepted" do - let(:accept) { :no } + context "when the bcache already exists on disk" do + let(:scenario) { "bcache1.xml" } - it "does not delete the bcache" do - device_graph.remove_bcache(Y2Storage::BlkDevice.find_by_name(device_graph, "/dev/bcache2")) - device_graph.remove_bcache(Y2Storage::BlkDevice.find_by_name(device_graph, "/dev/bcache0")) - subject.run - expect(Y2Storage::BlkDevice.find_by_name(device_graph, device_name)).to_not be_nil + let(:device_name) { "/dev/bcache1" } + + let(:system_device) { Y2Partitioner::DeviceGraphs.instance.system.find_device(device.sid) } + + context "and it had a caching set on disk" do + before do + # Only to ensure the pre-condition is fulfilled + expect(system_device.bcache_cset).to_not be_nil + end + + context "and its caching set was shared with other bcaches" do + before do + # Only to ensure the pre-condition is fulfilled + expect(system_device.bcache_cset.bcaches.size).to be > 1 + end + + it "shows an error message" do + expect(Yast2::Popup).to receive(:show).with(/Detaching is required/, headline: :error) + + subject.run + end + + include_examples "not delete bcache" + end + + context "and its caching set was not shared" do + before do + bcache_cset_only_for(system_device) + end + + include_examples "confirm and delete bcache" + end end - it "returns :back" do - device_graph.remove_bcache(Y2Storage::BlkDevice.find_by_name(device_graph, "/dev/bcache2")) - device_graph.remove_bcache(Y2Storage::BlkDevice.find_by_name(device_graph, "/dev/bcache0")) - expect(subject.run).to eq(:back) + context "and it had no caching set on disk" do + before do + system_device.remove_bcache_cset + end + + context "and currently it has not caching set either" do + before do + device.remove_bcache_cset + end + + include_examples "confirm and delete bcache" + end + + context "and currently it has a caching set" do + before do + # Only to ensure the pre-condition is fulfilled + expect(device.bcache_cset).to_not be_nil + end + + include_examples "confirm and delete bcache" + end end end - context "when the confirm message is accepted" do - let(:accept) { :yes } + context "when the bcache does not exist on disk" do + let(:scenario) { "bcache1.xml" } - it "deletes the bcache" do - device_graph.remove_bcache(Y2Storage::BlkDevice.find_by_name(device_graph, "/dev/bcache2")) - device_graph.remove_bcache(Y2Storage::BlkDevice.find_by_name(device_graph, "/dev/bcache0")) - subject.run - expect(Y2Storage::BlkDevice.find_by_name(device_graph, device_name)).to be_nil - end + let(:device_name) { "/dev/bcache99" } + + before do + vda1 = device_graph.find_by_name("/dev/vda1") - it "returns :finish" do - device_graph.remove_bcache(Y2Storage::BlkDevice.find_by_name(device_graph, "/dev/bcache2")) - device_graph.remove_bcache(Y2Storage::BlkDevice.find_by_name(device_graph, "/dev/bcache0")) - expect(subject.run).to eq(:finish) + vda1.create_bcache(device_name) end + + include_examples "confirm and delete bcache" end end end From 58ccc1f0fc20f042647659655f05e839429acab3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Fri, 15 Feb 2019 16:22:42 +0000 Subject: [PATCH 10/25] Add action for editing a bcache --- src/lib/y2partitioner/actions/edit_bcache.rb | 112 ++++++++ .../y2partitioner/actions/edit_bcache_test.rb | 253 ++++++++++++++++++ 2 files changed, 365 insertions(+) create mode 100644 src/lib/y2partitioner/actions/edit_bcache.rb create mode 100644 test/y2partitioner/actions/edit_bcache_test.rb diff --git a/src/lib/y2partitioner/actions/edit_bcache.rb b/src/lib/y2partitioner/actions/edit_bcache.rb new file mode 100644 index 0000000000..a9b4c9be9f --- /dev/null +++ b/src/lib/y2partitioner/actions/edit_bcache.rb @@ -0,0 +1,112 @@ +# encoding: utf-8 + +# Copyright (c) [2019] SUSE LLC +# +# All Rights Reserved. +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of version 2 of the GNU General Public License as published +# by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, contact SUSE LLC. +# +# To contact SUSE LLC about this file by physical or electronic mail, you may +# find current contact information at www.suse.com. + +require "yast" +require "y2partitioner/actions/base" +require "y2partitioner/actions/controllers/bcache" +require "y2partitioner/dialogs/bcache" + +module Y2Partitioner + module Actions + # Action for editing a bcache device, see {Actions::Base} + class EditBcache < Base + # Constructor + # + # @param bcache [Y2Storage::Bcache] + def initialize(bcache) + super() + + textdomain "storage" + + @controller = Controllers::Bcache.new(bcache) + end + + private + + # @return [Controllers::Bcache] + attr_reader :controller + + # List of errors that avoid to edit a Bcache + # + # @see Actions::Base#errors + # + # @return [Array] + def errors + (super + [flash_only_error, non_editable_error]).compact + end + + # Error when the bcache is Flash-only + # + # @return [String, nil] nil if the bcache is not Flash-only. + def flash_only_error + return nil unless flash_only_bcache? + + # TRANSLATORS: error message, %{name} is replaced by a bcache name (e.g., /dev/bcache0) + format( + _("The device %{name} is a Flash-only Bcache and its caching cannot be modified."), + name: controller.bcache.name + ) + end + + # Error when the caching device cannot be modified, see {#editable_bcache_cset?} + # + # @return [String, nil] nil if the caching device can be modified. + def non_editable_error + return nil if editable_bcache_cset? + + # TRANSLATORS: error message, %{name} is replaced by a bcache name (e.g., /dev/bcache0) + format( + _("The Bcache %{name} is already created on disk and its caching\n" \ + "cannot be modified. To modify the caching device, remove the Bcache\n" \ + "and create it again."), + name: controller.bcache.name + ) + end + + # Opens a dialog to edit a Bcache + # + # The Bcache is updated only if the dialog is accepted. + # + # @see Actions::Base#perform_action + def perform_action + dialog = Dialogs::Bcache.new(controller) + + return unless dialog.run == :next + + controller.update_bcache(dialog.caching_device, dialog.options) + end + + # Whether the bcache is Flash-only + # + # @return [Boolean] + def flash_only_bcache? + controller.bcache.flash_only? + end + + # Whether the caching set can be modified + # + # @return [Boolean] + def editable_bcache_cset? + !controller.committed_bcache? || !controller.committed_bcache_cset? + end + end + end +end diff --git a/test/y2partitioner/actions/edit_bcache_test.rb b/test/y2partitioner/actions/edit_bcache_test.rb new file mode 100644 index 0000000000..9ef27e736d --- /dev/null +++ b/test/y2partitioner/actions/edit_bcache_test.rb @@ -0,0 +1,253 @@ +#!/usr/bin/env rspec +# encoding: utf-8 + +# Copyright (c) [2019] SUSE LLC +# +# All Rights Reserved. +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of version 2 of the GNU General Public License as published +# by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, contact SUSE LLC. +# +# To contact SUSE LLC about this file by physical or electronic mail, you may +# find current contact information at www.suse.com + +require_relative "../test_helper" + +require "cwm/rspec" +require "y2partitioner/actions/edit_bcache" + +describe Y2Partitioner::Actions::EditBcache do + subject { described_class.new(device) } + + before do + devicegraph_stub(scenario) + + allow_any_instance_of(Y2Partitioner::Dialogs::Bcache).to receive(:run) + .and_return(dialog_result) + + allow_any_instance_of(Y2Partitioner::Dialogs::Bcache).to receive(:caching_device) + .and_return(selected_caching) + + allow_any_instance_of(Y2Partitioner::Dialogs::Bcache).to receive(:options) + .and_return(selected_options) + end + + let(:dialog_result) { nil } + + let(:selected_caching) { nil } + + let(:selected_options) { { cache_mode: Y2Storage::CacheMode::WRITEBACK } } + + let(:device) { device_graph.find_by_name(device_name) } + + let(:device_graph) { Y2Partitioner::DeviceGraphs.instance.current } + + describe "#run" do + before do + allow(Yast2::Popup).to receive(:show) + end + + shared_examples "not edit device" do + it "does not modify the caching device" do + bcache_cset = device.bcache_cset + + subject.run + + expect(device.bcache_cset).to eq(bcache_cset) + end + + it "does not modify the cache mode" do + cache_mode = device.cache_mode + + subject.run + + expect(device.cache_mode).to eq(cache_mode) + end + end + + shared_examples "not edit action" do + it "does not open a dialog to edit a bcache device" do + expect(Y2Partitioner::Dialogs::Bcache).to_not receive(:new) + + subject.run + end + + include_examples "not edit device" + + it "returns :back" do + expect(subject.run).to eq(:back) + end + end + + shared_examples "edit action" do + it "does not show an error popup" do + expect(Yast2::Popup).to_not receive(:show) + + subject.run + end + + it "opens a dialog to edit the bcache device" do + expect(Y2Partitioner::Dialogs::Bcache).to receive(:new).and_call_original + + subject.run + end + + context "when the dialog is accepted" do + let(:dialog_result) { :next } + + context "and a caching device was selected in the dialog" do + before do + # Only to ensure the pre-condition is fulfilled + expect(selected_caching).to_not be_nil + end + + it "attaches the selected caching device" do + subject.run + + expect(device.bcache_cset).to_not be_nil + expect(device.bcache_cset.blk_devices.first).to eq(selected_caching) + end + + it "sets the selected cache mode" do + subject.run + + expect(device.cache_mode).to eq(selected_options[:cache_mode]) + end + + it "returns :finish" do + expect(subject.run).to eq :finish + end + end + + context "when no caching device was selected in the dialog" do + let(:selected_caching) { nil } + + it "does not attach a caching device" do + subject.run + + expect(device.bcache_cset).to be_nil + end + + it "sets the selected cache mode" do + subject.run + + expect(device.cache_mode).to eq(selected_options[:cache_mode]) + end + + it "returns :finish" do + expect(subject.run).to eq :finish + end + end + end + + context "when the dialog is discarded" do + let(:dialog_result) { :back } + + include_examples "not edit device" + + it "returns :finish" do + expect(subject.run).to eq :finish + end + end + end + + context "when the device is a Flash-only bcache" do + let(:scenario) { "bcache2.xml" } + + let(:device_name) { "/dev/bcache1" } + + before do + # Only to ensure the pre-condition is fulfilled + expect(device.flash_only?).to eq(true) + end + + it "shows an error message" do + expect(Yast2::Popup).to receive(:show).with(/is a Flash-only/, headline: :error) + + subject.run + end + + include_examples "not edit action" + end + + context "when the bcache already exists on disk" do + let(:system_device) { Y2Partitioner::DeviceGraphs.instance.system.find_device(device.sid) } + + let(:scenario) { "bcache1.xml" } + + let(:device_name) { "/dev/bcache1" } + + # Preparing a value for examples that need it (see shared examples "edit bcache") + let(:selected_caching) { device_graph.find_by_name("/dev/vda2") } + + before do + # Only to ensure the pre-condition is fulfilled + expect(system_device).to_not be_nil + end + + context "and it had a caching set on disk" do + before do + # Only to ensure the pre-condition is fulfilled + expect(system_device.bcache_cset).to_not be_nil + end + + it "shows an error message" do + expect(Yast2::Popup).to receive(:show).with(/already created/, headline: :error) + + subject.run + end + + include_examples "not edit action" + end + + context "and it did not have a caching set on disk" do + before do + system_device.remove_bcache_cset + end + + context "and it currently has no caching set either" do + before do + device.remove_bcache_cset + end + + include_examples "edit action" + end + + context "and it currently has a caching set" do + before do + # Only to ensure the pre-condition is fulfilled + expect(device.bcache_cset).to_not be_nil + end + + include_examples "edit action" + end + end + end + + context "when the bcache does not exist on disk" do + let(:scenario) { "bcache1.xml" } + + before do + vda1 = device_graph.find_by_name("/dev/vda1") + + vda1.create_bcache(device_name) + end + + let(:device_name) { "/dev/bcache99" } + + # Preparing a value for examples that need it (see shared examples "edit bcache") + let(:selected_caching) { device_graph.find_by_name("/dev/vda2") } + + include_examples "edit action" + end + end +end From 753a3ce5ae151c71adef20ff6cbcd21f5cf5b1a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Fri, 15 Feb 2019 16:25:26 +0000 Subject: [PATCH 11/25] Add buttons for editing a bcache --- .../widgets/bcache_edit_button.rb | 53 ++++++++++++++++ .../widgets/bcache_modify_button.rb | 8 ++- .../widgets/bcache_edit_button_test.rb | 62 +++++++++++++++++++ .../widgets/bcache_modify_button_test.rb | 19 +++++- 4 files changed, 139 insertions(+), 3 deletions(-) create mode 100644 src/lib/y2partitioner/widgets/bcache_edit_button.rb create mode 100644 test/y2partitioner/widgets/bcache_edit_button_test.rb diff --git a/src/lib/y2partitioner/widgets/bcache_edit_button.rb b/src/lib/y2partitioner/widgets/bcache_edit_button.rb new file mode 100644 index 0000000000..f1b4c6b273 --- /dev/null +++ b/src/lib/y2partitioner/widgets/bcache_edit_button.rb @@ -0,0 +1,53 @@ +# encoding: utf-8 + +# Copyright (c) [2019] SUSE LLC +# +# All Rights Reserved. +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of version 2 of the GNU General Public License as published +# by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, contact SUSE LLC. +# +# To contact SUSE LLC about this file by physical or electronic mail, you may +# find current contact information at www.suse.com. + +require "yast" +require "y2partitioner/widgets/device_button" +require "y2partitioner/actions/edit_bcache" + +Yast.import "Popup" + +module Y2Partitioner + module Widgets + # Button for editing a bcache + class BcacheEditButton < DeviceButton + def initialize(*args) + super + textdomain "storage" + end + + # @macro seeAbstractWidget + def label + # TRANSLATORS: label for button to edit a bcache + _("Change Caching...") + end + + private + + # Returns the proper Actions class to edit the bcache + # + # @see Actions::EditBcache + def actions_class + Actions::EditBcache + end + end + end +end diff --git a/src/lib/y2partitioner/widgets/bcache_modify_button.rb b/src/lib/y2partitioner/widgets/bcache_modify_button.rb index 77e5aa119a..bd83b85c19 100644 --- a/src/lib/y2partitioner/widgets/bcache_modify_button.rb +++ b/src/lib/y2partitioner/widgets/bcache_modify_button.rb @@ -1,6 +1,6 @@ # encoding: utf-8 -# Copyright (c) [2018] SUSE LLC +# Copyright (c) [2018-2019] SUSE LLC # # All Rights Reserved. # @@ -22,6 +22,7 @@ require "yast" require "y2partitioner/widgets/device_menu_button" require "y2partitioner/actions/edit_blk_device" +require "y2partitioner/actions/edit_bcache" require "y2partitioner/actions/create_partition_table" module Y2Partitioner @@ -50,6 +51,11 @@ def actions label: _("Edit Bcache..."), class: Actions::EditBlkDevice }, + { + id: :caching, + label: _("Change Caching..."), + class: Actions::EditBcache + }, { id: :ptable, label: _("Create New Partition Table..."), diff --git a/test/y2partitioner/widgets/bcache_edit_button_test.rb b/test/y2partitioner/widgets/bcache_edit_button_test.rb new file mode 100644 index 0000000000..312f61d1d0 --- /dev/null +++ b/test/y2partitioner/widgets/bcache_edit_button_test.rb @@ -0,0 +1,62 @@ +#!/usr/bin/env rspec +# encoding: utf-8 + +# Copyright (c) [2019] SUSE LLC +# +# All Rights Reserved. +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of version 2 of the GNU General Public License as published +# by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, contact SUSE LLC. +# +# To contact SUSE LLC about this file by physical or electronic mail, you may +# find current contact information at www.suse.com + +require_relative "../test_helper" + +require "cwm/rspec" +require "y2partitioner/widgets/bcache_edit_button" + +describe Y2Partitioner::Widgets::BcacheEditButton do + before do + devicegraph_stub(scenario) + + allow(Y2Partitioner::Actions::EditBcache).to receive(:new).and_return(action) + end + + let(:action) { instance_double(Y2Partitioner::Actions::EditBcache, run: :finish) } + + subject(:button) { described_class.new(device: device) } + + let(:device) { device_graph.find_by_name(device_name) } + + let(:device_graph) { Y2Partitioner::DeviceGraphs.instance.current } + + let(:scenario) { "bcache1.xml" } + + let(:device_name) { "/dev/bcache0" } + + include_examples "CWM::PushButton" + + describe "#handle" do + it "returns :redraw if the action was successful" do + allow(action).to receive(:run).and_return(:finish) + + expect(button.handle).to eq(:redraw) + end + + it "returns nil if the action was not successful" do + allow(action).to receive(:run).and_return(:back) + + expect(button.handle).to be_nil + end + end +end diff --git a/test/y2partitioner/widgets/bcache_modify_button_test.rb b/test/y2partitioner/widgets/bcache_modify_button_test.rb index 94f6d3bfff..f9db6bcd3e 100644 --- a/test/y2partitioner/widgets/bcache_modify_button_test.rb +++ b/test/y2partitioner/widgets/bcache_modify_button_test.rb @@ -1,6 +1,6 @@ #!/usr/bin/env rspec # encoding: utf-8 -# Copyright (c) [2018] SUSE LLC +# Copyright (c) [2018-2019] SUSE LLC # # All Rights Reserved. # @@ -63,7 +63,7 @@ end end - context "when the option for editing the disk is selected" do + context "when the option for editing the bcache is selected" do let(:selected_option) { :"#{widget_id}_edit" } before do @@ -78,6 +78,21 @@ include_examples "handle bcache action result" end + context "when the option for changing the caching is selected" do + let(:selected_option) { :"#{widget_id}_caching" } + + before do + allow(Y2Partitioner::Actions::EditBcache).to receive(:new).and_return action + end + + it "opens the workflow for changing the caching options" do + expect(Y2Partitioner::Actions::EditBcache).to receive(:new) + button.handle(event) + end + + include_examples "handle bcache action result" + end + context "when the option for creating a partition table is selected" do let(:selected_option) { :"#{widget_id}_ptable" } From cf49c68a82f66a36832d7712dffd983a42037634 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Fri, 15 Feb 2019 16:29:37 +0000 Subject: [PATCH 12/25] Show delete button even for Flash-only bcaches --- .../widgets/device_buttons_set.rb | 10 +-- src/lib/y2partitioner/widgets/pages/bcache.rb | 15 ++--- .../widgets/device_buttons_set_test.rb | 67 ++++--------------- .../widgets/pages/bcache_test.rb | 28 ++++---- 4 files changed, 37 insertions(+), 83 deletions(-) diff --git a/src/lib/y2partitioner/widgets/device_buttons_set.rb b/src/lib/y2partitioner/widgets/device_buttons_set.rb index 9b4ee373c5..0a1dc85272 100644 --- a/src/lib/y2partitioner/widgets/device_buttons_set.rb +++ b/src/lib/y2partitioner/widgets/device_buttons_set.rb @@ -130,15 +130,11 @@ def software_raid_buttons # Buttons to display if {#device} is a bcache device def bcache_buttons - buttons = [ + [ BcacheModifyButton.new(device), - PartitionsButton.new(device, pager) + PartitionsButton.new(device, pager), + DeviceDeleteButton.new(pager: pager, device: device) ] - - # TODO: Allow to delete flash-only bcache devices - buttons << DeviceDeleteButton.new(pager: pager, device: device) unless device.flash_only? - - buttons end # Buttons to display if {#device} is a disk device diff --git a/src/lib/y2partitioner/widgets/pages/bcache.rb b/src/lib/y2partitioner/widgets/pages/bcache.rb index 15ea2f7576..96bf64e738 100644 --- a/src/lib/y2partitioner/widgets/pages/bcache.rb +++ b/src/lib/y2partitioner/widgets/pages/bcache.rb @@ -23,6 +23,7 @@ require "y2partitioner/icons" require "y2partitioner/widgets/bcache_device_description" require "y2partitioner/widgets/blk_device_edit_button" +require "y2partitioner/widgets/bcache_edit_button" require "y2partitioner/widgets/device_delete_button" require "y2partitioner/widgets/partition_table_add_button" require "y2partitioner/widgets/partitions_tab" @@ -102,14 +103,12 @@ def contents # @return [Array] def buttons - buttons = [BlkDeviceEditButton.new(device: @bcache)] - - # TODO: Allow to delete flash-only bcache devices - buttons << DeviceDeleteButton.new(device: @bcache) unless @bcache.flash_only? - - buttons << PartitionTableAddButton.new(device: @bcache) - - buttons + [ + BlkDeviceEditButton.new(device: @bcache), + BcacheEditButton.new(device: @bcache), + DeviceDeleteButton.new(device: @bcache), + PartitionTableAddButton.new(device: @bcache) + ] end end end diff --git a/test/y2partitioner/widgets/device_buttons_set_test.rb b/test/y2partitioner/widgets/device_buttons_set_test.rb index 49e639227d..d6b230c880 100644 --- a/test/y2partitioner/widgets/device_buttons_set_test.rb +++ b/test/y2partitioner/widgets/device_buttons_set_test.rb @@ -1,7 +1,7 @@ #!/usr/bin/env rspec # encoding: utf-8 -# Copyright (c) [2018] SUSE LLC +# Copyright (c) [2018-2019] SUSE LLC # # All Rights Reserved. # @@ -101,63 +101,20 @@ context "when targeting a Bcache device" do let(:scenario) { "bcache2.xml" } + let(:device) { device_graph.find_by_name("/dev/bcache0") } - let(:device) { device_graph.find_by_name(device_name) } - - context "and the device is not a flash-only bcache" do - let(:device_name) { "/dev/bcache0" } - - it "replaces the content with buttons to modify and to manage partitions" do - expect(widget).to receive(:replace) do |content| - widgets = Yast::CWM.widgets_in_contents([content]) - expect(widgets.map(&:class)).to include( - Y2Partitioner::Widgets::DeviceButtonsSet::ButtonsBox, - Y2Partitioner::Widgets::BcacheModifyButton, - Y2Partitioner::Widgets::PartitionsButton - ) - end - - widget.device = device - end - - it "includes a button to delete the device" do - expect(widget).to receive(:replace) do |content| - widgets = Yast::CWM.widgets_in_contents([content]) - expect(widgets.map(&:class)).to include( - Y2Partitioner::Widgets::DeviceDeleteButton - ) - end - - widget.device = device - end - end - - context "and the device is a flash-only bcache" do - let(:device_name) { "/dev/bcache1" } - - it "replaces the content with buttons to modify and to manage partitions" do - expect(widget).to receive(:replace) do |content| - widgets = Yast::CWM.widgets_in_contents([content]) - expect(widgets.map(&:class)).to contain_exactly( - Y2Partitioner::Widgets::DeviceButtonsSet::ButtonsBox, - Y2Partitioner::Widgets::BcacheModifyButton, - Y2Partitioner::Widgets::PartitionsButton - ) - end - - widget.device = device + it "replaces the content with buttons to modify, to delete and to manage partitions" do + expect(widget).to receive(:replace) do |content| + widgets = Yast::CWM.widgets_in_contents([content]) + expect(widgets.map(&:class)).to contain_exactly( + Y2Partitioner::Widgets::DeviceButtonsSet::ButtonsBox, + Y2Partitioner::Widgets::BcacheModifyButton, + Y2Partitioner::Widgets::DeviceDeleteButton, + Y2Partitioner::Widgets::PartitionsButton + ) end - it "does not include a button to delete the device" do - expect(widget).to receive(:replace) do |content| - widgets = Yast::CWM.widgets_in_contents([content]) - expect(widgets.map(&:class)).to_not include( - Y2Partitioner::Widgets::DeviceDeleteButton - ) - end - - widget.device = device - end + widget.device = device end end diff --git a/test/y2partitioner/widgets/pages/bcache_test.rb b/test/y2partitioner/widgets/pages/bcache_test.rb index 134826cc68..c794095c64 100644 --- a/test/y2partitioner/widgets/pages/bcache_test.rb +++ b/test/y2partitioner/widgets/pages/bcache_test.rb @@ -1,7 +1,7 @@ #!/usr/bin/env rspec # encoding: utf-8 -# Copyright (c) [2017] SUSE LLC +# Copyright (c) [2017-2019] SUSE LLC # # All Rights Reserved. # @@ -73,22 +73,24 @@ expect(description).to_not be_nil end - context "when the device is not a flash-only bcache" do - let(:device_name) { "/dev/bcache0" } + it "shows a button for editing the device" do + button = widgets.detect { |i| i.is_a?(Y2Partitioner::Widgets::BlkDeviceEditButton) } + expect(button).to_not be_nil + end - it "shows a button for deleting the device" do - button = widgets.detect { |i| i.is_a?(Y2Partitioner::Widgets::DeviceDeleteButton) } - expect(button).to_not be_nil - end + it "shows a button for changing the caching options" do + button = widgets.detect { |i| i.is_a?(Y2Partitioner::Widgets::BcacheEditButton) } + expect(button).to_not be_nil end - context "when the device is a flash-only bcache" do - let(:device_name) { "/dev/bcache1" } + it "shows a button for deleting the device" do + button = widgets.detect { |i| i.is_a?(Y2Partitioner::Widgets::DeviceDeleteButton) } + expect(button).to_not be_nil + end - it "does not show a button for deleting the device" do - button = widgets.detect { |i| i.is_a?(Y2Partitioner::Widgets::DeviceDeleteButton) } - expect(button).to be_nil - end + it "shows a button for configuring the partition table" do + button = widgets.detect { |i| i.is_a?(Y2Partitioner::Widgets::PartitionTableAddButton) } + expect(button).to_not be_nil end end end From 87778302f10fd48863230d6c3510017ddc861afc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Fri, 15 Feb 2019 16:30:27 +0000 Subject: [PATCH 13/25] Fix 'go back' for bcache --- src/lib/y2partitioner/ui_state.rb | 11 ++++++ src/lib/y2partitioner/widgets/pages.rb | 3 +- .../y2partitioner/widgets/pages/bcaches.rb | 3 +- test/y2partitioner/ui_state_test.rb | 36 +++++++++++++++++-- 4 files changed, 48 insertions(+), 5 deletions(-) diff --git a/src/lib/y2partitioner/ui_state.rb b/src/lib/y2partitioner/ui_state.rb index 80f7be520c..e657c1b629 100644 --- a/src/lib/y2partitioner/ui_state.rb +++ b/src/lib/y2partitioner/ui_state.rb @@ -55,6 +55,15 @@ def lvm_label _("Volume Management") end + # Title of the Bcache section + # + # @note See note on {.md_raids_label} about why this looks misplaced. + # + # @return [String] + def bcache_label + _("Bcache") + end + # @return [Integer, nil] if a row must be selected in a table with devices, # this returns the sid of the associated device attr_reader :row_sid @@ -156,6 +165,8 @@ def device_page_candidates(page) [device.sid, device.lvm_vg.sid] elsif device.is?(:lvm_vg) [device.sid, lvm_label] + elsif device.is?(:bcache) + [device.sid, bcache_label] else [device.sid] end diff --git a/src/lib/y2partitioner/widgets/pages.rb b/src/lib/y2partitioner/widgets/pages.rb index 407050809d..0ae5b52ede 100644 --- a/src/lib/y2partitioner/widgets/pages.rb +++ b/src/lib/y2partitioner/widgets/pages.rb @@ -1,6 +1,6 @@ # encoding: utf-8 -# Copyright (c) [2017] SUSE LLC +# Copyright (c) [2017-2019] SUSE LLC # # All Rights Reserved. # @@ -44,4 +44,5 @@ module Pages require "y2partitioner/widgets/pages/device_graph.rb" require "y2partitioner/widgets/pages/summary.rb" require "y2partitioner/widgets/pages/settings.rb" +require "y2partitioner/widgets/pages/bcache.rb" require "y2partitioner/widgets/pages/bcaches.rb" diff --git a/src/lib/y2partitioner/widgets/pages/bcaches.rb b/src/lib/y2partitioner/widgets/pages/bcaches.rb index f124dc158e..5e59182413 100644 --- a/src/lib/y2partitioner/widgets/pages/bcaches.rb +++ b/src/lib/y2partitioner/widgets/pages/bcaches.rb @@ -20,6 +20,7 @@ # find current contact information at www.suse.com. require "y2partitioner/icons" +require "y2partitioner/ui_state" require "y2partitioner/widgets/pages/devices_table" require "y2partitioner/widgets/bcache_add_button" @@ -43,7 +44,7 @@ def initialize(bcaches, pager) # @macro seeAbstractWidget def label - _("Bcache") + UIState.instance.bcache_label end private diff --git a/test/y2partitioner/ui_state_test.rb b/test/y2partitioner/ui_state_test.rb index f640347488..fe3f34eb02 100644 --- a/test/y2partitioner/ui_state_test.rb +++ b/test/y2partitioner/ui_state_test.rb @@ -1,7 +1,7 @@ #!/usr/bin/env rspec # encoding: utf-8 -# Copyright (c) [2017] SUSE LLC +# Copyright (c) [2017-2019] SUSE LLC # # All Rights Reserved. # @@ -27,7 +27,9 @@ describe Y2Partitioner::UIState do subject(:ui_state) { described_class.instance } - before { devicegraph_stub("complex-lvm-encrypt.yml") } + before { devicegraph_stub(scenario) } + + let(:scenario) { "complex-lvm-encrypt.yml" } let(:device) { Y2Storage::BlkDevice.find_by_name(fake_devicegraph, device_name) } @@ -64,9 +66,10 @@ let(:disks_page) { Y2Partitioner::Widgets::Pages::Disks.new(disks, pager) } let(:md_raids_page) { Y2Partitioner::Widgets::Pages::MdRaids.new(pager) } let(:lvm_page) { Y2Partitioner::Widgets::Pages::Lvm.new(pager) } + let(:bcaches_page) { Y2Partitioner::Widgets::Pages::Bcaches.new([], pager) } let(:btrfs_page) { Y2Partitioner::Widgets::Pages::Btrfs.new(pager) } - let(:pages) { [system_page, disks_page, md_raids_page, lvm_page, btrfs_page] } + let(:pages) { [system_page, disks_page, md_raids_page, lvm_page, bcaches_page, btrfs_page] } context "if the user has still not visited any node" do before { described_class.create_instance } @@ -218,6 +221,33 @@ end end end + + context "when the user has opened a Bcache page" do + let(:scenario) { "bcache1.xml" } + let(:device) { fake_devicegraph.find_by_name("/dev/bcache0") } + let(:another_bcache) { fake_devicegraph.find_by_name("/dev/bcache1") } + + let(:page) { Y2Partitioner::Widgets::Pages::Bcache.new(device, pager) } + let(:another_bcache_page) { Y2Partitioner::Widgets::Pages::Bcache.new(another_bcache, pager) } + + before { ui_state.go_to_tree_node(page) } + + context "if the Bcache is still there after redrawing" do + before { pages.concat [page, another_bcache_page] } + + it "selects the correct Bcache page" do + expect(ui_state.find_tree_node(pages)).to eq page + end + end + + context "if the Bcache is not longer there after redrawing" do + before { pages << another_bcache_page } + + it "selects the general Bcache page" do + expect(ui_state.find_tree_node(pages)).to eq bcaches_page + end + end + end end describe "#find_tab" do From 4ffa1ca4fdbe3012bcd9de0911a9d65e087ef464 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Mon, 18 Feb 2019 15:18:45 +0000 Subject: [PATCH 14/25] Remove backing device content when needed --- .../actions/controllers/bcache.rb | 36 +++++++++++++++++-- .../actions/controllers/bcache_test.rb | 35 ++++++++++++++++++ 2 files changed, 68 insertions(+), 3 deletions(-) diff --git a/src/lib/y2partitioner/actions/controllers/bcache.rb b/src/lib/y2partitioner/actions/controllers/bcache.rb index 18d9b753b3..b066147832 100644 --- a/src/lib/y2partitioner/actions/controllers/bcache.rb +++ b/src/lib/y2partitioner/actions/controllers/bcache.rb @@ -71,6 +71,8 @@ def create_bcache(backing_device, caching_device, options) BlkDeviceRestorer.new(backing_device).update_checkpoint + backing_device.remove_descendants if remove_backing_device_content?(backing_device) + @bcache = backing_device.create_bcache(Y2Storage::Bcache.find_free_name(current_graph)) apply_options(options) @@ -244,7 +246,7 @@ def create_bcache_cset(caching_device) caching_device.create_bcache_cset end - # Whether the caching set can be removed + # Whether the caching set should be removed # # @param bcache_cset [Y2Storage::BcacheCset] def remove_bcache_cset?(bcache_cset) @@ -264,6 +266,18 @@ def remove_bcache_cset(bcache_cset) BlkDeviceRestorer.new(caching_device).restore_from_checkpoint end + # Whether the content of the backing device should be removed + # + # The content of the backing device should be removed when the backing device + # already contains something on the disk (e.g., a filesystem). + # + # @return [Boolean] + def remove_backing_device_content?(backing_device) + return false unless committed_device?(backing_device) + + backing_device.descendants.any? { |d| committed_device?(d) } + end + # Bcache existing on disk # # @return [Y2Storage::Bcache, nil] nil if the bcache is being created or @@ -271,7 +285,7 @@ def remove_bcache_cset(bcache_cset) def committed_bcache return nil unless bcache - system_graph.find_device(bcache.sid) + committed_device(bcache) end # Caching set of the bcache existing on disk @@ -279,11 +293,27 @@ def committed_bcache # @return [Y2Storage::BcacheCset, nil] nil if the bcache does not exist # on disk or it has no caching set. def committed_bcache_cset - return nil unless committed_bcache + return nil unless committed_bcache? committed_bcache.bcache_cset end + # Whether the device already exists on disk + # + # @param device [Y2Storage::Device] + # @return [Boolean] + def committed_device?(device) + !committed_device(device).nil? + end + + # System version of the given device + # + # @param device [Y2Storage::Device] + # @return [Y2Storage::Device, nil] nil if the device does not exist on disk. + def committed_device(device) + system_graph.find_device(device.sid) + end + # Current devicegraph in which the action operates on # # @return [Y2Storage::Devicegraph] diff --git a/test/y2partitioner/actions/controllers/bcache_test.rb b/test/y2partitioner/actions/controllers/bcache_test.rb index 0d10e94e0f..1b3b23824f 100644 --- a/test/y2partitioner/actions/controllers/bcache_test.rb +++ b/test/y2partitioner/actions/controllers/bcache_test.rb @@ -233,6 +233,10 @@ def name_of_devices let(:options) { { cache_mode: Y2Storage::CacheMode::WRITEBACK } } + let(:system_backing_device) { system_graph.find_by_name(backing_device.name) } + + let(:system_graph) { Y2Partitioner::DeviceGraphs.instance.system } + it "creates a new bcache over the given backing device" do expect(current_graph.find_by_name("/dev/bcache3")).to be_nil @@ -255,6 +259,37 @@ def name_of_devices expect(bcache.cache_mode).to eq(options[:cache_mode]) end + context "when the given backing device has some content on disk" do + it "creates a new bcache without the content of the backing device" do + expect(system_backing_device.descendants).to_not be_empty + + subject.create_bcache(backing_device, caching_device, options) + + bcache = current_graph.find_by_name("/dev/bcache3") + + expect(bcache.descendants).to be_empty + end + end + + context "when the given backing device has content only in memory" do + before do + backing_device.remove_descendants + backing_device.create_filesystem(Y2Storage::Filesystems::Type::EXT4) + end + + it "creates a new bcache with the content of the backing device" do + content_before = backing_device.descendants + expect(content_before).to_not be_empty + + subject.create_bcache(backing_device, caching_device, options) + + bcache = current_graph.find_by_name("/dev/bcache3") + + expect(bcache.descendants).to_not be_empty + expect(bcache.descendants).to eq(content_before) + end + end + context "when non caching device is given" do let(:caching_device) { nil } From b639f547942a88bd2248b57e2eb91c1f54ee8d0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Mon, 18 Feb 2019 16:23:38 +0000 Subject: [PATCH 15/25] Update libstorage-ng dependency --- package/yast2-storage-ng.spec | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/package/yast2-storage-ng.spec b/package/yast2-storage-ng.spec index 586822cb7b..06e3a30794 100644 --- a/package/yast2-storage-ng.spec +++ b/package/yast2-storage-ng.spec @@ -26,8 +26,8 @@ Source: %{name}-%{version}.tar.bz2 Requires: yast2 >= 4.1.11 # for AbortException and handle direct abort Requires: yast2-ruby-bindings >= 4.0.6 -# Probing Flash-only Bcache devices -Requires: libstorage-ng-ruby >= 4.1.81 +# Bcache#remove_bcache_cset +Requires: libstorage-ng-ruby >= 4.1.89 # communicate with udisks Requires: rubygem(ruby-dbus) # Y2Packager::Repository @@ -36,8 +36,8 @@ Requires: yast2-packager >= 3.3.7 Requires: findutils BuildRequires: update-desktop-files -# Probing Flash-only Bcache devices -BuildRequires: libstorage-ng-ruby >= 4.1.81 +# Bcache#remove_bcache_cset +BuildRequires: libstorage-ng-ruby >= 4.1.89 BuildRequires: yast2-ruby-bindings BuildRequires: yast2-devtools # yast2-xml dependency is added by yast2 but ignored in the From 7fb96370dd275aec8dfc31e1d6068c79a9fb5e86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Mon, 18 Feb 2019 16:26:37 +0000 Subject: [PATCH 16/25] Update version and changelog --- package/yast2-storage-ng.changes | 6 ++++++ package/yast2-storage-ng.spec | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/package/yast2-storage-ng.changes b/package/yast2-storage-ng.changes index 1e3a083fcc..81d83abe37 100644 --- a/package/yast2-storage-ng.changes +++ b/package/yast2-storage-ng.changes @@ -1,3 +1,9 @@ +------------------------------------------------------------------- +Tue Feb 19 14:15:16 UTC 2019 - jlopez@suse.com + +- Partitioner: allow to edit bcache devices (part of fate#325346). +- 4.1.62 + ------------------------------------------------------------------- Tue Feb 19 13:39:49 UTC 2019 - Stefan Hundhammer diff --git a/package/yast2-storage-ng.spec b/package/yast2-storage-ng.spec index 06e3a30794..254459931b 100644 --- a/package/yast2-storage-ng.spec +++ b/package/yast2-storage-ng.spec @@ -16,7 +16,7 @@ # Name: yast2-storage-ng -Version: 4.1.61 +Version: 4.1.62 Release: 0 BuildRoot: %{_tmppath}/%{name}-%{version}-build From 3e3502285378557f8c81590cd4052fce72aea9d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Tue, 19 Feb 2019 16:20:07 +0000 Subject: [PATCH 17/25] Fix row selection for bcache --- src/lib/y2partitioner/actions/add_bcache.rb | 2 ++ src/lib/y2partitioner/actions/controllers/bcache.rb | 3 --- src/lib/y2partitioner/actions/delete_bcache.rb | 2 ++ src/lib/y2partitioner/actions/edit_bcache.rb | 2 ++ 4 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/lib/y2partitioner/actions/add_bcache.rb b/src/lib/y2partitioner/actions/add_bcache.rb index 3a37c750d4..06a0711646 100644 --- a/src/lib/y2partitioner/actions/add_bcache.rb +++ b/src/lib/y2partitioner/actions/add_bcache.rb @@ -23,6 +23,7 @@ require "y2partitioner/actions/base" require "y2partitioner/actions/controllers/bcache" require "y2partitioner/dialogs/bcache" +require "y2partitioner/ui_state" module Y2Partitioner module Actions @@ -71,6 +72,7 @@ def perform_action return unless dialog.run == :next controller.create_bcache(dialog.backing_device, dialog.caching_device, dialog.options) + UIState.instance.select_row(controller.bcache) end # Whether there is suitable backing devices diff --git a/src/lib/y2partitioner/actions/controllers/bcache.rb b/src/lib/y2partitioner/actions/controllers/bcache.rb index b066147832..b30c8c485b 100644 --- a/src/lib/y2partitioner/actions/controllers/bcache.rb +++ b/src/lib/y2partitioner/actions/controllers/bcache.rb @@ -22,7 +22,6 @@ require "yast" require "y2storage/bcache" require "y2partitioner/device_graphs" -require "y2partitioner/ui_state" require "y2partitioner/blk_device_restorer" module Y2Partitioner @@ -39,8 +38,6 @@ class Bcache # @param bcache [Y2Storage::Bcache, nil] nil if a new bcache is being created. def initialize(bcache = nil) @bcache = bcache - - UIState.instance.select_row(bcache) if bcache end # Suitable devices to be used as backing device diff --git a/src/lib/y2partitioner/actions/delete_bcache.rb b/src/lib/y2partitioner/actions/delete_bcache.rb index 6d3c42a272..91066647a4 100644 --- a/src/lib/y2partitioner/actions/delete_bcache.rb +++ b/src/lib/y2partitioner/actions/delete_bcache.rb @@ -22,6 +22,7 @@ require "yast" require "y2partitioner/actions/delete_device" require "y2partitioner/actions/controllers/bcache" +require "y2partitioner/ui_state" module Y2Partitioner module Actions @@ -36,6 +37,7 @@ def initialize(bcache) textdomain "storage" @bcache_controller = Controllers::Bcache.new(bcache) + UIState.instance.select_row(bcache) end private diff --git a/src/lib/y2partitioner/actions/edit_bcache.rb b/src/lib/y2partitioner/actions/edit_bcache.rb index a9b4c9be9f..8296ef0d2b 100644 --- a/src/lib/y2partitioner/actions/edit_bcache.rb +++ b/src/lib/y2partitioner/actions/edit_bcache.rb @@ -23,6 +23,7 @@ require "y2partitioner/actions/base" require "y2partitioner/actions/controllers/bcache" require "y2partitioner/dialogs/bcache" +require "y2partitioner/ui_state" module Y2Partitioner module Actions @@ -37,6 +38,7 @@ def initialize(bcache) textdomain "storage" @controller = Controllers::Bcache.new(bcache) + UIState.instance.select_row(bcache) end private From 7b45ea2a10da11498f0e410786e4333b096bd14d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Tue, 19 Feb 2019 16:21:21 +0000 Subject: [PATCH 18/25] Fix messages text --- src/lib/y2partitioner/actions/delete_bcache.rb | 4 ++-- src/lib/y2partitioner/actions/edit_bcache.rb | 6 +++--- test/y2partitioner/actions/delete_bcache_test.rb | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/lib/y2partitioner/actions/delete_bcache.rb b/src/lib/y2partitioner/actions/delete_bcache.rb index 91066647a4..6967e025ea 100644 --- a/src/lib/y2partitioner/actions/delete_bcache.rb +++ b/src/lib/y2partitioner/actions/delete_bcache.rb @@ -75,8 +75,8 @@ def unsafe_detach_error # TRANSLATORS: Error message when detach is not a safe action _( "The bcache cannot be deleted because it shares its cache set with other devices.\n" \ - "Detaching is required to avoid possible unreachable space, but detaching action\n" \ - "could take a very long time in some situations." + "Deleting it without detaching the device first can result in unreachable space.\n" \ + "Unfortunately detaching can take a very long time in some situations." ) end diff --git a/src/lib/y2partitioner/actions/edit_bcache.rb b/src/lib/y2partitioner/actions/edit_bcache.rb index 8296ef0d2b..61ac453e71 100644 --- a/src/lib/y2partitioner/actions/edit_bcache.rb +++ b/src/lib/y2partitioner/actions/edit_bcache.rb @@ -76,9 +76,9 @@ def non_editable_error # TRANSLATORS: error message, %{name} is replaced by a bcache name (e.g., /dev/bcache0) format( - _("The Bcache %{name} is already created on disk and its caching\n" \ - "cannot be modified. To modify the caching device, remove the Bcache\n" \ - "and create it again."), + _("The Bcache %{name} is already created on disk. Such device cannot be modified\n" \ + "because that might imply a detaching operation. Unfortunately detaching can take\n" \ + "a very long time in some situations."), name: controller.bcache.name ) end diff --git a/test/y2partitioner/actions/delete_bcache_test.rb b/test/y2partitioner/actions/delete_bcache_test.rb index fd6436c098..cc5e5551d2 100644 --- a/test/y2partitioner/actions/delete_bcache_test.rb +++ b/test/y2partitioner/actions/delete_bcache_test.rb @@ -148,7 +148,7 @@ def bcache_cset_only_for(bcache) end it "shows an error message" do - expect(Yast2::Popup).to receive(:show).with(/Detaching is required/, headline: :error) + expect(Yast2::Popup).to receive(:show).with(/cannot be deleted/, headline: :error) subject.run end From 0cce77686eaf5e5d83b8dfa032ca10e326fd1d5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Tue, 19 Feb 2019 16:21:47 +0000 Subject: [PATCH 19/25] Add missing documentation --- src/lib/y2storage/bcache.rb | 6 ++++++ src/lib/y2storage/devicegraph.rb | 6 +++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/lib/y2storage/bcache.rb b/src/lib/y2storage/bcache.rb index 784f320ce3..6be87a96aa 100644 --- a/src/lib/y2storage/bcache.rb +++ b/src/lib/y2storage/bcache.rb @@ -63,6 +63,12 @@ class Bcache < Partitionable # @param set [BcacheCset] set to attach storage_forward :add_bcache_cset + # @!method remove_bcache_cset + # This method does not make sense for Flash-only Bcache devices. + # + # @raise [storage::Exception] if detaching failed + # @raise [storage::LogicException] for a Flash-only Bcache device or when + # the Bcache device has no caching set. storage_forward :remove_bcache_cset # @!attribute cache_mode diff --git a/src/lib/y2storage/devicegraph.rb b/src/lib/y2storage/devicegraph.rb index 2fb341d6fe..9836a97d7e 100644 --- a/src/lib/y2storage/devicegraph.rb +++ b/src/lib/y2storage/devicegraph.rb @@ -381,12 +381,16 @@ def remove_bcache(bcache) remove_with_dependants(bcache_cset) if bcache_cset && bcache_cset.bcaches.empty? end + # Removes a caching set + # + # Bcache devices using this caching set are not removed. + # + # @raise [ArgumentError] if the caching set does not exist in the devicegraph def remove_bcache_cset(bcache_cset) if !(bcache_cset && bcache_cset.is?(:bcache_cset)) raise(ArgumentError, "Incorrect device #{bcache_cset.inspect}") end - # Bcaches using this caching set are not removed remove_device(bcache_cset) end From 019a65f1f2a2f5f6ad3ed56568c06f2a71f397c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Tue, 19 Feb 2019 16:23:27 +0000 Subject: [PATCH 20/25] Add help for bcache attributes --- .../widgets/bcache_device_description.rb | 35 +++++++++++++++---- .../widgets/disk_device_description.rb | 2 +- src/lib/y2partitioner/widgets/help.rb | 14 +++++++- 3 files changed, 43 insertions(+), 8 deletions(-) diff --git a/src/lib/y2partitioner/widgets/bcache_device_description.rb b/src/lib/y2partitioner/widgets/bcache_device_description.rb index d9a6d84f8c..403c8d7f06 100644 --- a/src/lib/y2partitioner/widgets/bcache_device_description.rb +++ b/src/lib/y2partitioner/widgets/bcache_device_description.rb @@ -44,16 +44,30 @@ def device_description # @return [String] def bcache_description output = Yast::HTML.Heading(_("Bcache Devices:")) - output << Yast::HTML.List([ - format(_("Backing Device: %s"), backing_device), - format(_("Caching UUID: %s"), uuid), - format(_("Caching Device: %s"), caching_device), - format(_("Cache mode: %s"), cache_mode) - ]) + output << Yast::HTML.List(bcache_attributes) + end + + # Fields to show in help + # + # @return [Array] + def help_fields + super + bcache_help_fields end private + # Attributes for describing a bcache device + # + # @return [Array] + def bcache_attributes + [ + format(_("Backing Device: %s"), backing_device), + format(_("Caching UUID: %s"), uuid), + format(_("Caching Device: %s"), caching_device), + format(_("Cache Mode: %s"), cache_mode) + ] + end + def uuid device.bcache_cset ? device.bcache_cset.uuid : "" end @@ -79,6 +93,15 @@ def cache_mode device.cache_mode.to_human_string end + + BCACHE_HELP_FIELDS = [:backing_device, :caching_uuid, :caching_device, :cache_mode].freeze + + # Help fields for a bcache device + # + # @return [Array] + def bcache_help_fields + BCACHE_HELP_FIELDS.dup + end end end end diff --git a/src/lib/y2partitioner/widgets/disk_device_description.rb b/src/lib/y2partitioner/widgets/disk_device_description.rb index 58c7a34970..f7fa1f0048 100644 --- a/src/lib/y2partitioner/widgets/disk_device_description.rb +++ b/src/lib/y2partitioner/widgets/disk_device_description.rb @@ -70,7 +70,7 @@ def disk_attributes # # @return [Array] def help_fields - blk_device_help_fields + disk_help_fields + blk_device_help_fields + disk_help_fields + filesystem_help_fields end DISK_HELP_FIELDS = [:vendor, :model, :bus, :sectors, :sector_size, :disk_label].freeze diff --git a/src/lib/y2partitioner/widgets/help.rb b/src/lib/y2partitioner/widgets/help.rb index d1e3028834..e3b93a3470 100644 --- a/src/lib/y2partitioner/widgets/help.rb +++ b/src/lib/y2partitioner/widgets/help.rb @@ -102,7 +102,19 @@ def included(_target) uuid: N_("UUID shows the Universally Unique\nIdentifier of the file " \ "system."), - vendor: N_("Vendor shows the device vendor.") + vendor: N_("Vendor shows the device vendor."), + + backing_device: N_("Backing Device shows the device used as backing device " \ + "for bcache."), + + caching_uuid: N_("Caching UUID shows the UUID of the used caching set. This " \ + "field is empty if no caching is used."), + + caching_device: N_("Caching Device shows the device used for caching. This field " \ + "is empty if no caching is used."), + + cache_mode: N_("Cache Mode shows the operating mode for bcache. Currently there " \ + "are four supported modes: Writethrough, Writeback, Writearound and None.") }.freeze # help texts that are appended to the common help only in Mode.normal From 132e2784878ac3d091cbba7be884dff7f95beda7 Mon Sep 17 00:00:00 2001 From: Ancor Gonzalez Sosa Date: Fri, 15 Feb 2019 17:49:11 +0100 Subject: [PATCH 21/25] Partitioner: button to activate crypt devices --- src/lib/y2partitioner/widgets/configure.rb | 20 +++- src/lib/y2partitioner/widgets/reprobe.rb | 22 ++-- test/support/partitioner_reprobe_examples.rb | 12 +++ test/y2partitioner/widgets/configure_test.rb | 102 ++++++++++++------- 4 files changed, 111 insertions(+), 45 deletions(-) diff --git a/src/lib/y2partitioner/widgets/configure.rb b/src/lib/y2partitioner/widgets/configure.rb index 1378657324..b77ba18d9b 100644 --- a/src/lib/y2partitioner/widgets/configure.rb +++ b/src/lib/y2partitioner/widgets/configure.rb @@ -66,8 +66,8 @@ def handle(event) return nil unless action return nil unless warning_accepted?(action) && availability_ensured?(action) - Yast::WFM.call(action.client) - reprobe + Yast::WFM.call(action.client) if action.client + reprobe(activate: action.activate) :redraw end @@ -183,6 +183,8 @@ def initialize(id, label, icon, client, pkgs) # Sorted list of actions ALL = [ + new(:crypt, N_("Provide Crypt &Passwords..."), "yast-encrypted", nil, + ["cryptsetup"]), new(:iscsi, N_("Configure &iSCSI..."), "yast-iscsi-client", "iscsi-client", ["yast2-iscsi-client"]), new(:fcoe, N_("Configure &FCoE..."), "fcoe", "fcoe-client", @@ -202,6 +204,10 @@ def initialize(id, label, icon, client, pkgs) # are indexed in this constant in order to reuse the existing # translations from yast2-storage WARNING_TEXTS = { + crypt: N_( + "Rescanning crypt devices cancels all current changes.\n" \ + "Really activate crypt devices?" + ), iscsi: N_( "Calling iSCSI configuration cancels all current changes.\n" \ "Really call iSCSI configuration?" @@ -275,6 +281,16 @@ def warning_text def supported? S390_IDS.include?(id) ? Yast::Arch.s390 : true end + + # Value for the 'activate' argument of {Reprobe#reprobe} + # + # For most cases this returns nil, which implies simply honoring the + # default behavior. + # + # @return [Boolean, nil] + def activate + id == :crypt ? true : nil + end end end end diff --git a/src/lib/y2partitioner/widgets/reprobe.rb b/src/lib/y2partitioner/widgets/reprobe.rb index 4075dd4770..5a4eeed4cf 100644 --- a/src/lib/y2partitioner/widgets/reprobe.rb +++ b/src/lib/y2partitioner/widgets/reprobe.rb @@ -41,10 +41,19 @@ module Reprobe # @raise [Y2Partitioner::ForcedAbortError] When there is an error during probing # and the user decides to abort, or probed devicegraph contains errors and the # user decides to not sanitize. - def reprobe + # + # @param activate [Boolean, nil] whether to perform an activation, if nil + # the (re)activation will be done only during installation + def reprobe(activate: nil) textdomain "storage" + + # By default, (re)activation is only done during installation. + # In installed systems, activation is only triggered for actions that + # explicitly force it. + activate = !!Yast::Stage.initial if activate.nil? + Yast::Popup.Feedback("", _("Rescanning disks...")) do - raise Y2Partitioner::ForcedAbortError unless activate_and_probe? + raise Y2Partitioner::ForcedAbortError unless activate_and_probe?(activate) probed = storage_manager.probed staging = storage_manager.staging @@ -59,13 +68,12 @@ def storage_manager # Performs storage reactivation (if needed) and reprobing # - # @note Activation is only done during installation, never in an already - # installed system - # + # @param activate [Boolean] whether to perform a reactivation of devices + # before the reprobing # @return [Boolean] false if something went wrong - def activate_and_probe? + def activate_and_probe?(activate) success = true - success &&= storage_manager.activate if Yast::Stage.initial + success &&= storage_manager.activate if activate success &&= storage_manager.probe success end diff --git a/test/support/partitioner_reprobe_examples.rb b/test/support/partitioner_reprobe_examples.rb index 22ad788cc2..0f80ca55e2 100644 --- a/test/support/partitioner_reprobe_examples.rb +++ b/test/support/partitioner_reprobe_examples.rb @@ -51,3 +51,15 @@ end end end + +RSpec.shared_examples "activation" do + it "runs activation (again)" do + expect(manager).to receive(:activate).and_return true + subject.handle(*handle_args) + end + + it "raises an exception if activation fails" do + allow(manager).to receive(:activate).and_return false + expect { subject.handle(*handle_args) }.to raise_error(Y2Partitioner::ForcedAbortError) + end +end diff --git a/test/y2partitioner/widgets/configure_test.rb b/test/y2partitioner/widgets/configure_test.rb index b5163c68fc..c85de3ea86 100644 --- a/test/y2partitioner/widgets/configure_test.rb +++ b/test/y2partitioner/widgets/configure_test.rb @@ -64,10 +64,11 @@ def menu_button_item_ids(button) context "if all the possible clients are available" do let(:missing_clients) { [] } - it "returns a menu button with buttons for iSCI, FCoE, DASD, zFCP and XPRAM" do + it "returns a menu button with buttons for crypt, iSCI, FCoE, DASD, zFCP and XPRAM" do term = widget.contents expect(term.value).to eq :MenuButton - expect(menu_button_item_ids(term)).to contain_exactly(:iscsi, :fcoe, :dasd, :zfcp, :xpram) + expect(menu_button_item_ids(term)) + .to contain_exactly(:crypt, :iscsi, :fcoe, :dasd, :zfcp, :xpram) end end @@ -77,7 +78,7 @@ def menu_button_item_ids(button) it "returns a menu button with buttons only for the available clients" do term = widget.contents expect(term.value).to eq :MenuButton - expect(menu_button_item_ids(term)).to contain_exactly(:fcoe, :zfcp, :xpram) + expect(menu_button_item_ids(term)).to contain_exactly(:crypt, :fcoe, :zfcp, :xpram) expect(menu_button_item_ids(term)).to_not include(:iscsi, :dasd) end end @@ -85,9 +86,10 @@ def menu_button_item_ids(button) context "if no client is available" do let(:missing_clients) { all_clients } - it "returns an empty term" do + it "returns a menu button with 'Crypt Passwords' as the only option" do term = widget.contents - expect(term.value).to eq :Empty + expect(term.value).to eq :MenuButton + expect(menu_button_item_ids(term)).to eq [:crypt] end end end @@ -98,10 +100,10 @@ def menu_button_item_ids(button) context "if all the possible clients are available" do let(:missing_clients) { [] } - it "returns a menu button with buttons for iSCI and FCoE" do + it "returns a menu button with buttons for crypt, iSCI and FCoE" do term = widget.contents expect(term.value).to eq :MenuButton - expect(menu_button_item_ids(term)).to contain_exactly(:iscsi, :fcoe) + expect(menu_button_item_ids(term)).to contain_exactly(:crypt, :iscsi, :fcoe) end end @@ -111,7 +113,7 @@ def menu_button_item_ids(button) it "returns a menu button with buttons only for the available clients" do term = widget.contents expect(term.value).to eq :MenuButton - expect(menu_button_item_ids(term)).to eq [:fcoe] + expect(menu_button_item_ids(term)).to eq [:crypt, :fcoe] expect(menu_button_item_ids(term)).to_not include(:iscsi) end end @@ -119,9 +121,10 @@ def menu_button_item_ids(button) context "if no client is available" do let(:missing_clients) { all_clients } - it "returns an empty term" do + it "returns a menu button with 'Crypt Passwords' as the only option" do term = widget.contents - expect(term.value).to eq :Empty + expect(term.value).to eq :MenuButton + expect(menu_button_item_ids(term)).to eq [:crypt] end end end @@ -136,20 +139,22 @@ def menu_button_item_ids(button) context "if all the possible clients are available" do let(:missing_clients) { [] } - it "returns a menu button with buttons for all clients (iSCI, FCoE, DASD, zFCP and XPRAM)" do + it "returns a menu button with buttons for all actions" do term = widget.contents expect(term.value).to eq :MenuButton - expect(menu_button_item_ids(term)).to contain_exactly(:iscsi, :fcoe, :dasd, :zfcp, :xpram) + expect(menu_button_item_ids(term)) + .to contain_exactly(:crypt, :iscsi, :fcoe, :dasd, :zfcp, :xpram) end end context "if some clients are not available" do let(:missing_clients) { ["iscsi-client", "dasd"] } - it "returns a menu button with buttons for all clients (iSCI, FCoE, DASD, zFCP and XPRAM)" do + it "returns a menu button with buttons for all actions" do term = widget.contents expect(term.value).to eq :MenuButton - expect(menu_button_item_ids(term)).to contain_exactly(:iscsi, :fcoe, :dasd, :zfcp, :xpram) + expect(menu_button_item_ids(term)) + .to contain_exactly(:crypt, :iscsi, :fcoe, :dasd, :zfcp, :xpram) end end end @@ -160,20 +165,20 @@ def menu_button_item_ids(button) context "if all the possible clients are available" do let(:missing_clients) { [] } - it "returns a menu button with buttons for all supported clients (iSCI and FCoE)" do + it "returns a menu button with buttons for all supported actions (crypt, iSCI and FCoE)" do term = widget.contents expect(term.value).to eq :MenuButton - expect(menu_button_item_ids(term)).to contain_exactly(:iscsi, :fcoe) + expect(menu_button_item_ids(term)).to contain_exactly(:crypt, :iscsi, :fcoe) end end context "if some clients are not available" do let(:missing_clients) { ["iscsi-client"] } - it "returns a menu button with buttons for all supported clients (iSCI and FCoE)" do + it "returns a menu button with buttons for all supported actions (crypt, iSCI and FCoE)" do term = widget.contents expect(term.value).to eq :MenuButton - expect(menu_button_item_ids(term)).to contain_exactly(:iscsi, :fcoe) + expect(menu_button_item_ids(term)).to contain_exactly(:crypt, :iscsi, :fcoe) end end end @@ -240,27 +245,35 @@ def event_for(id) context "if the user accepts the warning" do let(:accepted) { true } - it "calls the corresponding YaST client" do - expect(Yast::WFM).to receive(:call).with("iscsi-client") - widget.handle(event) - end + context "for an action performed via a separate client" do + let(:event) { event_for(:iscsi) } - include_examples "reprobing" + it "calls the corresponding YaST client" do + expect(Yast::WFM).to receive(:call).with("iscsi-client") + widget.handle(event) + end - it "runs activation again" do - expect(manager).to receive(:activate).and_return true - widget.handle(event) + include_examples "reprobing" + include_examples "activation" end - it "raises an exception if activation fails" do - allow(manager).to receive(:activate).and_return false - expect { subject.handle(event) }.to raise_error(Y2Partitioner::ForcedAbortError) + context "for activation of crypt devices" do + let(:event) { event_for(:crypt) } + + it "does not call any additional YaST client" do + expect(Yast::WFM).to_not receive(:call) + widget.handle(event) + end + + include_examples "reprobing" + include_examples "activation" end end end context "in an already installed system" do let(:install) { false } + let(:handle_args) { [event] } include_examples "show configure warning" @@ -292,16 +305,33 @@ def event_for(id) context "if the packages were installed or already there" do let(:installed_pkgs) { true } - it "calls the corresponding YaST client" do - expect(Yast::WFM).to receive(:call).with("iscsi-client") - widget.handle(event) + context "for an action performed via a separate client" do + let(:event) { event_for(:iscsi) } + + it "calls the corresponding YaST client" do + expect(Yast::WFM).to receive(:call).with("iscsi-client") + widget.handle(event) + end + + include_examples "reprobing" + + it "does not run activation" do + expect(manager).to_not receive(:activate) + widget.handle(event) + end end - include_examples "reprobing" + context "for activation of crypt devices" do + let(:event) { event_for(:crypt) } + before { allow(manager).to receive(:activate).and_return true } - it "does not run activation" do - expect(manager).to_not receive(:activate) - widget.handle(event) + it "does not call any additional YaST client" do + expect(Yast::WFM).to_not receive(:call) + widget.handle(event) + end + + include_examples "reprobing" + include_examples "activation" end end end From 889a2852e3a001e161eebdfd4c490115c2fa6eaa Mon Sep 17 00:00:00 2001 From: Ancor Gonzalez Sosa Date: Mon, 18 Feb 2019 18:01:21 +0100 Subject: [PATCH 22/25] Configure button: use inheritance to define the actions --- src/lib/y2partitioner/widgets/configure.rb | 281 ++++++++++++------- test/y2partitioner/widgets/configure_test.rb | 63 +++-- 2 files changed, 221 insertions(+), 123 deletions(-) diff --git a/src/lib/y2partitioner/widgets/configure.rb b/src/lib/y2partitioner/widgets/configure.rb index b77ba18d9b..354b7e4454 100644 --- a/src/lib/y2partitioner/widgets/configure.rb +++ b/src/lib/y2partitioner/widgets/configure.rb @@ -61,7 +61,7 @@ def contents # @return [:redraw, nil] :redraw when some configuration client was # executed; nil otherwise. def handle(event) - action = Action.find(event["ID"]) + action = find_action(event["ID"]) return nil unless action return nil unless warning_accepted?(action) && availability_ensured?(action) @@ -108,11 +108,11 @@ def actions @actions ||= if Yast::Stage.initial # In the inst-sys, check which clients are available - Action.supported.select { |action| Yast::WFM.ClientExists(action.client) } + supported_actions.reject(&:client_missing?) else # In the installed system, we don't care if the client is there or not # as the user will be prompted to install the pkg anyway (in #handle). - Action.supported + supported_actions end end @@ -156,130 +156,82 @@ def check_and_install_pkgs?(action) ret end + # Sorted list of actions + def supported_actions + @supported_actions ||= [ + CryptAction.new(_("Provide Crypt &Passwords..."), "yast-encrypted"), + IscsiAction.new(_("Configure &iSCSI..."), "yast-iscsi-client"), + FcoeAction.new(_("Configure &FCoE..."), "fcoe"), + DasdAction.new(_("Configure &DASD..."), "yast-dasd"), + ZfcpAction.new(_("Configure &zFCP..."), "yast-zfcp"), + XpramAction.new(_("Configure &XPRAM..."), "yast-xpram") + ].select(&:supported?) + end + + # Action with the given id + # + # @param id [Symbol] + # @return [Action] + def find_action(id) + supported_actions.find { |action| action.id == id } + end + # Each one of the configuration actions offered by the widget and that - # corresponds to a YaST client + # (usually) corresponds to a YaST client class Action include Yast::I18n - extend Yast::I18n - - textdomain "storage" # Constructor # - # @param id [Symbol] see {#id} - # @param label [String] string marked for translation to be used by {#label} + # @param label [String] see {#label} # @param icon [String] see {#icon} - # @param client [String] see {#client} - # @param pkgs [Array] see {#pkgs} - def initialize(id, label, icon, client, pkgs) + def initialize(label, icon) textdomain "storage" - @id = id @label = label @icon = icon - @client = client - @pkgs = pkgs - end - - # Sorted list of actions - ALL = [ - new(:crypt, N_("Provide Crypt &Passwords..."), "yast-encrypted", nil, - ["cryptsetup"]), - new(:iscsi, N_("Configure &iSCSI..."), "yast-iscsi-client", "iscsi-client", - ["yast2-iscsi-client"]), - new(:fcoe, N_("Configure &FCoE..."), "fcoe", "fcoe-client", - ["yast2-fcoe-client"]), - new(:dasd, N_("Configure &DASD..."), "yast-dasd", "dasd", - ["yast2-s390"]), - new(:zfcp, N_("Configure &zFCP..."), "yast-zfcp", "zfcp", - ["yast2-s390"]), - new(:xpram, N_("Configure &XPRAM..."), "yast-xpram", "xpram", - ["yast2-s390"]) - ].freeze - private_constant :ALL - - # Texts for {#warning_text} - # - # Although all texts are almost identical, the whole literal strings - # are indexed in this constant in order to reuse the existing - # translations from yast2-storage - WARNING_TEXTS = { - crypt: N_( - "Rescanning crypt devices cancels all current changes.\n" \ - "Really activate crypt devices?" - ), - iscsi: N_( - "Calling iSCSI configuration cancels all current changes.\n" \ - "Really call iSCSI configuration?" - ), - fcoe: N_( - "Calling FCoE configuration cancels all current changes.\n" \ - "Really call FCoE configuration?" - ), - dasd: N_( - "Calling DASD configuration cancels all current changes.\n" \ - "Really call DASD configuration?" - ), - fzcp: N_( - "Calling zFCP configuration cancels all current changes.\n" \ - "Really call zFCP configuration?" - ), - xpram: N_( - "Calling XPRAM configuration cancels all current changes.\n" \ - "Really call XPRAM configuration?" - ) - } - - # Actions that can only be executed on s390 systems - S390_IDS = [:dasd, :zfcp, :xpram].freeze - private_constant :S390_IDS - - # Action with the given id - # - # @param id [Symbol] - # @return [Action] - def self.find(id) - ALL.find { |action| action.id == id } end - # Actions that are supported in the current system - # - # @return [Array] - def self.supported - ALL.select(&:supported?) + # @return [Symbol] identifier for the action to use in the UI + def id + @id ||= self.class.name.split("::").last.to_sym end - # @return [Symbol] identifier for the action - attr_reader :id + # @return [String] Internationalized label + attr_reader :label # @return [String] name of the icon to display next to the label attr_reader :icon - # @return [Symbol] name of the YaST client implementing the action - attr_reader :client - - # @return [Array] name of the packages needed to run the action - attr_reader :pkgs - - # Internationalized label + # Internationalized text to be displayed as a warning before executing the action + # + # Although all texts are almost identical, the whole literal strings are used + # on each subclass in order to reuse the existing translations from yast2-storage # # @return [String] - def label - _(@label) + def warning_text + "" end - # Internationalized text to be displayed as a warning before executing the action + # Name of the YaST client implementing the action # - # @return [String] - def warning_text - _(WARNING_TEXTS[id]) + # @return [Symbol, nil] nil if no separate client needs to be called + def client + nil + end + + # Names of the packages needed to run the action + # + # @return [Array] + def pkgs + [] end # Whether the action is supported in the current system # # @return [Boolean] def supported? - S390_IDS.include?(id) ? Yast::Arch.s390 : true + true end # Value for the 'activate' argument of {Reprobe#reprobe} @@ -289,7 +241,140 @@ def supported? # # @return [Boolean, nil] def activate - id == :crypt ? true : nil + nil + end + + # Whether the client needed to run the action is missing + # + # @return [Boolean] + def client_missing? + return false unless client + + !Yast::WFM.ClientExists(client) + end + end + + # Specific class for the activation action + class CryptAction < Action + # @see Action#warning_text + def warning_text + _( + "Rescanning crypt devices cancels all current changes.\n" \ + "Really activate crypt devices?" + ) + end + + # @see Action#pkgs + def pkgs + ["cryptsetup"] + end + + # @see Action#activate + def activate + true + end + end + + # Specific action for running the iSCSI configuration client + class IscsiAction < Action + # @see Action#warning_text + def warning_text + _( + "Calling iSCSI configuration cancels all current changes.\n" \ + "Really call iSCSI configuration?" + ) + end + + # @see Action#client + def client + "iscsi-client" + end + + # @see Action#pkgs + def pkgs + ["yast2-iscsi-client"] + end + end + + # Specific action for running the FCoE configuration client + class FcoeAction < Action + # @see Action#warning_text + def warning_text + _( + "Calling FCoE configuration cancels all current changes.\n" \ + "Really call FCoE configuration?" + ) + end + + # @see Action#client + def client + "fcoe-client" + end + + # @see Action#pkgs + def pkgs + ["yast2-fcoe-client"] + end + end + + # Common base class for all the actions that are specific for S390 + class S390Action < Action + # @see Action#pkgs + def pkgs + ["yast2-s390"] + end + + # @see Action#supported? + def supported? + Yast::Arch.s390 + end + end + + # Specific action for running the DASD activation client + class DasdAction < S390Action + # @see Action#warning_text + def warning_text + _( + "Calling DASD configuration cancels all current changes.\n" \ + "Really call DASD configuration?" + ) + end + + # @see Action#client + def client + "dasd" + end + end + + # Specific action for running the zFCP configuration client + class ZfcpAction < S390Action + # @see Action#warning_text + def warning_text + _( + "Calling zFCP configuration cancels all current changes.\n" \ + "Really call zFCP configuration?" + ) + end + + # @see Action#client + def client + "zfcp" + end + end + + # Specific action for running the XPRAM configuration client + class XpramAction < S390Action + # @see Action#warning_text + def warning_text + _( + "Calling XPRAM configuration cancels all current changes.\n" \ + "Really call XPRAM configuration?" + ) + end + + # @see Action#client + def client + "xpram" end end end diff --git a/test/y2partitioner/widgets/configure_test.rb b/test/y2partitioner/widgets/configure_test.rb index c85de3ea86..4cb9bfd02b 100644 --- a/test/y2partitioner/widgets/configure_test.rb +++ b/test/y2partitioner/widgets/configure_test.rb @@ -67,8 +67,9 @@ def menu_button_item_ids(button) it "returns a menu button with buttons for crypt, iSCI, FCoE, DASD, zFCP and XPRAM" do term = widget.contents expect(term.value).to eq :MenuButton - expect(menu_button_item_ids(term)) - .to contain_exactly(:crypt, :iscsi, :fcoe, :dasd, :zfcp, :xpram) + expect(menu_button_item_ids(term)) .to contain_exactly( + :CryptAction, :IscsiAction, :FcoeAction, :DasdAction, :ZfcpAction, :XpramAction + ) end end @@ -78,8 +79,10 @@ def menu_button_item_ids(button) it "returns a menu button with buttons only for the available clients" do term = widget.contents expect(term.value).to eq :MenuButton - expect(menu_button_item_ids(term)).to contain_exactly(:crypt, :fcoe, :zfcp, :xpram) - expect(menu_button_item_ids(term)).to_not include(:iscsi, :dasd) + expect(menu_button_item_ids(term)).to contain_exactly( + :CryptAction, :FcoeAction, :ZfcpAction, :XpramAction + ) + expect(menu_button_item_ids(term)).to_not include(:IscsiAction, :DasdAction) end end @@ -89,7 +92,7 @@ def menu_button_item_ids(button) it "returns a menu button with 'Crypt Passwords' as the only option" do term = widget.contents expect(term.value).to eq :MenuButton - expect(menu_button_item_ids(term)).to eq [:crypt] + expect(menu_button_item_ids(term)).to eq [:CryptAction] end end end @@ -103,7 +106,9 @@ def menu_button_item_ids(button) it "returns a menu button with buttons for crypt, iSCI and FCoE" do term = widget.contents expect(term.value).to eq :MenuButton - expect(menu_button_item_ids(term)).to contain_exactly(:crypt, :iscsi, :fcoe) + expect(menu_button_item_ids(term)).to contain_exactly( + :CryptAction, :IscsiAction, :FcoeAction + ) end end @@ -113,8 +118,8 @@ def menu_button_item_ids(button) it "returns a menu button with buttons only for the available clients" do term = widget.contents expect(term.value).to eq :MenuButton - expect(menu_button_item_ids(term)).to eq [:crypt, :fcoe] - expect(menu_button_item_ids(term)).to_not include(:iscsi) + expect(menu_button_item_ids(term)).to eq [:CryptAction, :FcoeAction] + expect(menu_button_item_ids(term)).to_not include(:IscsiAction) end end @@ -124,7 +129,7 @@ def menu_button_item_ids(button) it "returns a menu button with 'Crypt Passwords' as the only option" do term = widget.contents expect(term.value).to eq :MenuButton - expect(menu_button_item_ids(term)).to eq [:crypt] + expect(menu_button_item_ids(term)).to eq [:CryptAction] end end end @@ -142,8 +147,9 @@ def menu_button_item_ids(button) it "returns a menu button with buttons for all actions" do term = widget.contents expect(term.value).to eq :MenuButton - expect(menu_button_item_ids(term)) - .to contain_exactly(:crypt, :iscsi, :fcoe, :dasd, :zfcp, :xpram) + expect(menu_button_item_ids(term)).to contain_exactly( + :CryptAction, :IscsiAction, :FcoeAction, :DasdAction, :ZfcpAction, :XpramAction + ) end end @@ -153,8 +159,9 @@ def menu_button_item_ids(button) it "returns a menu button with buttons for all actions" do term = widget.contents expect(term.value).to eq :MenuButton - expect(menu_button_item_ids(term)) - .to contain_exactly(:crypt, :iscsi, :fcoe, :dasd, :zfcp, :xpram) + expect(menu_button_item_ids(term)).to contain_exactly( + :CryptAction, :IscsiAction, :FcoeAction, :DasdAction, :ZfcpAction, :XpramAction + ) end end end @@ -168,7 +175,9 @@ def menu_button_item_ids(button) it "returns a menu button with buttons for all supported actions (crypt, iSCI and FCoE)" do term = widget.contents expect(term.value).to eq :MenuButton - expect(menu_button_item_ids(term)).to contain_exactly(:crypt, :iscsi, :fcoe) + expect(menu_button_item_ids(term)).to contain_exactly( + :CryptAction, :IscsiAction, :FcoeAction + ) end end @@ -178,7 +187,9 @@ def menu_button_item_ids(button) it "returns a menu button with buttons for all supported actions (crypt, iSCI and FCoE)" do term = widget.contents expect(term.value).to eq :MenuButton - expect(menu_button_item_ids(term)).to contain_exactly(:crypt, :iscsi, :fcoe) + expect(menu_button_item_ids(term)).to contain_exactly( + :CryptAction, :IscsiAction, :FcoeAction + ) end end end @@ -196,9 +207,9 @@ def event_for(id) end let(:accepted) { false } - let(:handle_args) { [event_for(:fcoe)] } + let(:handle_args) { [event_for(:FcoeAction)] } let(:manager) { Y2Storage::StorageManager.instance } - let(:event) { event_for(:iscsi) } + let(:event) { event_for(:IscsiAction) } RSpec.shared_examples "configure nothing" do it "returns nil" do @@ -225,8 +236,8 @@ def event_for(id) it "displays the corresponding warning message" do expect(Yast::Popup).to receive(:YesNo).with(/FCoE/).ordered expect(Yast::Popup).to receive(:YesNo).with(/iSCSI/).ordered - widget.handle(event_for(:fcoe)) - widget.handle(event_for(:iscsi)) + widget.handle(event_for(:FcoeAction)) + widget.handle(event_for(:IscsiAction)) end context "if the user rejects the warning" do @@ -246,7 +257,7 @@ def event_for(id) let(:accepted) { true } context "for an action performed via a separate client" do - let(:event) { event_for(:iscsi) } + let(:event) { event_for(:IscsiAction) } it "calls the corresponding YaST client" do expect(Yast::WFM).to receive(:call).with("iscsi-client") @@ -258,7 +269,7 @@ def event_for(id) end context "for activation of crypt devices" do - let(:event) { event_for(:crypt) } + let(:event) { event_for(:CryptAction) } it "does not call any additional YaST client" do expect(Yast::WFM).to_not receive(:call) @@ -274,6 +285,8 @@ def event_for(id) context "in an already installed system" do let(:install) { false } let(:handle_args) { [event] } + # Ensure all actions are available + before { allow(Yast::Arch).to receive(:s390).and_return true } include_examples "show configure warning" @@ -292,8 +305,8 @@ def event_for(id) .to receive(:CheckAndInstallPackages).with(["yast2-s390"]).ordered expect(Yast::PackageSystem) .to receive(:CheckAndInstallPackages).with(["yast2-fcoe-client"]).ordered - widget.handle(event_for(:dasd)) - widget.handle(event_for(:fcoe)) + widget.handle(event_for(:DasdAction)) + widget.handle(event_for(:FcoeAction)) end context "if the packages were not installed" do @@ -306,7 +319,7 @@ def event_for(id) let(:installed_pkgs) { true } context "for an action performed via a separate client" do - let(:event) { event_for(:iscsi) } + let(:event) { event_for(:IscsiAction) } it "calls the corresponding YaST client" do expect(Yast::WFM).to receive(:call).with("iscsi-client") @@ -322,7 +335,7 @@ def event_for(id) end context "for activation of crypt devices" do - let(:event) { event_for(:crypt) } + let(:event) { event_for(:CryptAction) } before { allow(manager).to receive(:activate).and_return true } it "does not call any additional YaST client" do From fa698374eb489a81aec4baabf9e328cbe4ed87ed Mon Sep 17 00:00:00 2001 From: Ancor Gonzalez Sosa Date: Tue, 19 Feb 2019 09:45:37 +0100 Subject: [PATCH 23/25] Version and changelog --- package/yast2-storage-ng.changes | 6 ++++++ package/yast2-storage-ng.spec | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/package/yast2-storage-ng.changes b/package/yast2-storage-ng.changes index 81d83abe37..1dff502f4f 100644 --- a/package/yast2-storage-ng.changes +++ b/package/yast2-storage-ng.changes @@ -1,3 +1,9 @@ +------------------------------------------------------------------- +Tue Feb 19 14:40:11 UTC 2019 - ancor@suse.com + +- Partitioner: new option "Provide Crypt Passwords" (bsc#1113515). +- 4.1.63 + ------------------------------------------------------------------- Tue Feb 19 14:15:16 UTC 2019 - jlopez@suse.com diff --git a/package/yast2-storage-ng.spec b/package/yast2-storage-ng.spec index 254459931b..c50ccc768a 100644 --- a/package/yast2-storage-ng.spec +++ b/package/yast2-storage-ng.spec @@ -16,7 +16,7 @@ # Name: yast2-storage-ng -Version: 4.1.62 +Version: 4.1.63 Release: 0 BuildRoot: %{_tmppath}/%{name}-%{version}-build From c1d6033079311b8d1b8660c2af2d52b3145c188d Mon Sep 17 00:00:00 2001 From: Ancor Gonzalez Sosa Date: Tue, 19 Feb 2019 14:00:30 +0100 Subject: [PATCH 24/25] Partitioner: help text for Configure button --- src/lib/y2partitioner/widgets/configure.rb | 14 ++++++++++++++ .../y2partitioner/widgets/rescan_devices_button.rb | 9 +++++++++ 2 files changed, 23 insertions(+) diff --git a/src/lib/y2partitioner/widgets/configure.rb b/src/lib/y2partitioner/widgets/configure.rb index 354b7e4454..bb406b56c7 100644 --- a/src/lib/y2partitioner/widgets/configure.rb +++ b/src/lib/y2partitioner/widgets/configure.rb @@ -71,6 +71,20 @@ def handle(event) :redraw end + # @macro seeAbstractWidget + def help + # TRANSLATORS: help text for the Partitioner + _( + "

The Configure button offers several options to activate devices " \ + "that were not detected by the initial system analysis. After activating the " \ + "devices of the choosen technology, the system will be rescanned and all the " \ + "information about storage devices will be refreshed. " \ + "Thus, the Provide Crypt Passwords option is also useful when no " \ + "encryption is involved, to activate devices of other technologies like LVM " \ + "or Multipath.

" + ) + end + private # @return [Array] diff --git a/src/lib/y2partitioner/widgets/rescan_devices_button.rb b/src/lib/y2partitioner/widgets/rescan_devices_button.rb index 0e7c00c48a..f9e2032a3e 100644 --- a/src/lib/y2partitioner/widgets/rescan_devices_button.rb +++ b/src/lib/y2partitioner/widgets/rescan_devices_button.rb @@ -52,6 +52,15 @@ def handle :redraw end + # @macro seeAbstractWidget + def help + # TRANSLATORS: help text for the Partitioner + _( + "

The Rescan Devices button refreshes the information about storage " \ + "devices.

" + ) + end + private def continue? From 487f879d25b7ee1569f600e4b62c401291f6a40f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Wed, 20 Feb 2019 08:44:54 +0000 Subject: [PATCH 25/25] Add missing arch mocking for bcache tests --- test/y2partitioner/actions/controllers/bcache_test.rb | 3 +++ test/y2partitioner/actions/edit_bcache_test.rb | 3 +++ test/y2partitioner/ui_state_test.rb | 3 +++ test/y2partitioner/widgets/bcache_edit_button_test.rb | 3 +++ test/y2storage/devicegraph_test.rb | 3 +++ 5 files changed, 15 insertions(+) diff --git a/test/y2partitioner/actions/controllers/bcache_test.rb b/test/y2partitioner/actions/controllers/bcache_test.rb index 1b3b23824f..e290206307 100644 --- a/test/y2partitioner/actions/controllers/bcache_test.rb +++ b/test/y2partitioner/actions/controllers/bcache_test.rb @@ -30,6 +30,9 @@ devicegraph_stub(scenario) end + # Bcache is only supported on x86 + let(:architecture) { :x86_64 } + subject(:controller) { described_class.new(device) } let(:device) { nil } diff --git a/test/y2partitioner/actions/edit_bcache_test.rb b/test/y2partitioner/actions/edit_bcache_test.rb index 9ef27e736d..78f10a2a17 100644 --- a/test/y2partitioner/actions/edit_bcache_test.rb +++ b/test/y2partitioner/actions/edit_bcache_test.rb @@ -41,6 +41,9 @@ .and_return(selected_options) end + # Bcache is only supported on x86 + let(:architecture) { :x86_64 } + let(:dialog_result) { nil } let(:selected_caching) { nil } diff --git a/test/y2partitioner/ui_state_test.rb b/test/y2partitioner/ui_state_test.rb index fe3f34eb02..1082617b07 100644 --- a/test/y2partitioner/ui_state_test.rb +++ b/test/y2partitioner/ui_state_test.rb @@ -223,6 +223,9 @@ end context "when the user has opened a Bcache page" do + # Bcache is only supported on x86 + let(:architecture) { :x86_64 } + let(:scenario) { "bcache1.xml" } let(:device) { fake_devicegraph.find_by_name("/dev/bcache0") } let(:another_bcache) { fake_devicegraph.find_by_name("/dev/bcache1") } diff --git a/test/y2partitioner/widgets/bcache_edit_button_test.rb b/test/y2partitioner/widgets/bcache_edit_button_test.rb index 312f61d1d0..0999209e94 100644 --- a/test/y2partitioner/widgets/bcache_edit_button_test.rb +++ b/test/y2partitioner/widgets/bcache_edit_button_test.rb @@ -32,6 +32,9 @@ allow(Y2Partitioner::Actions::EditBcache).to receive(:new).and_return(action) end + # Bcache is only supported on x86 + let(:architecture) { :x86_64 } + let(:action) { instance_double(Y2Partitioner::Actions::EditBcache, run: :finish) } subject(:button) { described_class.new(device: device) } diff --git a/test/y2storage/devicegraph_test.rb b/test/y2storage/devicegraph_test.rb index eac82388f9..c5055b5a7a 100644 --- a/test/y2storage/devicegraph_test.rb +++ b/test/y2storage/devicegraph_test.rb @@ -735,6 +735,9 @@ def with_sda2_deleted(initial_graph) fake_scenario("bcache1.xml") end + # Bcache is only supported on x86 + let(:architecture) { :x86_64 } + it "removes the given caching set device" do bcache_cset = Y2Storage::BcacheCset.all(devicegraph).first expect(bcache_cset).to_not be_nil