[libvirt] [PATCH v4 0/3] bhyve: implement PCI address allocation

Changes from v3: - Instead of adding pci allocation code to utils/virpci.[ch], create new files conf/domain_addr.[ch] to contain only domain device addresses management Changes from v2: - Base on PCI allocation code from Qemu driver Changes from v1: - Reserve slot 1 for LPC PCI-ISA bridge, used by console device - Respect addresses provided by user in domain xml file - Fix tests so 'make check' passes Roman Bogorodskiy (3): qemu: extract PCI handling structs qemu: extract common PCI handling functions bhyve: implement PCI address allocation po/POTFILES.in | 2 + src/Makefile.am | 5 + src/bhyve/bhyve_command.c | 142 ++-- 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 +- src/conf/domain_addr.c | 566 ++++++++++++++++ src/conf/domain_addr.h | 149 ++++ src/libvirt_private.syms | 17 + src/qemu/qemu_command.c | 754 +++------------------ src/qemu/qemu_command.h | 49 +- src/qemu/qemu_domain.c | 3 +- src/qemu/qemu_domain.h | 6 +- src/qemu/qemu_hotplug.c | 8 +- src/qemu/qemu_process.c | 2 +- .../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 + 29 files changed, 1287 insertions(+), 782 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 create mode 100644 src/conf/domain_addr.c create mode 100644 src/conf/domain_addr.h -- 1.9.0

