12:53 a.m.
Hi,
This is the first attempt to implement PCI address allocation for the
bhyve driver. This patch is by no means a complete version and the idea
of this is to understand if I'm moving in a right direction.
This code is based on the one from QEMU driver. Even though bhyve currently
has no support for PCI bridges, it should be possible to add that
support without major rewrites when this feature will be available in bhyve.
So, currently we have the following. For a domain like that:
https://gist.github.com/novel/10569989#file-domainin-xml
The processed domain looks this way:
https://gist.github.com/novel/10569989#file-domainout-xml
and the command is:
https://gist.github.com/novel/10569989#file-cmd
Please let me know if it's uncomfortable to follow gist links; I didn't
put domain xml inline as it's pretty lengthy.
Open questions are:
* What's the best way to deal with the 0:0,hostbridge device, should it
be explicitly added to the domain definition?
* How to handle lpc device, that's required for console. From bhyve point
of view it looks like this:
-s 31,lpc -l com1,ttydev
That is, LPC PCI-ISA bridge on PCI slot and com1 port on that bridge.
Things that need to be fixed in that patch, but are obvious:
* Fix 'make check' which fails for obvious reasons of new addresses in domain
xml
* As we support more than one disk device, respect boot order
Roman Bogorodskiy (1):
bhyve: implement PCI address allocation
po/POTFILES.in | 1 +
src/Makefile.am | 4 +
src/bhyve/bhyve_command.c | 128 +++++++++++-----------
src/bhyve/bhyve_device.c | 272 ++++++++++++++++++++++++++++++++++++++++++++++
src/bhyve/bhyve_device.h | 41 +++++++
src/bhyve/bhyve_domain.c | 75 +++++++++++++
src/bhyve/bhyve_domain.h | 41 +++++++
src/bhyve/bhyve_driver.c | 9 +-
8 files changed, 503 insertions(+), 68 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
--
1.9.0
12:53 a.m.
New subject: [libvirt] [PATCH] bhyve: implement PCI address allocation
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 | 129 ++++++++++------------
src/bhyve/bhyve_device.c | 272 ++++++++++++++++++++++++++++++++++++++++++++++
src/bhyve/bhyve_device.h | 41 +++++++
src/bhyve/bhyve_domain.c | 75 +++++++++++++
src/bhyve/bhyve_domain.h | 41 +++++++
src/bhyve/bhyve_driver.c | 9 +-
8 files changed, 502 insertions(+), 70 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
diff --git a/po/POTFILES.in b/po/POTFILES.in
index 122b853..d1eefef 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -9,6 +9,7 @@ gnulib/lib/regcomp.c
src/access/viraccessdriverpolkit.c
src/access/viraccessmanager.c
src/bhyve/bhyve_command.c
+src/bhyve/bhyve_device.c
src/bhyve/bhyve_driver.c
src/bhyve/bhyve_process.c
src/conf/capabilities.c
diff --git a/src/Makefile.am b/src/Makefile.am
index f6690b6..443ae93 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -780,6 +780,10 @@ PARALLELS_DRIVER_SOURCES = \
BHYVE_DRIVER_SOURCES = \
bhyve/bhyve_command.c \
bhyve/bhyve_command.h \
+ bhyve/bhyve_device.c \
+ bhyve/bhyve_device.h \
+ bhyve/bhyve_domain.c \
+ bhyve/bhyve_domain.h \
bhyve/bhyve_driver.h \
bhyve/bhyve_driver.c \
bhyve/bhyve_process.c \
diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c
index a2da34a..560a45e 100644
--- a/src/bhyve/bhyve_command.c
+++ b/src/bhyve/bhyve_command.c
@@ -41,21 +41,14 @@ VIR_LOG_INIT("bhyve.bhyve_command");
static int
bhyveBuildNetArgStr(const virDomainDef *def, virCommandPtr cmd)
{
- virDomainNetDefPtr net = NULL;
- char *brname = NULL;
- char *realifname = NULL;
- int *tapfd = NULL;
char macaddr[VIR_MAC_STRING_BUFLEN];
+ size_t i;
- if (def->nnets != 1) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("domain should have one and only one net defined"));
- return -1;
- }
-
- net = def->nets[0];
-
- if (net) {
+ for (i = 0; i < def->nnets; i++) {
+ char *realifname = NULL;
+ int *tapfd = NULL;
+ char *brname = NULL;
+ virDomainNetDefPtr net = net = def->nets[i];;
int actualType = virDomainNetGetActualType(net);
if (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) {
@@ -87,30 +80,31 @@ bhyveBuildNetArgStr(const virDomainDef *def, virCommandPtr cmd)
VIR_FREE(brname);
return -1;
}
- }
- realifname = virNetDevTapGetRealDeviceName(net->ifname);
+ realifname = virNetDevTapGetRealDeviceName(net->ifname);
- if (realifname == NULL) {
- VIR_FREE(net->ifname);
- VIR_FREE(brname);
- return -1;
- }
+ if (realifname == NULL) {
+ VIR_FREE(net->ifname);
+ VIR_FREE(brname);
+ return -1;
+ }
- VIR_DEBUG("%s -> %s", net->ifname, realifname);
- /* hack on top of other hack: we need to set
- * interface to 'UP' again after re-opening to find its
- * name
- */
- if (virNetDevSetOnline(net->ifname, true) != 0) {
- VIR_FREE(net->ifname);
- VIR_FREE(brname);
- return -1;
- }
+ VIR_DEBUG("%s -> %s", net->ifname, realifname);
+ /* hack on top of other hack: we need to set
+ * interface to 'UP' again after re-opening to find its
+ * name
+ */
+ if (virNetDevSetOnline(net->ifname, true) != 0) {
+ VIR_FREE(net->ifname);
+ VIR_FREE(brname);
+ return -1;
+ }
- virCommandAddArg(cmd, "-s");
- virCommandAddArgFormat(cmd, "1:0,virtio-net,%s,mac=%s",
- realifname, virMacAddrFormat(&net->mac, macaddr));
+ virCommandAddArg(cmd, "-s");
+ virCommandAddArgFormat(cmd, "%d:0,virtio-net,%s,mac=%s",
+ net->info.addr.pci.slot,
+ realifname, virMacAddrFormat(&net->mac, macaddr));
+ }
return 0;
}
@@ -150,46 +144,43 @@ bhyveBuildConsoleArgStr(const virDomainDef *def, virCommandPtr cmd)
static int
bhyveBuildDiskArgStr(const virDomainDef *def, virCommandPtr cmd)
{
- virDomainDiskDefPtr disk;
const char *bus_type;
+ size_t i;
+
+ for (i = 0; i < def->ndisks; i++) {
+ virDomainDiskDefPtr disk = def->disks[i];
+
+ switch (disk->bus) {
+ case VIR_DOMAIN_DISK_BUS_SATA:
+ bus_type = "ahci-hd";
+ break;
+ case VIR_DOMAIN_DISK_BUS_VIRTIO:
+ bus_type = "virtio-blk";
+ break;
+ default:
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("unsupported disk bus type"));
+ return -1;
+ }
- if (def->ndisks != 1) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("domain should have one and only one disk defined"));
- return -1;
- }
-
- disk = def->disks[0];
-
- switch (disk->bus) {
- case VIR_DOMAIN_DISK_BUS_SATA:
- bus_type = "ahci-hd";
- break;
- case VIR_DOMAIN_DISK_BUS_VIRTIO:
- bus_type = "virtio-blk";
- break;
- default:
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("unsupported disk bus type"));
- return -1;
- }
+ if (disk->device != VIR_DOMAIN_DISK_DEVICE_DISK) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("unsupported disk device"));
+ return -1;
+ }
- if (disk->device != VIR_DOMAIN_DISK_DEVICE_DISK) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("unsupported disk device"));
- return -1;
- }
+ if (virDomainDiskGetType(disk) != VIR_STORAGE_TYPE_FILE) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("unsupported disk type"));
+ return -1;
+ }
- if (virDomainDiskGetType(disk) != VIR_STORAGE_TYPE_FILE) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("unsupported disk type"));
- return -1;
+ virCommandAddArg(cmd, "-s");
+ virCommandAddArgFormat(cmd, "%d:0,%s,%s",
+ disk->info.addr.pci.slot, bus_type,
+ virDomainDiskGetSource(disk));
}
- virCommandAddArg(cmd, "-s");
- virCommandAddArgFormat(cmd, "2:0,%s,%s", bus_type,
- virDomainDiskGetSource(disk));
-
return 0;
}
@@ -272,9 +263,9 @@ virBhyveProcessBuildLoadCmd(bhyveConnPtr driver ATTRIBUTE_UNUSED,
virCommandPtr cmd;
virDomainDiskDefPtr disk;
- if (vm->def->ndisks != 1) {
+ if (vm->def->ndisks < 1) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("domain should have one and only one disk defined"));
+ _("domain should have more than one disk defined"));
return NULL;
}
diff --git a/src/bhyve/bhyve_device.c b/src/bhyve/bhyve_device.c
new file mode 100644
index 0000000..b4a0438
--- /dev/null
+++ b/src/bhyve/bhyve_device.c
@@ -0,0 +1,272 @@
+/*
+ * bhyve_device.c: bhyve device management
+ *
+ * Copyright (C) 2014 Roman Bogorodskiy
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ * <http://www.gnu.org/licenses/>;.
+ *
+ * Author: Roman Bogorodskiy
+ */
+
+#include <config.h>
+
+#include "bhyve_device.h"
+#include "viralloc.h"
+#include "virlog.h"
+#include "virstring.h"
+
+#define VIR_FROM_THIS VIR_FROM_BHYVE
+
+VIR_LOG_INIT("bhyve.bhyve_device");
+
+#define BHYVE_PCI_ADDRESS_SLOT_LAST 31
+
+typedef struct {
+ virDomainControllerModelPCI model;
+
+ size_t minSlot, maxSlot; /* usually 0,0 or 1,31 */
+ uint8_t slots[BHYVE_PCI_ADDRESS_SLOT_LAST + 1];
+} bhyveDomainPCIAddressBus;
+
+typedef bhyveDomainPCIAddressBus *bhyveDomainPCIAddressBusPtr;
+
+struct _bhyveDomainPCIAddressSet {
+ bhyveDomainPCIAddressBus *buses;
+ size_t nbuses;
+ virDevicePCIAddress lastaddr;
+};
+
+static char *
+bhyveDomainPCIAddressAsString(virDevicePCIAddressPtr addr)
+{
+ char *str;
+
+ ignore_value(virAsprintf(&str, "%.4x:%.2x:%.2x.%.1x",
+ addr->domain,
+ addr->bus,
+ addr->slot,
+ addr->function));
+
+ return str;
+}
+
+static int
+bhyveDomainPCIAddressBusSetModel(bhyveDomainPCIAddressBusPtr bus,
+ virDomainControllerModelPCI model)
+{
+ switch (model) {
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
+ bus->minSlot = 1;
+ bus->maxSlot = BHYVE_PCI_ADDRESS_SLOT_LAST;
+ break;
+ default:
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Invalid PCI controller model %d"), model);
+ return -1;
+ }
+
+ bus->model = model;
+ return 0;
+}
+
+bhyveDomainPCIAddressSetPtr
+bhyveDomainPCIAddressSetCreate(virDomainDefPtr def ATTRIBUTE_UNUSED, unsigned int
nbuses)
+{
+ bhyveDomainPCIAddressSetPtr addrs;
+
+ if (VIR_ALLOC(addrs) < 0)
+ goto error;
+
+ if (VIR_ALLOC_N(addrs->buses, nbuses) < 0)
+ goto error;
+
+ addrs->nbuses = nbuses;
+
+ if (bhyveDomainPCIAddressBusSetModel(&addrs->buses[0],
+ VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) < 0)
+ goto error;
+
+ return addrs;
+
+ error:
+ bhyveDomainPCIAddressSetFree(addrs);
+ return NULL;
+}
+
+static int
+bhyveAssignDevicePCISlots(virDomainDefPtr def,
+ bhyveDomainPCIAddressSetPtr addrs)
+{
+ size_t i;
+
+ for (i = 0; i < def->ndisks; i++) {
+ if (bhyveDomainPCIAddressReserveNextSlot(addrs,
+ &def->disks[i]->info) < 0)
+ goto error;
+ }
+
+ for (i = 0; i < def->nnets; i++) {
+ if (bhyveDomainPCIAddressReserveNextSlot(addrs,
+ &def->nets[i]->info) < 0)
+ goto error;
+ }
+
+ for (i = 0; i < def->ncontrollers; i++) {
+ if (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) {
+ if (def->controllers[i]->info.type !=
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
+ continue;
+ if (def->controllers[i]->model ==
VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT)
+ continue;
+
+ if (bhyveDomainPCIAddressReserveNextSlot(addrs,
+ &def->controllers[i]->info)
< 0)
+ goto error;
+ }
+
+ }
+
+ return 0;
+
+ error:
+ return -1;
+}
+
+int bhyveDomainAssignPCIAddresses(virDomainDefPtr def,
+ virDomainObjPtr obj)
+{
+ bhyveDomainPCIAddressSetPtr addrs = NULL;
+ bhyveDomainObjPrivatePtr priv = NULL;
+
+ int ret = -1;
+
+ if (!(addrs = bhyveDomainPCIAddressSetCreate(def, 1)))
+ goto cleanup;
+
+ if (bhyveAssignDevicePCISlots(def, addrs) < 0)
+ goto cleanup;
+
+ if (obj && obj->privateData) {
+ priv = obj->privateData;
+ if (addrs) {
+ bhyveDomainPCIAddressSetFree(priv->pciaddrs);
+ priv->persistentAddrs = 1;
+ priv->pciaddrs = addrs;
+ } else {
+ priv->persistentAddrs = 0;
+ }
+ }
+
+ ret = 0;
+
+ cleanup:
+ return ret;
+}
+
+int bhyveDomainAssignAddresses(virDomainDefPtr def, virDomainObjPtr obj)
+{
+ return bhyveDomainAssignPCIAddresses(def, obj);
+}
+
+static bool
+bhyveDomainPCIAddressSlotInUse(bhyveDomainPCIAddressSetPtr addrs,
+ virDevicePCIAddressPtr addr)
+{
+ return !!addrs->buses[addr->bus].slots[addr->slot];
+}
+
+
+static int
+bhyveDomainPCIAddressGetNextSlot(bhyveDomainPCIAddressSetPtr addrs,
+ virDevicePCIAddressPtr next_addr,
+ int opaque ATTRIBUTE_UNUSED)
+{
+ virDevicePCIAddress a = { 0, 0, 0, 0, false };
+
+ if (addrs->nbuses == 0) {
+ virReportError(VIR_ERR_XML_ERROR, "%s", _("No PCI buses
available"));
+ return -1;
+ }
+
+ for (a.slot++; a.bus < addrs->nbuses; a.bus++) {
+ for (; a.slot <= BHYVE_PCI_ADDRESS_SLOT_LAST; a.slot++) {
+ if (!bhyveDomainPCIAddressSlotInUse(addrs, &a))
+ goto success;
+
+ VIR_INFO("PCI slot %.4x:%.2x:%.2x already in use",
+ a.domain, a.bus, a.slot);
+ }
+ }
+
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s", _("No more available PCI slots"));
+
+ success:
+ VIR_INFO("Found free PCI slot %.4x:%.2x:%.2x",
+ a.domain, a.bus, a.slot);
+ *next_addr = a;
+ return 0;
+}
+
+static int
+bhyveDomainPCIAddressReserveSlot(bhyveDomainPCIAddressSetPtr addrs,
+ virDevicePCIAddressPtr addr,
+ int opaque ATTRIBUTE_UNUSED)
+{
+ int ret = -1;
+ char *addrStr = NULL;
+ bhyveDomainPCIAddressBusPtr bus = &addrs->buses[addr->bus];
+
+ if (!(addrStr = bhyveDomainPCIAddressAsString(addr)))
+ goto cleanup;
+
+ if (bus->slots[addr->slot]) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("Attempted double use of PCI slot %s"), addrStr);
+ goto cleanup;
+ }
+
+ bus->slots[addr->slot] = 0xFF;
+ ret = 0;
+
+ cleanup:
+ VIR_FREE(addrStr);
+ return ret;
+}
+
+int
+bhyveDomainPCIAddressReserveNextSlot(bhyveDomainPCIAddressSetPtr addrs,
+ virDomainDeviceInfoPtr dev)
+{
+ virDevicePCIAddress addr;
+ if (bhyveDomainPCIAddressGetNextSlot(addrs, &addr, 0) < 0)
+ return -1;
+
+ if (bhyveDomainPCIAddressReserveSlot(addrs, &addr, 0) < 0)
+ return -1;
+
+ dev->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
+ dev->addr.pci = addr;
+ addrs->lastaddr = addr;
+ return 0;
+}
+
+void bhyveDomainPCIAddressSetFree(bhyveDomainPCIAddressSetPtr addrs)
+{
+ if (!addrs)
+ return;
+
+ VIR_FREE(addrs->buses);
+ VIR_FREE(addrs);
+}
diff --git a/src/bhyve/bhyve_device.h b/src/bhyve/bhyve_device.h
new file mode 100644
index 0000000..adc25e9
--- /dev/null
+++ b/src/bhyve/bhyve_device.h
@@ -0,0 +1,41 @@
+/*
+ * bhyve_device.h: bhyve device management headers
+ *
+ * Copyright (C) 2014 Roman Bogorodskiy
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ * <http://www.gnu.org/licenses/>;.
+ *
+ * Author: Roman Bogorodskiy
+ */
+
+#ifndef __BHYVE_DEVICE_H__
+# define __BHYVE_DEVICE_H__
+
+# include "domain_conf.h"
+# include "bhyve_domain.h"
+
+int bhyveDomainAssignPCIAddresses(virDomainDefPtr def, virDomainObjPtr obj);
+
+bhyveDomainPCIAddressSetPtr bhyveDomainPCIAddressSetCreate(virDomainDefPtr def,
+ unsigned int nbuses);
+
+int bhyveDomainAssignAddresses(virDomainDefPtr def, virDomainObjPtr obj)
+ ATTRIBUTE_NONNULL(1);
+
+void bhyveDomainPCIAddressSetFree(bhyveDomainPCIAddressSetPtr addrs);
+
+int bhyveDomainPCIAddressReserveNextSlot(bhyveDomainPCIAddressSetPtr addrs,
virDomainDeviceInfoPtr dev);
+
+#endif /* __BHYVE_DEVICE_H__ */
diff --git a/src/bhyve/bhyve_domain.c b/src/bhyve/bhyve_domain.c
new file mode 100644
index 0000000..284140c
--- /dev/null
+++ b/src/bhyve/bhyve_domain.c
@@ -0,0 +1,75 @@
+/*
+ * bhyve_domain.c: bhyve domain private state
+ *
+ * Copyright (C) 2014 Roman Bogorodskiy
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ * <http://www.gnu.org/licenses/>;.
+ *
+ * Author: Roman Bogorodskiy
+ */
+
+#include <config.h>
+
+#include "bhyve_device.h"
+#include "bhyve_domain.h"
+#include "viralloc.h"
+#include "virlog.h"
+
+#define VIR_FROM_THIS VIR_FROM_BHYVE
+
+VIR_LOG_INIT("bhyve.bhyve_domain");
+
+static void *
+bhyveDomainObjPrivateAlloc(void)
+{
+ bhyveDomainObjPrivatePtr priv;
+
+ if (VIR_ALLOC(priv) < 0)
+ return NULL;
+
+ return priv;
+}
+
+static void
+bhyveDomainObjPrivateFree(void *data)
+{
+ bhyveDomainObjPrivatePtr priv = data;
+
+ bhyveDomainPCIAddressSetFree(priv->pciaddrs);
+
+ VIR_FREE(priv);
+}
+
+virDomainXMLPrivateDataCallbacks virBhyveDriverPrivateDataCallbacks = {
+ .alloc = bhyveDomainObjPrivateAlloc,
+ .free = bhyveDomainObjPrivateFree,
+};
+
+static int
+bhyveDomainDefPostParse(virDomainDefPtr def,
+ virCapsPtr caps ATTRIBUTE_UNUSED,
+ void *opaque ATTRIBUTE_UNUSED)
+{
+ /* Add an implicit PCI root controller */
+ if (virDomainDefMaybeAddController(def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 0,
+ VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) < 0)
+ return -1;
+
+ return 0;
+}
+
+virDomainDefParserConfig virBhyveDriverDomainDefParserConfig = {
+ .domainPostParseCallback = bhyveDomainDefPostParse,
+};
diff --git a/src/bhyve/bhyve_domain.h b/src/bhyve/bhyve_domain.h
new file mode 100644
index 0000000..5085759
--- /dev/null
+++ b/src/bhyve/bhyve_domain.h
@@ -0,0 +1,41 @@
+/*
+ * bhyve_domain.h: bhyve domain private state headers
+ *
+ * Copyright (C) 2014 Roman Bogorodskiy
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ * <http://www.gnu.org/licenses/>;.
+ *
+ * Author: Roman Bogorodskiy
+ */
+
+#ifndef __BHYVE_DOMAIN_H__
+# define __BHYVE_DOMAIN_H__
+
+# include "domain_conf.h"
+
+typedef struct _bhyveDomainPCIAddressSet bhyveDomainPCIAddressSet;
+typedef bhyveDomainPCIAddressSet *bhyveDomainPCIAddressSetPtr;
+
+typedef struct _bhyveDomainObjPrivate bhyveDomainObjPrivate;
+typedef bhyveDomainObjPrivate *bhyveDomainObjPrivatePtr;
+struct _bhyveDomainObjPrivate {
+ bhyveDomainPCIAddressSetPtr pciaddrs;
+ int persistentAddrs;
+};
+
+extern virDomainXMLPrivateDataCallbacks virBhyveDriverPrivateDataCallbacks;
+extern virDomainDefParserConfig virBhyveDriverDomainDefParserConfig;
+
+#endif /* __BHYVE_DOMAIN_H__ */
diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
index 461a070..eb615c8 100644
--- a/src/bhyve/bhyve_driver.c
+++ b/src/bhyve/bhyve_driver.c
@@ -51,7 +51,9 @@
#include "viraccessapicheck.h"
#include "nodeinfo.h"
+#include "bhyve_device.h"
#include "bhyve_driver.h"
+#include "bhyve_domain.h"
#include "bhyve_process.h"
#include "bhyve_utils.h"
@@ -457,6 +459,9 @@ bhyveDomainDefineXML(virConnectPtr conn, const char *xml)
if (virDomainDefineXMLEnsureACL(conn, def) < 0)
goto cleanup;
+ if (bhyveDomainAssignAddresses(def, NULL) < 0)
+ goto cleanup;
+
if (!(vm = virDomainObjListAdd(privconn->domains, def,
privconn->xmlopt,
0, &oldDef)))
@@ -872,7 +877,9 @@ bhyveStateInitialize(bool priveleged ATTRIBUTE_UNUSED,
if (!(bhyve_driver->caps = bhyveBuildCapabilities()))
goto cleanup;
- if (!(bhyve_driver->xmlopt = virDomainXMLOptionNew(NULL, NULL, NULL)))
+ if (!(bhyve_driver->xmlopt =
virDomainXMLOptionNew(&virBhyveDriverDomainDefParserConfig,
+
&virBhyveDriverPrivateDataCallbacks,
+ NULL)))
goto cleanup;
if (!(bhyve_driver->domains = virDomainObjListNew()))
--
1.9.0
4:16 a.m.
On 04/13/2014 08:53 AM, Roman Bogorodskiy wrote:
Hi,
This is the first attempt to implement PCI address allocation for the
bhyve driver. This patch is by no means a complete version and the idea
of this is to understand if I'm moving in a right direction.
This code is based on the one from QEMU driver.
Rather than doing a cut/paste of the qemu code, it would be greatly more
desirable to move the pci address allocation from qemu into a library
that is accessible from both drivers. This will eliminate dual
maintenance headaches in the future.
Projects like this can tend to be painful, but in the end it means that
when someone using qemu finds and fixes a bug, it will automatically be
fixed for bhyve. And. for example, when the qemu driver adds support for
PCIe "root-port" controllers and upstream/downstream switches[*], the
bhyve driver will automatically get address allocation support for those
controllers too (if/when it implements the controllers).
Or is there some particular reason that makes it better to keep it
separate (or maybe some other conversation that I missed, or have
forgotten)?
[*]For reference, here is a description of the various PCI controller
types - the design in this email is fairly close to what was eventually
implemented, except that the actual code uses a "model" attribute to the
controller element, rather than a separate <model> subelement:
https://www.redhat.com/archives/libvir-list/2013-April/msg01144.html
(root-port, upstream-switch-port, and downstream-switch-port weren't
implemented at the time, because they weren't immediately useful)
Even though bhyve currently
has no support for PCI bridges, it should be possible to add that
support without major rewrites when this feature will be available in bhyve.
So, currently we have the following. For a domain like that:
https://gist.github.com/novel/10569989#file-domainin-xml
The processed domain looks this way:
https://gist.github.com/novel/10569989#file-domainout-xml
and the command is:
https://gist.github.com/novel/10569989#file-cmd
Please let me know if it's uncomfortable to follow gist links; I didn't
put domain xml inline as it's pretty lengthy.
Open questions are:
* What's the best way to deal with the 0:0,hostbridge device, should it
be explicitly added to the domain definition?
No. Slot 0 on each PCI controller is considered to be reserved, and
doesn't show up in the domain definition. The address allocation code
takes that into account.
* How to handle lpc device, that's required for console. From
bhyve point
of view it looks like this:
-s 31,lpc -l com1,ttydev
That is, LPC PCI-ISA bridge on PCI slot and com1 port on that bridge.
We don't model the ISA bus for qemu, and also don't support adding any
device to the ISA bus (although qemu has one). I think you can just
directly define a serial character device with port='0':
<serial type='pty'>
<target port='0'/>
</serial>
Things that need to be fixed in that patch, but are obvious:
* Fix 'make check' which fails for obvious reasons of new addresses in domain
xml
* As we support more than one disk device, respect boot order
Roman Bogorodskiy (1):
bhyve: implement PCI address allocation
po/POTFILES.in | 1 +
src/Makefile.am | 4 +
src/bhyve/bhyve_command.c | 128 +++++++++++-----------
src/bhyve/bhyve_device.c | 272 ++++++++++++++++++++++++++++++++++++++++++++++
src/bhyve/bhyve_device.h | 41 +++++++
src/bhyve/bhyve_domain.c | 75 +++++++++++++
src/bhyve/bhyve_domain.h | 41 +++++++
src/bhyve/bhyve_driver.c | 9 +-
8 files changed, 503 insertions(+), 68 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
6:26 a.m.
Laine Stump wrote:
On 04/13/2014 08:53 AM, Roman Bogorodskiy wrote:
> Hi,
>
> This is the first attempt to implement PCI address allocation for the
> bhyve driver. This patch is by no means a complete version and the idea
> of this is to understand if I'm moving in a right direction.
>
> This code is based on the one from QEMU driver.
Rather than doing a cut/paste of the qemu code, it would be greatly more
desirable to move the pci address allocation from qemu into a library
that is accessible from both drivers. This will eliminate dual
maintenance headaches in the future.
Extracting common code into a library makes a perfect sense. But before
doing that I'd like to have a complete and functional implementation for
bhyve (not necessarily pushed). When it's done, it'd be more or less
obvious which parts of the code should be shared, otherwise there's a
risk of extracting the code from Qemu which would not be used by Bhyve.
Projects like this can tend to be painful, but in the end it means
that
when someone using qemu finds and fixes a bug, it will automatically be
fixed for bhyve. And. for example, when the qemu driver adds support for
PCIe "root-port" controllers and upstream/downstream switches[*], the
bhyve driver will automatically get address allocation support for those
controllers too (if/when it implements the controllers).
Or is there some particular reason that makes it better to keep it
separate (or maybe some other conversation that I missed, or have
forgotten)?
I don't see a reason to keep it separate at this point, but as I've
mentioned above, I think it'd make sense for me to complete the current
patch in a way it as and then analyze what parts need to be shared.
I don't think you've missed any conversations on that topic.
[*]For reference, here is a description of the various PCI
controller
types - the design in this email is fairly close to what was eventually
implemented, except that the actual code uses a "model" attribute to the
controller element, rather than a separate <model> subelement:
https://www.redhat.com/archives/libvir-list/2013-April/msg01144.html
(root-port, upstream-switch-port, and downstream-switch-port weren't
implemented at the time, because they weren't immediately useful)
> Even though bhyve currently
> has no support for PCI bridges, it should be possible to add that
> support without major rewrites when this feature will be available in bhyve.
>
> So, currently we have the following. For a domain like that:
>
> https://gist.github.com/novel/10569989#file-domainin-xml
>
> The processed domain looks this way:
>
> https://gist.github.com/novel/10569989#file-domainout-xml
>
> and the command is:
>
> https://gist.github.com/novel/10569989#file-cmd
>
> Please let me know if it's uncomfortable to follow gist links; I didn't
> put domain xml inline as it's pretty lengthy.
>
> Open questions are:
>
> * What's the best way to deal with the 0:0,hostbridge device, should it
> be explicitly added to the domain definition?
No. Slot 0 on each PCI controller is considered to be reserved, and
doesn't show up in the domain definition. The address allocation code
takes that into account.
Good.
> * How to handle lpc device, that's required for console.
From bhyve point
> of view it looks like this:
>
> -s 31,lpc -l com1,ttydev
>
> That is, LPC PCI-ISA bridge on PCI slot and com1 port on that bridge.
We don't model the ISA bus for qemu, and also don't support adding any
device to the ISA bus (although qemu has one). I think you can just
directly define a serial character device with port='0':
<serial type='pty'>
<target port='0'/>
</serial>
Bhyve uses 'nmdm' type console now, and it requires the ISA bridge and
ISA bus (more details on that in commit
6c91134de46ea481fa36c008c0a3667cbd088f1c), unfortunately.
Thanks,
Roman Bogorodskiy
3875
days inactive
3876
days old
3 comments
2 participants
participants (2)
-
Laine Stump -
Roman Bogorodskiy