Skip to content

umount: Resolve btrfs subvolume mountpoints - #1521

Open
wyu71 wants to merge 2 commits into
storaged-project:masterfrom
wyu71:fix/btrfs-subvol-umount
Open

wyu71 wants to merge 2 commits into
storaged-project:masterfrom
wyu71:fix/btrfs-subvol-umount

Conversation

@wyu71

@wyu71 wyu71 commented Jun 3, 2026

Copy link
Copy Markdown

When umount.udisks2 is invoked for a btrfs subvolume mountpoint, stat() may return the anonymous btrfs device number instead of the backing block device. This makes the helper fail to find the matching UDisks object.

Look up the btrfs mount source from /proc/self/mountinfo before falling back to the mountpoint device number.

Fixes #1495

When umount.udisks2 is invoked for a btrfs subvolume mountpoint,
stat() may return the anonymous btrfs device number instead of the
backing block device. This makes the helper fail to find the matching
UDisks object.

Look up the btrfs mount source from /proc/self/mountinfo before falling
back to the mountpoint device number.

Fixes storaged-project#1495
@wyu71

wyu71 commented Jun 3, 2026

Copy link
Copy Markdown
Author

Testing Farm failure is unrelated to this change.

The failure is in EXFATTestCase.test_* tests:

No such interface 'org.freedesktop.UDisks2.Filesystem' on object at /org/freedesktop/UDisks2/block_devices/sdd

This PR only touches the umount.udisks2 helper and does not affect filesystem interface detection. The issue appears to be a test environment problem with exFAT support on the /dev/sdd device.

Could you re-run the Testing Farm check?

@vojtechtrefny

Copy link
Copy Markdown
Member

Testing Farm failure is unrelated to this change.

The failure is in EXFATTestCase.test_* tests:

That's caused by some changes in the latest exfatprogs, I am already working on fixing this (storaged-project/libblockdev#1199), you can ignore these failures.

@vojtechtrefny

Copy link
Copy Markdown
Member

/packit test

1 similar comment
@wyu71

wyu71 commented Jul 17, 2026

Copy link
Copy Markdown
Author

/packit test

@jlmxyz

jlmxyz commented Jul 21, 2026 •

Copy link
Copy Markdown

is there any review planned? so that it's integrated into a release? it is problematic to have to umount the disk by root user
I tested it as a local patch using PCManFM-Qt and it works for umount, however there is still a error when mounting "the operation was cancelled" but this don't affect the mount itself (just have to reselect the drive after and access content)

@wyu71

wyu71 commented Jul 22, 2026

Copy link
Copy Markdown
Author

Thanks for testing and confirming the umount fix works.

The mount-side "operation was cancelled" error looks unrelated to this PR — this change only touches the umount.udisks2 helper and doesn't affect the mount code path. It might be worth opening a separate issue for the mount behavior so it can be looked into on its own.

@tbzatek

tbzatek commented Aug 25, 2026

Copy link
Copy Markdown
Member

The code is copy-pasted from

/* Temporary work-around for btrfs, see
*
* https://bugzilla.redhat.com/show_bug.cgi?id=495152#c31
* http://article.gmane.org/gmane.comp.file-systems.btrfs/2851
*
* for details.
*/
if (major == 0)
{
const gchar *sep;
sep = strstr (lines[n], " - ");
if (sep != NULL)
{
gchar fstype[PATH_MAX + 1];
gchar mount_source[PATH_MAX + 1];
struct stat statbuf;
if (sscanf (sep + 3, PATH_MAX_FMT " " PATH_MAX_FMT, fstype, mount_source) != 2)
{
udisks_warning ("Error parsing things past - for '%s'", lines[n]);
continue;
}
fstype[sizeof fstype - 1] = '\0';
mount_source[sizeof mount_source - 1] = '\0';
if (g_strcmp0 (fstype, "btrfs") != 0)
continue;
if (!g_str_has_prefix (mount_source, "/dev/"))
continue;
if (stat (mount_source, &statbuf) != 0)
{
udisks_warning ("Error statting %s: %m", mount_source);
continue;
}
if (!S_ISBLK (statbuf.st_mode))
{
udisks_warning ("%s is not a block device", mount_source);
continue;
}
dev = statbuf.st_rdev;
}
, correct?

Would be nice to have this as a common function somewhere to avoid code duplication. While umount_udisks2 is linking against libudisks2.la, we don't want a new symbol there. It makes sense to separate this in a new header and just include that file while linking statically.

@wyu71

wyu71 commented Aug 31, 2026

Copy link
Copy Markdown
Author

Thanks for the review. I extracted the btrfs mountinfo parsing into the private src/udisksmountutil.h header as a static helper and included it from both the mount monitor and umount.udisks2. This avoids code duplication without adding a new symbol to libudisks2.

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.

udisk fail to umount btrfs partion if default btrfs subvol is not "root"

4 participants