Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

btrfs-progs: fix overly strict is_same_loop_file() #938

Open
wants to merge 1 commit into
base: devel
Choose a base branch
from

Conversation

maharmstone
Copy link
Contributor

is_same_loop_file() protects us from doing mkfs.btrfs on a loopback device that's already mounted. Unfortunately it's too strict, erroring out even in some cases where mkfs would be safe. This has resulted in systemd having to do some ugly code to work around this (see systemd/systemd#28033).

Get the offset and sizelimit of each loopback device, and allow mkfs if there's no overlap. Fixes
#640.

is_same_loop_file() protects us from doing mkfs.btrfs on a loopback
device that's already mounted. Unfortunately it's too strict, erroring
out even in some cases where mkfs would be safe. This has resulted in
systemd having to do some ugly code to work around this
(see systemd/systemd#28033).

Get the offset and sizelimit of each loopback device, and allow mkfs if
there's no overlap. Fixes
kdave#640.

Signed-off-by: Mark Harmstone <[email protected]>
@DaanDeMeyer
Copy link

DaanDeMeyer commented Jan 14, 2025

Trying this out I get the following:

~/systemd (main)> sudo ~/.local/bin/mkosi --runtime-size=15G boot
Opened '/work/home/daandemeyer/systemd/build/mkosi.output/image' in O_RDONLY access mode, with O_DIRECT enabled.
Determined sector size 512 based on discovered partition table.
loop1: Acquired exclusive lock.
Successfully acquired /dev/loop1, devno=7:1, nr=1, diskseq=152
Kernel was quicker than us in adding partition 1.
Dissecting esp partition with label esp and UUID 1a42d708-b379-42a1-a4fe-352671054494
Opened /dev/loop1p1 (fd=3, whole_block_devnum=7:1, diskseq=152).
Kernel was quicker than us in adding partition 2.
Dissecting root partition with label root-x86-64 and UUID 8f26912a-a13a-4155-91b1-00229a0e558b
Opened /dev/loop1p2 (fd=4, whole_block_devnum=7:1, diskseq=152).
Probed fstype 'btrfs' on partition /dev/loop1p2.
Mounting /proc/self/fd/4 (btrfs) on /run/systemd/mount-rootfs (MS_RDONLY|MS_NODEV "")...
Mounting /proc/self/fd/3 (vfat) on /run/systemd/mount-rootfs/boot (MS_RDONLY|MS_NOSUID|MS_NODEV|MS_NOEXEC|MS_NOSYMFOLLOW "fmask=0177,dmask=0077")...
No machine ID set, using randomized partition UUIDs.
File '/work/home/daandemeyer/systemd/build/mkosi.output/image' already is of requested size or larger, not growing. (15G >= 15G)
Determined sector size 512 based on discovered partition table.
Sector size of device is 512 bytes. Using filesystem sector size of 4096 and grain size of 4096.
Applying changes to /work/home/daandemeyer/systemd/build/mkosi.output/image.
Successfully wiped file system signatures from future partition 2.
Successfully discarded data from future partition 2.
Successfully discarded gap after partition 2.
loop6: Acquired exclusive lock.
Successfully acquired /dev/loop6, devno=7:6, nr=6, diskseq=153
Formatting future partition 2.
Executing mkfs command: mkfs.btrfs -L usr-x86-64 -U 5895d7bf-bcb7-47e6-ae64-15dd3f866f59 --sectorsize=4096 /dev/loop6
Successfully forked off '(mkfs)' as PID 295882.
btrfs-progs v6.12 
See https://btrfs.readthedocs.io for more information.

ERROR: /dev/loop6 is mounted
(mkfs) failed with exit status 1.
Failed to umount /run/systemd/mount-rootfs, ignoring: Device or resource busy
‣ "systemd-repart --image /work/home/daandemeyer/systemd/build/mkosi.output/image --size=16106127360 --no-pager --dry-run=no --offline=no --pretty=no /work/home/daandemeyer/systemd/build/mkosi.output/image" returned non-zero exit code 1.

@adam900710
Copy link
Collaborator

Can we just simply get rid of the complex (and unnecessary to me at least) loop back device lookup?

If there are multiple loopback devices pointing to the same file, it's obviously something wrong with the end user.
And I even doubt if the existing checks is enough, e.g. using hard links to evade the path based checks.

Just treat /dev/loop0p1 as our check boundary is good enough to me.

@adam900710
Copy link
Collaborator

@DaanDeMeyer Mind to give this branch a try?
#940

@maharmstone
Copy link
Contributor Author

Can we just simply get rid of the complex (and unnecessary to me at least) loop back device lookup?

If there are multiple loopback devices pointing to the same file, it's obviously something wrong with the end user. And I even doubt if the existing checks is enough, e.g. using hard links to evade the path based checks.

Just treat /dev/loop0p1 as our check boundary is good enough to me.

This would be fine by me too.

@DaanDeMeyer
Copy link

@adam900710 Will do, can you also take a look at https://lore.kernel.org/linux-fsdevel/[email protected]/T/#u? Your patch to remove /proc/self/fd from /proc/mounts caused a regression in systemd-repart which Boris has been investigating and that email thread has some suggestions on possible fixes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants