Skip to content

Commit

Permalink
Treat btrfs subvolumes like all other mounts
Browse files Browse the repository at this point in the history
  • Loading branch information
@shundhammer
shundhammer committed Aug 24, 2021
1 parent 25d78de commit 3429419
Show file tree
Hide file tree
Showing 2 changed files with 82 additions and 58 deletions.
71 changes: 27 additions & 44 deletions src/lib/installation/unmounter.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,15 +56,6 @@ def initialize(device, mount_path, fs_type = "", mount_opt = "")
@mount_opt = mount_opt
end

# Check if this is a mount for a btrfs.
# @return [Boolean] true if btrfs, false if not
#
def btrfs?
return false if @fs_type.nil?

@fs_type == "btrfs"
end

# Format this mount as a string for logging.
#
def to_s
Expand Down Expand Up @@ -102,7 +93,7 @@ def read_mounts_file(file_name)
end

# Parse one entry of /proc/mounts and add it to @mounts
# if it meets the criteria (mount prefix, no btrfs subvolume)
# if it meets the criteria (mount prefix)
#
# @param line [String] one line of /proc/mounts
# @return [Mount,nil] parsed mount if relevant
Expand All @@ -111,7 +102,7 @@ def add_mount(line)
mount = parse_mount(line)
return nil if mount.nil? # Empty or comment

if ignore?(mount)
if !mount.mount_path.start_with?(@mnt_prefix)
@ignored_mounts << mount
return nil
end
Expand All @@ -121,29 +112,6 @@ def add_mount(line)
mount
end

# Check if a mount should be ignored, i.e. if the path either doesn't start
# with the mount prefix (usually "/mnt") or if it is a btrfs subvolume.
#
# A subvolume cannot be unmounted while its parent main volume is still mounted; that will result in a
#
# @return [Boolean] ignore
#
def ignore?(mount)
return true unless mount.mount_path.start_with?(@mnt_prefix)

if mount.btrfs? && mount_for_device(mount.device)
# We already have a mount for a Btrfs on this device,
# so any more mount for that device must be a subvolume.
#
# Notice that this relies on /proc/mounts being in the correct order:
# Btrfs main mount first, all its subvolumes after that.
log.info("-- Ignoring btrfs subvolume #{mount}")
return true
end

false # don't ignore
end

# Parse one entry of /proc/mounts.
#
# @param line [String] one line of /proc/mounts
Expand All @@ -157,15 +125,6 @@ def parse_mount(line)
Mount.new(device, mount_path, fs_type, mount_opt)
end

# Return the mount for a specified device or nil if there is none.
#
# @param device [String]
# @return [Mount,nil] Matching mount
#
def mount_for_device(device)
@mounts.find { |mount| mount.device == device }
end

# Return the paths to be unmounted in the correct unmount order.
#
# This makes use of the fact that /proc/mounts is already sorted in
Expand All @@ -187,7 +146,31 @@ def ignored_paths
@ignored_mounts.map(&:mount_path)
end

# Actually execute all the pending unmount operations.
# Actually execute all the pending unmount operations in the right
# sequence.
#
# This iterates over all relevant mounts and invokes the external "umount"
# command for each one separately. Notice that while "umount -R path" also
# exists, it will stop executing when it first encounters any mount that
# cannot be unmounted ("filesystem busy"), even if mounts that come after
# that could safely be unmounted.
#
# If unmounting a mount fails, this does not attempt to remount read-only
# ("umount -r"), by force ("umount -f") or lazily ("umount -l"):
#
# - Remounting read-only ("umount -r" or "mount -o remount,ro") typically
# also fails if unmounting fails. It would have to be a rare coincidence
# that a filesystem has only open files in read-only mode already; only
# then it would have a chance to succeed.
#
# - Force-unmounting ("umount -f") realistically only works for NFS mounts.
# It is intended for cases when the NFS server has become unreachable.
#
# - Lazy unmounting ("umount -l") mostly removes the entry for this
# filesytem from /proc/mounts; it actually only unmounts when the pending
# operations that prevent a simple unmount are finished which may take a
# long time; or forever. And there is no way to find out if or when this
# has ever happened, so the next mount for this filesystem may fail.
#
def execute
unmount_paths.each do |path|
Expand Down
69 changes: 55 additions & 14 deletions test/unmounter_test.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,34 +101,75 @@ def stored_proc_mounts(scenario)
let(:subject) { described_class.new(PREFIX, proc_mounts) }

