-
-
Notifications
You must be signed in to change notification settings - Fork 220
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
Conversation
d2318ab
to
7af7249
Compare
7af7249
to
8d071db
Compare
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 ;) |
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 |
LUKS support would be nice. |
There was a problem hiding this 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 ¯\(ツ)/¯
There was a problem hiding this 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.
Co-authored-by: Jörg Thalheim <[email protected]>
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 |
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:
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