leres retitled this revision from Add an option to edit the ZFS pool creation options. to zfsboot: Add an option to edit the ZFS pool creation options..Nov 10 2024, 11:35 PM

emaste added a subscriber: emaste.Nov 11 2024, 8:51 PM

I just tested this option to disable the bclone feature for the root pool. This worked fine. I think this is a nice option to have.

grahamperrin added a subscriber: grahamperrin.Nov 16 2024, 12:01 AM

dim published this revision for review.Dec 16 2024, 3:35 PM

I think this is a handy feature, and can't really find any problems with it. I know that I personally prefer zstd compression, and currently I have to start the bsdinstaller myself using:

BSDINSTALL_CONFIGCURRENT=yes ZFSBOOT_POOL_CREATE_OPTIONS="-O compress=zstd -O atime=off" /usr/sbin/bsdinstall

but it would be nicer if you could directly do this in the dialog...

Generally looks good to me.

usr.sbin/bsdinstall/scripts/zfsboot
380	It might nice if this showed the current options or is that potentially too large?

This revision is now accepted and ready to land.Dec 17 2024, 2:21 PM

In D47478#1097126, @jhb wrote:

It might nice if this showed the current options or is that potentially too large?

Do you mean the entire command line, all command arguments, or all options/flags? The current version does all the options/flags (see attached).

(And I don't think we need to worry about input width; dialog seems to do a good job of dealing when the contents of the input box is large, you can use arrow keys to scroll left and right.)

guest-patmaddox added a subscriber: guest-patmaddox.Dec 18 2024, 10:09 AM

dim added inline comments.Dec 18 2024, 6:54 PM

usr.sbin/bsdinstall/scripts/zfsboot
329	Is there supposed to be a space at the end of the message here? I don't see it in the other prompts.
380	I think what jhb means here is that instead of `'*'` you might want to use `$ZFSBOOT_POOL_CREATE_OPTIONS` ?

In D47478#1097191, @leres wrote:

In D47478#1097126, @jhb wrote:

It might nice if this showed the current options or is that potentially too large?

Do you mean the entire command line, all command arguments, or all options/flags? The current version does all the options/flags (see attached).

Btw I can't view F105565279, at least not via e.g. https://reviews.freebsd.org/F105565279. There is probably some sort of permission issue.

I tried putting the options in the menu but I was worried they would quickly get too wide. But I guess if the options are too wide to view the user can make the window wider

The blank is a typo, I'll fix it.

I think I fixed the permissions on F105565279.

Address review feedback:
Show ZFSBOOT_POOL_CREATE_OPTIONS in main menu.
Remove extraneous blank in message.

This revision now requires review to proceed.Dec 18 2024, 7:12 PM

Harbormaster completed remote builds in B61245: Diff 148209.Dec 18 2024, 7:12 PM

So I wonder what the dialog looks like now by default on an 80x25 console? Does bsddialog do something sane to truncate the option list if it is too long or does it render oddly?

In D47478#1097625, @jhb wrote:

So I wonder what the dialog looks like now by default on an 80x25 console? Does bsddialog do something sane to truncate the option list if it is too long or does it render oddly?

I created some frame grabs with a 80x34 xterm (my default since the 80's)

zfsboot2 is with the installation default.
zfsboot3 is with some extra (bogus) flags.
zfsboot4 is the same as zfsboot3 but with a 121x34 window.

leres attached a referenced file: F105664779: zfsboot4.png. (Show Details)Dec 18 2024, 9:57 PM

leres attached a referenced file: F105664767: zfsboot3.png. (Show Details)

leres attached a referenced file: F105664753: zfsboot2.png. (Show Details)

In D47478#1097638, @leres wrote:

In D47478#1097625, @jhb wrote:

So I wonder what the dialog looks like now by default on an 80x25 console? Does bsddialog do something sane to truncate the option list if it is too long or does it render oddly?

I created some frame grabs with a 80x34 xterm (my default since the 80's)

zfsboot2 is with the installation default.
zfsboot3 is with some extra (bogus) flags.
zfsboot4 is the same as zfsboot3 but with a 121x34 window.

That’s definitely a build of main? That looks to me more like GPL dialog than bsddialog.

In D47478#1097722, @jrtc27 wrote:

That’s definitely a build of main? That looks to me more like GPL dialog than bsddialog.

I guess it's using dialog; if I force it to use bsddialog the output doesn't look much different to me (just the last line).

leres attached a referenced file: F105675890: zfsboot5.png. (Show Details)Dec 19 2024, 1:15 AM

So dialog/bsddialog silently truncates overly wide output. That's better than garbling the display somehow. It might be nice if bsddialog had some sort of option to replace the end of an overly-long item with a "..." trailer, but this is certainly fine, and in particular the current default easily fits in 80 columns.

This revision is now accepted and ready to land.Dec 19 2024, 3:06 PM

Thanks for grabbing all the screen shots btw.

If nobody has any other objections, I will commit this later today.

Oh actually @leres can commit this :)

In D47478#1098383, @dim wrote:

Oh actually @leres can commit this :)

Happy to.

I'm a ports, not a src committer. Looking at the last src commit I made (c4cbf1fb) I'll need these headers, right?

PR: 282745
Approved by: jhb
Differential Revision: https://reviews.freebsd.org/D47478

In D47478#1098395, @leres wrote:
In D47478#1098383, @dim wrote:

Oh actually @leres can commit this :)

