New manual page for vmrun.sh

[trhodes]

Summary

Someone on Twitter said "vmrun.sh should have a manual page" and I decided to write one. It provides the list of accepted flags, though the unused one is hidden and noted in BUGS, and an example of creating and installing FreeBSD in a new guest.

[khng]

Here is my quick go through.

vmrun.8

27

Nit pick: extra line

60–63

66

70–72

There are a number of ways to load this (in terms of configuration, plus manually loading), not only kldload(8).

74

From the script, it is that you must at least create and/or configure a tap0 if you do not specify the -t option at all.

75–78

The tap(4), which defaults to tap0 if unspecified, is only for the script to run. Whether you must bridge it with another NIC is another question.

84

The virtio-blk paravirtualization device implementation is irrelevant to the virtio(4) kernel driver, since virtio(4) is on the guest's perspective rather than the host's perspective, so I don't think it is appropriate to cross-reference virtio(4).

96–102

114–115

Do not mention it if it simply does not exist for now.

189–196

200–203

I don't see it anywhere in the script, and the commit I am talking about is 9cfd8046a48e.

[khng]

By the way, we refer to the machine created under bhyve as guest rather than guest host.

[0mp]

That's definitely a nice addition. Thanks for working on that.

vmrun.8

2

We are missing the SPDX tag here as well.

4

This part of the license can be dropped. See https://reviews.freebsd.org/D26980 for example.

36

Per style(9), options should be sorted like this: -AaEhiTv. Capital letters should go first.

83

86

91

Arguments to other flags need to be added as well.

93

187

[trhodes]

Here is a new version based on all the feedback from everyone, keep the good comments coming please!

diff_vmrun.diff5 KBDownload

vmrun.8

36

Duh, thank you, I fixed this.

60–63

Slightly different version worked up, please let me know what you think (new diff attached).

70–72

True, I cleaned that up since we don't need to explain every one.

74

I altered the wording a little bit.

84

Okay, I used the virtio_blk 4 xref.

86

Thank you, surprised I didn't catch that.

91

Done - although, because some of the arguments are a little long in the SYNOPSIS, I reduced the wording a bit. It may be preferred or not, maybe SYNOPSIS should be updated? I'm open to suggestions.

93

I went back and forth on "two" versus "2" because in writing we use the former, but parameters as a literal would be the latter. I'm going to use "2" because it's technically a parameter.

96–102

I used a slightly different version to avoid repeating "device" twice in the same sentence. It still reads a tad bit awkward and I might need to kick it around a bit unless we like my suggestion.

114–115

This is interesting - the option is in usage(), but not part of the OPTARGs. I'm 50/50 here because the catch all will print the message that "-h" should be printing. But at the same time, a quick help menu either through the catch all or "-h" is certainly useful. And it's a very minor change:

• current/share/examples/bhyve/vmrun.sh 2021-11-14 17:40:52.943682000 -0500

+++ vmrun.sh 2021-11-15 14:53:31.233837000 -0500
@@ -165,6 +165,9 @@

	F)
		vncsize="${OPTARG}"
		;;

+ h)
+ usage
+ ;;

	H)
		host_base=`realpath ${OPTARG}`
		;;

187

I'm sorry, I don't understand this, I thought it was whitespace but I cannot find it in my version.

200–203

I removed the entire section because, after a second look at the script, I observed what I stated above. I really think the "-h" flag was intended. The "-h" flag is in the version you linked, it's just near the bottom of usage. Easy to miss because you expect "-H" and "-h" to be grouped together, and I think it's what tripped me up as well.

[0mp]

In D32985#745570, @trhodes wrote:

Here is a new version based on all the feedback from everyone, keep the good comments coming please!

diff_vmrun.diff5 KBDownload

Thanks for the update! Would it be possible for you to post the new diff to Phabricator? It's slightly easier to post comments in the web UI.

[trhodes]

I'm sorry - I realized that updating the diff is done differently here. I made it work with a different review but added it incorrectly here. Thank you for asking!

[bcr]

Any updates on this one?

[pauamma_gundo.com]

Arguably a matter of taste, but maybe s/utiliz(e|es|ing|ed)/us\1/g too?

All minor, fixable on commit.

(Also, if the lack of a manpages group reviewer isn't deliberate, consider this manpages approval.)

vmrun.8

27

Bump.

112

Comma splice.