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

Add support for ZFS delegation #12056

Merged
merged 22 commits into from
Jul 27, 2023
Merged

Add support for ZFS delegation #12056

merged 22 commits into from
Jul 27, 2023

Conversation

stgraber
Copy link
Contributor

@stgraber stgraber commented Jul 20, 2023

This adds support for the ZFS dataset delegation feature found in OpenZFS 2.2.

On the LXD front, this is done through a new zfs.delegate configuration option on storage volumes.
One can therefore do:

lxc init images:ubuntu/22.04 u1
lxc storage volume set default container/u1 zfs.delegate=true
lxc start u1

To get a container that has delegated ZFS access to its dataset and anything underneath it.
The same works as expected with custom storage volumes which can therefore allow multiple containers to interact with the dataset.

As part of implementing this, the ZFS storage driver had to be updated to handle recursive operations throughout, so that those nested datasets don't get lost during a backup or migration.

Lastly, the way this is implemented is through new DelegateInstance and DelegateCustomVolume functions on the storage backend which are called after the instance is running (as the PID is required). This allowed for the work to be mostly contained to the ZFS storage driver. That's with the exception of the logic enabling /dev/zfs in the container as that did need to be directly to the LXC driver and has to be conditional on the correct ZFS version being on the system as one doesn't want to expose /dev/zfs to containers on prior versions (potential security risk there).

This work was sponsored by Buddy (https://buddy.works) who also sponsored the matching OpenZFS work.

Closes #11796

@github-actions github-actions bot added Documentation Documentation needs updating API Changes to the REST API labels Jul 20, 2023
lxd/storage/backend_lxd.go Outdated Show resolved Hide resolved
lxd/storage/backend_lxd.go Outdated Show resolved Hide resolved
lxd/storage/backend_lxd.go Outdated Show resolved Hide resolved
lxd/storage/backend_lxd.go Outdated Show resolved Hide resolved
@tomponline
Copy link
Member

@stgraber does this close #11796 ?

@stgraber
Copy link
Contributor Author

@tomponline it does indeed

@stgraber
Copy link
Contributor Author

Review feedback addressed

@stgraber
Copy link
Contributor Author

@tomponline rebased following changes to main branch

@tomponline
Copy link
Member

Thanks @stgraber I'm going to look at this once I've got the LXD 5.16 release done and dusted. Cheers

We've also got #12067 which might cause this to need to be rebased again once its resolved.

Copy link
Member

@tomponline tomponline left a comment

Choose a reason for hiding this comment

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

Thanks @stgraber I'm looking forward to this!

I've added a few comments/questions.

WRT to the recursion changes, we could land that as a separate PR as it feels like that is a self contained precursor to the actual ZFS delegation?

test/suites/storage_driver_zfs.sh Show resolved Hide resolved
lxd/storage/drivers/driver_zfs_volumes.go Show resolved Hide resolved
lxd/storage/drivers/driver_zfs_volumes.go Show resolved Hide resolved
lxd/storage/drivers/driver_zfs_utils.go Show resolved Hide resolved
lxd/storage/drivers/driver_zfs_volumes.go Outdated Show resolved Hide resolved
lxd/instance/drivers/driver_lxc.go Show resolved Hide resolved
lxd/storage/drivers/driver_zfs_volumes.go Outdated Show resolved Hide resolved
test/suites/storage_driver_zfs.sh Show resolved Hide resolved
lxd/storage/backend_lxd.go Outdated Show resolved Hide resolved
@stgraber
Copy link
Contributor Author

WRT to the recursion changes, we could land that as a separate PR as it feels like that is a self contained precursor to the actual ZFS delegation?

We could but it wouldn't be testable on its own as the only case where recursion makes sense is when delegation is also possible. So it feels like having the two merged together (in separate commits as they are here) is appropriate.

stgraber added 20 commits July 26, 2023 11:54
Signed-off-by: Stéphane Graber <[email protected]>
Sponsored-by: Buddy (https://buddy.works)
Signed-off-by: Stéphane Graber <[email protected]>
Sponsored-by: Buddy (https://buddy.works)
Signed-off-by: Stéphane Graber <[email protected]>
Sponsored-by: Buddy (https://buddy.works)
Signed-off-by: Stéphane Graber <[email protected]>
Sponsored-by: Buddy (https://buddy.works)
Signed-off-by: Stéphane Graber <[email protected]>
Sponsored-by: Buddy (https://buddy.works)
Signed-off-by: Stéphane Graber <[email protected]>
Sponsored-by: Buddy (https://buddy.works)
Signed-off-by: Stéphane Graber <[email protected]>
Sponsored-by: Buddy (https://buddy.works)
Signed-off-by: Stéphane Graber <[email protected]>
Sponsored-by: Buddy (https://buddy.works)
Signed-off-by: Stéphane Graber <[email protected]>
Sponsored-by: Buddy (https://buddy.works)
Signed-off-by: Stéphane Graber <[email protected]>
Sponsored-by: Buddy (https://buddy.works)
Signed-off-by: Stéphane Graber <[email protected]>
Sponsored-by: Buddy (https://buddy.works)
Signed-off-by: Stéphane Graber <[email protected]>
Sponsored-by: Buddy (https://buddy.works)
Signed-off-by: Stéphane Graber <[email protected]>
Sponsored-by: Buddy (https://buddy.works)
Signed-off-by: Stéphane Graber <[email protected]>
Sponsored-by: Buddy (https://buddy.works)
stgraber added 2 commits July 26, 2023 11:55
Signed-off-by: Stéphane Graber <[email protected]>
Sponsored-by: Buddy (https://buddy.works)
Signed-off-by: Stéphane Graber <[email protected]>
Sponsored-by: Buddy (https://buddy.works)
@stgraber
Copy link
Contributor Author

@tomponline I believe that's all feedback addressed, just waiting to confirm I didn't break anything with those MountInfo changes :)

@stgraber
Copy link
Contributor Author

@tomponline all tests passed, ready to go

Copy link
Member

@tomponline tomponline left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@tomponline tomponline merged commit 6fd175c into canonical:main Jul 27, 2023
@dalbani
Copy link

dalbani commented Jul 27, 2023

First, it's really great that you implemented it 👍
Out of curiosity, which release of LXD will this feature find its way in?
And I suppose that a backport to LXD 5.0 is not to be expected?

@tomponline
Copy link
Member

This will be in lxd 5.17.

As for backporting. As it depends on a very recent version of zfs and kernel, it'll be a while before we back port and we'll need to switch to the newer core22 base first too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Changes to the REST API Documentation Documentation needs updating
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support ZFS delegation to container
4 participants