Happy to.

I'm a ports, not a src committer. Looking at the last src commit I made (c4cbf1fb) I'll need these headers, right?
PR: 282745
Approved by: jhb
Differential Revision: https://reviews.freebsd.org/D47478

I think normally arc land adds Reviewed by:, but it doesn't make too much difference I guess.

In D47478#1098397, @dim wrote:

I think normally arc land adds Reviewed by:, but it doesn't make too much difference I guess.

Sorry to be such a noob. git doesn't like my .netrc (I think this is due to a recent change to curl) and when I remove my .netrc I get a 403 trying to push the change.

arc-land.txt1 KBDownload

leres attached a referenced file: F105872526: arc-land.txt. (Show Details)Dec 21 2024, 10:00 PM

Maybe you can just avoid arc, and do a regular commit on top of main, rebasing whatever you have now. Just ensure the Differential Revision: https://reviews.freebsd.org/D47478 line is in there, then Phabricator can close this review.

When I started today, the phabricator (returning either 502 bad gateway, a broken page, or a correct looking page that wouldn't let me post a comment). I decided it would be best to wait for that to resolve...

Next, I think 2a3bac31 fixed ftp/curl's parsing of .netrc, certainly git no longer balks at my mine.

Finally, thinking about this a bit I believe my 403 problem was due to my lack of a src commit bit and no "Approved by" header. My first attempt to "arc land" consolidated the two commits in my git tree (and added a "Differential Revision" header); I used "git commit --amend" to add other headers. At that point "arc land" was happy.

consolidate commits after (failed) attempts to run "arc land"

This revision now requires review to proceed.Dec 22 2024, 8:50 PM

Harbormaster completed remote builds in B61306: Diff 148335.Dec 22 2024, 8:51 PM

Even with the "Approved by" header "arc land" still gives me 403 (as does "git push").

It's probably best if one of you guys does the commit.

This revision was not accepted when it landed; it landed in state Needs Review.Thu, Dec 26, 2:50 PM

Closed by commit rGca7e12f7df22: bsdinstall zfsboot: Add an option to edit the ZFS pool creation options (authored by leres, committed by dim). · Explain Why

This revision was automatically updated to reflect the committed changes.

dim added a commit: rGca7e12f7df22: bsdinstall zfsboot: Add an option to edit the ZFS pool creation options.

dim added a commit: rG7d6ae5874ede: bsdinstall zfsboot: Add an option to edit the ZFS pool creation options.Mon, Dec 30, 7:21 PM

dim added a commit: rG6258b5bf0670: bsdinstall zfsboot: Add an option to edit the ZFS pool creation options.