Introduce new files (domain_addr.[ch]) to provide an API for domain device handling that could be shared across the drivers. A list of data types were extracted and moved there: qemuDomainPCIAddressBus -> virDomainPCIAddressBus qemuDomainPCIAddressBusPtr -> virDomainPCIAddressBusPtr _qemuDomainPCIAddressSet -> virDomainPCIAddressSet qemuDomainPCIAddressSetPtr -> virDomainPCIAddressSetPtr qemuDomainPCIConnectFlags -> virDomainPCIConnectFlags Also, move the related definitions and macros. --- src/Makefile.am | 1 + src/conf/domain_addr.c | 24 +++++++ src/conf/domain_addr.h | 79 ++++++++++++++++++++++ src/qemu/qemu_command.c | 171 +++++++++++++++++++++--------------------------- src/qemu/qemu_command.h | 49 ++++---------- src/qemu/qemu_domain.h | 6 +- 6 files changed, 193 insertions(+), 137 deletions(-) create mode 100644 src/conf/domain_addr.c create mode 100644 src/conf/domain_addr.h diff --git a/src/Makefile.am b/src/Makefile.am index e9dc9e0..cfb7097 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -247,6 +247,7 @@ NETDEV_CONF_SOURCES = \ # Domain driver generic impl APIs DOMAIN_CONF_SOURCES = \ conf/capabilities.c conf/capabilities.h \ + conf/domain_addr.c conf/domain_addr.h \ conf/domain_conf.c conf/domain_conf.h \ conf/domain_audit.c conf/domain_audit.h \ conf/domain_nwfilter.c conf/domain_nwfilter.h \ diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c new file mode 100644 index 0000000..73e17aa --- /dev/null +++ b/src/conf/domain_addr.c @@ -0,0 +1,24 @@ +/* + * domain_addr.c: helper APIs for managing domain device addresses + * + * Copyright (C) 2006-2014 Red Hat, Inc. + * Copyright (C) 2006 Daniel P. Berrange + * + * 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: Daniel P. Berrange <berrange@redhat.com> + */ + +#include <config.h> diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h new file mode 100644 index 0000000..f5a5199 --- /dev/null +++ b/src/conf/domain_addr.h @@ -0,0 +1,79 @@ +/* + * domain_addr.h: helper APIs for managing domain device addresses + * + * Copyright (C) 2006-2014 Red Hat, Inc. + * Copyright (C) 2006 Daniel P. Berrange + * + * 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: Daniel P. Berrange <berrange@redhat.com> + */ + +#ifndef __DOMAIN_ADDR_H__ +# define __DOMAIN_ADDR_H__ + +# include "domain_conf.h" + +# define VIR_PCI_ADDRESS_SLOT_LAST 31 +# define VIR_PCI_ADDRESS_FUNCTION_LAST 7 + +typedef enum { + VIR_PCI_CONNECT_HOTPLUGGABLE = 1 << 0, + /* This bus supports hot-plug */ + VIR_PCI_CONNECT_SINGLESLOT = 1 << 1, + /* This "bus" has only a single downstream slot/port */ + + VIR_PCI_CONNECT_TYPE_PCI = 1 << 2, + /* PCI devices can connect to this bus */ + VIR_PCI_CONNECT_TYPE_PCIE = 1 << 3, + /* PCI Express devices can connect to this bus */ + VIR_PCI_CONNECT_TYPE_EITHER_IF_CONFIG = 1 << 4, + /* PCI *and* PCIe devices allowed, if the address + * was specified in the config by the user + */ +} virDomainPCIConnectFlags; + +typedef struct { + virDomainControllerModelPCI model; + /* flags an min/max can be computed from model, but + * having them ready makes life easier. + */ + virDomainPCIConnectFlags flags; + size_t minSlot, maxSlot; /* usually 0,0 or 1,31 */ + /* Each bit in a slot represents one function on that slot. If the + * bit is set, that function is in use by a device. + */ + uint8_t slots[VIR_PCI_ADDRESS_SLOT_LAST + 1]; +} virDomainPCIAddressBus; +typedef virDomainPCIAddressBus *virDomainPCIAddressBusPtr; + +struct _virDomainPCIAddressSet { + virDomainPCIAddressBus *buses; + size_t nbuses; + virDevicePCIAddress lastaddr; + virDomainPCIConnectFlags lastFlags; + bool dryRun; /* on a dry run, new buses are auto-added + and addresses aren't saved in device infos */ +}; +typedef struct _virDomainPCIAddressSet virDomainPCIAddressSet; +typedef virDomainPCIAddressSet *virDomainPCIAddressSetPtr; + +/* a combination of all bit that describe the type of connections + * allowed, e.g. PCI, PCIe, switch + */ +# define VIR_PCI_CONNECT_TYPES_MASK \ + (VIR_PCI_CONNECT_TYPE_PCI | VIR_PCI_CONNECT_TYPE_PCIE) + +#endif /* __DOMAIN_ADDR_H__ */ diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index dd8e40a..ebf17a8 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -39,6 +39,7 @@ #include "viruuid.h" #include "c-ctype.h" #include "domain_nwfilter.h" +#include "domain_addr.h" #include "domain_audit.h" #include "domain_conf.h" #include "snapshot_conf.h" @@ -1443,47 +1444,21 @@ int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def, return ret; } -#define QEMU_PCI_ADDRESS_SLOT_LAST 31 -#define QEMU_PCI_ADDRESS_FUNCTION_LAST 7 - -typedef struct { - virDomainControllerModelPCI model; - /* flags an min/max can be computed from model, but - * having them ready makes life easier. - */ - qemuDomainPCIConnectFlags flags; - size_t minSlot, maxSlot; /* usually 0,0 or 1,31 */ - /* Each bit in a slot represents one function on that slot. If the - * bit is set, that function is in use by a device. - */ - uint8_t slots[QEMU_PCI_ADDRESS_SLOT_LAST + 1]; -} qemuDomainPCIAddressBus; -typedef qemuDomainPCIAddressBus *qemuDomainPCIAddressBusPtr; - -struct _qemuDomainPCIAddressSet { - qemuDomainPCIAddressBus *buses; - size_t nbuses; - virDevicePCIAddress lastaddr; - qemuDomainPCIConnectFlags lastFlags; - bool dryRun; /* on a dry run, new buses are auto-added - and addresses aren't saved in device infos */ -}; - static bool qemuDomainPCIAddressFlagsCompatible(virDevicePCIAddressPtr addr, const char *addrStr, - qemuDomainPCIConnectFlags busFlags, - qemuDomainPCIConnectFlags devFlags, + virDomainPCIConnectFlags busFlags, + virDomainPCIConnectFlags devFlags, bool reportError, bool fromConfig) { virErrorNumber errType = (fromConfig ? VIR_ERR_XML_ERROR : VIR_ERR_INTERNAL_ERROR); - qemuDomainPCIConnectFlags flagsMatchMask = QEMU_PCI_CONNECT_TYPES_MASK; + virDomainPCIConnectFlags flagsMatchMask = VIR_PCI_CONNECT_TYPES_MASK; if (fromConfig) - flagsMatchMask |= QEMU_PCI_CONNECT_TYPE_EITHER_IF_CONFIG; + flagsMatchMask |= VIR_PCI_CONNECT_TYPE_EITHER_IF_CONFIG; /* If this bus doesn't allow the type of connection (PCI * vs. PCIe) required by the device, or if the device requires @@ -1491,13 +1466,13 @@ qemuDomainPCIAddressFlagsCompatible(virDevicePCIAddressPtr addr, */ if (!(devFlags & busFlags & flagsMatchMask)) { if (reportError) { - if (devFlags & QEMU_PCI_CONNECT_TYPE_PCI) { + if (devFlags & VIR_PCI_CONNECT_TYPE_PCI) { virReportError(errType, _("PCI bus is not compatible with the device " "at %s. Device requires a standard PCI slot, " "which is not provided by bus %.4x:%.2x"), addrStr, addr->domain, addr->bus); - } else if (devFlags & QEMU_PCI_CONNECT_TYPE_PCIE) { + } else if (devFlags & VIR_PCI_CONNECT_TYPE_PCIE) { virReportError(errType, _("PCI bus is not compatible with the device " "at %s. Device requires a PCI Express slot, " @@ -1514,8 +1489,8 @@ qemuDomainPCIAddressFlagsCompatible(virDevicePCIAddressPtr addr, } return false; } - if ((devFlags & QEMU_PCI_CONNECT_HOTPLUGGABLE) && - !(busFlags & QEMU_PCI_CONNECT_HOTPLUGGABLE)) { + if ((devFlags & VIR_PCI_CONNECT_HOTPLUGGABLE) && + !(busFlags & VIR_PCI_CONNECT_HOTPLUGGABLE)) { if (reportError) { virReportError(errType, _("PCI bus is not compatible with the device " @@ -1534,13 +1509,13 @@ qemuDomainPCIAddressFlagsCompatible(virDevicePCIAddressPtr addr, * comparing the flags). */ static bool -qemuDomainPCIAddressValidate(qemuDomainPCIAddressSetPtr addrs, +qemuDomainPCIAddressValidate(virDomainPCIAddressSetPtr addrs, virDevicePCIAddressPtr addr, const char *addrStr, - qemuDomainPCIConnectFlags flags, + virDomainPCIConnectFlags flags, bool fromConfig) { - qemuDomainPCIAddressBusPtr bus; + virDomainPCIAddressBusPtr bus; virErrorNumber errType = (fromConfig ? VIR_ERR_XML_ERROR : VIR_ERR_INTERNAL_ERROR); @@ -1585,10 +1560,10 @@ qemuDomainPCIAddressValidate(qemuDomainPCIAddressSetPtr addrs, addrStr, bus->maxSlot); return false; } - if (addr->function > QEMU_PCI_ADDRESS_FUNCTION_LAST) { + if (addr->function > VIR_PCI_ADDRESS_FUNCTION_LAST) { virReportError(errType, _("Invalid PCI address %s. function must be <= %u"), - addrStr, QEMU_PCI_ADDRESS_FUNCTION_LAST); + addrStr, VIR_PCI_ADDRESS_FUNCTION_LAST); return false; } return true; @@ -1596,33 +1571,33 @@ qemuDomainPCIAddressValidate(qemuDomainPCIAddressSetPtr addrs, static int -qemuDomainPCIAddressBusSetModel(qemuDomainPCIAddressBusPtr bus, +qemuDomainPCIAddressBusSetModel(virDomainPCIAddressBusPtr bus, virDomainControllerModelPCI model) { switch (model) { case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE: case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT: - bus->flags = (QEMU_PCI_CONNECT_HOTPLUGGABLE | - QEMU_PCI_CONNECT_TYPE_PCI); + bus->flags = (VIR_PCI_CONNECT_HOTPLUGGABLE | + VIR_PCI_CONNECT_TYPE_PCI); bus->minSlot = 1; - bus->maxSlot = QEMU_PCI_ADDRESS_SLOT_LAST; + bus->maxSlot = VIR_PCI_ADDRESS_SLOT_LAST; break; case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT: /* slots 1 - 31, no hotplug, PCIe only unless the address was * specified in user config *and* the particular device being * attached also allows it */ - bus->flags = (QEMU_PCI_CONNECT_TYPE_PCIE | - QEMU_PCI_CONNECT_TYPE_EITHER_IF_CONFIG); + bus->flags = (VIR_PCI_CONNECT_TYPE_PCIE | + VIR_PCI_CONNECT_TYPE_EITHER_IF_CONFIG); bus->minSlot = 1; - bus->maxSlot = QEMU_PCI_ADDRESS_SLOT_LAST; + bus->maxSlot = VIR_PCI_ADDRESS_SLOT_LAST; break; case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE: /* slots 1 - 31, standard PCI slots, * but *not* hot-pluggable */ - bus->flags = QEMU_PCI_CONNECT_TYPE_PCI; + bus->flags = VIR_PCI_CONNECT_TYPE_PCI; bus->minSlot = 1; - bus->maxSlot = QEMU_PCI_ADDRESS_SLOT_LAST; + bus->maxSlot = VIR_PCI_ADDRESS_SLOT_LAST; break; default: virReportError(VIR_ERR_INTERNAL_ERROR, @@ -1646,9 +1621,9 @@ qemuDomainPCIAddressBusSetModel(qemuDomainPCIAddressBusPtr bus, * >0 = number of buses added */ static int -qemuDomainPCIAddressSetGrow(qemuDomainPCIAddressSetPtr addrs, +qemuDomainPCIAddressSetGrow(virDomainPCIAddressSetPtr addrs, virDevicePCIAddressPtr addr, - qemuDomainPCIConnectFlags flags) + virDomainPCIConnectFlags flags) { int add; size_t i; @@ -1659,7 +1634,7 @@ qemuDomainPCIAddressSetGrow(qemuDomainPCIAddressSetPtr addrs, return 0; /* auto-grow only works when we're adding plain PCI devices */ - if (!(flags & QEMU_PCI_CONNECT_TYPE_PCI)) { + if (!(flags & VIR_PCI_CONNECT_TYPE_PCI)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot automatically add a new PCI bus for a " "device requiring a slot other than standard PCI.")); @@ -1702,13 +1677,13 @@ qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, virDomainDeviceInfoPtr info, void *opaque) { - qemuDomainPCIAddressSetPtr addrs = opaque; + virDomainPCIAddressSetPtr addrs = opaque; int ret = -1; virDevicePCIAddressPtr addr = &info->addr.pci; bool entireSlot; /* flags may be changed from default below */ - qemuDomainPCIConnectFlags flags = (QEMU_PCI_CONNECT_HOTPLUGGABLE | - QEMU_PCI_CONNECT_TYPE_PCI); + virDomainPCIConnectFlags flags = (VIR_PCI_CONNECT_HOTPLUGGABLE | + VIR_PCI_CONNECT_TYPE_PCI); if ((info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) || ((device->type == VIR_DOMAIN_DEVICE_HOSTDEV) && @@ -1732,13 +1707,13 @@ qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, /* pci-bridge needs a PCI slot, but it isn't * hot-pluggable, so it doesn't need a hot-pluggable slot. */ - flags = QEMU_PCI_CONNECT_TYPE_PCI; + flags = VIR_PCI_CONNECT_TYPE_PCI; break; case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE: /* pci-bridge needs a PCIe slot, but it isn't * hot-pluggable, so it doesn't need a hot-pluggable slot. */ - flags = QEMU_PCI_CONNECT_TYPE_PCIE; + flags = VIR_PCI_CONNECT_TYPE_PCIE; break; default: break; @@ -1749,7 +1724,7 @@ qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, /* SATA controllers aren't hot-plugged, and can be put in * either a PCI or PCIe slot */ - flags = QEMU_PCI_CONNECT_TYPE_PCI | QEMU_PCI_CONNECT_TYPE_PCIE; + flags = VIR_PCI_CONNECT_TYPE_PCI | VIR_PCI_CONNECT_TYPE_PCIE; break; case VIR_DOMAIN_CONTROLLER_TYPE_USB: @@ -1763,14 +1738,14 @@ qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI2: case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI3: case VIR_DOMAIN_CONTROLLER_MODEL_USB_VT82C686B_UHCI: - flags = (QEMU_PCI_CONNECT_TYPE_PCI | - QEMU_PCI_CONNECT_TYPE_EITHER_IF_CONFIG); + flags = (VIR_PCI_CONNECT_TYPE_PCI | + VIR_PCI_CONNECT_TYPE_EITHER_IF_CONFIG); break; case VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI: /* should this be PCIE-only? Or do we need to allow PCI * for backward compatibility? */ - flags = QEMU_PCI_CONNECT_TYPE_PCI | QEMU_PCI_CONNECT_TYPE_PCIE; + flags = VIR_PCI_CONNECT_TYPE_PCI | VIR_PCI_CONNECT_TYPE_PCIE; break; case VIR_DOMAIN_CONTROLLER_MODEL_USB_PCI_OHCI: case VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI: @@ -1785,8 +1760,8 @@ qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, switch (device->data.sound->model) { case VIR_DOMAIN_SOUND_MODEL_ICH6: case VIR_DOMAIN_SOUND_MODEL_ICH9: - flags = (QEMU_PCI_CONNECT_TYPE_PCI | - QEMU_PCI_CONNECT_TYPE_EITHER_IF_CONFIG); + flags = (VIR_PCI_CONNECT_TYPE_PCI | + VIR_PCI_CONNECT_TYPE_EITHER_IF_CONFIG); break; } break; @@ -1795,7 +1770,7 @@ qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, /* video cards aren't hot-plugged, and can be put in either a * PCI or PCIe slot */ - flags = QEMU_PCI_CONNECT_TYPE_PCI | QEMU_PCI_CONNECT_TYPE_PCIE; + flags = VIR_PCI_CONNECT_TYPE_PCI | VIR_PCI_CONNECT_TYPE_PCIE; break; } @@ -1827,7 +1802,7 @@ qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, * commandline, but that don't really care if a PCI bus * actually exists. */ if (addrs->nbuses > 0 && - !(addrs->buses[0].flags & QEMU_PCI_CONNECT_TYPE_PCI)) { + !(addrs->buses[0].flags & VIR_PCI_CONNECT_TYPE_PCI)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Bus 0 must be PCI for integrated PIIX3 " "USB or IDE controllers")); @@ -1868,7 +1843,7 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, virDomainObjPtr obj) { int ret = -1; - qemuDomainPCIAddressSetPtr addrs = NULL; + virDomainPCIAddressSetPtr addrs = NULL; qemuDomainObjPrivatePtr priv = NULL; if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { @@ -1876,7 +1851,7 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, int nbuses = 0; size_t i; int rv; - qemuDomainPCIConnectFlags flags = QEMU_PCI_CONNECT_TYPE_PCI; + virDomainPCIConnectFlags flags = VIR_PCI_CONNECT_TYPE_PCI; for (i = 0; i < def->ncontrollers; i++) { if (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) { @@ -1901,7 +1876,7 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, goto cleanup; for (i = 1; i < addrs->nbuses; i++) { - qemuDomainPCIAddressBusPtr bus = &addrs->buses[i]; + virDomainPCIAddressBusPtr bus = &addrs->buses[i]; if ((rv = virDomainDefMaybeAddController( def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, @@ -1975,12 +1950,12 @@ int qemuDomainAssignAddresses(virDomainDefPtr def, } -qemuDomainPCIAddressSetPtr +virDomainPCIAddressSetPtr qemuDomainPCIAddressSetCreate(virDomainDefPtr def, unsigned int nbuses, bool dryRun) { - qemuDomainPCIAddressSetPtr addrs; + virDomainPCIAddressSetPtr addrs; size_t i; if (VIR_ALLOC(addrs) < 0) @@ -2038,7 +2013,7 @@ qemuDomainPCIAddressSetCreate(virDomainDefPtr def, /* * Check if the PCI slot is used by another device. */ -static bool qemuDomainPCIAddressSlotInUse(qemuDomainPCIAddressSetPtr addrs, +static bool qemuDomainPCIAddressSlotInUse(virDomainPCIAddressSetPtr addrs, virDevicePCIAddressPtr addr) { return !!addrs->buses[addr->bus].slots[addr->slot]; @@ -2055,15 +2030,15 @@ static bool qemuDomainPCIAddressSlotInUse(qemuDomainPCIAddressSetPtr addrs, * XML). */ int -qemuDomainPCIAddressReserveAddr(qemuDomainPCIAddressSetPtr addrs, +qemuDomainPCIAddressReserveAddr(virDomainPCIAddressSetPtr addrs, virDevicePCIAddressPtr addr, - qemuDomainPCIConnectFlags flags, + virDomainPCIConnectFlags flags, bool reserveEntireSlot, bool fromConfig) { int ret = -1; char *addrStr = NULL; - qemuDomainPCIAddressBusPtr bus; + virDomainPCIAddressBusPtr bus; virErrorNumber errType = (fromConfig ? VIR_ERR_XML_ERROR : VIR_ERR_INTERNAL_ERROR); @@ -2117,14 +2092,14 @@ qemuDomainPCIAddressReserveAddr(qemuDomainPCIAddressSetPtr addrs, int -qemuDomainPCIAddressReserveSlot(qemuDomainPCIAddressSetPtr addrs, +qemuDomainPCIAddressReserveSlot(virDomainPCIAddressSetPtr addrs, virDevicePCIAddressPtr addr, - qemuDomainPCIConnectFlags flags) + virDomainPCIConnectFlags flags) { return qemuDomainPCIAddressReserveAddr(addrs, addr, flags, true, false); } -int qemuDomainPCIAddressEnsureAddr(qemuDomainPCIAddressSetPtr addrs, +int qemuDomainPCIAddressEnsureAddr(virDomainPCIAddressSetPtr addrs, virDomainDeviceInfoPtr dev) { int ret = -1; @@ -2134,8 +2109,8 @@ int qemuDomainPCIAddressEnsureAddr(qemuDomainPCIAddressSetPtr addrs, * function is only used for hot-plug, though, and hot-plug is * only supported for standard PCI devices, so we can safely use * the setting below */ - qemuDomainPCIConnectFlags flags = (QEMU_PCI_CONNECT_HOTPLUGGABLE | - QEMU_PCI_CONNECT_TYPE_PCI); + virDomainPCIConnectFlags flags = (VIR_PCI_CONNECT_HOTPLUGGABLE | + VIR_PCI_CONNECT_TYPE_PCI); if (!(addrStr = qemuDomainPCIAddressAsString(&dev->addr.pci))) goto cleanup; @@ -2166,7 +2141,7 @@ int qemuDomainPCIAddressEnsureAddr(qemuDomainPCIAddressSetPtr addrs, } -int qemuDomainPCIAddressReleaseAddr(qemuDomainPCIAddressSetPtr addrs, +int qemuDomainPCIAddressReleaseAddr(virDomainPCIAddressSetPtr addrs, virDevicePCIAddressPtr addr) { addrs->buses[addr->bus].slots[addr->slot] &= ~(1 << addr->function); @@ -2174,13 +2149,13 @@ int qemuDomainPCIAddressReleaseAddr(qemuDomainPCIAddressSetPtr addrs, } static int -qemuDomainPCIAddressReleaseSlot(qemuDomainPCIAddressSetPtr addrs, +qemuDomainPCIAddressReleaseSlot(virDomainPCIAddressSetPtr addrs, virDevicePCIAddressPtr addr) { /* permit any kind of connection type in validation, since we * already had it, and are giving it back. */ - qemuDomainPCIConnectFlags flags = QEMU_PCI_CONNECT_TYPES_MASK; + virDomainPCIConnectFlags flags = VIR_PCI_CONNECT_TYPES_MASK; int ret = -1; char *addrStr = NULL; @@ -2197,7 +2172,7 @@ qemuDomainPCIAddressReleaseSlot(qemuDomainPCIAddressSetPtr addrs, return ret; } -void qemuDomainPCIAddressSetFree(qemuDomainPCIAddressSetPtr addrs) +void qemuDomainPCIAddressSetFree(virDomainPCIAddressSetPtr addrs) { if (!addrs) return; @@ -2208,9 +2183,9 @@ void qemuDomainPCIAddressSetFree(qemuDomainPCIAddressSetPtr addrs) static int -qemuDomainPCIAddressGetNextSlot(qemuDomainPCIAddressSetPtr addrs, +qemuDomainPCIAddressGetNextSlot(virDomainPCIAddressSetPtr addrs, virDevicePCIAddressPtr next_addr, - qemuDomainPCIConnectFlags flags) + virDomainPCIConnectFlags flags) { /* default to starting the search for a free slot from * 0000:00:00.0 @@ -2241,7 +2216,7 @@ qemuDomainPCIAddressGetNextSlot(qemuDomainPCIAddressSetPtr addrs, a.domain, a.bus); continue; } - for (; a.slot <= QEMU_PCI_ADDRESS_SLOT_LAST; a.slot++) { + for (; a.slot <= VIR_PCI_ADDRESS_SLOT_LAST; a.slot++) { if (!qemuDomainPCIAddressSlotInUse(addrs, &a)) goto success; @@ -2271,7 +2246,7 @@ qemuDomainPCIAddressGetNextSlot(qemuDomainPCIAddressSetPtr addrs, a.domain, a.bus); continue; } - for (a.slot = 1; a.slot <= QEMU_PCI_ADDRESS_SLOT_LAST; a.slot++) { + for (a.slot = 1; a.slot <= VIR_PCI_ADDRESS_SLOT_LAST; a.slot++) { if (!qemuDomainPCIAddressSlotInUse(addrs, &a)) goto success; @@ -2296,9 +2271,9 @@ qemuDomainPCIAddressGetNextSlot(qemuDomainPCIAddressSetPtr addrs, } int -qemuDomainPCIAddressReserveNextSlot(qemuDomainPCIAddressSetPtr addrs, +qemuDomainPCIAddressReserveNextSlot(virDomainPCIAddressSetPtr addrs, virDomainDeviceInfoPtr dev, - qemuDomainPCIConnectFlags flags) + virDomainPCIConnectFlags flags) { virDevicePCIAddress addr; if (qemuDomainPCIAddressGetNextSlot(addrs, &addr, flags) < 0) @@ -2354,14 +2329,14 @@ qemuDomainReleaseDeviceAddress(virDomainObjPtr vm, static int qemuValidateDevicePCISlotsPIIX3(virDomainDefPtr def, virQEMUCapsPtr qemuCaps, - qemuDomainPCIAddressSetPtr addrs) + virDomainPCIAddressSetPtr addrs) { int ret = -1; size_t i; virDevicePCIAddress tmp_addr; bool qemuDeviceVideoUsable = virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIDEO_PRIMARY); char *addrStr = NULL; - qemuDomainPCIConnectFlags flags = QEMU_PCI_CONNECT_HOTPLUGGABLE | QEMU_PCI_CONNECT_TYPE_PCI; + virDomainPCIConnectFlags flags = VIR_PCI_CONNECT_HOTPLUGGABLE | VIR_PCI_CONNECT_TYPE_PCI; /* Verify that first IDE and USB controllers (if any) is on the PIIX3, fn 1 */ for (i = 0; i < def->ncontrollers; i++) { @@ -2507,14 +2482,14 @@ qemuDomainMachineIsI440FX(virDomainDefPtr def) static int qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, virQEMUCapsPtr qemuCaps, - qemuDomainPCIAddressSetPtr addrs) + virDomainPCIAddressSetPtr addrs) { int ret = -1; size_t i; virDevicePCIAddress tmp_addr; bool qemuDeviceVideoUsable = virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIDEO_PRIMARY); char *addrStr = NULL; - qemuDomainPCIConnectFlags flags = QEMU_PCI_CONNECT_TYPE_PCIE; + virDomainPCIConnectFlags flags = VIR_PCI_CONNECT_TYPE_PCIE; for (i = 0; i < def->ncontrollers; i++) { switch (def->controllers[i]->type) { @@ -2690,10 +2665,10 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, int qemuAssignDevicePCISlots(virDomainDefPtr def, virQEMUCapsPtr qemuCaps, - qemuDomainPCIAddressSetPtr addrs) + virDomainPCIAddressSetPtr addrs) { size_t i, j; - qemuDomainPCIConnectFlags flags; + virDomainPCIConnectFlags flags; virDevicePCIAddress tmp_addr; if ((STRPREFIX(def->os.machine, "pc-0.") || @@ -2725,16 +2700,16 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, /* pci-bridge doesn't require hot-plug * (although it does provide hot-plug in its slots) */ - flags = QEMU_PCI_CONNECT_TYPE_PCI; + flags = VIR_PCI_CONNECT_TYPE_PCI; break; case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE: /* dmi-to-pci-bridge requires a non-hotplug PCIe * slot */ - flags = QEMU_PCI_CONNECT_TYPE_PCIE; + flags = VIR_PCI_CONNECT_TYPE_PCIE; break; default: - flags = QEMU_PCI_CONNECT_HOTPLUGGABLE | QEMU_PCI_CONNECT_TYPE_PCI; + flags = VIR_PCI_CONNECT_HOTPLUGGABLE | VIR_PCI_CONNECT_TYPE_PCI; break; } if (qemuDomainPCIAddressReserveNextSlot(addrs, @@ -2744,7 +2719,7 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, } } - flags = QEMU_PCI_CONNECT_HOTPLUGGABLE | QEMU_PCI_CONNECT_TYPE_PCI; + flags = VIR_PCI_CONNECT_HOTPLUGGABLE | VIR_PCI_CONNECT_TYPE_PCI; for (i = 0; i < def->nfss; i++) { if (def->fss[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 45f29a0..29da1ca 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -24,6 +24,7 @@ #ifndef __QEMU_COMMAND_H__ # define __QEMU_COMMAND_H__ +# include "domain_addr.h" # include "domain_conf.h" # include "vircommand.h" # include "capabilities.h" @@ -234,55 +235,33 @@ void qemuDomainReleaseDeviceAddress(virDomainObjPtr vm, virDomainDeviceInfoPtr info, const char *devstr); -typedef enum { - QEMU_PCI_CONNECT_HOTPLUGGABLE = 1 << 0, - /* This bus supports hot-plug */ - QEMU_PCI_CONNECT_SINGLESLOT = 1 << 1, - /* This "bus" has only a single downstream slot/port */ - - QEMU_PCI_CONNECT_TYPE_PCI = 1 << 2, - /* PCI devices can connect to this bus */ - QEMU_PCI_CONNECT_TYPE_PCIE = 1 << 3, - /* PCI Express devices can connect to this bus */ - QEMU_PCI_CONNECT_TYPE_EITHER_IF_CONFIG = 1 << 4, - /* PCI *and* PCIe devices allowed, if the address - * was specified in the config by the user - */ -} qemuDomainPCIConnectFlags; - -/* a combination of all bit that describe the type of connections - * allowed, e.g. PCI, PCIe, switch - */ -# define QEMU_PCI_CONNECT_TYPES_MASK \ - (QEMU_PCI_CONNECT_TYPE_PCI | QEMU_PCI_CONNECT_TYPE_PCIE) - int qemuDomainAssignPCIAddresses(virDomainDefPtr def, virQEMUCapsPtr qemuCaps, virDomainObjPtr obj); -qemuDomainPCIAddressSetPtr qemuDomainPCIAddressSetCreate(virDomainDefPtr def, - unsigned int nbuses, - bool dryRun); -int qemuDomainPCIAddressReserveSlot(qemuDomainPCIAddressSetPtr addrs, +virDomainPCIAddressSetPtr qemuDomainPCIAddressSetCreate(virDomainDefPtr def, + unsigned int nbuses, + bool dryRun); +int qemuDomainPCIAddressReserveSlot(virDomainPCIAddressSetPtr addrs, virDevicePCIAddressPtr addr, - qemuDomainPCIConnectFlags flags); -int qemuDomainPCIAddressReserveAddr(qemuDomainPCIAddressSetPtr addrs, + virDomainPCIConnectFlags flags); +int qemuDomainPCIAddressReserveAddr(virDomainPCIAddressSetPtr addrs, virDevicePCIAddressPtr addr, - qemuDomainPCIConnectFlags flags, + virDomainPCIConnectFlags flags, bool reserveEntireSlot, bool fromConfig); -int qemuDomainPCIAddressReserveNextSlot(qemuDomainPCIAddressSetPtr addrs, +int qemuDomainPCIAddressReserveNextSlot(virDomainPCIAddressSetPtr addrs, virDomainDeviceInfoPtr dev, - qemuDomainPCIConnectFlags flags); -int qemuDomainPCIAddressEnsureAddr(qemuDomainPCIAddressSetPtr addrs, + virDomainPCIConnectFlags flags); +int qemuDomainPCIAddressEnsureAddr(virDomainPCIAddressSetPtr addrs, virDomainDeviceInfoPtr dev); -int qemuDomainPCIAddressReleaseAddr(qemuDomainPCIAddressSetPtr addrs, +int qemuDomainPCIAddressReleaseAddr(virDomainPCIAddressSetPtr addrs, virDevicePCIAddressPtr addr); -void qemuDomainPCIAddressSetFree(qemuDomainPCIAddressSetPtr addrs); +void qemuDomainPCIAddressSetFree(virDomainPCIAddressSetPtr addrs); int qemuAssignDevicePCISlots(virDomainDefPtr def, virQEMUCapsPtr qemuCaps, - qemuDomainPCIAddressSetPtr addrs); + virDomainPCIAddressSetPtr addrs); int qemuDomainCCWAddressAssign(virDomainDeviceInfoPtr dev, qemuDomainCCWAddressSetPtr addrs, bool autoassign); diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index ab27f15..29b9e7c 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -26,6 +26,7 @@ # include "virthread.h" # include "vircgroup.h" +# include "domain_addr.h" # include "domain_conf.h" # include "snapshot_conf.h" # include "qemu_monitor.h" @@ -116,9 +117,6 @@ struct qemuDomainJobObj { bool asyncAbort; /* abort of async job requested */ }; -typedef struct _qemuDomainPCIAddressSet qemuDomainPCIAddressSet; -typedef qemuDomainPCIAddressSet *qemuDomainPCIAddressSetPtr; - typedef void (*qemuDomainCleanupCallback)(virQEMUDriverPtr driver, virDomainObjPtr vm); typedef struct _qemuDomainCCWAddressSet qemuDomainCCWAddressSet; @@ -146,7 +144,7 @@ struct _qemuDomainObjPrivate { int nvcpupids; int *vcpupids; - qemuDomainPCIAddressSetPtr pciaddrs; + virDomainPCIAddressSetPtr pciaddrs; qemuDomainCCWAddressSetPtr ccwaddrs; int persistentAddrs; -- 1.9.0

