[libvirt] [PATCH v5] bhyve: implement PCI address allocation
Changes from v4: - Qemu-related part of sharing the common code is complete and pushed, so patch lives on its own now - virBhyveProcessBuildBhyveCmd() modified to loop over disks and nets and call bhyveBuildDiskArgStr() and bhyveBuildNetArgStr() for an individual device to make the latter more readable - Fix double assing / double semicolon typo - Fix spelling in virReportError() for disks - Make bhyveAssignDevicePCISlots() assign nets before disks to reproduce the old behaviour when one net and one disk were supported - Make persistentAddrs of _bhyveDomainObjPrivate bool. Roman Bogorodskiy (1): bhyve: implement PCI address allocation 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 -- 1.9.0
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 diff --git a/po/POTFILES.in b/po/POTFILES.in index 6e8d465..ef2f114 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 cfb7097..dc7f794 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -787,6 +787,10 @@ BHYVE_DRIVER_SOURCES = \ bhyve/bhyve_capabilities.h \ 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 91a8731..d3b3f69 100644 --- 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) { - virDomainNetDefPtr net = NULL; - char *brname = NULL; + char macaddr[VIR_MAC_STRING_BUFLEN]; char *realifname = NULL; int *tapfd = NULL; - char macaddr[VIR_MAC_STRING_BUFLEN]; + 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")); + 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; } - net = def->nets[0]; - - if (net) { - int actualType = virDomainNetGetActualType(net); - - 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)); + if (!net->ifname || + STRPREFIX(net->ifname, VIR_NET_GENERATED_PREFIX) || + strchr(net->ifname, '%')) { + VIR_FREE(net->ifname); + if (VIR_STRDUP(net->ifname, VIR_NET_GENERATED_PREFIX "%d") < 0) { + VIR_FREE(brname); return -1; } + } - if (!net->ifname || - STRPREFIX(net->ifname, VIR_NET_GENERATED_PREFIX) || - strchr(net->ifname, '%')) { + if (!dryRun) { + if (virNetDevTapCreateInBridgePort(brname, &net->ifname, &net->mac, + def->uuid, tapfd, 1, + virDomainNetGetActualVirtPortProfile(net), + virDomainNetGetActualVlan(net), + VIR_NETDEV_TAP_CREATE_IFUP | VIR_NETDEV_TAP_CREATE_PERSIST) < 0) { VIR_FREE(net->ifname); - if (VIR_STRDUP(net->ifname, VIR_NET_GENERATED_PREFIX "%d") < 0) { - VIR_FREE(brname); - return -1; - } + VIR_FREE(brname); + return -1; } - if (!dryRun) - if (virNetDevTapCreateInBridgePort(brname, &net->ifname, &net->mac, - def->uuid, tapfd, 1, - virDomainNetGetActualVirtPortProfile(net), - virDomainNetGetActualVlan(net), - VIR_NETDEV_TAP_CREATE_IFUP | VIR_NETDEV_TAP_CREATE_PERSIST) < 0) { - VIR_FREE(net->ifname); - VIR_FREE(brname); - return -1; - } - } - - if (!dryRun) { realifname = virNetDevTapGetRealDeviceName(net->ifname); if (realifname == NULL) { @@ -105,6 +95,7 @@ bhyveBuildNetArgStr(const virDomainDef *def, virCommandPtr cmd, bool dryRun) * name */ if (virNetDevSetOnline(net->ifname, true) != 0) { + VIR_FREE(realifname); VIR_FREE(net->ifname); VIR_FREE(brname); return -1; @@ -116,8 +107,10 @@ bhyveBuildNetArgStr(const virDomainDef *def, virCommandPtr cmd, bool dryRun) virCommandAddArg(cmd, "-s"); - virCommandAddArgFormat(cmd, "1:0,virtio-net,%s,mac=%s", + virCommandAddArgFormat(cmd, "%d:0,virtio-net,%s,mac=%s", + net->info.addr.pci.slot, realifname, virMacAddrFormat(&net->mac, macaddr)); + VIR_FREE(realifname); return 0; } @@ -146,7 +139,7 @@ bhyveBuildConsoleArgStr(const virDomainDef *def, virCommandPtr cmd) return -1; } - virCommandAddArgList(cmd, "-s", "31,lpc", NULL); + virCommandAddArgList(cmd, "-s", "1,lpc", NULL); virCommandAddArg(cmd, "-l"); virCommandAddArgFormat(cmd, "com%d,%s", chr->target.port + 1, chr->source.data.file.path); @@ -155,19 +148,12 @@ bhyveBuildConsoleArgStr(const virDomainDef *def, virCommandPtr cmd) } static int -bhyveBuildDiskArgStr(const virDomainDef *def, virCommandPtr cmd) +bhyveBuildDiskArgStr(const virDomainDef *def ATTRIBUTE_UNUSED, + virDomainDiskDefPtr disk, + virCommandPtr cmd) { - virDomainDiskDefPtr disk; const char *bus_type; - 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"; @@ -194,7 +180,8 @@ bhyveBuildDiskArgStr(const virDomainDef *def, virCommandPtr cmd) } virCommandAddArg(cmd, "-s"); - virCommandAddArgFormat(cmd, "2:0,%s,%s", bus_type, + virCommandAddArgFormat(cmd, "%d:0,%s,%s", + disk->info.addr.pci.slot, bus_type, virDomainDiskGetSource(disk)); return 0; @@ -212,6 +199,8 @@ virBhyveProcessBuildBhyveCmd(bhyveConnPtr driver ATTRIBUTE_UNUSED, * -S 31,uart,stdio \ * vm0 */ + size_t i; + virCommandPtr cmd = virCommandNew(BHYVE); /* CPUs */ @@ -245,10 +234,17 @@ virBhyveProcessBuildBhyveCmd(bhyveConnPtr driver ATTRIBUTE_UNUSED, virCommandAddArgList(cmd, "-s", "0:0,hostbridge", NULL); /* Devices */ - if (bhyveBuildNetArgStr(def, cmd, dryRun) < 0) - goto error; - if (bhyveBuildDiskArgStr(def, cmd) < 0) - goto error; + for (i = 0; i < def->nnets; i++) { + virDomainNetDefPtr net = def->nets[i]; + if (bhyveBuildNetArgStr(def, net, cmd, dryRun) < 0) + goto error; + } + for (i = 0; i < def->ndisks; i++) { + virDomainDiskDefPtr disk = def->disks[i]; + + if (bhyveBuildDiskArgStr(def, disk, cmd) < 0) + goto error; + } if (bhyveBuildConsoleArgStr(def, cmd) < 0) goto error; virCommandAddArg(cmd, def->name); @@ -279,9 +275,9 @@ virBhyveProcessBuildLoadCmd(bhyveConnPtr driver ATTRIBUTE_UNUSED, virCommandPtr cmd; virDomainDiskDefPtr disk; - if (def->ndisks != 1) { + if (def->ndisks < 1) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("domain should have one and only one disk defined")); + _("domain should have at least 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..fa266de --- /dev/null +++ b/src/bhyve/bhyve_device.c @@ -0,0 +1,174 @@ +/* + * 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 "domain_addr.h" +#include "viralloc.h" +#include "virlog.h" +#include "virstring.h" + +#define VIR_FROM_THIS VIR_FROM_BHYVE + +VIR_LOG_INIT("bhyve.bhyve_device"); + +static int +bhyveCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, + virDomainDeviceDefPtr device ATTRIBUTE_UNUSED, + virDomainDeviceInfoPtr info, + void *opaque) +{ + int ret = -1; + virDomainPCIAddressSetPtr addrs = opaque; + virDevicePCIAddressPtr addr = &info->addr.pci; + + if (addr->domain == 0 && addr->bus == 0) { + if (addr->slot == 0) { + return 0; + } else if (addr->slot == 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("PCI bus 0 slot 1 is reserved for the implicit " + "LPC PCI-ISA bridge")); + return -1; + } + } + + if (virDomainPCIAddressReserveSlot(addrs, addr, VIR_PCI_CONNECT_TYPE_PCI) < 0) + goto cleanup; + + ret = 0; + cleanup: + return ret; +} + +virDomainPCIAddressSetPtr +bhyveDomainPCIAddressSetCreate(virDomainDefPtr def ATTRIBUTE_UNUSED, unsigned int nbuses) +{ + virDomainPCIAddressSetPtr addrs; + + if ((addrs = virDomainPCIAddressSetAlloc(nbuses)) == NULL) + return NULL; + + if (virDomainPCIAddressBusSetModel(&addrs->buses[0], + VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) < 0) + goto error; + + if (virDomainDeviceInfoIterate(def, bhyveCollectPCIAddress, addrs) < 0) + goto error; + + return addrs; + + error: + virDomainPCIAddressSetFree(addrs); + return NULL; +} + +static int +bhyveAssignDevicePCISlots(virDomainDefPtr def, + virDomainPCIAddressSetPtr addrs) +{ + size_t i; + virDevicePCIAddress lpc_addr; + + /* explicitly reserve slot 1 for LPC-ISA bridge */ + memset(&lpc_addr, 0, sizeof(lpc_addr)); + lpc_addr.slot = 0x1; + + if (virDomainPCIAddressReserveSlot(addrs, &lpc_addr, VIR_PCI_CONNECT_TYPE_PCI) < 0) + goto error; + + for (i = 0; i < def->nnets; i++) { + if (def->nets[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) + continue; + if (virDomainPCIAddressReserveNextSlot(addrs, + &def->nets[i]->info, + VIR_PCI_CONNECT_TYPE_PCI) < 0) + goto error; + } + + for (i = 0; i < def->ndisks; i++) { + if (def->disks[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && + def->disks[i]->info.addr.pci.slot != 0) + continue; + if (virDomainPCIAddressReserveNextSlot(addrs, + &def->disks[i]->info, + VIR_PCI_CONNECT_TYPE_PCI) < 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 (virDomainPCIAddressReserveNextSlot(addrs, + &def->controllers[i]->info, + VIR_PCI_CONNECT_TYPE_PCI) < 0) + goto error; + } + + } + + return 0; + + error: + return -1; +} + +int bhyveDomainAssignPCIAddresses(virDomainDefPtr def, + virDomainObjPtr obj) +{ + virDomainPCIAddressSetPtr 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) { + virDomainPCIAddressSetFree(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); +} diff --git a/src/bhyve/bhyve_device.h b/src/bhyve/bhyve_device.h new file mode 100644 index 0000000..1144f51 --- /dev/null +++ b/src/bhyve/bhyve_device.h @@ -0,0 +1,38 @@ +/* + * 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 "virpci.h" +# include "bhyve_domain.h" + +int bhyveDomainAssignPCIAddresses(virDomainDefPtr def, virDomainObjPtr obj); + +virDomainPCIAddressSetPtr bhyveDomainPCIAddressSetCreate(virDomainDefPtr def, + unsigned int nbuses); + +int bhyveDomainAssignAddresses(virDomainDefPtr def, virDomainObjPtr obj) + ATTRIBUTE_NONNULL(1); + +#endif /* __BHYVE_DEVICE_H__ */ diff --git a/src/bhyve/bhyve_domain.c b/src/bhyve/bhyve_domain.c new file mode 100644 index 0000000..7c7bec3 --- /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; + + virDomainPCIAddressSetFree(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..b8ef22a --- /dev/null +++ b/src/bhyve/bhyve_domain.h @@ -0,0 +1,39 @@ +/* + * 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_addr.h" +# include "domain_conf.h" + +typedef struct _bhyveDomainObjPrivate bhyveDomainObjPrivate; +typedef bhyveDomainObjPrivate *bhyveDomainObjPrivatePtr; +struct _bhyveDomainObjPrivate { + virDomainPCIAddressSetPtr pciaddrs; + bool 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 580b124..43d3209 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -52,8 +52,10 @@ #include "viraccessapicheck.h" #include "nodeinfo.h" +#include "bhyve_device.h" #include "bhyve_driver.h" #include "bhyve_command.h" +#include "bhyve_domain.h" #include "bhyve_process.h" #include "bhyve_utils.h" #include "bhyve_capabilities.h" @@ -500,6 +502,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))) @@ -660,6 +665,9 @@ bhyveConnectDomainXMLToNative(virConnectPtr conn, VIR_DOMAIN_XML_INACTIVE))) goto cleanup; + if (bhyveDomainAssignAddresses(def, NULL) < 0) + goto cleanup; + if (!(loadcmd = virBhyveProcessBuildLoadCmd(privconn, def))) goto cleanup; @@ -1108,7 +1116,9 @@ bhyveStateInitialize(bool priveleged ATTRIBUTE_UNUSED, if (!(bhyve_driver->caps = virBhyveCapsBuild())) 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())) diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.args b/tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.args index 60a56b9..79f8e88 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.args @@ -1,3 +1,3 @@ /usr/sbin/bhyve -c 1 -m 214 -A -I -H -P -s 0:0,hostbridge \ --s 1:0,virtio-net,faketapdev,mac=52:54:00:00:00:00 \ +-s 3:0,virtio-net,faketapdev,mac=52:54:00:00:00:00 \ -s 2:0,ahci-hd,/tmp/freebsd.img bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.xml index b429fef..2be970e 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.xml +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.xml @@ -15,10 +15,12 @@ <driver name='file' type='raw'/> <source file='/tmp/freebsd.img'/> <target dev='hda' bus='sata'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> </disk> <interface type='bridge'> <model type='virtio'/> <source bridge="virbr0"/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </interface> </devices> </domain> diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-base.args b/tests/bhyvexml2argvdata/bhyvexml2argv-base.args index 9d4faa5..4122e62 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-base.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-base.args @@ -1,3 +1,3 @@ /usr/sbin/bhyve -c 1 -m 214 -H -P -s 0:0,hostbridge \ --s 1:0,virtio-net,faketapdev,mac=52:54:00:00:00:00 \ +-s 3:0,virtio-net,faketapdev,mac=52:54:00:00:00:00 \ -s 2:0,ahci-hd,/tmp/freebsd.img bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-base.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-base.xml index 8c96f77..3d23375 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-base.xml +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-base.xml @@ -11,10 +11,12 @@ <driver name='file' type='raw'/> <source file='/tmp/freebsd.img'/> <target dev='hda' bus='sata'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> </disk> <interface type='bridge'> <model type='virtio'/> <source bridge="virbr0"/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </interface> </devices> </domain> diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-console.args b/tests/bhyvexml2argvdata/bhyvexml2argv-console.args index 1e09fb4..df50290 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-console.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-console.args @@ -1,4 +1,4 @@ /usr/sbin/bhyve -c 1 -m 214 -H -P -s 0:0,hostbridge \ --s 1:0,virtio-net,faketapdev,mac=52:54:00:00:00:00 \ +-s 3:0,virtio-net,faketapdev,mac=52:54:00:00:00:00 \ -s 2:0,ahci-hd,/tmp/freebsd.img \ --s 31,lpc -l com1,/dev/nmdm0A bhyve +-s 1,lpc -l com1,/dev/nmdm0A bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-console.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-console.xml index 64073f0..35206b5 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-console.xml +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-console.xml @@ -11,10 +11,12 @@ <driver name='file' type='raw'/> <source file='/tmp/freebsd.img'/> <target dev='hda' bus='sata'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> </disk> <interface type='bridge'> <model type='virtio'/> <source bridge="virbr0"/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </interface> <console type='nmdm'> <source master='/dev/nmdm0A' slave='/dev/nmdm0B'/> diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.args b/tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.args index 54ad2b8..1638d54 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.args @@ -1,3 +1,3 @@ /usr/sbin/bhyve -c 1 -m 214 -H -P -s 0:0,hostbridge \ --s 1:0,virtio-net,faketapdev,mac=52:54:00:00:00:00 \ +-s 3:0,virtio-net,faketapdev,mac=52:54:00:00:00:00 \ -s 2:0,virtio-blk,/tmp/freebsd.img bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.xml index 8cfb518..773d55e 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.xml +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.xml @@ -11,10 +11,12 @@ <driver name='file' type='raw'/> <source file='/tmp/freebsd.img'/> <target dev='vda' bus='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> </disk> <interface type='bridge'> <model type='virtio'/> <source bridge="virbr0"/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </interface> </devices> </domain> diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.args b/tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.args index 1a06abd..f914865 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.args @@ -1,3 +1,3 @@ /usr/sbin/bhyve -c 1 -m 214 -H -P -s 0:0,hostbridge \ --s 1:0,virtio-net,faketapdev,mac=52:54:00:22:ee:11 \ +-s 3:0,virtio-net,faketapdev,mac=52:54:00:22:ee:11 \ -s 2:0,ahci-hd,/tmp/freebsd.img bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.xml index 41a42b0..b262eb7 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.xml +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.xml @@ -11,11 +11,13 @@ <driver name='file' type='raw'/> <source file='/tmp/freebsd.img'/> <target dev='hda' bus='sata'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> </disk> <interface type='bridge'> <mac address="52:54:00:22:ee:11"/> <model type='virtio'/> <source bridge="virbr0"/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </interface> </devices> </domain> diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-serial.args b/tests/bhyvexml2argvdata/bhyvexml2argv-serial.args index 1e09fb4..df50290 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-serial.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-serial.args @@ -1,4 +1,4 @@ /usr/sbin/bhyve -c 1 -m 214 -H -P -s 0:0,hostbridge \ --s 1:0,virtio-net,faketapdev,mac=52:54:00:00:00:00 \ +-s 3:0,virtio-net,faketapdev,mac=52:54:00:00:00:00 \ -s 2:0,ahci-hd,/tmp/freebsd.img \ --s 31,lpc -l com1,/dev/nmdm0A bhyve +-s 1,lpc -l com1,/dev/nmdm0A bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-serial.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-serial.xml index bfecbb9..cd4f25b 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-serial.xml +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-serial.xml @@ -11,10 +11,12 @@ <driver name='file' type='raw'/> <source file='/tmp/freebsd.img'/> <target dev='hda' bus='sata'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> </disk> <interface type='bridge'> <model type='virtio'/> <source bridge="virbr0"/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </interface> <serial type='nmdm'> <source master='/dev/nmdm0A' slave='/dev/nmdm0B'/> -- 1.9.0
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
--- 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?
+ 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
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
On 05/26/2014 06:24 PM, Roman Bogorodskiy wrote:
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?
Yes, it would be nice not to break upgrades :) In QEMU driver, this is done by calling AssignAddresses from QemuProcessStart.
--- 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 bhyve is able to run the inaccessible domain, I think we should allow it. Jan
Ján Tomko wrote:
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?
Yes, it would be nice not to break upgrades :) In QEMU driver, this is done by calling AssignAddresses from QemuProcessStart.
Thanks, I'll take a look how Qemu driver does that. PS Looks like I'm late with getting this feature into the current release as it's unlikely I'll be able to get to it until this weekend. Roman Bogorodskiy
Roman Bogorodskiy wrote:
Ján Tomko wrote:
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?
Yes, it would be nice not to break upgrades :) In QEMU driver, this is done by calling AssignAddresses from QemuProcessStart.
Thanks, I'll take a look how Qemu driver does that.
PS Looks like I'm late with getting this feature into the current release as it's unlikely I'll be able to get to it until this weekend.
I've uploaded v6 that assigns addresses in ProcessStart. While looking at the code from Qemu I also noticed that I've missed address assignment in CreateXML function as well. Roman Bogorodskiy
participants (2)
-
Ján Tomko -
Roman Bogorodskiy