Refactor configuration management in bhyve.

[jhb]

Summary

Replace the existing ad-hoc configuration via various global variables
with a small database of key-value pairs. The database supports
heirarchical keys using a MIB-like syntax to name the path to a given
key. Values are always stored as strings. The API used to manage
configuation values does include wrappers to handling boolean values.
Other values use non-string types require parsing by consumers.

The configuration values are stored in a tree using nvlists. Leaf
nodes hold string values. Configuration values are permitted to
reference other configuration values using '%(name)'. This permits
constructing template configurations.

All existing command line arguments now set configuration values. For
devices, the "-s" option parses its option argument to generate a list
of key-value pairs for the given device.

A new '-o' command line option permits setting an individual
configuration variable. The key name is always given as a full path
of dot-separated components.

A new '-k' command line option parses a simple configuration file.
This configuration file holds a flat list of 'key=value' lines where
the 'key' is the full path of a configuration variable. Lines
starting with a '#' are comments.

In general, bhyve starts by parsing command line options in sequence
and applying those settings to configuration values. Once this is
complete, bhyve then begins initializing its state based on the
configuration values. This means that subsequent configuration
options or files may override or supplement previously given settings.

A special 'config.dump' configuration value can be set to true to help
debug configuration issues. When this value is set, bhyve will print
out the configuration tree and exit after all command line argumnts
have been parsed but before starting a virtual machine.

Most command line argments map to a single configuration variable,
e.g. '-w' sets the 'x86.strictmsr' value to false. A few command
line arguments have less obvious effects:

• Multiple '-p' options append their values (as a comma-seperated list) to "vcpu.N.cpuset" values (where N is a decimal vcpu number).

• For '-s' options, a pci.<bus>.<slot>.<function> node is created. The first argument to '-s' (the device type) is used as the value of a "device" variable. Additional comma-separated arguments are then parsed into 'key=value' pairs and used to set additional variables under the device node. A PCI device emulation driver can provide its own hook to override the parsing of the additonal '-s' arguments after the device type.
  
  After the configuration phase as completed, the init_pci hook then walks the "pci.<bus>.<slot>.<func>" nodes. It uses the "device" value to find the device model to use. The device model's init routine is passed a reference to its nvlist node in the configuration tree which it can query for specific variables.
  
  The result is that a lot of the string parsing is removed from the device models and centralized. In addition, adding a new variable just requires teaching the model to look for the new variable.

• For '-l' options, a similar model is used where the string is parsed into values that are later read during initialization. One key note here is that the serial ports use the commonly used lowercase names from existing documentation and examples (e.g. "lpc.com1") instead of the uppercase names previously used internally in bhyve.

Test Plan

• tested a bit with an UEFI based config though I don't have any existing UEFI VMs

[wanpengqian_gmail.com]

usr.sbin/bhyve/bhyverun.c

1370

Setting config.dump=0 still dump the config and exit.

[wanpengqian_gmail.com]

In D26035#577636, @araujo wrote:

The configuration file is not intuitive at all, how I will know what pci slot my virtio-blk shall live?
Why not simplify it instead of complicate more? The configuration file supposed to be something to simplify the command line, and instead is getting more complex than bhyve cli itself. Perhaps we will need another io<something> wrapper to simplify the creation of bhyve configuration files.

Could we use a similar approach of the configuration file syntax like everybody else is using? I know make things simple is hard.

The config file here is for bhyve internal, it is accurate but complex.
Maybe other hypervisor manager, such as vm-bhyve will read their simple config file and output this config style for bhyve, then launch the bhyve process.

[araujo]

In D26035#577644, @wanpengqian_gmail.com wrote:

In D26035#577636, @araujo wrote:

The configuration file is not intuitive at all, how I will know what pci slot my virtio-blk shall live?
Why not simplify it instead of complicate more? The configuration file supposed to be something to simplify the command line, and instead is getting more complex than bhyve cli itself. Perhaps we will need another io<something> wrapper to simplify the creation of bhyve configuration files.

Could we use a similar approach of the configuration file syntax like everybody else is using? I know make things simple is hard.

