Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Merge 12e9ec5 into 55d6962
  • Loading branch information
jreidinger committed Oct 16, 2018
2 parents 55d6962 + 12e9ec5 commit 8b8d625
Show file tree
Hide file tree
Showing 6 changed files with 85 additions and 2 deletions.
20 changes: 20 additions & 0 deletions doc/bcache.md
@@ -0,0 +1,20 @@
## Bcache

bcache (abbreviated from block cache) is a cache in the Linux kernel's block layer,
which is used for accessing secondary storage devices. It allows one or more
fast storage devices, such as flash-based solid-state drives (SSDs), to act as
a cache for one or more slower storage devices, such as hard disk drives (HDDs);
this effectively creates hybrid volumes and provides performance improvements.

### Limitations in YaST

As its name suggests, in general it can be used on top of any block device. But YaST limits its usage.
YaST does not allow the creation of bcache devices on top of other bcache devices, even indirectly.
Such setup would not make much sense from a practical point of view and it can cause troubles with
the bcache metadata.

Several bcache operations are asynchronous and can take a significant amount of time.
YaST also prevents actions that would trigger such operations. For example, YaST limits
editing or resizing an existing bcache device and removing a bcache device that shares
its caching set. All those actions could imply detaching a cache, which is one of those slow
and asynchronous processes.
7 changes: 7 additions & 0 deletions package/yast2-storage-ng.changes
@@ -1,3 +1,10 @@
-------------------------------------------------------------------
Tue Oct 16 12:58:10 UTC 2018 - jreidinger@suse.com

- Improve filtering of possible backing devices for bcache and
limit deletion of bcache to only safe cases (fate#325346)
- 4.1.25

-------------------------------------------------------------------
Thu Oct 11 15:03:26 UTC 2018 - ancor@suse.com

Expand Down
2 changes: 1 addition & 1 deletion package/yast2-storage-ng.spec
Expand Up @@ -16,7 +16,7 @@
#

Name: yast2-storage-ng
Version: 4.1.24
Version: 4.1.25
Release: 0

BuildRoot: %{_tmppath}/%{name}-%{version}-build
Expand Down
4 changes: 3 additions & 1 deletion src/lib/y2partitioner/actions/add_bcache.rb
Expand Up @@ -70,7 +70,9 @@ 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?)
(!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
end

Expand Down
24 changes: 24 additions & 0 deletions src/lib/y2partitioner/actions/delete_bcache.rb
Expand Up @@ -21,6 +21,7 @@

require "yast"
require "y2partitioner/actions/delete_device"
require "y2partitioner/device_graphs"

module Y2Partitioner
module Actions
Expand All @@ -35,6 +36,29 @@ def initialize(*args)

private

# @see DeleteDevice#errors
def errors
errors = super + [shared_cset]

errors.compact
end

# Error when the bcache shares cset with any other bcache.
#
# @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)

# TRANSLATORS: Error when trying to modify an existing bcache that shares caches.
_(
"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."
)
end

# Deletes the indicated Bcache (see {DeleteDevice#device})
def delete
log.info "deleting bcache raid #{device}"
Expand Down
30 changes: 30 additions & 0 deletions test/y2partitioner/actions/delete_bcache_test.rb
Expand Up @@ -47,10 +47,30 @@

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

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
end
end

context "when deleting a bcache without associated devices" do
let(:device_name) { "/dev/bcache1" }

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
end
Expand All @@ -68,6 +88,8 @@
let(:device_name) { "/dev/bcache2" }

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

Expand All @@ -79,11 +101,15 @@
let(:accept) { :no }

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
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)
end
end
Expand All @@ -92,11 +118,15 @@
let(:accept) { :yes }

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

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)
end
end
Expand Down

0 comments on commit 8b8d625

Please sign in to comment.