Re: [libvirt] [PATCH v5] bhyve: implement PCI address allocation
Ján Tomko wrote:
On 05/17/2014 07:49 PM, Roman Bogorodskiy wrote:
> Automatically allocate PCI addresses for devices instead
> of hardcoding them in the driver code. The current
> allocation schema is to dedicate an entire slot for each devices.
>
> Also, allow having arbitrary number of devices.
> ---
> po/POTFILES.in | 1 +
> src/Makefile.am | 4 +
> src/bhyve/bhyve_command.c | 112 +++++++------
> src/bhyve/bhyve_device.c | 174 +++++++++++++++++++++
> src/bhyve/bhyve_device.h | 38 +++++
> src/bhyve/bhyve_domain.c | 75 +++++++++
> src/bhyve/bhyve_domain.h | 39 +++++
> src/bhyve/bhyve_driver.c | 12 +-
> .../bhyvexml2argvdata/bhyvexml2argv-acpiapic.args | 2 +-
> tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.xml | 2 +
> tests/bhyvexml2argvdata/bhyvexml2argv-base.args | 2 +-
> tests/bhyvexml2argvdata/bhyvexml2argv-base.xml | 2 +
> tests/bhyvexml2argvdata/bhyvexml2argv-console.args | 4 +-
> tests/bhyvexml2argvdata/bhyvexml2argv-console.xml | 2 +
> .../bhyvexml2argv-disk-virtio.args | 2 +-
> .../bhyvexml2argv-disk-virtio.xml | 2 +
> tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.args | 2 +-
> tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.xml | 2 +
> tests/bhyvexml2argvdata/bhyvexml2argv-serial.args | 4 +-
> tests/bhyvexml2argvdata/bhyvexml2argv-serial.xml | 2 +
> 20 files changed, 416 insertions(+), 67 deletions(-)
> create mode 100644 src/bhyve/bhyve_device.c
> create mode 100644 src/bhyve/bhyve_device.h
> create mode 100644 src/bhyve/bhyve_domain.c
> create mode 100644 src/bhyve/bhyve_domain.h
>
ACK
Thanks!
One more thing that bothers me: after this change user needs to
re-define a domain, otherwise domain will have no addresses and will
fail to start.
Could/should it be handled somehow?
> --- a/src/bhyve/bhyve_command.c
> +++ b/src/bhyve/bhyve_command.c
> @@ -39,58 +39,48 @@
> VIR_LOG_INIT("bhyve.bhyve_command");
>
> static int
> -bhyveBuildNetArgStr(const virDomainDef *def, virCommandPtr cmd, bool dryRun)
> +bhyveBuildNetArgStr(const virDomainDef *def,
> + virDomainNetDefPtr net,
> + virCommandPtr cmd,
> + bool dryRun)
> {
> + char *brname = NULL;
> + int actualType = virDomainNetGetActualType(net);
>
> - if (def->nnets != 1) {
> - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> - _("domain should have one and only one net
defined"));
I don't see this check re-added anywhere. Is it okay to have a domain with no
net now?
Interesting question. Initially, I've added this requirement because it
was the only (easy) way to get into the guest. Now, as we have serial
console support in bhyve, it's possible to get into domain without
having any network interfaces, so user has a choice.
However, user could still not add serial console and network interface
and end up with a unaccessible domain. Does it make sense from the user
experience point of view to prevent this situation from happening?
> + if (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) {
> + if (VIR_STRDUP(brname, virDomainNetGetActualBridgeName(net)) < 0)
> + return -1;
> + } else {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("Network type %d is not supported"),
> + virDomainNetGetActualType(net));
> return -1;
> }
>
Jan
Roman Bogorodskiy
Attachments:
- attachment.sig (application/pgp-signature — 488 bytes)