The config file here is for bhyve internal, it is accurate but complex.
Maybe other hypervisor manager, such as vm-bhyve will read their simple config file and output this config style for bhyve, then launch the bhyve process.

In D26035#577644, @wanpengqian_gmail.com wrote:

In D26035#577636, @araujo wrote:

The configuration file is not intuitive at all, how I will know what pci slot my virtio-blk shall live?
Why not simplify it instead of complicate more? The configuration file supposed to be something to simplify the command line, and instead is getting more complex than bhyve cli itself. Perhaps we will need another io<something> wrapper to simplify the creation of bhyve configuration files.

Could we use a similar approach of the configuration file syntax like everybody else is using? I know make things simple is hard.

The config file here is for bhyve internal, it is accurate but complex.
Maybe other hypervisor manager, such as vm-bhyve will read their simple config file and output this config style for bhyve, then launch the bhyve process.

Maybe you are not familiar enough with the bhyve configuration fie attempt implementations!!!
I will drop these slides here: https://docs.google.com/presentation/d/15s9rWCe8ILJB-wTP_cZFFWZrAPI0IzG3jKvi_2zHgss/edit?usp=sharing

[wanpengqian_gmail.com]

In D26035#577645, @araujo wrote:

Maybe you are not familiar enough with the bhyve configuration fie attempt implementations!!!
I will drop these slides here: https://docs.google.com/presentation/d/15s9rWCe8ILJB-wTP_cZFFWZrAPI0IzG3jKvi_2zHgss/edit?usp=sharing

I apologize I didn't know there is a history of bhyve configuration.
Now I have read the slides you wrote before. and I think your proposal and this one has no conflict at all.

I have done some modifications to pci_ahci.c and pci_nvme.c before, and that is a pain, every file has its own parser.
And for virtual machine hypervisors, we have to provide more closed syntax than real computer topology.
while this MIB-like syntax is the best for bhyve(hypervisor) ever in my opinion. when it stores value to nvlists, the consumer(each soft controller) will be happy.
while it can do some modifications without affecting others.

Making syntax simple, that requires bhyve do more computing on runtime. while changing the pci slot/func etc. will affect the running virtual machine.(For example, changing a nic slot number, will make Windows guests recognize it as new nic, and lose the static IP setting.) so It is better to fine control this by the user.

As your UCL like configuration RFC, it can provide more friendly to users while this one provides more friendly to bhyve internal functions(programmer).
We will be happy if both can cooperate together in the future.

[wanpengqian_gmail.com]

usr.sbin/bhyve/pci_fbuf.c

256

Already plus one in line 249.

275

Already plus one in line 268.

[freqlabs]

