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

tests: detect loop devices with deleted files as an invariant #14568

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

zyga
Copy link
Contributor

@zyga zyga commented Oct 2, 2024

When a loop device is created with a given backing file, the file itself may be deleted. The kernel keeps the reference to the file alive but it also adjusts the name of the file to have the suffix " (deleted)".

We should not have loop devices with this property across tests. They might indicate that a mount sticks around and leaks through a reference somehow, or that the low-level loop-back utilities are buggy.

When a loop device is created with a given backing file, the file itself
may be deleted. The kernel keeps the reference to the file alive but it
also adjusts the name of the file to have the suffix " (deleted)".

We should not have loop devices with this property across tests.
They might indicate that a mount sticks around and leaks through a
reference somehow, or that the low-level loop-back utilities are buggy.

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
@zyga zyga force-pushed the tweak/deleted-loop-invariant branch from 4daf969 to 346c55e Compare October 2, 2024 13:14
Copy link

codecov bot commented Oct 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.87%. Comparing base (ac897ee) to head (346c55e).
Report is 47 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #14568      +/-   ##
==========================================
+ Coverage   78.85%   78.87%   +0.01%     
==========================================
  Files        1079     1083       +4     
  Lines      145615   146105     +490     
==========================================
+ Hits       114828   115239     +411     
- Misses      23601    23671      +70     
- Partials     7186     7195       +9     
Flag Coverage Δ
unittests 78.87% <ø> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

check_deleted_loop_devices() {
n="$1" # invariant name

losetup -l | grep -F ' (deleted)' > "$TESTSTMP/tests.invariant.$n"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(deleted) is unlikely to shwo up in device name or file name, so we can be less picky about the format I think

Suggested change
losetup -l | grep -F ' (deleted)' > "$TESTSTMP/tests.invariant.$n"
losetup -l | grep -F '(deleted)' > "$TESTSTMP/tests.invariant.$n"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this part is relevant. The space is not quoted otherwise. I'd keep this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did an experiment and I ran into another fragile part of the userspace API where escaping is just not done. The files are literally called like that, neither is deleted:

zyga@falka:~/snapd$ losetup -l
NAME       SIZELIMIT OFFSET AUTOCLEAR RO BACK-FILE                         DIO LOG-SEC
/dev/loop1         0      0         0  0 /home/zyga/snapd/potato (deleted)   0     512
/dev/loop0         0      0         0  0 /home/zyga/snapd/potato             0     512
zyga@falka:~/snapd$ cat /sys/class/block/loop[01]/loop/backing_file
/home/zyga/snapd/potato
/home/zyga/snapd/potato (deleted)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

zyga@falka:~/snapd$ losetup -l
NAME       SIZELIMIT OFFSET AUTOCLEAR RO BACK-FILE                                   DIO LOG-SEC
/dev/loop1         0      0         0  0 /home/zyga/snapd/potato (deleted) (deleted)   0     512
/dev/loop0         0      0         0  0 /home/zyga/snapd/potato (deleted)             0     512

The joke is that text formats are easy for humans and machines to parse.

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

Successfully merging this pull request may close these issues.

2 participants