On 05/11/2014 08:48 AM, Roman Bogorodskiy wrote:
Introduce new files (domain_addr.[ch]) to provide an API for domain device handling that could be shared across the drivers.
A list of data types were extracted and moved there:
qemuDomainPCIAddressBus -> virDomainPCIAddressBus qemuDomainPCIAddressBusPtr -> virDomainPCIAddressBusPtr _qemuDomainPCIAddressSet -> virDomainPCIAddressSet qemuDomainPCIAddressSetPtr -> virDomainPCIAddressSetPtr qemuDomainPCIConnectFlags -> virDomainPCIConnectFlags
Also, move the related definitions and macros. --- src/Makefile.am | 1 + src/conf/domain_addr.c | 24 +++++++ src/conf/domain_addr.h | 79 ++++++++++++++++++++++ src/qemu/qemu_command.c | 171 +++++++++++++++++++++--------------------------- src/qemu/qemu_command.h | 49 ++++---------- src/qemu/qemu_domain.h | 6 +- 6 files changed, 193 insertions(+), 137 deletions(-) create mode 100644 src/conf/domain_addr.c create mode 100644 src/conf/domain_addr.h
diff --git a/src/Makefile.am b/src/Makefile.am index e9dc9e0..cfb7097 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -247,6 +247,7 @@ NETDEV_CONF_SOURCES = \ # Domain driver generic impl APIs DOMAIN_CONF_SOURCES = \ conf/capabilities.c conf/capabilities.h \ + conf/domain_addr.c conf/domain_addr.h \ conf/domain_conf.c conf/domain_conf.h \ conf/domain_audit.c conf/domain_audit.h \ conf/domain_nwfilter.c conf/domain_nwfilter.h \ diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c new file mode 100644 index 0000000..73e17aa --- /dev/null +++ b/src/conf/domain_addr.c @@ -0,0 +1,24 @@ +/* + * domain_addr.c: helper APIs for managing domain device addresses + * + * Copyright (C) 2006-2014 Red Hat, Inc. + * Copyright (C) 2006 Daniel P. Berrange + * + * 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: Daniel P. Berrange <berrange@redhat.com>
This file addition would look better in the second patch.
+ */ + +#include <config.h> diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h new file mode 100644 index 0000000..f5a5199 --- /dev/null +++ b/src/conf/domain_addr.h @@ -0,0 +1,79 @@ +/* + * domain_addr.h: helper APIs for managing domain device addresses + * + * Copyright (C) 2006-2014 Red Hat, Inc. + * Copyright (C) 2006 Daniel P. Berrange + * + * 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: Daniel P. Berrange <berrange@redhat.com> + */ +
I think you should be the author, since you created the file. Most of the structs and functions were (re)written by Laine anyway and we have the copyrights and git history for attributions. But I'd like to hear a second opinion on this. The rest is just code movement and renaming, ACK. Jan

On 05/13/2014 03:21 AM, Ján Tomko wrote:
On 05/11/2014 08:48 AM, Roman Bogorodskiy wrote:
Introduce new files (domain_addr.[ch]) to provide an API for domain device handling that could be shared across the drivers.
+ * Author: Daniel P. Berrange <berrange@redhat.com> + */ +
I think you should be the author, since you created the file. Most of the structs and functions were (re)written by Laine anyway and we have the copyrights and git history for attributions. But I'd like to hear a second opinion on this.
Per-file authorship is an awkward concept in the age of multiple contributors. When copying and pasting from another file, I tend to preserve the original author if I used most of the code intact, or drop the line entirely if I'm going to do a rewrite. The git log is much more authoratative than an author comment in the file, so I don't care enough to require you to change it - if you are comfortable claiming the work, put your own name.
The rest is just code movement and renaming, ACK.
-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Eric Blake wrote:
On 05/13/2014 03:21 AM, Ján Tomko wrote:
On 05/11/2014 08:48 AM, Roman Bogorodskiy wrote:
Introduce new files (domain_addr.[ch]) to provide an API for domain device handling that could be shared across the drivers.
+ * Author: Daniel P. Berrange <berrange@redhat.com> + */ +
I think you should be the author, since you created the file. Most of the structs and functions were (re)written by Laine anyway and we have the copyrights and git history for attributions. But I'd like to hear a second opinion on this.
Per-file authorship is an awkward concept in the age of multiple contributors. When copying and pasting from another file, I tend to preserve the original author if I used most of the code intact, or drop the line entirely if I'm going to do a rewrite. The git log is much more authoratative than an author comment in the file, so I don't care enough to require you to change it - if you are comfortable claiming the work, put your own name.
Yes, as the change is mostly moving and renaming, I decided to keep the original authorship as in the files I moved the code from. I think I'll keep it as is then. Roman Bogorodskiy

