search

kdave / btrfs-progs

Development of userspace BTRFS tools
GNU General Public License v2.0
527 stars 239 forks source link

libbtrfsutil: add btrfs_util_get_label #736

Open jelly opened 5 months ago

jelly commented 5 months ago

Extend libbtrfs with a method to obtain to btrfs filesystem label.

Hey, this is a first PR of hopefully more to come. I'm looking at extending libbtrfsutil as we intend to use it in libblockdev to avoid parsing btrfs CLI output.

This is just a simple starter PR, some more interesting such as volume information (devices, RAID data, etc. basically btrfs filesystem show), creating a volume and adding/removing devices. And of course, btrfs_util_set_label happy to include that in this PR if required.

kdave commented 5 months ago

Thanks. I'm not sure if you're aware of my attempt to do implement the label get/set support, it looks it's mostly the same https://github.com/kdave/btrfs-progs/commits/dev/libbtrfsutil/labels-wip/ .

However one thing is missing from your code, this is adding a new symbol to the library and must be thus versioned. It would be best to add more new calls than one by one (each requiring a version update). Thre's also a pending update of the API naming scheme https://github.com/kdave/btrfs-progs/issues/574 .

jelly commented 5 months ago

Hi,

Thanks. I'm not sure if you're aware of my attempt to do implement the label get/set support, it looks it's mostly the same https://github.com/kdave/btrfs-progs/commits/dev/libbtrfsutil/labels-wip/ .

Woops, I was fully unaware of this I've looked through the issues and pull requests but not other branches.

However one thing is missing from your code, this is adding a new symbol to the library and must be thus versioned. It would be best to add more new calls than one by one (each requiring a version update). Thre's also a pending update of the API naming scheme #574 .

Shall I take your commit and work on a branch with for example adding AddDevice/RemoveDevice/CreateVolume that should be fairly trivial.

Something I really would like however is to replace our usage of btrfs filesystem show parsed by regex, so something like list devices of a btrfs volume. That will require some more API design, I guess it's useful to think of how the Python API should look like and then create the C API?

kdave commented 3 months ago

I've pushed library updated 1.3 that updates the naming and sets the ground for future extensions. The release 6.7 will have only that.

Regarding the label, yes use my patch as a base, it needs some changes and I don't remember if it's complete (C code is, not sure about python and tests). This can be in 1.4 update along with some other easy ioctl wrappers, there are too many to choose from so it can be anything.

jelly commented 3 months ago

The Python code for the label has some issues, the tests don't succeed as there is extra data after the label. I'll take a look at it.

I have been looking into potential new functions to expose in libbtrfsutil which would be useful for libblockdev but I've found out that none of them are really trivial to implement:

The command line utility discards when required, detects if there is a filesystem on the target and optionally can enqueue by checking the sysfs file /sys/fs/btrfs/$fs_id/exclusive_operation . As no code can be re-used, it would either have to be written again or not handled.

btrfs_util_device_add("/dev/sda", "/mountpoint", force?, enque?, nodiscard?)

Remove seems more trivial, but not entirely useful with an add I'd say.

btrfs_util_device_remove("/dev/sda", "/mountpoint", enqueue)

Maybe a better alternative for my use case is to add json output to btrfs filesystem show. I assume that is a welcome change regardless.

So for further extending libbttrfsutil adding the following functionality would probably be "easier":