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

zfs: add ZFS "topology" features like explicit vdevs, cache, and special #723

Merged
merged 10 commits into from
Sep 3, 2024

Conversation

dmadisetti
Copy link
Contributor

@dmadisetti dmadisetti commented Aug 1, 2024

Addresses vdev definitions, partially addressing #298

I don't think this is the most elegant solution, so happy to churn over this a few times. Happy to receive feedback.
TODOs as I see it:

  • More comprehensive tests, currently just a stub
  • Clean up. I'm not exceptionally happy with this and would love some feedback, but I tried to stick the spec in ZFS features support #298

I think it might be worth having a lib/types/zfs folder. The vdev submodule could then be moved to be a standalone file without polluting the diskoTypes. Might be worth doing in another PR.

stakeholders: @victorklos @nbraud @luishfonseca @Mic92 @Lassulus

@dmadisetti dmadisetti force-pushed the dm/zfs-extra branch 2 times, most recently from d2318ab to 7af7249 Compare August 2, 2024 01:55
@victorklos
Copy link

How very nice that you are working on this! Glad to confirm your code is working and allowed me to use disko on a previously unsupported ZFS layout.

A couple of things remain, of which one is critical: the members of a vdev can either be whole disks or partitions.

Are you interested in feedback on a broader usability/logical level, or is your goal to produce something that is usable? Just asking before I get carried away ;)

@dmadisetti
Copy link
Contributor Author

Thanks @victorklos ! This was more so that I could utilize Disko myself in a similar "hack it together" context.

I would like a potential refactor and before upstreaming though.

I would love some feedback, maybe in the context of the comments that @ Mic92 has already left. I think they are good suggestions, I just haven't gotten around to it

@0x450x6c
Copy link
Contributor

LUKS support would be nice.

Copy link
Contributor Author

@dmadisetti dmadisetti left a comment

Choose a reason for hiding this comment

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

Addressed comments, and pulled in commit from @0x450x6c

I can clean up the commit history, but seems like everything gets merged anyway ¯\(ツ)

Copy link
Member

@Mic92 Mic92 left a comment

Choose a reason for hiding this comment

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

Looks overall pretty good.

@dmadisetti dmadisetti marked this pull request as ready for review August 27, 2024 12:29
@dmadisetti dmadisetti changed the title WIP: add ZFS "topology" features like explicit vdevs, cache, and special zfs: add ZFS "topology" features like explicit vdevs, cache, and special Aug 27, 2024
@dmadisetti
Copy link
Contributor Author

Checks are not deterministic locally, and I cannot reliably cause them to intentionally fail.

I'm OK with what's here, there is more work to be done to support all vdevs, and I think some of the edgecase tests can be picked up with that.

As such, taking off draft

@Mic92 Mic92 merged commit 37c83c0 into nix-community:master Sep 3, 2024
3 checks passed
@Mic92 Mic92 mentioned this pull request Sep 3, 2024
2 tasks
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.

4 participants