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: setting a label is only supported on a mount point #966

Open
jelly opened this issue Oct 3, 2023 · 3 comments
Open

btrfs: setting a label is only supported on a mount point #966

jelly opened this issue Oct 3, 2023 · 3 comments

Comments

@jelly
Copy link
Contributor

jelly commented Oct 3, 2023

I noticed that setting a label is only supported via a mountpoint for btrfs, while in theory a label can be set on a device.

btrfs filesystem label [<device>|<mountpoint>] [<newlabel>]

Most other set_label functions seem to use the device. Which in Cockpit leads a somewhat inconsistent experience when the filesystem is not mounted. (In theory we can work around it by first mounting it temporary, set label and umount).

Changing the API over to a mountpoint would break libblockdev's API so I guess that is a not an option and this isn't a real "breaking" or high priority issue for us. (Read: no issue at all)

@jelly
Copy link
Contributor Author

jelly commented Oct 3, 2023

Sorry, this report seems to be wrong, a mounted btrfs partition can't set it's label in udisks likely because it uses the device name /dev/sda which does not work when it's mounted:

[root@fedora-38-127-0-0-2-2201 ~]# btrfs filesystem label /dev/sda wookie
ERROR: device /dev/sda is mounted, use mount point
[root@fedora-38-127-0-0-2-2201 ~]# btrfs filesystem label /foo wookie
[root@fedora-38-127-0-0-2-2201 ~]# btrfs filesystem show /dev/sda
Label: 'wookie'  uuid: ac07d160-1028-4873-881e-91939924e08b
        Total devices 1 FS bytes used 144.00KiB
        devid    1 size 10.00GiB used 536.00MiB path /dev/sda

So I suppose the comment in btrfs.c is outdated and that confused me :)

@tbzatek
Copy link
Member

tbzatek commented Oct 9, 2023

In theory we can work around it by first mounting it temporary, set label and umount

This should be avoided whenever possible, we've been banished by the community when doing things like this. Better to find a way around and address the device some other way. Setting label is not that severe comparing to retrieving info though. If available, a private mount namespace should be used for such things (currently unimplemented, see #894).

So mountpoint should be passed in when mounted, device file otherwise? I'm not sure whether UDisks is prepared for such things, the libblockdev fs API sounds vague in this specific case too.

@vojtechtrefny
Copy link
Member

The "generic" FS functions do the temporary mount when needed for the operation so bd_fs_set_label works with device for btrfs. We discussed this before and we decided to not change the filesystem specific functions in these cases, bd_fs_xfs_resize for example also expects mount point and not device.

In theory we can work around it by first mounting it temporary, set label and umount

This should be avoided whenever possible, we've been banished by the community when doing things like this.

This is a problem for "automatic" functions (like _get_info), I don't think it is that problematic for actions that are started manually by the user like resize or label change.

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

No branches or pull requests

3 participants