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

Only able to create a subvolume in the first mount point #1242

Open
jelly opened this issue Jan 16, 2024 · 6 comments
Open

Only able to create a subvolume in the first mount point #1242

jelly opened this issue Jan 16, 2024 · 6 comments

Comments

@jelly
Copy link
Contributor

jelly commented Jan 16, 2024

When a user has multiple subvol's mounted from the same blockdevice it is not possible to create a subvolume on a specific mount point. For example:

subvolumes:

/left
/right

mounts:

/dev/sda on /run/left type btrfs (rw,relatime,seclabel,discard=async,space_cache=v2,subvolid=256,subvol=/left)
/dev/sda on /run/right type btrfs (rw,relatime,seclabel,discard=async,space_cache=v2,subvolid=257,subvol=/right)

As the udisks code always grabs the same mount point you can't create a subvolume on /run/right as /run/left was mounted first.

Reproducer:

mkfs.btrfs -L foo /dev/sda
mkdir -p /mnt/butter
mount /dev/sda /mnt/butter
btrfs subvolume create /mnt/butter/left
btrfs subvolume create /mnt/butter/right
umount /mnt/butter

mkdir /run/{left,right}
mount -o subvol=left /dev/sda /run/left/
mount -o subvol=right /dev/sda /run/right/
[root@fedora-39-127-0-0-2-2201 ~]# busctl call org.freedesktop.UDisks2 /org/freedesktop/UDisks2/block_devices/sda org.freedesktop.UDisks2.Filesystem.BTRFS CreateSubvolume sa{sv} "foo" 0
[root@fedora-39-127-0-0-2-2201 ~]# ls -lh /run/left/
total 0
drwxr-xr-x. 1 root root 0 Jan 17 09:56 foo
[root@fedora-39-127-0-0-2-2201 ~]# ls -lh /run/right/
total 0
[root@fedora-39-127-0-0-2-2201 ~]# busctl call org.freedesktop.UDisks2 /org/freedesktop/UDisks2/block_devices/sda org.freedesktop.UDisks2.Filesystem.BTRFS CreateSubvolume sa{sv} "/mnt/right/foo" 0
Call failed: Process reported exit code 1: ERROR: cannot access '/run/left//mnt/right': No such file or directory
[root@fedora-39-127-0-0-2-2201 ~]# busctl call org.freedesktop.UDisks2 /org/freedesktop/UDisks2/block_devices/sda org.freedesktop.UDisks2.Filesystem.BTRFS CreateSubvolume sa{sv} "right/foo" 0
Call failed: Process reported exit code 1: ERROR: cannot access '/run/left/right': No such file or directory
@jelly
Copy link
Contributor Author

jelly commented Jan 16, 2024

cc @mvollmer

@jelly
Copy link
Contributor Author

jelly commented Jan 17, 2024

This is likely very hard to fix without breaking the API and either making CreateSubvolume require the full path or adding a Subvolume object to UDisks, so GetSubvolumes() would return a new Object which we could call CreateSubvolume on.

@tbzatek
Copy link
Member

tbzatek commented Jan 17, 2024

Oh, pardon my ignorance, is it necessary to use mountpoint as an identification? Wouldn't volume UUID be enough for that?

@jelly
Copy link
Contributor Author

jelly commented Jan 17, 2024

Oh, pardon my ignorance, is it necessary to use mountpoint as an identification? Wouldn't volume UUID be enough for that?

volume UUID as in the btrfs volume? Or the disk' UUID? Because a btrfs volume can have multiple mountpoints.

@tbzatek
Copy link
Member

tbzatek commented Jan 23, 2024

I hereby declare that I have no clue how btrfs works, so I'll leave it to you, @jelly.

As for the API - one possibility is to use the options argument for passing extra options, like a mountpoint in this case. It's a bit of a hack and not the cleanest way of extending existing API. So depends on how many other (intrusive) changes are coming - we may perhaps opt for a new module implementation.

FYI, the module naming may be extended in form of btrfs:gen2 e.g...

@jelly
Copy link
Contributor Author

jelly commented Mar 6, 2024

@mvollmer and I discussed this a while ago and we came up with an potential solution (Or more or less how we solved it in Cockpit).

CreateSubvolume will take (parent_subvolume, "newsubvolumename") as arguments (or just a path). And will look up the parent_subvolume's mount_point. We solved this in JavaScript wit the data from udisks:

function get_mount_point_in_parent(volume, subvol) {
    const block = client.blocks[volume.path];
    const subvols = client.uuids_btrfs_subvols[volume.data.uuid];
    if (!subvols)
        return null;

    for (const p of subvols) {
        const has_parent_subvol = (p.pathname == "/" && subvol.pathname !== "/") ||
                                  (subvol.pathname.substring(0, p.pathname.length) == p.pathname &&
                                   subvol.pathname[p.pathname.length] == "/");
        if (has_parent_subvol && is_mounted(client, block, p)) {
            const [, pmp, opts] = get_fstab_config_with_client(client, block, false, p);
            const opt_ro = extract_option(parse_options(opts), "ro");
            if (!opt_ro) {
                if (p.pathname == "/")
                    return pmp + "/" + subvol.pathname;
                else
                    return pmp + subvol.pathname.substring(p.pathname.length);
            }
        }
    }
    return null;
}

The libblockdev create_subvolume invocation would just be $subvolume_parent_dir / $subvolumename then.

Our Deletion code works similar but also allows for recursive deletion or subvolumes. Something which would be trivial to support once libblockdev uses libbtrfsutil which supports passing a RECURSIVE_DELETE flag.

What do you think? This would hide the complexity of figuring out the full path with mount point from the developer and you will always need to know the parent subvolume when creating one, except I suppose when you make a new subvolume on top of the root subvolume (id 5) so I guess the api should allow passing just a subvolume name for that.

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

2 participants