Move sharable PCI handling functions to domain_addr.[ch], and change theirs prefix from 'qemu' to 'vir': - virDomainPCIAddressAsString; - virDomainPCIAddressBusSetModel; - virDomainPCIAddressEnsureAddr; - virDomainPCIAddressFlagsCompatible; - virDomainPCIAddressGetNextSlot; - virDomainPCIAddressReleaseSlot; - virDomainPCIAddressReserveAddr; - virDomainPCIAddressReserveNextSlot; - virDomainPCIAddressReserveSlot; - virDomainPCIAddressSetFree; - virDomainPCIAddressSetGrow; - virDomainPCIAddressSlotInUse; - virDomainPCIAddressValidate; The only change here is function names, the implementation itself stays untouched. Extract common allocation code from DomainPCIAddressSetCreate into virDomainPCIAddressSetAlloc. --- po/POTFILES.in | 1 + src/conf/domain_addr.c | 542 ++++++++++++++++++++++++++++++++++++++ src/conf/domain_addr.h | 70 +++++ src/libvirt_private.syms | 17 ++ src/qemu/qemu_command.c | 671 ++++++----------------------------------------- src/qemu/qemu_command.h | 18 +- src/qemu/qemu_domain.c | 3 +- src/qemu/qemu_hotplug.c | 8 +- src/qemu/qemu_process.c | 2 +- 9 files changed, 718 insertions(+), 614 deletions(-) diff --git a/po/POTFILES.in b/po/POTFILES.in index e35eb82..6e8d465 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -14,6 +14,7 @@ src/bhyve/bhyve_process.c src/conf/capabilities.c src/conf/cpu_conf.c src/conf/device_conf.c +src/conf/domain_addr.c src/conf/domain_conf.c src/conf/domain_event.c src/conf/interface_conf.c diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index 73e17aa..34a3fac 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -22,3 +22,545 @@ */ #include <config.h> + +#include "viralloc.h" +#include "virlog.h" +#include "virstring.h" +#include "domain_addr.h" + +#define VIR_FROM_THIS VIR_FROM_DOMAIN + +VIR_LOG_INIT("conf.domain_addr"); + +bool +virDomainPCIAddressFlagsCompatible(virDevicePCIAddressPtr addr, + const char *addrStr, + virDomainPCIConnectFlags busFlags, + virDomainPCIConnectFlags devFlags, + bool reportError, + bool fromConfig) +{ + virErrorNumber errType = (fromConfig + ? VIR_ERR_XML_ERROR : VIR_ERR_INTERNAL_ERROR); + virDomainPCIConnectFlags flagsMatchMask = VIR_PCI_CONNECT_TYPES_MASK; + + if (fromConfig) + flagsMatchMask |= VIR_PCI_CONNECT_TYPE_EITHER_IF_CONFIG; + + /* If this bus doesn't allow the type of connection (PCI + * vs. PCIe) required by the device, or if the device requires + * hot-plug and this bus doesn't have it, return false. + */ + if (!(devFlags & busFlags & flagsMatchMask)) { + if (reportError) { + if (devFlags & VIR_PCI_CONNECT_TYPE_PCI) { + virReportError(errType, + _("PCI bus is not compatible with the device " + "at %s. Device requires a standard PCI slot, " + "which is not provided by bus %.4x:%.2x"), + addrStr, addr->domain, addr->bus); + } else if (devFlags & VIR_PCI_CONNECT_TYPE_PCIE) { + virReportError(errType, + _("PCI bus is not compatible with the device " + "at %s. Device requires a PCI Express slot, " + "which is not provided by bus %.4x:%.2x"), + addrStr, addr->domain, addr->bus); + } else { + /* this should never happen. If it does, there is a + * bug in the code that sets the flag bits for devices. + */ + virReportError(errType, + _("The device information for %s has no PCI " + "connection types listed"), addrStr); + } + } + return false; + } + if ((devFlags & VIR_PCI_CONNECT_HOTPLUGGABLE) && + !(busFlags & VIR_PCI_CONNECT_HOTPLUGGABLE)) { + if (reportError) { + virReportError(errType, + _("PCI bus is not compatible with the device " + "at %s. Device requires hot-plug capability, " + "which is not provided by bus %.4x:%.2x"), + addrStr, addr->domain, addr->bus); + } + return false; + } + return true; +} + + +/* Verify that the address is in bounds for the chosen bus, and + * that the bus is of the correct type for the device (via + * comparing the flags). + */ +bool +virDomainPCIAddressValidate(virDomainPCIAddressSetPtr addrs, + virDevicePCIAddressPtr addr, + const char *addrStr, + virDomainPCIConnectFlags flags, + bool fromConfig) +{ + virDomainPCIAddressBusPtr bus; + virErrorNumber errType = (fromConfig + ? VIR_ERR_XML_ERROR : VIR_ERR_INTERNAL_ERROR); + + if (addrs->nbuses == 0) { + virReportError(errType, "%s", _("No PCI buses available")); + return false; + } + if (addr->domain != 0) { + virReportError(errType, + _("Invalid PCI address %s. " + "Only PCI domain 0 is available"), + addrStr); + return false; + } + if (addr->bus >= addrs->nbuses) { + virReportError(errType, + _("Invalid PCI address %s. " + "Only PCI buses up to %zu are available"), + addrStr, addrs->nbuses - 1); + return false; + } + + bus = &addrs->buses[addr->bus]; + + /* assure that at least one of the requested connection types is + * provided by this bus + */ + if (!virDomainPCIAddressFlagsCompatible(addr, addrStr, bus->flags, + flags, true, fromConfig)) + return false; + + /* some "buses" are really just a single port */ + if (bus->minSlot && addr->slot < bus->minSlot) { + virReportError(errType, + _("Invalid PCI address %s. slot must be >= %zu"), + addrStr, bus->minSlot); + return false; + } + if (addr->slot > bus->maxSlot) { + virReportError(errType, + _("Invalid PCI address %s. slot must be <= %zu"), + addrStr, bus->maxSlot); + return false; + } + if (addr->function > VIR_PCI_ADDRESS_FUNCTION_LAST) { + virReportError(errType, + _("Invalid PCI address %s. function must be <= %u"), + addrStr, VIR_PCI_ADDRESS_FUNCTION_LAST); + return false; + } + return true; +} + + +int +virDomainPCIAddressBusSetModel(virDomainPCIAddressBusPtr bus, + virDomainControllerModelPCI model) +{ + switch (model) { + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE: + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT: + bus->flags = (VIR_PCI_CONNECT_HOTPLUGGABLE | + VIR_PCI_CONNECT_TYPE_PCI); + bus->minSlot = 1; + bus->maxSlot = VIR_PCI_ADDRESS_SLOT_LAST; + break; + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT: + /* slots 1 - 31, no hotplug, PCIe only unless the address was + * specified in user config *and* the particular device being + * attached also allows it + */ + bus->flags = (VIR_PCI_CONNECT_TYPE_PCIE | + VIR_PCI_CONNECT_TYPE_EITHER_IF_CONFIG); + bus->minSlot = 1; + bus->maxSlot = VIR_PCI_ADDRESS_SLOT_LAST; + break; + case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE: + /* slots 1 - 31, standard PCI slots, + * but *not* hot-pluggable */ + bus->flags = VIR_PCI_CONNECT_TYPE_PCI; + bus->minSlot = 1; + bus->maxSlot = VIR_PCI_ADDRESS_SLOT_LAST; + break; + default: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid PCI controller model %d"), model); + return -1; + } + + bus->model = model; + return 0; +} + + +/* Ensure addr fits in the address set, by expanding it if needed. + * This will only grow if the flags say that we need a normal + * hot-pluggable PCI slot. If we need a different type of slot, it + * will fail. + * + * Return value: + * -1 = OOM + * 0 = no action performed + * >0 = number of buses added + */ +int +virDomainPCIAddressSetGrow(virDomainPCIAddressSetPtr addrs, + virDevicePCIAddressPtr addr, + virDomainPCIConnectFlags flags) +{ + int add; + size_t i; + + add = addr->bus - addrs->nbuses + 1; + i = addrs->nbuses; + if (add <= 0) + return 0; + + /* auto-grow only works when we're adding plain PCI devices */ + if (!(flags & VIR_PCI_CONNECT_TYPE_PCI)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot automatically add a new PCI bus for a " + "device requiring a slot other than standard PCI.")); + return -1; + } + + if (VIR_EXPAND_N(addrs->buses, addrs->nbuses, add) < 0) + return -1; + + for (; i < addrs->nbuses; i++) { + /* Any time we auto-add a bus, we will want a multi-slot + * bus. Currently the only type of bus we will auto-add is a + * pci-bridge, which is hot-pluggable and provides standard + * PCI slots. + */ + virDomainPCIAddressBusSetModel(&addrs->buses[i], + VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE); + } + return add; +} + + +char * +virDomainPCIAddressAsString(virDevicePCIAddressPtr addr) +{ + char *str; + + ignore_value(virAsprintf(&str, "%.4x:%.2x:%.2x.%.1x", + addr->domain, + addr->bus, + addr->slot, + addr->function)); + return str; +} + + +/* + * Check if the PCI slot is used by another device. + */ +bool +virDomainPCIAddressSlotInUse(virDomainPCIAddressSetPtr addrs, + virDevicePCIAddressPtr addr) +{ + return !!addrs->buses[addr->bus].slots[addr->slot]; +} + + +/* + * Reserve a slot (or just one function) for a device. If + * reserveEntireSlot is true, all functions for the slot are reserved, + * otherwise only one. If fromConfig is true, the address being + * requested came directly from the config and errors should be worded + * appropriately. If fromConfig is false, the address was + * automatically created by libvirt, so it is an internal error (not + * XML). + */ +int +virDomainPCIAddressReserveAddr(virDomainPCIAddressSetPtr addrs, + virDevicePCIAddressPtr addr, + virDomainPCIConnectFlags flags, + bool reserveEntireSlot, + bool fromConfig) +{ + int ret = -1; + char *addrStr = NULL; + virDomainPCIAddressBusPtr bus; + virErrorNumber errType = (fromConfig + ? VIR_ERR_XML_ERROR : VIR_ERR_INTERNAL_ERROR); + + if (!(addrStr = virDomainPCIAddressAsString(addr))) + goto cleanup; + + /* Add an extra bus if necessary */ + if (addrs->dryRun && virDomainPCIAddressSetGrow(addrs, addr, flags) < 0) + goto cleanup; + /* Check that the requested bus exists, is the correct type, and we + * are asking for a valid slot + */ + if (!virDomainPCIAddressValidate(addrs, addr, addrStr, flags, fromConfig)) + goto cleanup; + + bus = &addrs->buses[addr->bus]; + + if (reserveEntireSlot) { + if (bus->slots[addr->slot]) { + virReportError(errType, + _("Attempted double use of PCI slot %s " + "(may need \"multifunction='on'\" for " + "device on function 0)"), addrStr); + goto cleanup; + } + bus->slots[addr->slot] = 0xFF; /* reserve all functions of slot */ + VIR_DEBUG("Reserving PCI slot %s (multifunction='off')", addrStr); + } else { + if (bus->slots[addr->slot] & (1 << addr->function)) { + if (addr->function == 0) { + virReportError(errType, + _("Attempted double use of PCI Address %s"), + addrStr); + } else { + virReportError(errType, + _("Attempted double use of PCI Address %s " + "(may need \"multifunction='on'\" " + "for device on function 0)"), addrStr); + } + goto cleanup; + } + bus->slots[addr->slot] |= (1 << addr->function); + VIR_DEBUG("Reserving PCI address %s", addrStr); + } + + ret = 0; + cleanup: + VIR_FREE(addrStr); + return ret; +} + + +int +virDomainPCIAddressReserveSlot(virDomainPCIAddressSetPtr addrs, + virDevicePCIAddressPtr addr, + virDomainPCIConnectFlags flags) +{ + return virDomainPCIAddressReserveAddr(addrs, addr, flags, true, false); +} + +int +virDomainPCIAddressEnsureAddr(virDomainPCIAddressSetPtr addrs, + virDomainDeviceInfoPtr dev) +{ + int ret = -1; + char *addrStr = NULL; + /* Flags should be set according to the particular device, + * but only the caller knows the type of device. Currently this + * function is only used for hot-plug, though, and hot-plug is + * only supported for standard PCI devices, so we can safely use + * the setting below */ + virDomainPCIConnectFlags flags = (VIR_PCI_CONNECT_HOTPLUGGABLE | + VIR_PCI_CONNECT_TYPE_PCI); + + if (!(addrStr = virDomainPCIAddressAsString(&dev->addr.pci))) + goto cleanup; + + if (dev->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { + /* We do not support hotplug multi-function PCI device now, so we should + * reserve the whole slot. The function of the PCI device must be 0. + */ + if (dev->addr.pci.function != 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Only PCI device addresses with function=0" + " are supported")); + goto cleanup; + } + + if (!virDomainPCIAddressValidate(addrs, &dev->addr.pci, + addrStr, flags, true)) + goto cleanup; + + ret = virDomainPCIAddressReserveSlot(addrs, &dev->addr.pci, flags); + } else { + ret = virDomainPCIAddressReserveNextSlot(addrs, dev, flags); + } + + cleanup: + VIR_FREE(addrStr); + return ret; +} + + +int +virDomainPCIAddressReleaseAddr(virDomainPCIAddressSetPtr addrs, + virDevicePCIAddressPtr addr) +{ + addrs->buses[addr->bus].slots[addr->slot] &= ~(1 << addr->function); + return 0; +} + +int +virDomainPCIAddressReleaseSlot(virDomainPCIAddressSetPtr addrs, + virDevicePCIAddressPtr addr) +{ + /* permit any kind of connection type in validation, since we + * already had it, and are giving it back. + */ + virDomainPCIConnectFlags flags = VIR_PCI_CONNECT_TYPES_MASK; + int ret = -1; + char *addrStr = NULL; + + if (!(addrStr = virDomainPCIAddressAsString(addr))) + goto cleanup; + + if (!virDomainPCIAddressValidate(addrs, addr, addrStr, flags, false)) + goto cleanup; + + addrs->buses[addr->bus].slots[addr->slot] = 0; + ret = 0; + cleanup: + VIR_FREE(addrStr); + return ret; +} + + +virDomainPCIAddressSetPtr +virDomainPCIAddressSetAlloc(unsigned int nbuses) +{ + virDomainPCIAddressSetPtr addrs; + + if (VIR_ALLOC(addrs) < 0) + goto error; + + if (VIR_ALLOC_N(addrs->buses, nbuses) < 0) + goto error; + + addrs->nbuses = nbuses; + return addrs; + + error: + virDomainPCIAddressSetFree(addrs); + return NULL; +} + + +void +virDomainPCIAddressSetFree(virDomainPCIAddressSetPtr addrs) +{ + if (!addrs) + return; + + VIR_FREE(addrs->buses); + VIR_FREE(addrs); +} + + +int +virDomainPCIAddressGetNextSlot(virDomainPCIAddressSetPtr addrs, + virDevicePCIAddressPtr next_addr, + virDomainPCIConnectFlags flags) +{ + /* default to starting the search for a free slot from + * 0000:00:00.0 + */ + virDevicePCIAddress a = { 0, 0, 0, 0, false }; + char *addrStr = NULL; + + /* except if this search is for the exact same type of device as + * last time, continue the search from the previous match + */ + if (flags == addrs->lastFlags) + a = addrs->lastaddr; + + if (addrs->nbuses == 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", _("No PCI buses available")); + goto error; + } + + /* Start the search at the last used bus and slot */ + for (a.slot++; a.bus < addrs->nbuses; a.bus++) { + if (!(addrStr = virDomainPCIAddressAsString(&a))) + goto error; + if (!virDomainPCIAddressFlagsCompatible(&a, addrStr, + addrs->buses[a.bus].flags, + flags, false, false)) { + VIR_FREE(addrStr); + VIR_DEBUG("PCI bus %.4x:%.2x is not compatible with the device", + a.domain, a.bus); + continue; + } + for (; a.slot <= VIR_PCI_ADDRESS_SLOT_LAST; a.slot++) { + if (!virDomainPCIAddressSlotInUse(addrs, &a)) + goto success; + + VIR_DEBUG("PCI slot %.4x:%.2x:%.2x already in use", + a.domain, a.bus, a.slot); + } + a.slot = 1; + VIR_FREE(addrStr); + } + + /* There were no free slots after the last used one */ + if (addrs->dryRun) { + /* a is already set to the first new bus and slot 1 */ + if (virDomainPCIAddressSetGrow(addrs, &a, flags) < 0) + goto error; + goto success; + } else if (flags == addrs->lastFlags) { + /* Check the buses from 0 up to the last used one */ + for (a.bus = 0; a.bus <= addrs->lastaddr.bus; a.bus++) { + addrStr = NULL; + if (!(addrStr = virDomainPCIAddressAsString(&a))) + goto error; + if (!virDomainPCIAddressFlagsCompatible(&a, addrStr, + addrs->buses[a.bus].flags, + flags, false, false)) { + VIR_DEBUG("PCI bus %.4x:%.2x is not compatible with the device", + a.domain, a.bus); + continue; + } + for (a.slot = 1; a.slot <= VIR_PCI_ADDRESS_SLOT_LAST; a.slot++) { + if (!virDomainPCIAddressSlotInUse(addrs, &a)) + goto success; + + VIR_DEBUG("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")); + error: + VIR_FREE(addrStr); + return -1; + + success: + VIR_DEBUG("Found free PCI slot %.4x:%.2x:%.2x", + a.domain, a.bus, a.slot); + *next_addr = a; + VIR_FREE(addrStr); + return 0; +} + +int +virDomainPCIAddressReserveNextSlot(virDomainPCIAddressSetPtr addrs, + virDomainDeviceInfoPtr dev, + virDomainPCIConnectFlags flags) +{ + virDevicePCIAddress addr; + if (virDomainPCIAddressGetNextSlot(addrs, &addr, flags) < 0) + return -1; + + if (virDomainPCIAddressReserveSlot(addrs, &addr, flags) < 0) + return -1; + + if (!addrs->dryRun) { + dev->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; + dev->addr.pci = addr; + } + + addrs->lastaddr = addr; + addrs->lastFlags = flags; + return 0; +} diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h index f5a5199..c59ef85 100644 --- a/src/conf/domain_addr.h +++ b/src/conf/domain_addr.h @@ -76,4 +76,74 @@ typedef virDomainPCIAddressSet *virDomainPCIAddressSetPtr; # define VIR_PCI_CONNECT_TYPES_MASK \ (VIR_PCI_CONNECT_TYPE_PCI | VIR_PCI_CONNECT_TYPE_PCIE) +char *virDomainPCIAddressAsString(virDevicePCIAddressPtr addr) + ATTRIBUTE_NONNULL(1); + +virDomainPCIAddressSetPtr virDomainPCIAddressSetAlloc(unsigned int nbuses); + +void virDomainPCIAddressSetFree(virDomainPCIAddressSetPtr addrs); + +bool virDomainPCIAddressFlagsCompatible(virDevicePCIAddressPtr addr, + const char *addrStr, + virDomainPCIConnectFlags busFlags, + virDomainPCIConnectFlags devFlags, + bool reportError, + bool fromConfig) + ATTRIBUTE_NONNULL(1); + +bool virDomainPCIAddressValidate(virDomainPCIAddressSetPtr addrs, + virDevicePCIAddressPtr addr, + const char *addrStr, + virDomainPCIConnectFlags flags, + bool fromConfig) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + + +int virDomainPCIAddressBusSetModel(virDomainPCIAddressBusPtr bus, + virDomainControllerModelPCI model) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + +bool virDomainPCIAddressSlotInUse(virDomainPCIAddressSetPtr addrs, + virDevicePCIAddressPtr addr) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + +int virDomainPCIAddressSetGrow(virDomainPCIAddressSetPtr addrs, + virDevicePCIAddressPtr addr, + virDomainPCIConnectFlags flags) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + +int virDomainPCIAddressReserveAddr(virDomainPCIAddressSetPtr addrs, + virDevicePCIAddressPtr addr, + virDomainPCIConnectFlags flags, + bool reserveEntireSlot, + bool fromConfig) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + +int virDomainPCIAddressReserveSlot(virDomainPCIAddressSetPtr addrs, + virDevicePCIAddressPtr addr, + virDomainPCIConnectFlags flags) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + +int virDomainPCIAddressEnsureAddr(virDomainPCIAddressSetPtr addrs, + virDomainDeviceInfoPtr dev) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + +int virDomainPCIAddressReleaseAddr(virDomainPCIAddressSetPtr addrs, + virDevicePCIAddressPtr addr) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + +int virDomainPCIAddressReleaseSlot(virDomainPCIAddressSetPtr addrs, + virDevicePCIAddressPtr addr) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + +int virDomainPCIAddressGetNextSlot(virDomainPCIAddressSetPtr addrs, + virDevicePCIAddressPtr next_addr, + virDomainPCIConnectFlags flags) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + +int virDomainPCIAddressReserveNextSlot(virDomainPCIAddressSetPtr addrs, + virDomainDeviceInfoPtr dev, + virDomainPCIConnectFlags flags) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + #endif /* __DOMAIN_ADDR_H__ */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index b4dd338..0a5fb14 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -85,6 +85,23 @@ virDevicePCIAddressIsValid; virDevicePCIAddressParseXML; +# conf/domain_addr.h +virDomainPCIAddressAsString; +virDomainPCIAddressBusSetModel; +virDomainPCIAddressEnsureAddr; +virDomainPCIAddressFlagsCompatible; +virDomainPCIAddressGetNextSlot; +virDomainPCIAddressReleaseSlot; +virDomainPCIAddressReserveAddr; +virDomainPCIAddressReserveNextSlot; +virDomainPCIAddressReserveSlot; +virDomainPCIAddressSetAlloc; +virDomainPCIAddressSetFree; +virDomainPCIAddressSetGrow; +virDomainPCIAddressSlotInUse; +virDomainPCIAddressValidate; + + # conf/domain_audit.h virDomainAuditCgroup; virDomainAuditCgroupMajor; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index ebf17a8..aeaadbd 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1240,7 +1240,7 @@ void qemuDomainCCWAddressSetFree(qemuDomainCCWAddressSetPtr addrs) static qemuDomainCCWAddressSetPtr qemuDomainCCWAddressSetCreate(void) { - qemuDomainCCWAddressSetPtr addrs = NULL; + qemuDomainCCWAddressSetPtr addrs = NULL; if (VIR_ALLOC(addrs) < 0) goto error; @@ -1445,232 +1445,6 @@ int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def, } -static bool -qemuDomainPCIAddressFlagsCompatible(virDevicePCIAddressPtr addr, - const char *addrStr, - virDomainPCIConnectFlags busFlags, - virDomainPCIConnectFlags devFlags, - bool reportError, - bool fromConfig) -{ - virErrorNumber errType = (fromConfig - ? VIR_ERR_XML_ERROR : VIR_ERR_INTERNAL_ERROR); - virDomainPCIConnectFlags flagsMatchMask = VIR_PCI_CONNECT_TYPES_MASK; - - if (fromConfig) - flagsMatchMask |= VIR_PCI_CONNECT_TYPE_EITHER_IF_CONFIG; - - /* If this bus doesn't allow the type of connection (PCI - * vs. PCIe) required by the device, or if the device requires - * hot-plug and this bus doesn't have it, return false. - */ - if (!(devFlags & busFlags & flagsMatchMask)) { - if (reportError) { - if (devFlags & VIR_PCI_CONNECT_TYPE_PCI) { - virReportError(errType, - _("PCI bus is not compatible with the device " - "at %s. Device requires a standard PCI slot, " - "which is not provided by bus %.4x:%.2x"), - addrStr, addr->domain, addr->bus); - } else if (devFlags & VIR_PCI_CONNECT_TYPE_PCIE) { - virReportError(errType, - _("PCI bus is not compatible with the device " - "at %s. Device requires a PCI Express slot, " - "which is not provided by bus %.4x:%.2x"), - addrStr, addr->domain, addr->bus); - } else { - /* this should never happen. If it does, there is a - * bug in the code that sets the flag bits for devices. - */ - virReportError(errType, - _("The device information for %s has no PCI " - "connection types listed"), addrStr); - } - } - return false; - } - if ((devFlags & VIR_PCI_CONNECT_HOTPLUGGABLE) && - !(busFlags & VIR_PCI_CONNECT_HOTPLUGGABLE)) { - if (reportError) { - virReportError(errType, - _("PCI bus is not compatible with the device " - "at %s. Device requires hot-plug capability, " - "which is not provided by bus %.4x:%.2x"), - addrStr, addr->domain, addr->bus); - } - return false; - } - return true; -} - - -/* Verify that the address is in bounds for the chosen bus, and - * that the bus is of the correct type for the device (via - * comparing the flags). - */ -static bool -qemuDomainPCIAddressValidate(virDomainPCIAddressSetPtr addrs, - virDevicePCIAddressPtr addr, - const char *addrStr, - virDomainPCIConnectFlags flags, - bool fromConfig) -{ - virDomainPCIAddressBusPtr bus; - virErrorNumber errType = (fromConfig - ? VIR_ERR_XML_ERROR : VIR_ERR_INTERNAL_ERROR); - - if (addrs->nbuses == 0) { - virReportError(errType, "%s", _("No PCI buses available")); - return false; - } - if (addr->domain != 0) { - virReportError(errType, - _("Invalid PCI address %s. " - "Only PCI domain 0 is available"), - addrStr); - return false; - } - if (addr->bus >= addrs->nbuses) { - virReportError(errType, - _("Invalid PCI address %s. " - "Only PCI buses up to %zu are available"), - addrStr, addrs->nbuses - 1); - return false; - } - - bus = &addrs->buses[addr->bus]; - - /* assure that at least one of the requested connection types is - * provided by this bus - */ - if (!qemuDomainPCIAddressFlagsCompatible(addr, addrStr, bus->flags, - flags, true, fromConfig)) - return false; - - /* some "buses" are really just a single port */ - if (bus->minSlot && addr->slot < bus->minSlot) { - virReportError(errType, - _("Invalid PCI address %s. slot must be >= %zu"), - addrStr, bus->minSlot); - return false; - } - if (addr->slot > bus->maxSlot) { - virReportError(errType, - _("Invalid PCI address %s. slot must be <= %zu"), - addrStr, bus->maxSlot); - return false; - } - if (addr->function > VIR_PCI_ADDRESS_FUNCTION_LAST) { - virReportError(errType, - _("Invalid PCI address %s. function must be <= %u"), - addrStr, VIR_PCI_ADDRESS_FUNCTION_LAST); - return false; - } - return true; -} - - -static int -qemuDomainPCIAddressBusSetModel(virDomainPCIAddressBusPtr bus, - virDomainControllerModelPCI model) -{ - switch (model) { - case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE: - case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT: - bus->flags = (VIR_PCI_CONNECT_HOTPLUGGABLE | - VIR_PCI_CONNECT_TYPE_PCI); - bus->minSlot = 1; - bus->maxSlot = VIR_PCI_ADDRESS_SLOT_LAST; - break; - case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT: - /* slots 1 - 31, no hotplug, PCIe only unless the address was - * specified in user config *and* the particular device being - * attached also allows it - */ - bus->flags = (VIR_PCI_CONNECT_TYPE_PCIE | - VIR_PCI_CONNECT_TYPE_EITHER_IF_CONFIG); - bus->minSlot = 1; - bus->maxSlot = VIR_PCI_ADDRESS_SLOT_LAST; - break; - case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE: - /* slots 1 - 31, standard PCI slots, - * but *not* hot-pluggable */ - bus->flags = VIR_PCI_CONNECT_TYPE_PCI; - bus->minSlot = 1; - bus->maxSlot = VIR_PCI_ADDRESS_SLOT_LAST; - break; - default: - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Invalid PCI controller model %d"), model); - return -1; - } - - bus->model = model; - return 0; -} - - -/* Ensure addr fits in the address set, by expanding it if needed. - * This will only grow if the flags say that we need a normal - * hot-pluggable PCI slot. If we need a different type of slot, it - * will fail. - * - * Return value: - * -1 = OOM - * 0 = no action performed - * >0 = number of buses added - */ -static int -qemuDomainPCIAddressSetGrow(virDomainPCIAddressSetPtr addrs, - virDevicePCIAddressPtr addr, - virDomainPCIConnectFlags flags) -{ - int add; - size_t i; - - add = addr->bus - addrs->nbuses + 1; - i = addrs->nbuses; - if (add <= 0) - return 0; - - /* auto-grow only works when we're adding plain PCI devices */ - if (!(flags & VIR_PCI_CONNECT_TYPE_PCI)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot automatically add a new PCI bus for a " - "device requiring a slot other than standard PCI.")); - return -1; - } - - if (VIR_EXPAND_N(addrs->buses, addrs->nbuses, add) < 0) - return -1; - - for (; i < addrs->nbuses; i++) { - /* Any time we auto-add a bus, we will want a multi-slot - * bus. Currently the only type of bus we will auto-add is a - * pci-bridge, which is hot-pluggable and provides standard - * PCI slots. - */ - qemuDomainPCIAddressBusSetModel(&addrs->buses[i], - VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE); - } - return add; -} - - -static char * -qemuDomainPCIAddressAsString(virDevicePCIAddressPtr addr) -{ - char *str; - - ignore_value(virAsprintf(&str, "%.4x:%.2x:%.2x.%.1x", - addr->domain, - addr->bus, - addr->slot, - addr->function)); - return str; -} - - static int qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, virDomainDeviceDefPtr device, @@ -1816,8 +1590,8 @@ qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, entireSlot = (addr->function == 0 && addr->multi != VIR_DEVICE_ADDRESS_PCI_MULTI_ON); - if (qemuDomainPCIAddressReserveAddr(addrs, addr, flags, - entireSlot, true) < 0) + if (virDomainPCIAddressReserveAddr(addrs, addr, flags, + entireSlot, true) < 0) goto cleanup; ret = 0; @@ -1872,7 +1646,7 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, if (qemuAssignDevicePCISlots(def, qemuCaps, addrs) < 0) goto cleanup; /* Reserve 1 extra slot for a (potential) bridge */ - if (qemuDomainPCIAddressReserveNextSlot(addrs, &info, flags) < 0) + if (virDomainPCIAddressReserveNextSlot(addrs, &info, flags) < 0) goto cleanup; for (i = 1; i < addrs->nbuses; i++) { @@ -1883,12 +1657,12 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, i, bus->model)) < 0) goto cleanup; /* If we added a new bridge, we will need one more address */ - if (rv > 0 && qemuDomainPCIAddressReserveNextSlot(addrs, &info, - flags) < 0) + if (rv > 0 && virDomainPCIAddressReserveNextSlot(addrs, &info, + flags) < 0) goto cleanup; } nbuses = addrs->nbuses; - qemuDomainPCIAddressSetFree(addrs); + virDomainPCIAddressSetFree(addrs); addrs = NULL; } else if (max_idx > 0) { @@ -1911,7 +1685,7 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, priv = obj->privateData; if (addrs) { /* if this is the live domain object, we persist the PCI addresses*/ - qemuDomainPCIAddressSetFree(priv->pciaddrs); + virDomainPCIAddressSetFree(priv->pciaddrs); priv->persistentAddrs = 1; priv->pciaddrs = addrs; addrs = NULL; @@ -1923,7 +1697,7 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, ret = 0; cleanup: - qemuDomainPCIAddressSetFree(addrs); + virDomainPCIAddressSetFree(addrs); return ret; } @@ -1958,11 +1732,8 @@ qemuDomainPCIAddressSetCreate(virDomainDefPtr def, virDomainPCIAddressSetPtr addrs; size_t i; - if (VIR_ALLOC(addrs) < 0) - goto error; - - if (VIR_ALLOC_N(addrs->buses, nbuses) < 0) - goto error; + if ((addrs = virDomainPCIAddressSetAlloc(nbuses)) == NULL) + return NULL; addrs->nbuses = nbuses; addrs->dryRun = dryRun; @@ -1975,11 +1746,11 @@ qemuDomainPCIAddressSetCreate(virDomainDefPtr def, * */ if (nbuses > 0) - qemuDomainPCIAddressBusSetModel(&addrs->buses[0], - VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT); + virDomainPCIAddressBusSetModel(&addrs->buses[0], + VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT); for (i = 1; i < nbuses; i++) { - qemuDomainPCIAddressBusSetModel(&addrs->buses[i], - VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE); + virDomainPCIAddressBusSetModel(&addrs->buses[i], + VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE); } for (i = 0; i < def->ncontrollers; i++) { @@ -1995,8 +1766,8 @@ qemuDomainPCIAddressSetCreate(virDomainDefPtr def, goto error; } - if (qemuDomainPCIAddressBusSetModel(&addrs->buses[idx], - def->controllers[i]->model) < 0) + if (virDomainPCIAddressBusSetModel(&addrs->buses[idx], + def->controllers[i]->model) < 0) goto error; } @@ -2006,292 +1777,10 @@ qemuDomainPCIAddressSetCreate(virDomainDefPtr def, return addrs; error: - qemuDomainPCIAddressSetFree(addrs); + virDomainPCIAddressSetFree(addrs); return NULL; } -/* - * Check if the PCI slot is used by another device. - */ -static bool qemuDomainPCIAddressSlotInUse(virDomainPCIAddressSetPtr addrs, - virDevicePCIAddressPtr addr) -{ - return !!addrs->buses[addr->bus].slots[addr->slot]; -} - - -/* - * Reserve a slot (or just one function) for a device. If - * reserveEntireSlot is true, all functions for the slot are reserved, - * otherwise only one. If fromConfig is true, the address being - * requested came directly from the config and errors should be worded - * appropriately. If fromConfig is false, the address was - * automatically created by libvirt, so it is an internal error (not - * XML). - */ -int -qemuDomainPCIAddressReserveAddr(virDomainPCIAddressSetPtr addrs, - virDevicePCIAddressPtr addr, - virDomainPCIConnectFlags flags, - bool reserveEntireSlot, - bool fromConfig) -{ - int ret = -1; - char *addrStr = NULL; - virDomainPCIAddressBusPtr bus; - virErrorNumber errType = (fromConfig - ? VIR_ERR_XML_ERROR : VIR_ERR_INTERNAL_ERROR); - - if (!(addrStr = qemuDomainPCIAddressAsString(addr))) - goto cleanup; - - /* Add an extra bus if necessary */ - if (addrs->dryRun && qemuDomainPCIAddressSetGrow(addrs, addr, flags) < 0) - goto cleanup; - /* Check that the requested bus exists, is the correct type, and we - * are asking for a valid slot - */ - if (!qemuDomainPCIAddressValidate(addrs, addr, addrStr, flags, fromConfig)) - goto cleanup; - - bus = &addrs->buses[addr->bus]; - - if (reserveEntireSlot) { - if (bus->slots[addr->slot]) { - virReportError(errType, - _("Attempted double use of PCI slot %s " - "(may need \"multifunction='on'\" for " - "device on function 0)"), addrStr); - goto cleanup; - } - bus->slots[addr->slot] = 0xFF; /* reserve all functions of slot */ - VIR_DEBUG("Reserving PCI slot %s (multifunction='off')", addrStr); - } else { - if (bus->slots[addr->slot] & (1 << addr->function)) { - if (addr->function == 0) { - virReportError(errType, - _("Attempted double use of PCI Address %s"), - addrStr); - } else { - virReportError(errType, - _("Attempted double use of PCI Address %s " - "(may need \"multifunction='on'\" " - "for device on function 0)"), addrStr); - } - goto cleanup; - } - bus->slots[addr->slot] |= (1 << addr->function); - VIR_DEBUG("Reserving PCI address %s", addrStr); - } - - ret = 0; - cleanup: - VIR_FREE(addrStr); - return ret; -} - - -int -qemuDomainPCIAddressReserveSlot(virDomainPCIAddressSetPtr addrs, - virDevicePCIAddressPtr addr, - virDomainPCIConnectFlags flags) -{ - return qemuDomainPCIAddressReserveAddr(addrs, addr, flags, true, false); -} - -int qemuDomainPCIAddressEnsureAddr(virDomainPCIAddressSetPtr addrs, - virDomainDeviceInfoPtr dev) -{ - int ret = -1; - char *addrStr = NULL; - /* Flags should be set according to the particular device, - * but only the caller knows the type of device. Currently this - * function is only used for hot-plug, though, and hot-plug is - * only supported for standard PCI devices, so we can safely use - * the setting below */ - virDomainPCIConnectFlags flags = (VIR_PCI_CONNECT_HOTPLUGGABLE | - VIR_PCI_CONNECT_TYPE_PCI); - - if (!(addrStr = qemuDomainPCIAddressAsString(&dev->addr.pci))) - goto cleanup; - - if (dev->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { - /* We do not support hotplug multi-function PCI device now, so we should - * reserve the whole slot. The function of the PCI device must be 0. - */ - if (dev->addr.pci.function != 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Only PCI device addresses with function=0" - " are supported")); - goto cleanup; - } - - if (!qemuDomainPCIAddressValidate(addrs, &dev->addr.pci, - addrStr, flags, true)) - goto cleanup; - - ret = qemuDomainPCIAddressReserveSlot(addrs, &dev->addr.pci, flags); - } else { - ret = qemuDomainPCIAddressReserveNextSlot(addrs, dev, flags); - } - - cleanup: - VIR_FREE(addrStr); - return ret; -} - - -int qemuDomainPCIAddressReleaseAddr(virDomainPCIAddressSetPtr addrs, - virDevicePCIAddressPtr addr) -{ - addrs->buses[addr->bus].slots[addr->slot] &= ~(1 << addr->function); - return 0; -} - -static int -qemuDomainPCIAddressReleaseSlot(virDomainPCIAddressSetPtr addrs, - virDevicePCIAddressPtr addr) -{ - /* permit any kind of connection type in validation, since we - * already had it, and are giving it back. - */ - virDomainPCIConnectFlags flags = VIR_PCI_CONNECT_TYPES_MASK; - int ret = -1; - char *addrStr = NULL; - - if (!(addrStr = qemuDomainPCIAddressAsString(addr))) - goto cleanup; - - if (!qemuDomainPCIAddressValidate(addrs, addr, addrStr, flags, false)) - goto cleanup; - - addrs->buses[addr->bus].slots[addr->slot] = 0; - ret = 0; - cleanup: - VIR_FREE(addrStr); - return ret; -} - -void qemuDomainPCIAddressSetFree(virDomainPCIAddressSetPtr addrs) -{ - if (!addrs) - return; - - VIR_FREE(addrs->buses); - VIR_FREE(addrs); -} - - -static int -qemuDomainPCIAddressGetNextSlot(virDomainPCIAddressSetPtr addrs, - virDevicePCIAddressPtr next_addr, - virDomainPCIConnectFlags flags) -{ - /* default to starting the search for a free slot from - * 0000:00:00.0 - */ - virDevicePCIAddress a = { 0, 0, 0, 0, false }; - char *addrStr = NULL; - - /* except if this search is for the exact same type of device as - * last time, continue the search from the previous match - */ - if (flags == addrs->lastFlags) - a = addrs->lastaddr; - - if (addrs->nbuses == 0) { - virReportError(VIR_ERR_XML_ERROR, "%s", _("No PCI buses available")); - goto error; - } - - /* Start the search at the last used bus and slot */ - for (a.slot++; a.bus < addrs->nbuses; a.bus++) { - if (!(addrStr = qemuDomainPCIAddressAsString(&a))) - goto error; - if (!qemuDomainPCIAddressFlagsCompatible(&a, addrStr, - addrs->buses[a.bus].flags, - flags, false, false)) { - VIR_FREE(addrStr); - VIR_DEBUG("PCI bus %.4x:%.2x is not compatible with the device", - a.domain, a.bus); - continue; - } - for (; a.slot <= VIR_PCI_ADDRESS_SLOT_LAST; a.slot++) { - if (!qemuDomainPCIAddressSlotInUse(addrs, &a)) - goto success; - - VIR_DEBUG("PCI slot %.4x:%.2x:%.2x already in use", - a.domain, a.bus, a.slot); - } - a.slot = 1; - VIR_FREE(addrStr); - } - - /* There were no free slots after the last used one */ - if (addrs->dryRun) { - /* a is already set to the first new bus and slot 1 */ - if (qemuDomainPCIAddressSetGrow(addrs, &a, flags) < 0) - goto error; - goto success; - } else if (flags == addrs->lastFlags) { - /* Check the buses from 0 up to the last used one */ - for (a.bus = 0; a.bus <= addrs->lastaddr.bus; a.bus++) { - addrStr = NULL; - if (!(addrStr = qemuDomainPCIAddressAsString(&a))) - goto error; - if (!qemuDomainPCIAddressFlagsCompatible(&a, addrStr, - addrs->buses[a.bus].flags, - flags, false, false)) { - VIR_DEBUG("PCI bus %.4x:%.2x is not compatible with the device", - a.domain, a.bus); - continue; - } - for (a.slot = 1; a.slot <= VIR_PCI_ADDRESS_SLOT_LAST; a.slot++) { - if (!qemuDomainPCIAddressSlotInUse(addrs, &a)) - goto success; - - VIR_DEBUG("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")); - error: - VIR_FREE(addrStr); - return -1; - - success: - VIR_DEBUG("Found free PCI slot %.4x:%.2x:%.2x", - a.domain, a.bus, a.slot); - *next_addr = a; - VIR_FREE(addrStr); - return 0; -} - -int -qemuDomainPCIAddressReserveNextSlot(virDomainPCIAddressSetPtr addrs, - virDomainDeviceInfoPtr dev, - virDomainPCIConnectFlags flags) -{ - virDevicePCIAddress addr; - if (qemuDomainPCIAddressGetNextSlot(addrs, &addr, flags) < 0) - return -1; - - if (qemuDomainPCIAddressReserveSlot(addrs, &addr, flags) < 0) - return -1; - - if (!addrs->dryRun) { - dev->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; - dev->addr.pci = addr; - } - - addrs->lastaddr = addr; - addrs->lastFlags = flags; - return 0; -} - void qemuDomainReleaseDeviceAddress(virDomainObjPtr vm, @@ -2311,8 +1800,8 @@ qemuDomainReleaseDeviceAddress(virDomainObjPtr vm, NULLSTR(devstr)); else if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI && virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) && - qemuDomainPCIAddressReleaseSlot(priv->pciaddrs, - &info->addr.pci) < 0) + virDomainPCIAddressReleaseSlot(priv->pciaddrs, + &info->addr.pci) < 0) VIR_WARN("Unable to release PCI address on %s", NULLSTR(devstr)); } @@ -2388,7 +1877,7 @@ qemuValidateDevicePCISlotsPIIX3(virDomainDefPtr def, if (addrs->nbuses) { memset(&tmp_addr, 0, sizeof(tmp_addr)); tmp_addr.slot = 1; - if (qemuDomainPCIAddressReserveSlot(addrs, &tmp_addr, flags) < 0) + if (virDomainPCIAddressReserveSlot(addrs, &tmp_addr, flags) < 0) goto cleanup; } @@ -2404,17 +1893,17 @@ qemuValidateDevicePCISlotsPIIX3(virDomainDefPtr def, memset(&tmp_addr, 0, sizeof(tmp_addr)); tmp_addr.slot = 2; - if (!(addrStr = qemuDomainPCIAddressAsString(&tmp_addr))) + if (!(addrStr = virDomainPCIAddressAsString(&tmp_addr))) goto cleanup; - if (!qemuDomainPCIAddressValidate(addrs, &tmp_addr, - addrStr, flags, false)) + if (!virDomainPCIAddressValidate(addrs, &tmp_addr, + addrStr, flags, false)) goto cleanup; - if (qemuDomainPCIAddressSlotInUse(addrs, &tmp_addr)) { + if (virDomainPCIAddressSlotInUse(addrs, &tmp_addr)) { if (qemuDeviceVideoUsable) { - if (qemuDomainPCIAddressReserveNextSlot(addrs, - &primaryVideo->info, - flags) < 0) + if (virDomainPCIAddressReserveNextSlot(addrs, + &primaryVideo->info, + flags) < 0) goto cleanup; } else { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -2423,7 +1912,7 @@ qemuValidateDevicePCISlotsPIIX3(virDomainDefPtr def, goto cleanup; } } else { - if (qemuDomainPCIAddressReserveSlot(addrs, &tmp_addr, flags) < 0) + if (virDomainPCIAddressReserveSlot(addrs, &tmp_addr, flags) < 0) goto cleanup; primaryVideo->info.addr.pci = tmp_addr; primaryVideo->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; @@ -2444,12 +1933,12 @@ qemuValidateDevicePCISlotsPIIX3(virDomainDefPtr def, memset(&tmp_addr, 0, sizeof(tmp_addr)); tmp_addr.slot = 2; - if (qemuDomainPCIAddressSlotInUse(addrs, &tmp_addr)) { + if (virDomainPCIAddressSlotInUse(addrs, &tmp_addr)) { VIR_DEBUG("PCI address 0:0:2.0 in use, future addition of a video" " device will not be possible without manual" " intervention"); virResetLastError(); - } else if (qemuDomainPCIAddressReserveSlot(addrs, &tmp_addr, flags) < 0) { + } else if (virDomainPCIAddressReserveSlot(addrs, &tmp_addr, flags) < 0) { goto cleanup; } } @@ -2527,9 +2016,9 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, */ memset(&tmp_addr, 0, sizeof(tmp_addr)); tmp_addr.slot = 0x1E; - if (!qemuDomainPCIAddressSlotInUse(addrs, &tmp_addr)) { - if (qemuDomainPCIAddressReserveAddr(addrs, &tmp_addr, - flags, true, false) < 0) + if (!virDomainPCIAddressSlotInUse(addrs, &tmp_addr)) { + if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, + flags, true, false) < 0) goto cleanup; def->controllers[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; def->controllers[i]->info.addr.pci.domain = 0; @@ -2552,13 +2041,13 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, tmp_addr.slot = 0x1F; tmp_addr.function = 0; tmp_addr.multi = 1; - if (qemuDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags, - false, false) < 0) + if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags, + false, false) < 0) goto cleanup; tmp_addr.function = 3; tmp_addr.multi = 0; - if (qemuDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags, - false, false) < 0) + if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags, + false, false) < 0) goto cleanup; } @@ -2574,17 +2063,17 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, memset(&tmp_addr, 0, sizeof(tmp_addr)); tmp_addr.slot = 1; - if (!(addrStr = qemuDomainPCIAddressAsString(&tmp_addr))) + if (!(addrStr = virDomainPCIAddressAsString(&tmp_addr))) goto cleanup; - if (!qemuDomainPCIAddressValidate(addrs, &tmp_addr, - addrStr, flags, false)) + if (!virDomainPCIAddressValidate(addrs, &tmp_addr, + addrStr, flags, false)) goto cleanup; - if (qemuDomainPCIAddressSlotInUse(addrs, &tmp_addr)) { + if (virDomainPCIAddressSlotInUse(addrs, &tmp_addr)) { if (qemuDeviceVideoUsable) { - if (qemuDomainPCIAddressReserveNextSlot(addrs, - &primaryVideo->info, - flags) < 0) + if (virDomainPCIAddressReserveNextSlot(addrs, + &primaryVideo->info, + flags) < 0) goto cleanup; } else { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -2593,7 +2082,7 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, goto cleanup; } } else { - if (qemuDomainPCIAddressReserveSlot(addrs, &tmp_addr, flags) < 0) + if (virDomainPCIAddressReserveSlot(addrs, &tmp_addr, flags) < 0) goto cleanup; primaryVideo->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; primaryVideo->info.addr.pci = tmp_addr; @@ -2614,12 +2103,12 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, memset(&tmp_addr, 0, sizeof(tmp_addr)); tmp_addr.slot = 1; - if (qemuDomainPCIAddressSlotInUse(addrs, &tmp_addr)) { + if (virDomainPCIAddressSlotInUse(addrs, &tmp_addr)) { VIR_DEBUG("PCI address 0:0:1.0 in use, future addition of a video" " device will not be possible without manual" " intervention"); virResetLastError(); - } else if (qemuDomainPCIAddressReserveSlot(addrs, &tmp_addr, flags) < 0) { + } else if (virDomainPCIAddressReserveSlot(addrs, &tmp_addr, flags) < 0) { goto cleanup; } } @@ -2712,9 +2201,9 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, flags = VIR_PCI_CONNECT_HOTPLUGGABLE | VIR_PCI_CONNECT_TYPE_PCI; break; } - if (qemuDomainPCIAddressReserveNextSlot(addrs, - &def->controllers[i]->info, - flags) < 0) + if (virDomainPCIAddressReserveNextSlot(addrs, + &def->controllers[i]->info, + flags) < 0) goto error; } } @@ -2727,8 +2216,8 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, /* Only support VirtIO-9p-pci so far. If that changes, * we might need to skip devices here */ - if (qemuDomainPCIAddressReserveNextSlot(addrs, &def->fss[i]->info, - flags) < 0) + if (virDomainPCIAddressReserveNextSlot(addrs, &def->fss[i]->info, + flags) < 0) goto error; } @@ -2742,8 +2231,8 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, (def->nets[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)) { continue; } - if (qemuDomainPCIAddressReserveNextSlot(addrs, &def->nets[i]->info, - flags) < 0) + if (virDomainPCIAddressReserveNextSlot(addrs, &def->nets[i]->info, + flags) < 0) goto error; } @@ -2756,8 +2245,8 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, def->sounds[i]->model == VIR_DOMAIN_SOUND_MODEL_PCSPK) continue; - if (qemuDomainPCIAddressReserveNextSlot(addrs, &def->sounds[i]->info, - flags) < 0) + if (virDomainPCIAddressReserveNextSlot(addrs, &def->sounds[i]->info, + flags) < 0) goto error; } @@ -2819,7 +2308,7 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, if (addr.slot == 0) { /* This is the first part of the controller, so need * to find a free slot & then reserve a function */ - if (qemuDomainPCIAddressGetNextSlot(addrs, &tmp_addr, flags) < 0) + if (virDomainPCIAddressGetNextSlot(addrs, &tmp_addr, flags) < 0) goto error; addr.bus = tmp_addr.bus; @@ -2830,16 +2319,16 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, addrs->lastaddr.multi = 0; } /* Finally we can reserve the slot+function */ - if (qemuDomainPCIAddressReserveAddr(addrs, &addr, flags, - false, false) < 0) + if (virDomainPCIAddressReserveAddr(addrs, &addr, flags, + false, false) < 0) goto error; def->controllers[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; def->controllers[i]->info.addr.pci = addr; } else { - if (qemuDomainPCIAddressReserveNextSlot(addrs, - &def->controllers[i]->info, - flags) < 0) + if (virDomainPCIAddressReserveNextSlot(addrs, + &def->controllers[i]->info, + flags) < 0) goto error; } } @@ -2864,8 +2353,8 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, goto error; } - if (qemuDomainPCIAddressReserveNextSlot(addrs, &def->disks[i]->info, - flags) < 0) + if (virDomainPCIAddressReserveNextSlot(addrs, &def->disks[i]->info, + flags) < 0) goto error; } @@ -2877,9 +2366,9 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, def->hostdevs[i]->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) continue; - if (qemuDomainPCIAddressReserveNextSlot(addrs, - def->hostdevs[i]->info, - flags) < 0) + if (virDomainPCIAddressReserveNextSlot(addrs, + def->hostdevs[i]->info, + flags) < 0) goto error; } @@ -2887,9 +2376,9 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, if (def->memballoon && def->memballoon->model == VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO && def->memballoon->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { - if (qemuDomainPCIAddressReserveNextSlot(addrs, - &def->memballoon->info, - flags) < 0) + if (virDomainPCIAddressReserveNextSlot(addrs, + &def->memballoon->info, + flags) < 0) goto error; } @@ -2897,8 +2386,8 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, if (def->rng && def->rng->model == VIR_DOMAIN_RNG_MODEL_VIRTIO && def->rng->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { - if (qemuDomainPCIAddressReserveNextSlot(addrs, - &def->rng->info, flags) < 0) + if (virDomainPCIAddressReserveNextSlot(addrs, + &def->rng->info, flags) < 0) goto error; } @@ -2906,8 +2395,8 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, if (def->watchdog && def->watchdog->model != VIR_DOMAIN_WATCHDOG_MODEL_IB700 && def->watchdog->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { - if (qemuDomainPCIAddressReserveNextSlot(addrs, &def->watchdog->info, - flags) < 0) + if (virDomainPCIAddressReserveNextSlot(addrs, &def->watchdog->info, + flags) < 0) goto error; } @@ -2915,8 +2404,8 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, * assigned address. */ if (def->nvideos > 0 && def->videos[0]->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { - if (qemuDomainPCIAddressReserveNextSlot(addrs, &def->videos[0]->info, - flags) < 0) + if (virDomainPCIAddressReserveNextSlot(addrs, &def->videos[0]->info, + flags) < 0) goto error; } /* Further non-primary video cards which have to be qxl type */ @@ -2928,8 +2417,8 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, } if (def->videos[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) continue; - if (qemuDomainPCIAddressReserveNextSlot(addrs, &def->videos[i]->info, - flags) < 0) + if (virDomainPCIAddressReserveNextSlot(addrs, &def->videos[i]->info, + flags) < 0) goto error; } for (i = 0; i < def->ninputs; i++) { @@ -2976,7 +2465,7 @@ qemuBuildDeviceAddressStr(virBufferPtr buf, const char *contAlias = NULL; size_t i; - if (!(devStr = qemuDomainPCIAddressAsString(&info->addr.pci))) + if (!(devStr = virDomainPCIAddressAsString(&info->addr.pci))) goto cleanup; for (i = 0; i < domainDef->ncontrollers; i++) { virDomainControllerDefPtr cont = domainDef->controllers[i]; diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 29da1ca..afbd6ff 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -242,23 +242,7 @@ int qemuDomainAssignPCIAddresses(virDomainDefPtr def, virDomainPCIAddressSetPtr qemuDomainPCIAddressSetCreate(virDomainDefPtr def, unsigned int nbuses, bool dryRun); -int qemuDomainPCIAddressReserveSlot(virDomainPCIAddressSetPtr addrs, - virDevicePCIAddressPtr addr, - virDomainPCIConnectFlags flags); -int qemuDomainPCIAddressReserveAddr(virDomainPCIAddressSetPtr addrs, - virDevicePCIAddressPtr addr, - virDomainPCIConnectFlags flags, - bool reserveEntireSlot, - bool fromConfig); -int qemuDomainPCIAddressReserveNextSlot(virDomainPCIAddressSetPtr addrs, - virDomainDeviceInfoPtr dev, - virDomainPCIConnectFlags flags); -int qemuDomainPCIAddressEnsureAddr(virDomainPCIAddressSetPtr addrs, - virDomainDeviceInfoPtr dev); -int qemuDomainPCIAddressReleaseAddr(virDomainPCIAddressSetPtr addrs, - virDevicePCIAddressPtr addr); - -void qemuDomainPCIAddressSetFree(virDomainPCIAddressSetPtr addrs); + int qemuAssignDevicePCISlots(virDomainDefPtr def, virQEMUCapsPtr qemuCaps, virDomainPCIAddressSetPtr addrs); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 34cc736..8415f1a 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -34,6 +34,7 @@ #include "cpu/cpu.h" #include "viruuid.h" #include "virfile.h" +#include "domain_addr.h" #include "domain_event.h" #include "virtime.h" #include "virstoragefile.h" @@ -245,7 +246,7 @@ qemuDomainObjPrivateFree(void *data) virObjectUnref(priv->qemuCaps); virCgroupFree(&priv->cgroup); - qemuDomainPCIAddressSetFree(priv->pciaddrs); + virDomainPCIAddressSetFree(priv->pciaddrs); qemuDomainCCWAddressSetFree(priv->ccwaddrs); virDomainChrSourceDefFree(priv->monConfig); qemuDomainObjFreeJob(priv); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 7c8830e..125a2db 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -284,7 +284,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, goto error; } else if (!disk->info.type || disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { - if (qemuDomainPCIAddressEnsureAddr(priv->pciaddrs, &disk->info) < 0) + if (virDomainPCIAddressEnsureAddr(priv->pciaddrs, &disk->info) < 0) goto error; } releaseaddr = true; @@ -386,7 +386,7 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr driver, if (controller->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE || controller->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { - if (qemuDomainPCIAddressEnsureAddr(priv->pciaddrs, &controller->info) < 0) + if (virDomainPCIAddressEnsureAddr(priv->pciaddrs, &controller->info) < 0) goto cleanup; } else if (controller->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { if (qemuDomainCCWAddressAssign(&controller->info, priv->ccwaddrs, @@ -940,7 +940,7 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("virtio-s390 net device cannot be hotplugged.")); else if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) && - qemuDomainPCIAddressEnsureAddr(priv->pciaddrs, &net->info) < 0) + virDomainPCIAddressEnsureAddr(priv->pciaddrs, &net->info) < 0) goto cleanup; releaseaddr = true; @@ -1230,7 +1230,7 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver, if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { if (qemuAssignDeviceHostdevAlias(vm->def, hostdev, -1) < 0) goto error; - if (qemuDomainPCIAddressEnsureAddr(priv->pciaddrs, hostdev->info) < 0) + if (virDomainPCIAddressEnsureAddr(priv->pciaddrs, hostdev->info) < 0) goto error; releaseaddr = true; if (backend != VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO && diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 592e3b7..606478e 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4390,7 +4390,7 @@ void qemuProcessStop(virQEMUDriverPtr driver, virDomainDefClearDeviceAliases(vm->def); if (!priv->persistentAddrs) { virDomainDefClearPCIAddresses(vm->def); - qemuDomainPCIAddressSetFree(priv->pciaddrs); + virDomainPCIAddressSetFree(priv->pciaddrs); priv->pciaddrs = NULL; virDomainDefClearCCWAddresses(vm->def); qemuDomainCCWAddressSetFree(priv->ccwaddrs); -- 1.9.0

