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.
Details
Diff Detail
- Repository
- R9 FreeBSD doc repository
- Lint
Lint Skipped - Unit
Tests Skipped
Event Timeline
Here is my quick go through.
vmrun.8 | ||
---|---|---|
26 | Nit pick: extra line | |
59–62 | ||
65 | ||
69–71 | There are a number of ways to load this (in terms of configuration, plus manually loading), not only kldload(8). | |
73 | 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. | |
74–77 | 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. | |
83 | 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). | |
95–101 | ||
113–114 | Do not mention it if it simply does not exist for now. | |
188–195 | ||
199–202 | I don't see it anywhere in the script, and the commit I am talking about is 9cfd8046a48e. |
By the way, we refer to the machine created under bhyve as guest rather than guest host.
That's definitely a nice addition. Thanks for working on that.
vmrun.8 | ||
---|---|---|
1 | We are missing the SPDX tag here as well. | |
3 | This part of the license can be dropped. See https://reviews.freebsd.org/D26980 for example. | |
35 | Per style(9), options should be sorted like this: -AaEhiTv. Capital letters should go first. | |
82 | ||
85 | ||
90 | Arguments to other flags need to be added as well. | |
92 | ||
186 |
Here is a new version based on all the feedback from everyone, keep the good comments coming please!
vmrun.8 | ||
---|---|---|
35 | Duh, thank you, I fixed this. | |
59–62 | Slightly different version worked up, please let me know what you think (new diff attached). | |
69–71 | True, I cleaned that up since we don't need to explain every one. | |
73 | I altered the wording a little bit. | |
83 | Okay, I used the virtio_blk 4 xref. | |
85 | Thank you, surprised I didn't catch that. | |
90 | 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. | |
92 | 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. | |
95–101 | 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. | |
113–114 | 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:
+++ vmrun.sh 2021-11-15 14:53:31.233837000 -0500 F) vncsize="${OPTARG}" ;; + h) H) host_base=`realpath ${OPTARG}` ;; | |
186 | I'm sorry, I don't understand this, I thought it was whitespace but I cannot find it in my version. | |
199–202 | 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. |
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.
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!