context "partition-based btrfs with subvolumes, no separate /home" do
let(:proc_mounts) { stored_proc_mounts("btrfs") }
let(:proc_mounts) { stored_proc_mounts("btrfs") } # see data/proc/mounts/
let(:expected_result) do
%w(/mnt/run
/mnt/sys
/mnt/proc
/mnt/dev
/mnt/var
/mnt/usr/local
/mnt/tmp
/mnt/srv
/mnt/root
/mnt/opt
/mnt/home
/mnt/boot/grub2/x86_64-efi
/mnt/boot/grub2/i386-pc
/mnt/.snapshots
/mnt)
end

it "will unmount /mnt/run, /mnt/sys, /mnt/proc, /mnt/dev, /mnt" do
expect(subject.unmount_paths).to eq ["/mnt/run", "/mnt/sys", "/mnt/proc", "/mnt/dev", "/mnt"]
it "will unmount /mnt/run, /mnt/sys, /mnt/proc, /mnt/dev, all subvolumes, /mnt" do
expect(subject.unmount_paths).to eq expected_result
end
end

context "partition-based btrfs with subvolumes and separate xfs /home" do
let(:proc_mounts) { stored_proc_mounts("btrfs-xfs-home") }

it "will unmount /mnt/run, /mnt/sys, /mnt/proc, /mnt/dev, /mnt/home, /mnt" do
expect(subject.unmount_paths).to eq ["/mnt/run", "/mnt/sys", "/mnt/proc", "/mnt/dev", "/mnt/home", "/mnt"]
let(:expected_result) do
%w(/mnt/run
/mnt/sys
/mnt/proc
/mnt/dev
/mnt/var
/mnt/usr/local
/mnt/tmp
/mnt/srv
/mnt/root
/mnt/opt
/mnt/home
/mnt/boot/grub2/x86_64-efi
/mnt/boot/grub2/i386-pc
/mnt)
end
end

context "partition-based btrfs with subvolumes and separate btrfs /home" do
let(:proc_mounts) { stored_proc_mounts("btrfs-xfs-home") }

it "will unmount /mnt/run, /mnt/sys, /mnt/proc, /mnt/dev, /mnt/home, /mnt" do
expect(subject.unmount_paths).to eq ["/mnt/run", "/mnt/sys", "/mnt/proc", "/mnt/dev", "/mnt/home", "/mnt"]
it "will unmount /mnt/run, /mnt/sys, /mnt/proc, /mnt/dev, all subvolumes, /mnt/home, /mnt" do
expect(subject.unmount_paths).to eq expected_result
end
end

context "encrypted LVM with btrfs with subvolumes and separate btrfs /home" do
let(:proc_mounts) { stored_proc_mounts("btrfs-xfs-home") }
let(:expected_result) do
%w(/mnt/run
/mnt/sys
/mnt/proc
/mnt/dev
/mnt/var
/mnt/usr/local
/mnt/tmp
/mnt/srv
/mnt/root
/mnt/opt
/mnt/home
/mnt/boot/grub2/x86_64-efi
/mnt/boot/grub2/i386-pc
/mnt)
end

it "will unmount /mnt/run, /mnt/sys, /mnt/proc, /mnt/dev, /mnt/home, /mnt" do
expect(subject.unmount_paths).to eq ["/mnt/run", "/mnt/sys", "/mnt/proc", "/mnt/dev", "/mnt/home", "/mnt"]
it "will unmount /mnt/run, /mnt/sys, /mnt/proc, /mnt/dev, all subvolumes, /mnt/home, /mnt" do
expect(subject.unmount_paths).to eq expected_result
end
end
end
Expand Down

0 comments on commit 3429419

Please sign in to comment.