Various work on OpenZFS and ZFS/FreeBSD.
Nov 15 2021
Nov 14 2021
Also add to Makefile.inc1 and SUBDIR_DEPEND_*. Should always work now.
Alright, I incorporated these last feedback items. I'm going to commit this shortly.
Thanks to all reviewers, your feedback and suggestions helped a lot in improving the text!
Nov 13 2021
As far as I'm concerned, I think you can go ahead and commit at your discretion after this round of (very minor) suggestions. Getting to diminishing returns here.
Oct 30 2021
A buildworld fails after applying this patch to main, seems to be while linking libavl:
Oct 29 2021
Change an instance of "alone" to "only" in the L2ARC description to not confuse people about adding multiple devices there.
Reply with confirmation about the L2ARC vdevs question.
Oct 27 2021
I'll have another look at the whole thing, but it will likely be a few days before I can.
Update with some of my own replies.
Yet another update addressing some of @pauamma_gundo.com's comments.
Oct 19 2021
Looks right to me.
Oct 16 2021
Remove libzfs_core dep from zfsd
This looks good, but I'm surprised that there aren't more transitive dependencies that can be removed. For example, zfsd doesn't use libzfs_core directly, only through libzfs. Can libzfs_core be removed from its LIBADD?
Sep 27 2021
Aaaaaaaand done! (Split glossary to 1 sentence per line, please.)
Sep 26 2021
Mentioning it here to avoid repeating it a gazillion times: 1 sentence per line.
Sep 14 2021
Sep 12 2021
Sounds like a bug in the upstream module. I’d prefer to see that fixed rather than using autofs to work around the zfs_key pam module bug. Or see if it’s feasible to implement a pure autofs solution (i.e., autofs loads and unloads the key for a given dataset). If neither of the above options are possible, I’m more inclined to oppose this change until a more streamlined solution can be found.
Sep 10 2021
Sep 9 2021
According to the OpenZFS commit, the zfs_key pam module already unmounts the dataset and unloads the key when a session is closed.
Sep 8 2021
Does it make sense to have autofs unload zfs keys if it doesn't even know how to load them?
Sep 7 2021
Sep 6 2021
Abandoning, due to the existence of the upstream module in cddl
In general I don't think it's a good idea to add an option to opt-in unload of crypto keys: it should always be done unless explicitly opted out (or can't be opted out), so I'd recommend either removing the option and default to be safe, or make it an opt-out option (when specified, do not unload crypto key).
This proposed module have a lot of behavior that is very different from the Oracle and the OpenZFS implementation and since we are naming it the same way I don't think it's a good idea to diverge from the other implementations, especially the most commonly used parameters like homes= and the lack of mounting of the datasets.
Sep 5 2021
Update including the lastest suggestions to the lower last part of the chapter.
Another big update, including both @ygy's and @pauamma_gundo.com's comments.
I should have kept the diff much smaller in retrospect, sorry about that. We're getting close to the final piece now, thanks to your efforts!
The line breaks were caused by my editor trying to be smarter than it should be.
Sep 4 2021
Taking a break before tackling the wall of text, but splitting lines into sentences first would make needed edits easier to spot. Pretty please?
Sep 3 2021
Hoping to finish reviewing it tonight. 4 installments is pushing it.
I've given it another once-over, and am pretty happy with it, so unless anyone else has any interjections, I say it's good to go.
New version that includes suggestions by @pauamma_gundo.com
Phew, a lot of changes, most of which I concur with and changed. Thanks!
Updated patch follows.
Sep 1 2021
Taking me longer than i thought (when doesn't it for anyone?) but getting there.
Aug 30 2021
A few comments, mostly agreeing with the suggestion.
Another fine set of changes, including most (if not all) suggestions by @pauamma_gundo.com
Second installment. (Let me know if I'm too nitpicky or overwhelming.)
Aug 29 2021
Update diff to incorporate changes from @debdrup and @pauamma_gundo.com . Looking forward to the second half of changes. I know it's tedious and I should have kept it shorter. Will consider than for future changes like this on other chapters.