On 05/11/2014 08:48 AM, Roman Bogorodskiy wrote:
Move sharable PCI handling functions to domain_addr.[ch], and change theirs prefix from 'qemu' to 'vir':
- virDomainPCIAddressAsString; - virDomainPCIAddressBusSetModel; - virDomainPCIAddressEnsureAddr; - virDomainPCIAddressFlagsCompatible; - virDomainPCIAddressGetNextSlot; - virDomainPCIAddressReleaseSlot; - virDomainPCIAddressReserveAddr; - virDomainPCIAddressReserveNextSlot; - virDomainPCIAddressReserveSlot; - virDomainPCIAddressSetFree; - virDomainPCIAddressSetGrow; - virDomainPCIAddressSlotInUse; - virDomainPCIAddressValidate;
The only change here is function names, the implementation itself stays untouched.
Extract common allocation code from DomainPCIAddressSetCreate into virDomainPCIAddressSetAlloc. --- po/POTFILES.in | 1 + src/conf/domain_addr.c | 542 ++++++++++++++++++++++++++++++++++++++ src/conf/domain_addr.h | 70 +++++ src/libvirt_private.syms | 17 ++ src/qemu/qemu_command.c | 671 ++++++----------------------------------------- src/qemu/qemu_command.h | 18 +- src/qemu/qemu_domain.c | 3 +- src/qemu/qemu_hotplug.c | 8 +- src/qemu/qemu_process.c | 2 +- 9 files changed, 718 insertions(+), 614 deletions(-)
+ +int +virDomainPCIAddressEnsureAddr(virDomainPCIAddressSetPtr addrs, + virDomainDeviceInfoPtr dev) +{ + int ret = -1; + char *addrStr = NULL; + /* Flags should be set according to the particular device, + * but only the caller knows the type of device. Currently this + * function is only used for hot-plug, though, and hot-plug is + * only supported for standard PCI devices, so we can safely use + * the setting below */ + virDomainPCIConnectFlags flags = (VIR_PCI_CONNECT_HOTPLUGGABLE | + VIR_PCI_CONNECT_TYPE_PCI); + + if (!(addrStr = virDomainPCIAddressAsString(&dev->addr.pci))) + goto cleanup; + + if (dev->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { + /* We do not support hotplug multi-function PCI device now, so we should + * reserve the whole slot. The function of the PCI device must be 0. + */ + if (dev->addr.pci.function != 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Only PCI device addresses with function=0" + " are supported")); + goto cleanup; + } + + if (!virDomainPCIAddressValidate(addrs, &dev->addr.pci, + addrStr, flags, true))
Indentation is off.
+ goto cleanup; + + ret = virDomainPCIAddressReserveSlot(addrs, &dev->addr.pci, flags); + } else { + ret = virDomainPCIAddressReserveNextSlot(addrs, dev, flags); + } + + cleanup: + VIR_FREE(addrStr); + return ret; +} +
diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h index f5a5199..c59ef85 100644 --- a/src/conf/domain_addr.h +++ b/src/conf/domain_addr.h @@ -76,4 +76,74 @@ typedef virDomainPCIAddressSet *virDomainPCIAddressSetPtr;
+bool virDomainPCIAddressFlagsCompatible(virDevicePCIAddressPtr addr, + const char *addrStr, + virDomainPCIConnectFlags busFlags, + virDomainPCIConnectFlags devFlags, + bool reportError, + bool fromConfig) + ATTRIBUTE_NONNULL(1); + +bool virDomainPCIAddressValidate(virDomainPCIAddressSetPtr addrs, + virDevicePCIAddressPtr addr, + const char *addrStr, + virDomainPCIConnectFlags flags, + bool fromConfig) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
For both of these, addrStr can be marked as ATTRIBUTE_NONNULL too.
+ + +int virDomainPCIAddressBusSetModel(virDomainPCIAddressBusPtr bus, + virDomainControllerModelPCI model) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
model is an enum, 0 is a valid value.
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index ebf17a8..aeaadbd 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c
@@ -1240,7 +1240,7 @@ void qemuDomainCCWAddressSetFree(qemuDomainCCWAddressSetPtr addrs) static qemuDomainCCWAddressSetPtr qemuDomainCCWAddressSetCreate(void) { - qemuDomainCCWAddressSetPtr addrs = NULL; + qemuDomainCCWAddressSetPtr addrs = NULL;
if (VIR_ALLOC(addrs) < 0) goto error;
Unrelated whitespace change. ACK with the ATTRIBUTE_NONNULL removed for model. Jan

Ján Tomko wrote:
On 05/11/2014 08:48 AM, Roman Bogorodskiy wrote:
Move sharable PCI handling functions to domain_addr.[ch], and change theirs prefix from 'qemu' to 'vir':
- virDomainPCIAddressAsString; - virDomainPCIAddressBusSetModel; - virDomainPCIAddressEnsureAddr; - virDomainPCIAddressFlagsCompatible; - virDomainPCIAddressGetNextSlot; - virDomainPCIAddressReleaseSlot; - virDomainPCIAddressReserveAddr; - virDomainPCIAddressReserveNextSlot; - virDomainPCIAddressReserveSlot; - virDomainPCIAddressSetFree; - virDomainPCIAddressSetGrow; - virDomainPCIAddressSlotInUse; - virDomainPCIAddressValidate;
The only change here is function names, the implementation itself stays untouched.
Extract common allocation code from DomainPCIAddressSetCreate into virDomainPCIAddressSetAlloc.
Unrelated whitespace change.
ACK with the ATTRIBUTE_NONNULL removed for model.
Pushed 1/3 and 2/3 with the fixes applied; thanks. Roman Bogorodskiy

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 | 142 ++++++++--------- 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, 429 insertions(+), 84 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..bc7ec5c 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, bool dryRun) { - 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) { @@ -78,7 +71,7 @@ bhyveBuildNetArgStr(const virDomainDef *def, virCommandPtr cmd, bool dryRun) } } - if (!dryRun) + if (!dryRun) { if (virNetDevTapCreateInBridgePort(brname, &net->ifname, &net->mac, def->uuid, tapfd, 1, virDomainNetGetActualVirtPortProfile(net), @@ -88,36 +81,38 @@ bhyveBuildNetArgStr(const virDomainDef *def, virCommandPtr cmd, bool dryRun) VIR_FREE(brname); return -1; } - } - if (!dryRun) { - 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(realifname); + VIR_FREE(net->ifname); + VIR_FREE(brname); + return -1; + } + } else { + if (VIR_STRDUP(realifname, "tap0") < 0) + return -1; } - } else { - if (VIR_STRDUP(realifname, "tap0") < 0) - 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)); + VIR_FREE(realifname); + } return 0; } @@ -146,7 +141,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); @@ -157,46 +152,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; } @@ -279,9 +271,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 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..c18665f --- /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->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->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->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..0d56fb7 --- /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..d59fe58 --- /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; + 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 67571d2..5e171bb 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" @@ -473,6 +475,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))) @@ -633,6 +638,9 @@ bhyveConnectDomainXMLToNative(virConnectPtr conn, VIR_DOMAIN_XML_INACTIVE))) goto cleanup; + if (bhyveDomainAssignAddresses(def, NULL) < 0) + goto cleanup; + if (!(loadcmd = virBhyveProcessBuildLoadCmd(privconn, def))) goto cleanup; @@ -1080,7 +1088,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/11/2014 08:48 AM, 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.
I think this can be split in a separate patch.
--- po/POTFILES.in | 1 + src/Makefile.am | 4 + src/bhyve/bhyve_command.c | 142 ++++++++--------- 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, 429 insertions(+), 84 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..bc7ec5c 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, bool dryRun) { - 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++) {
Moving this loop to the caller of the function would reduce the extra indentation level.
+ char *realifname = NULL; + int *tapfd = NULL; + char *brname = NULL; + virDomainNetDefPtr net = net = def->nets[i];;
No need to assign it twice and one semicolon would be enough. :)
int actualType = virDomainNetGetActualType(net);
if (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) {
@@ -146,7 +141,7 @@ bhyveBuildConsoleArgStr(const virDomainDef *def, virCommandPtr cmd) return -1; }
- virCommandAddArgList(cmd, "-s", "31,lpc", NULL); + virCommandAddArgList(cmd, "-s", "1,lpc", NULL);
Are both slot numbers arbitrary? And do we care about backwards compatibility?
virCommandAddArg(cmd, "-l"); virCommandAddArgFormat(cmd, "com%d,%s", chr->target.port + 1, chr->source.data.file.path); @@ -157,46 +152,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));
Do SATA disks use a PCI slot too?
}
- virCommandAddArg(cmd, "-s"); - virCommandAddArgFormat(cmd, "2:0,%s,%s", bus_type, - virDomainDiskGetSource(disk)); - return 0; }
@@ -279,9 +271,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 more than one disk defined"));
s/more than/at least/
return NULL; }
diff --git a/src/bhyve/bhyve_device.c b/src/bhyve/bhyve_device.c new file mode 100644 index 0000000..c18665f --- /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"));
It's only implied when the console is present. Does it make sense to let someone use the address if they don't want the ISA bridge? Maybe we should generate an explicit <controller> element for it too.
+ return -1; + } + } + + if (virDomainPCIAddressReserveSlot(addrs, addr, VIR_PCI_CONNECT_TYPE_PCI) < 0) + goto cleanup; + + ret = 0; + cleanup: + return ret; +} +
+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->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->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; + }
Assigning nets before disks would match the hardcoded slots that were used by a domain with one net and one disk before.
+ + 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; + }
I think unsupported controllers don't need PCI slots.
+ + } + + return 0; + + error: + return -1; +} +
diff --git a/src/bhyve/bhyve_domain.h b/src/bhyve/bhyve_domain.h new file mode 100644 index 0000000..d59fe58 --- /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; + int persistentAddrs;
bool would be enough.
+}; + +extern virDomainXMLPrivateDataCallbacks virBhyveDriverPrivateDataCallbacks; +extern virDomainDefParserConfig virBhyveDriverDomainDefParserConfig; + +#endif /* __BHYVE_DOMAIN_H__ */
Jan

