freebsd-update.8: Document CreateBootEnv Also, add bectl(8) to section "See Also". [1] PR: 261716 MFC after: 1 week Fixes: f28f13890541 freebsd-update: create a ZFS boot environment on install Differential Revision: https://reviews.freebsd.org/D34169 Co-authored-by: Tobias Rehbein <tobias.rehbein@web.de> [1]
Details
Details
Diff Detail
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| share/man/man5/freebsd-update.conf.5 | ||
|---|---|---|
| 282 | Someone thought along the same lines (https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=261716) but suggested bectl in see-also, which looks good to me. | |
Comment Actions
Other than the nit and the question of when it should be pushed, this change looks good to me.
| share/man/man5/freebsd-update.conf.5 | ||
|---|---|---|
| 222 | CreateBootEnv is the parameter, yes/no is the value. | |
| 256 | There have been reports of boot environments being created if freebsd-update is run outside of a jail, but is meant to update a jail. I'm wondering if it's better to wait for that to be fixed by @kevans or whether it should be noted in a BUGS section (I'd personally lean towards the first, especially if the fix gets an errata notice). | |
Comment Actions
Thanks, @pauamma_gundo.com. Please accept the revision once you are happy with it. Otherwise I'm not sure if I should add you to the list of reviewers or not ;)
| share/man/man5/freebsd-update.conf.5 | ||
|---|---|---|
| 235 | I'm not sure what RTC is. It seems to be the timezone on the host. I added a command. It should be clearer now. | |
| 256 | ||
| 282 | Done. | |
Comment Actions
LGTM. (Is that what you meant by "accept"? I've never done the one reviewers who are also committers do, which I assume it the item in the Add Action menu, but isn't described in the reviewer workflow (https://wiki.freebsd.org/Phabricator#Review_Revision_and_Add_Feedback). Am I allowed to?
Comment Actions
Anybody can 'accept' to indicate they have reviewed the changes and approve of the change. It isn't 'approval' in the sense of granting permission, though, which is why anybody can do it.
It is up to the committer that makes the commit how much weight to put on any review.
Might not be a bad idea to clarify the phabricator info... and migrate it into the handbook. It's an established part of the project now and so should move from the wiki which generally is for more experimental things...
Comment Actions
Yes, feel free. There's not a whole lot you can't/shouldn't do in Phab as a non-committer; about the only thing I can think of is commandeering or editing somebody else's review ("Edit Revision" / "Update Diff" is the specific verbiage) without prior discussion/consent. Adding yourself as a blocking reviewer is also often poor form, but it's very unlikely to elicit more than somebody questioning if the 'blocking' aspect is really appropriate.
| share/man/man5/freebsd-update.conf.5 | ||
|---|---|---|
| 252 | I would probably change this to "ZFS is not used" and add "the ZFS root is not setup for boot environments". The bootenv still will use the recently added bectl check to determine that we the pool layout looks sane for boot environments, which may not be the case for some non-standard or maybe super legacy layouts. It's probably okay to be a bit hand-wavy there and xref bectl.8 in a "caveats may be listed in bectl(8)" kind of way. | |
| 256 | IMO I'd go ahead and omit it; I'm going to fix that particular bug tomorrow (today, in your timezone and certainly by the time you read this) and I'll add the additional caveat above: If the basedir (-b) or jail (-j) options are used to update other roots. I'll probably go ahead and EN this manpage update while I'm at it. | |
Comment Actions
- Upadate the reasons why the CreateBootEnv option is sometimes a nop (as suggested by @kevans).
Comment Actions
- Update the description of how the bootenv name is generated after e01e8f911b935eabcc35b4d121951e4e21042ee5
| share/man/man5/freebsd-update.conf.5 | ||
|---|---|---|
| 235 | I meant the battery-backed hardware clock. (I think the IBM PC/AT technical reference documentation called it "RTC", but mine is long-lost.) | |
| 241 | Hmm. This means "whichever value (or none) sudo/su/doas/root login sets TZ to", which may not be what the sysadmin expects. This review may be the wrong place to ask, but would it be better to toss in a -u or a %Z? | |
| 246 | Maybe a more recent version, like 12.2? May be my taste, though. | |
Comment Actions
I think that all the comments have been addressed. I'll wait with committing this patch for a few more days to let you review it and accept it.