Agreed it's a shame this isn't UCL, it could be much closer to jail.conf (which should be made UCL someday, but it's very nearly compatible on a surface level). It doesn't look like any features unique to libnv are used.

The config file overriding options passed on the command-line seems backwards. I'd expect to be able to override config file settings explicitly on the command line.

[rgrimes]

To all those harping about it not being foo or bar or zed, this *is* an underlying frame work that is very flexible and allows one to implement what ever foo, bar or zed configuration you want on top of it. This leaves json, xml, or any other implementation out of the base layer code and those can be implemented in a layer above this. This has been discussed for a some time extensively by those that attend the semi-monthly bhyve call.

[jhb]

In D26035#577636, @araujo wrote:

The configuration file is not intuitive at all, how I will know what pci slot my virtio-blk shall live?
Why not simplify it instead of complicate more? The configuration file supposed to be something to simplify the command line, and instead is getting more complex than bhyve cli itself. Perhaps we will need another io<something> wrapper to simplify the creation of bhyve configuration files.

This is much more complex than bhyve cli itself.
pci.0.0.0.device=hostbridge
pci.0.2.0.device=virtio-net
pci.0.2.0.backend=%(tap)
pci.0.3.0.device=virtio-blk
pci.0.3.0.path=/dev/zvol/zroot/bhyve/%(name)
pci.0.4.0.device=virtio-rnd
pci.0.31.0.device=lpc
lpc.com1.device=stdio
lpc.com2.device=/dev/nmdm%(name)2B
lpc.bootrom=/usr/local/share/uefi-firmware/BHYVE_UEFI.fd

Could we use a similar approach of the configuration file syntax like everybody else is using? I know make things simple is hard.

To be clear, one can build a UCL parser (I think I mentioned this on the list) that uses rules to write a backing config. I explicitly mentioned there that the UCL syntax could auto-compute the PCI slots to use while having an optional keyword to force a specific PCI slot if desired. Reading the config file is just a means of translating the config the UCL file likes into a set of configuration variable settings. That is, you could very much do something like:

disk {
      path = "/foo";
      type = "virtio-blk"
}

And the UCL parser would use an internal routine in bhyve to say "give me the next free PCI slot" and use that to pick the right "pci.<b>.<s>.<f>" node to hang its explicit variables off of. Some users of bhyve have expressed a desire for a flat config store (and the ability to set arbitrary options via '-o' that override what it is in a base config file), so the backing store for the config needs that level of detail, but you can populate the config in various ways.

[jhb]

In D26035#577801, @freqlabs wrote:

Agreed it's a shame this isn't UCL, it could be much closer to jail.conf (which should be made UCL someday, but it's very nearly compatible on a surface level). It doesn't look like any features unique to libnv are used.

The config file overriding options passed on the command-line seems backwards. I'd expect to be able to override config file settings explicitly on the command line.

Please read the mail on virtualization@. It is very feasible to add a UCL config file syntax (and that is even a desired plan, just not currently implemented). Using libnv is more convenient than writing trees by hand in C.

I don't follow your last comment. The example I gave uses command line options to augment what is in the config file as it had "template" values. However, in general the rule is that each command line argument is processed in sequence and each command line argument results in zero or more modifications to the configuration. An option occurring before a '-f <file>' is processed first and can be overridden by the config file, and an option after the '-f' is processed later and will override settings in the config file. You can also have multiple '-f' files if you wanted to (or multiple UCL files in the future).

In case the template part isn't clear, the same config file can be used for different VMs, e.g.:

bhyve -f uefi.cfg -o tap=tap0 head
bhyve -f uefi.cfg -o tap=tap1 fbsd12
bhyve -f uefi.cfg -o tap=tap2 fbsd11

Where each of these VMs uses /dev/zvol/zroot/bhyve/{head,fbsd12,fbsd11} as its disk and /dev/nmdm{head,fbsd12,fbsd11}2B for com2 (which I use for kgdb over serial) without having to have separate config files. Instead, the config accepts parameters. Note that the template thing would also work with a UCL config file, e.g.

disk {
      path = "/dev/zvol/zroot/bhyve/%(name)";
}

usr.sbin/bhyve/bhyverun.c

1370

Hah, so it does. It is debatable if this feature should even make it into the tree, but I suppose it it does it should be a real boolean.

[freqlabs]

In D26035#578018, @jhb wrote:

In D26035#577801, @freqlabs wrote:

Agreed it's a shame this isn't UCL, it could be much closer to jail.conf (which should be made UCL someday, but it's very nearly compatible on a surface level). It doesn't look like any features unique to libnv are used.

The config file overriding options passed on the command-line seems backwards. I'd expect to be able to override config file settings explicitly on the command line.

Please read the mail on virtualization@. It is very feasible to add a UCL config file syntax (and that is even a desired plan, just not currently implemented). Using libnv is more convenient than writing trees by hand in C.

Certainly, I guess I'm not subscribed to that list yet, I'll fix that :)
libucl is unsavory to work with, I know. libnv also has its shortcomings. I have somewhere stashed code to convert between the two, but the features don't map very well so it wasn't a particularly useful exercise; educational, though. But like I said, it doesn't look like anything specific to libnv is being used. If the long-term goal is to eventually support a more canonical config file format in addition to this format, I'm all for it. My main concern was that introducing a strange new config format might preclude us from that. It doesn't sound like that will be the case. May be as simple as interpreting *.cfg one way and *.conf another way.

I don't follow your last comment. The example I gave uses command line options to augment what is in the config file as it had "template" values. However, in general the rule is that each command line argument is processed in sequence and each command line argument results in zero or more modifications to the configuration. An option occurring before a '-f <file>' is processed first and can be overridden by the config file, and an option after the '-f' is processed later and will override settings in the config file. You can also have multiple '-f' files if you wanted to (or multiple UCL files in the future).

The description in the summary confused me, then. I got the impression that configuration parsing happened after command line options, but you've cleared up that misunderstanding for me, thanks.

[jhb]

• Don't skip : before port twice.
• Treat config.dump as a boolean.

[jhb]

In D26035#578022, @freqlabs wrote:

In D26035#578018, @jhb wrote:

In D26035#577801, @freqlabs wrote:

Agreed it's a shame this isn't UCL, it could be much closer to jail.conf (which should be made UCL someday, but it's very nearly compatible on a surface level). It doesn't look like any features unique to libnv are used.

The config file overriding options passed on the command-line seems backwards. I'd expect to be able to override config file settings explicitly on the command line.

Please read the mail on virtualization@. It is very feasible to add a UCL config file syntax (and that is even a desired plan, just not currently implemented). Using libnv is more convenient than writing trees by hand in C.

Certainly, I guess I'm not subscribed to that list yet, I'll fix that :)
libucl is unsavory to work with, I know. libnv also has its shortcomings. I have somewhere stashed code to convert between the two, but the features don't map very well so it wasn't a particularly useful exercise; educational, though. But like I said, it doesn't look like anything specific to libnv is being used. If the long-term goal is to eventually support a more canonical config file format in addition to this format, I'm all for it. My main concern was that introducing a strange new config format might preclude us from that. It doesn't sound like that will be the case. May be as simple as interpreting *.cfg one way and *.conf another way.

I was probably thinking more like -F vs -f or the like. Perhaps different letters if necessary to avoid being too confusing. I also wouldn't mind UCL being the "preferred" format for end-users and the flat style being intended more for orchestration frameworks that want to generate configs.

[grehan]

Given the config file syntax will likely change but the "-f" option will be around forever, can a magic number or first-line metadata comment be used to distinguish this key/value file format from a future version ?

[araujo]

In D26035#578006, @jhb wrote:

In D26035#577636, @araujo wrote:

The configuration file is not intuitive at all, how I will know what pci slot my virtio-blk shall live?
Why not simplify it instead of complicate more? The configuration file supposed to be something to simplify the command line, and instead is getting more complex than bhyve cli itself. Perhaps we will need another io<something> wrapper to simplify the creation of bhyve configuration files.

This is much more complex than bhyve cli itself.
pci.0.0.0.device=hostbridge
pci.0.2.0.device=virtio-net
pci.0.2.0.backend=%(tap)
pci.0.3.0.device=virtio-blk
pci.0.3.0.path=/dev/zvol/zroot/bhyve/%(name)
pci.0.4.0.device=virtio-rnd
pci.0.31.0.device=lpc
lpc.com1.device=stdio
lpc.com2.device=/dev/nmdm%(name)2B
lpc.bootrom=/usr/local/share/uefi-firmware/BHYVE_UEFI.fd

Could we use a similar approach of the configuration file syntax like everybody else is using? I know make things simple is hard.

To be clear, one can build a UCL parser (I think I mentioned this on the list) that uses rules to write a backing config. I explicitly mentioned there that the UCL syntax could auto-compute the PCI slots to use while having an optional keyword to force a specific PCI slot if desired. Reading the config file is just a means of translating the config the UCL file likes into a set of configuration variable settings. That is, you could very much do something like:

Definitely that is what will happen, somebody else will write a wrapper for this configuration file!!! The problem I see here is the standard(syntax and etc), it does not look like any configuration file I have ever see, and you know after you commit it, it will live there for forever, because we need to respect POLA. See bhyvectl, supossed to be a cli but it is so complex that we call it as a developer tool, because nobody knows how to use half of its features.

Have you take a look on the OpenBSD VM configuration file?

We should try to unify and simplify the hundreds of different configuration files syntax we have in the system, but no, we are adding another band-aid to the Frankenstein. I have no rights to blame the work you have done, however I'm the one that spent a good chunk of time going around the bhyve configuration file and failed to get a standard accepted. I feel frustrated to see the configuration file going in this direction masked by a nvlist implementation. The nvlist implementation is nice, but the configuration file syntax really sucks(Sorry my French).

disk {
      path = "/foo";
      type = "virtio-blk"
}

And the UCL parser would use an internal routine in bhyve to say "give me the next free PCI slot" and use that to pick the right "pci.<b>.<s>.<f>" node to hang its explicit variables off of. Some users of bhyve have expressed a desire for a flat config store (and the ability to set arbitrary options via '-o' that override what it is in a base config file), so the backing store for the config needs that level of detail, but you can populate the config in various ways.

[jhb]

In D26035#578062, @grehan wrote:

Given the config file syntax will likely change but the "-f" option will be around forever, can a magic number or first-line metadata comment be used to distinguish this key/value file format from a future version ?

I would be fine with using a more obscure option for the flat config file (if we did long opts it could be a --flat-config or some such without a short letter even, but we don't do any long opts so not quite as straightforward to pick an obscure one). I don't really expect the syntax of that to change, but I think having '-f' be used for a more friendly UCL syntax might be more useful and I would be happy to leave "-f" free for the UCL thing to use. The UCL thing might itself include a version number as a field for the UCL parser to use that I think would permit some level of compatibility in the future.

[jhb]

Definitely that is what will happen, somebody else will write a wrapper for this configuration file!!! The problem I see here is the standard(syntax and etc), it does not look like any configuration file I have ever see, and you know after you commit it, it will live there for forever, because we need to respect POLA.

I think you are conflating a few things. First, the syntax to me at least seems rather straight forward and even rather common (e.g. Windows INI files, the OpenSSL config file, even rc.conf) all use 'name=value' lines. It doesn't have [group] lines. I suppose if you really wanted that you could do that for nesting instead of a MIB name, but that seems equally unreadable.

Also, to be clear, the main thrust here is not really having a config file (that is more thrown in as a PoC of how one can combine both files and command line options to build a config), but it is really the nvlist tree and separating config parsing from acting on the config. That removes lots of dubious string parsing code from device models and also makes it much easier (IMO) to add new config variables in the future with minimal code changes. One of the things this model permits is adding config options to USB devices with previously were not permitted by the limited syntax of '-s' options used for XHCI controllers. The "flat" config file is basically the simplest type of config file format one could have (direct key=value without additional implicit knowledge). Right now we already require people using '-s' to know about which PCI slot devices live on, so the "flat" config file isn't worse, just equally non-ideal in that regard.

I think your actual concern is not the syntax, but the fact that the file maps 1:1 onto the set of config values and the stability of how the config tree is managed over time. Devices in particular I think are one of the hardest to reason about. I had originally toyed with trying to do a device namespace that looked a bit more like this:

device.0.type="ahci"
device.0.location="pci0:1:0"
device.0.path="disk.img"

etc. However, while more abstract, it is harder to deal with this sanely in bhyve itself as there may be parent-child dependencies this doesn't capture. Even the current 'pci.<bus>.<slot>.<function>' scheme isn't really ideal if we ever have support for PCI-PCI bridges (though it is doable, you do pci bus 0 first and do a depth-first traversal using the bridge bus numbers to look for devices instead of the current iteration over all possible values).

The other scheme I considered would be to have the device nodes be an actual tree reflecting their layout much like ACPI or FDT. USB devices are already following this somewhat by being hung under the xhci node, whereas lpc devices are not (they are not children of the lpc device). For PCI devices this would be more invasive if PCI-PCI bridges were added since that would then become more of a tree rather than the current flat layout it is now. However, this would also be more complex than the current layout which more naturally fit the existing device model initialization.

[grehan]

In D26035#578286, @jhb wrote:

In D26035#578062, @grehan wrote:

Given the config file syntax will likely change but the "-f" option will be around forever, can a magic number or first-line metadata comment be used to distinguish this key/value file format from a future version ?

I would be fine with using a more obscure option for the flat config file (if we did long opts it could be a --flat-config or some such without a short letter even, but we don't do any long opts so not quite as straightforward to pick an obscure one). I don't really expect the syntax of that to change, but I think having '-f' be used for a more friendly UCL syntax might be more useful and I would be happy to leave "-f" free for the UCL thing to use. The UCL thing might itself include a version number as a field for the UCL parser to use that I think would permit some level of compatibility in the future.

-k ? ("k"ey-value)

[araujo]

This comment was removed by araujo.

[araujo]

In D26035#578290, @jhb wrote:

Definitely that is what will happen, somebody else will write a wrapper for this configuration file!!! The problem I see here is the standard(syntax and etc), it does not look like any configuration file I have ever see, and you know after you commit it, it will live there for forever, because we need to respect POLA.

I think you are conflating a few things. First, the syntax to me at least seems rather straight forward and even rather common (e.g. Windows INI files, the OpenSSL config file, even rc.conf) all use 'name=value' lines. It doesn't have [group] lines. I suppose if you really wanted that you could do that for nesting instead of a MIB name, but that seems equally unreadable.

Also, to be clear, the main thrust here is not really having a config file (that is more thrown in as a PoC of how one can combine both files and command line options to build a config), but it is really the nvlist tree and separating config parsing from acting on the config. That removes lots of dubious string parsing code from device models and also makes it much easier (IMO) to add new config variables in the future with minimal code changes. One of the things this model permits is adding config options to USB devices with previously were not permitted by the limited syntax of '-s' options used for XHCI controllers. The "flat" config file is basically the simplest type of config file format one could have (direct key=value without additional implicit knowledge). Right now we already require people using '-s' to know about which PCI slot devices live on, so the "flat" config file isn't worse, just equally non-ideal in that regard.

I think your actual concern is not the syntax, but the fact that the file maps 1:1 onto the set of config values and the stability of how the config tree is managed over time. Devices in particular I think are one of the hardest to reason about. I had originally toyed with trying to do a device namespace that looked a bit more like this:

device.0.type="ahci"
device.0.location="pci0:1:0"
device.0.path="disk.img"

etc. However, while more abstract, it is harder to deal with this sanely in bhyve itself as there may be parent-child dependencies this doesn't capture. Even the current 'pci.<bus>.<slot>.<function>' scheme isn't really ideal if we ever have support for PCI-PCI bridges (though it is doable, you do pci bus 0 first and do a depth-first traversal using the bridge bus numbers to look for devices instead of the current iteration over all possible values).

The other scheme I considered would be to have the device nodes be an actual tree reflecting their layout much like ACPI or FDT. USB devices are already following this somewhat by being hung under the xhci node, whereas lpc devices are not (they are not children of the lpc device). For PCI devices this would be more invasive if PCI-PCI bridges were added since that would then become more of a tree rather than the current flat layout it is now. However, this would also be more complex than the current layout which more naturally fit the existing device model initialization.

Thanks for all the explanation, after reading all of it twice, it would make much more sense to open two different reviews, one for the nvlist implementation that is very nice, and we have discussed that before and then another one for the config parser implementation. It sounds and looks like for me you want to force this configuration file syntax, masking it on the nvlist tree implementation. Although the internals and the concept looks nice, you are just ignoring all the benefits of UCL, json, xml and flat format that it provides without even considering or even trying it, also it provides tools out of the box. Again, the configuration file is supposed to be something simple that users can understand and opt to use instead of the complex bhyve command line, not something so complicated like what you are proposing!!!

I can't stop you to committing it, and I can envision, after years of discussions about what is the best configuration file syntax, after years of people complaining that bhyve needed a shell script to be run, every conference people asking about "Why we don't have a configuration file", we will end up with this. And again, we will need to rely on third-party software to facilitate the usage of bhyve, because we just have no capability to make things simple.

PS: Apologise for the "request changes" for this review, actually I choose the wrong "add action".

[grehan]

usr.sbin/bhyve/net_backends.c

936–937

Should be if (value == NULL)

[afedorov]

I really like the changes related to option handling.

[grehan]

usr.sbin/bhyve/pci_ahci.c

2865

Perhaps a flag could be used to indicate a legacy-only device name and retire the -hd and -cd names. It's now possible to create an ahci-cd node with "type=hd".

[editor_callfortesting.org]

Great work! Will 'config.dump' or another flag do a dry-run dump of the bhyve command to be executed? This would be very helpful. Apologies if I am overlooking it.

[jhb]

usr.sbin/bhyve/pci_ahci.c

2865

Not quite sure what you mean? Do you want '-s 0,ahci,<path>' to work without an hd: or cd: prefix? Or rather, did I break an existing feature that let you do '-s 0,ahci-cd,hd:<path>'? The current code assumes that ahci-cd and ahci-hd are only used with exactly one port (pci_ahci_hd_legacy_config only creates a single port and doesn't parse a string of drives the way pci_ahci_legacy_config does).

[jhb]

• Rebase
• Fix correct variable name.
• Use -k for the key-value config file instead of -f.
• Change config.dump to write out a valid key-value config file.

[jhb]

• Rebase.
• Convert 9p.

[jhb]

• Rebase.

[jhb]

• Update pctestdev.
• Fixup for snapshot vmname set.
• Keep config.dump.
• Handle non-present pctest-dev setting.
• Add documentation.

[jhb]

usr.sbin/bhyve/bhyve_config.5

492

Upon further reflection, I think I should probably change this to be more like AHCI, so that you have:

port.0.name=console
port.0.path=/some/socket
port.1.name=foo
port.1.path=/other/socket
...

Rather than the current:

ports.console=/some/socket
ports.foo=/other/socket

In addition, a second bug is that I don't see why these devices have to only work with UNIX domain sockets. It should be reasonable to hook these up to a /dev/ndmX device, or to use "stdio" like we do for UART devices? However, that seems orthogonal.

[grehan]

usr.sbin/bhyve/bhyve_config.5

492

The initial use of this was as an out-of-bound messaging path for guest tools, not so much as a serial port.

virtio-console can't operate in polled-mode so isn't of much use as an actual guest console.

[jhb]

usr.sbin/bhyve/bhyve_config.5

492

Huh, ok. I might still clean up the config to be more like AHCI.

[jhb]

• Switch virtio console over to using port.N nodes.
• Use "path" for a UART backend.
• Adjust column sizes.
• Complete adaptation of vtcon to port.N.
• Rework the USB device configuration tree.

[jhb]

• Add a concept of PCI emulation aliases.
• Always invoke pe_legacy_config.

[jhb]

• Rebase for removal of bvmconsole and bvmdebug.

[jhb]

• Add wrappers for bool variables with a default value.
• Use get_config_bool_default for global settings only checked once.

[rew]

usr.sbin/bhyve/bhyve_config.5

15

missed a space

525–526

missed a number, netgraph(4).

usr.sbin/bhyve/pci_lpc.c

79–80

The following gets printed to terminal on boot:

/tmp/bhyve.lXJmgyl    656:         Device (com1)
Remark   2182 -                              ^ At least one lower case letter found in NameSeg, ASL is case insensitive - converting to upper case (COM1)

/tmp/bhyve.lXJmgyl    673:         Device (com2)
Remark   2182 -                              ^ At least one lower case letter found in NameSeg, ASL is case insensitive - converting to upper case (COM2)

From main(), acpi_build() gets called, then basl_compile() where iasl(8) gets called via system(). The iasl command has stdout going /dev/null but not stderr (which are where the messages are printed to).

passing the -vi flag to iasl(8) will suppress the warnings, here's a diff for context, https://reviews.freebsd.org/differential/diff/82835/

I haven't figured out how to add a suggestion on a file that isn't in the review, hence the provided diff.

[jhb]

• Rebase and update for com3/4.
• Use uppercase names for UART devices in the DSDT.
• Fixes from Rob Wing and update for com3/4.

[jhb]

usr.sbin/bhyve/pci_lpc.c

79–80

I decided to just fix the warning by using uppercase names in the DSDT instead. Thanks for reporting it though, I had missed it.

[editor_callfortesting.org]

In testing this against 13.0-BETA1, stderr redirection does not appear to work:

bhyve -k win.cfg 2>/dev/null

The boot string that the configuration file is based on works fine with stderr redirection.

Suggestion: Unless it is required by the parser or something, remove the period in "config.dump=1" or replace it with a dash to be more like other bhyve parameters and perhaps safer parsing with /bin/sh. Do also consider adding it to the manual page as it is very useful.

Great work!

[jhb]

In D26035#641414, @editor_callfortesting.org wrote:

In testing this against 13.0-BETA1, stderr redirection does not appear to work:

bhyve -k win.cfg 2>/dev/null

This is a bit unusual. Can you show which errors you are getting on stderr that you expect to not be getting?

The boot string that the configuration file is based on works fine with stderr redirection.

Suggestion: Unless it is required by the parser or something, remove the period in "config.dump=1" or replace it with a dash to be more like other bhyve parameters and perhaps safer parsing with /bin/sh. Do also consider adding it to the manual page as it is very useful.

The period is basically required in that the config variables are a MIB like sysctl names where '.' is the delimiter. It is also documented in bhyve_config(5).

[editor_callfortesting.org]

In D26035#642819, @jhb wrote:

In D26035#641414, @editor_callfortesting.org wrote:

In testing this against 13.0-BETA1, stderr redirection does not appear to work:

bhyve -k win.cfg 2>/dev/null

This is a bit unusual. Can you show which errors you are getting on stderr that you expect to not be getting?

The boot string that the configuration file is based on works fine with stderr redirection.

This is my bhyve command for Windows Server 2019 which produces a SAC> prompt on stdio without the stderr messages redirected:

/usr/sbin/bhyve -c 2 -m 4G -H -w -P \

-l com1,stdio \
-l bootrom,/root/windows-bhyve-2021/bcran-UEFI-CFT/BHYVE_UEFI.fd \
-s 0:0,hostbridge \
-s 2:0,e1000,tap0 \
-s 3:0,ahci-hd,/root/win.raw \
-s 30:0,fbuf,tcp=10.0.0.14:5900,w=1024,h=768 \
-s 31:0,lpc \
windows 2> /dev/null

Adding the config dump flag gives:

acpi_tables=false
memory.size=4G
x86.vmexit_on_hlt=true
x86.strictmsr=false
x86.vmexit_on_pause=true
cpus=2
#config.dump=1
lpc.com1.path=stdio
lpc.bootrom=/root/windows-bhyve-2021/bcran-UEFI-CFT/BHYVE_UEFI.fd
pci.0.0.0.device=hostbridge
pci.0.2.0.device=e1000
pci.0.2.0.backend=tap0
pci.0.3.0.device=ahci
pci.0.3.0.port.0.type=hd
pci.0.3.0.port.0.path=/root/win.raw
pci.0.30.0.device=fbuf
pci.0.30.0.tcp=10.0.0.14:5900
pci.0.30.0.w=1024
pci.0.30.0.h=768
pci.0.31.0.device=lpc
name=windows

Redirecting this to win.cfg can be launched with:

bhyve -k win.cfg 2> /dev/null

Results in stdio flooded with messages like "rdmsr to register 0x198 on vcpu 1" where previously it was a clear SAC> console.

Perhaps I have the syntax wrong?

[jhb]

Hmm, I tested with bhyveload (rather than the UEFI ROM), and stderr was blocked ok for me. I used a FreeBSD guest and used 'cpucontrol -m 0x198 /dev/cpuctl0' after 'kldload cpuctl' to provoke the error. One thing to check perhaps is getting 'procstat -f' output of the running bhyve process so we can see what fd 2 is set to.