Ján Tomko wrote:
On 05/11/2014 08:48 AM, 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.
I think this can be split in a separate patch.
Sounds good. Originally, I decided to send it in a single series to justify the list of functions and types moved. But now as the first two patches are ACKed, I'll push them (hopefully today or tomorrow) and continue iterating with the bhyve one.
diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index 91a8731..bc7ec5c 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, bool dryRun) { - 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++) {
Moving this loop to the caller of the function would reduce the extra indentation level.
Makes sense, will fix.
+ char *realifname = NULL; + int *tapfd = NULL; + char *brname = NULL; + virDomainNetDefPtr net = net = def->nets[i];;
No need to assign it twice and one semicolon would be enough. :)
Must we a fat finger of mine while messing with vim. Funny that it is still a valid code.
int actualType = virDomainNetGetActualType(net);
if (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) {
@@ -146,7 +141,7 @@ bhyveBuildConsoleArgStr(const virDomainDef *def, virCommandPtr cmd) return -1; }
- virCommandAddArgList(cmd, "-s", "31,lpc", NULL); + virCommandAddArgList(cmd, "-s", "1,lpc", NULL);
Are both slot numbers arbitrary? And do we care about backwards compatibility?
Yes, both these slot numbers are arbitrary. I'm not sure we need to care about backwards compatibility because guest console configuration does not depend on the pci slot used for that PCI-ISA bridge.
virCommandAddArg(cmd, "-l"); virCommandAddArgFormat(cmd, "com%d,%s", chr->target.port + 1, chr->source.data.file.path); @@ -157,46 +152,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));
Do SATA disks use a PCI slot too?
Yes: http://www.freebsd.org/cgi/man.cgi?query=bhyve&sektion=0&manpath=FreeBSD+10.0-RELEASE&format=html Basically, all the devices added through the "-s" flag are pci devices that require the slot number (and function, optionally).
+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"));
It's only implied when the console is present. Does it make sense to let someone use the address if they don't want the ISA bridge?
Maybe we should generate an explicit <controller> element for it too.
As we currently do not have explicit support for PCI-ISA bridges, and in bhyve it's used for console only (and unlikely will be used for something else), the idea was to just reserve this slot to get a consistent and reproducible pci addresses allocation and also not to deal with a new controller type just for that single feature.
+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->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->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; + }
Assigning nets before disks would match the hardcoded slots that were used by a domain with one net and one disk before.
Good point.
+ + 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; + }
I think unsupported controllers don't need PCI slots.
I will take a look into that. Thanks a lot for reviewing this rather huge patchset! Roman Bogorodskiy

Ján Tomko wrote:
+ 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; + }
I think unsupported controllers don't need PCI slots.
I've been thinking about that and appears that block could be dropped completely. The only controller we explicitly operate with is of type 'PCI'. Possible models for pci controllers are: pci-root, pcie-root, pci-bridge and dmi-to-pci-bridge. Bhyve only supports pci-root that doesn't need an address. So appears this block of code should be dropped? Roman Bogorodskiy
participants (3)
-
Eric Blake
-
Ján Tomko
-
Roman Bogorodskiy