These functions don't use any QEMU's specific data, so they can be moved to a file which is shared between all of the hypervisors. Their names are changed from qemu* to vir*. More hypervisors will probably implement the possibility of explicitly stating the address of devices, so it makes sense to be prepared for it by making address assignment generic. After moving the rest of the functions, I will restore the "static" keyword wherever applicable. --- src/conf/domain_addr.c | 477 +++++++++++++++++++++++++++++++++++++ src/conf/domain_addr.h | 19 ++ src/libvirt_private.syms | 7 + src/qemu/qemu_domain_address.c | 517 ++--------------------------------------- 4 files changed, 519 insertions(+), 501 deletions(-) diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index e482328..3570a62 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -1229,3 +1229,480 @@ virDomainVirtioSerialAddrRelease(virDomainVirtioSerialAddrSetPtr addrs, VIR_FREE(str); return ret; } + +int +virDomainAssignVirtioSerialAddresses(virDomainDefPtr def) +{ + int ret = -1; + size_t i; + virDomainVirtioSerialAddrSetPtr addrs = NULL; + + if (!(addrs = virDomainVirtioSerialAddrSetCreate())) + goto cleanup; + + if (virDomainVirtioSerialAddrSetAddControllers(addrs, def) < 0) + goto cleanup; + + if (virDomainDeviceInfoIterate(def, virDomainVirtioSerialAddrReserve, + addrs) < 0) + goto cleanup; + + VIR_DEBUG("Finished reserving existing ports"); + + for (i = 0; i < def->nconsoles; i++) { + virDomainChrDefPtr chr = def->consoles[i]; + if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE && + chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO && + !virDomainVirtioSerialAddrIsComplete(&chr->info) && + virDomainVirtioSerialAddrAutoAssign(def, addrs, &chr->info, true) < 0) + goto cleanup; + } + + for (i = 0; i < def->nchannels; i++) { + virDomainChrDefPtr chr = def->channels[i]; + if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL && + chr->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO && + !virDomainVirtioSerialAddrIsComplete(&chr->info) && + virDomainVirtioSerialAddrAutoAssign(def, addrs, &chr->info, false) < 0) + goto cleanup; + } + + /* we persist the addresses */ + virDomainVirtioSerialAddrSetFree(def->vioserialaddrs); + def->vioserialaddrs = addrs; + addrs = NULL; + + ret = 0; + + cleanup: + virDomainVirtioSerialAddrSetFree(addrs); + return ret; +} + +static int +virDomainSpaprVIOFindByReg(virDomainDefPtr def ATTRIBUTE_UNUSED, + virDomainDeviceDefPtr device ATTRIBUTE_UNUSED, + virDomainDeviceInfoPtr info, void *opaque) +{ + virDomainDeviceInfoPtr target = opaque; + + if (info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO) + return 0; + + /* Match a dev that has a reg, is not us, and has a matching reg */ + if (info->addr.spaprvio.has_reg && info != target && + info->addr.spaprvio.reg == target->addr.spaprvio.reg) + /* Has to be < 0 so virDomainDeviceInfoIterate() will exit */ + return -1; + + return 0; +} + +int +virDomainAssignSpaprVIOAddress(virDomainDefPtr def, + virDomainDeviceInfoPtr info, + unsigned long long default_reg) +{ + bool user_reg; + int ret; + + if (info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO) + return 0; + + /* Check if the user has assigned the reg already, if so use it */ + user_reg = info->addr.spaprvio.has_reg; + if (!user_reg) { + info->addr.spaprvio.reg = default_reg; + info->addr.spaprvio.has_reg = true; + } + + ret = virDomainDeviceInfoIterate(def, virDomainSpaprVIOFindByReg, info); + while (ret != 0) { + if (user_reg) { + virReportError(VIR_ERR_XML_ERROR, + _("spapr-vio address %#llx already in use"), + info->addr.spaprvio.reg); + return -EEXIST; + } + + /* We assigned the reg, so try a new value */ + info->addr.spaprvio.reg += 0x1000; + ret = virDomainDeviceInfoIterate(def, virDomainSpaprVIOFindByReg, + info); + } + + return 0; +} + +void +virDomainPrimeVirtioDeviceAddresses(virDomainDefPtr def, + virDomainDeviceAddressType type) +{ + /* + declare address-less virtio devices to be of address type 'type' + disks, networks, consoles, controllers, memballoon and rng in this + order + if type is ccw filesystem devices are declared to be of address type ccw + */ + size_t i; + + for (i = 0; i < def->ndisks; i++) { + if (def->disks[i]->bus == VIR_DOMAIN_DISK_BUS_VIRTIO && + def->disks[i]->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) + def->disks[i]->info.type = type; + } + + for (i = 0; i < def->nnets; i++) { + if (STREQ(def->nets[i]->model, "virtio") && + def->nets[i]->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { + def->nets[i]->info.type = type; + } + } + + for (i = 0; i < def->ninputs; i++) { + if (def->inputs[i]->bus == VIR_DOMAIN_DISK_BUS_VIRTIO && + def->inputs[i]->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) + def->inputs[i]->info.type = type; + } + + for (i = 0; i < def->ncontrollers; i++) { + if ((def->controllers[i]->type == + VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL || + def->controllers[i]->type == + VIR_DOMAIN_CONTROLLER_TYPE_SCSI) && + def->controllers[i]->info.type == + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) + def->controllers[i]->info.type = type; + } + + if (def->memballoon && + def->memballoon->model == VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO && + def->memballoon->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) + def->memballoon->info.type = type; + + for (i = 0; i < def->nrngs; i++) { + if (def->rngs[i]->model == VIR_DOMAIN_RNG_MODEL_VIRTIO && + def->rngs[i]->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) + def->rngs[i]->info.type = type; + } + + if (type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { + for (i = 0; i < def->nfss; i++) { + if (def->fss[i]->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) + def->fss[i]->info.type = type; + } + } +} + +static int +virDomainCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, + virDomainDeviceDefPtr device, + virDomainDeviceInfoPtr info, + void *opaque) +{ + virDomainPCIAddressSetPtr addrs = opaque; + int ret = -1; + virPCIDeviceAddressPtr addr = &info->addr.pci; + bool entireSlot; + /* flags may be changed from default below */ + virDomainPCIConnectFlags flags = (VIR_PCI_CONNECT_HOTPLUGGABLE | + VIR_PCI_CONNECT_TYPE_PCI_DEVICE); + + if (!virDeviceInfoPCIAddressPresent(info) || + ((device->type == VIR_DOMAIN_DEVICE_HOSTDEV) && + (device->data.hostdev->parent.type != VIR_DOMAIN_DEVICE_NONE))) { + /* If a hostdev has a parent, its info will be a part of the + * parent, and will have its address collected during the scan + * of the parent's device type. + */ + return 0; + } + + /* Change flags according to differing requirements of different + * devices. + */ + switch (device->type) { + case VIR_DOMAIN_DEVICE_CONTROLLER: + switch (device->data.controller->type) { + case VIR_DOMAIN_CONTROLLER_TYPE_PCI: + flags = virDomainPCIControllerModelToConnectType(device->data.controller->model); + break; + + case VIR_DOMAIN_CONTROLLER_TYPE_SATA: + /* SATA controllers aren't hot-plugged, and can be put in + * either a PCI or PCIe slot + */ + flags = (VIR_PCI_CONNECT_TYPE_PCI_DEVICE + | VIR_PCI_CONNECT_TYPE_PCIE_DEVICE); + break; + + case VIR_DOMAIN_CONTROLLER_TYPE_USB: + /* allow UHCI and EHCI controllers to be manually placed on + * the PCIe bus (but don't put them there automatically) + */ + switch (device->data.controller->model) { + case VIR_DOMAIN_CONTROLLER_MODEL_USB_EHCI: + case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_EHCI1: + case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI1: + 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 = VIR_PCI_CONNECT_TYPE_PCI_DEVICE; + 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 = (VIR_PCI_CONNECT_TYPE_PCI_DEVICE + | VIR_PCI_CONNECT_TYPE_PCIE_DEVICE); + break; + case VIR_DOMAIN_CONTROLLER_MODEL_USB_PCI_OHCI: + case VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI: + case VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX4_UHCI: + /* Allow these for PCI only */ + break; + } + } + break; + + case VIR_DOMAIN_DEVICE_SOUND: + switch (device->data.sound->model) { + case VIR_DOMAIN_SOUND_MODEL_ICH6: + case VIR_DOMAIN_SOUND_MODEL_ICH9: + flags = VIR_PCI_CONNECT_TYPE_PCI_DEVICE; + break; + } + break; + + case VIR_DOMAIN_DEVICE_VIDEO: + /* video cards aren't hot-plugged, and can be put in either a + * PCI or PCIe slot + */ + flags = (VIR_PCI_CONNECT_TYPE_PCI_DEVICE + | VIR_PCI_CONNECT_TYPE_PCIE_DEVICE); + break; + } + + /* Ignore implicit controllers on slot 0:0:1.0: + * implicit IDE controller on 0:0:1.1 (no qemu command line) + * implicit USB controller on 0:0:1.2 (-usb) + * + * If the machine does have a PCI bus, they will get reserved + * in qemuDomainAssignDevicePCISlots(). + */ + + /* These are the IDE and USB controllers in the PIIX3, hardcoded + * to bus 0 slot 1. They cannot be attached to a PCIe slot, only + * PCI. + */ + if (device->type == VIR_DOMAIN_DEVICE_CONTROLLER && addr->domain == 0 && + addr->bus == 0 && addr->slot == 1) { + virDomainControllerDefPtr cont = device->data.controller; + + if ((cont->type == VIR_DOMAIN_CONTROLLER_TYPE_IDE && cont->idx == 0 && + addr->function == 1) || + (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB && cont->idx == 0 && + (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI || + cont->model == -1) && addr->function == 2)) { + /* Note the check for nbuses > 0 - if there are no PCI + * buses, we skip this check. This is a quirk required for + * some machinetypes such as s390, which pretend to have a + * PCI bus for long enough to generate the "-usb" on the + * commandline, but that don't really care if a PCI bus + * actually exists. */ + if (addrs->nbuses > 0 && + !(addrs->buses[0].flags & VIR_PCI_CONNECT_TYPE_PCI_DEVICE)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Bus 0 must be PCI for integrated PIIX3 " + "USB or IDE controllers")); + return -1; + } else { + return 0; + } + } + } + + entireSlot = (addr->function == 0 && + addr->multi != VIR_TRISTATE_SWITCH_ON); + + if (virDomainPCIAddressReserveAddr(addrs, addr, flags, + entireSlot, true) < 0) + goto cleanup; + + ret = 0; + cleanup: + return ret; +} + +virDomainPCIAddressSetPtr +virDomainPCIAddressSetCreate(virDomainDefPtr def, + unsigned int nbuses, + bool dryRun) +{ + virDomainPCIAddressSetPtr addrs; + size_t i; + + if ((addrs = virDomainPCIAddressSetAlloc(nbuses)) == NULL) + return NULL; + + addrs->nbuses = nbuses; + addrs->dryRun = dryRun; + + /* As a safety measure, set default model='pci-root' for first pci + * controller and 'pci-bridge' for all subsequent. After setting + * those defaults, then scan the config and set the actual model + * for all addrs[idx]->bus that already have a corresponding + * controller in the config. + * + */ + if (nbuses > 0) + virDomainPCIAddressBusSetModel(&addrs->buses[0], + VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT); + for (i = 1; i < nbuses; i++) { + virDomainPCIAddressBusSetModel(&addrs->buses[i], + VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE); + } + + for (i = 0; i < def->ncontrollers; i++) { + size_t idx = def->controllers[i]->idx; + + if (def->controllers[i]->type != VIR_DOMAIN_CONTROLLER_TYPE_PCI) + continue; + + if (idx >= addrs->nbuses) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Inappropriate new pci controller index %zu " + "not found in addrs"), idx); + goto error; + } + + if (virDomainPCIAddressBusSetModel(&addrs->buses[idx], + def->controllers[i]->model) < 0) + goto error; + } + + if (virDomainDeviceInfoIterate(def, virDomainCollectPCIAddress, addrs) < 0) + goto error; + + return addrs; + + error: + virDomainPCIAddressSetFree(addrs); + return NULL; +} + +bool +virDomainPCIBusFullyReserved(virDomainPCIAddressBusPtr bus) +{ + size_t i; + + for (i = bus->minSlot; i <= bus->maxSlot; i++) + if (!bus->slots[i]) + return false; + + return true; +} + +void +virDomainPCIControllerSetDefaultModelName(virDomainControllerDefPtr cont) +{ + int *modelName = &cont->opts.pciopts.modelName; + + /* make sure it's not already set */ + if (*modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) + return; + switch ((virDomainControllerModelPCI)cont->model) { + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE: + *modelName = VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCI_BRIDGE; + break; + case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE: + *modelName = VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_I82801B11_BRIDGE; + break; + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT: + *modelName = VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_IOH3420; + break; + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT: + *modelName = VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_X3130_UPSTREAM; + break; + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT: + *modelName = VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_XIO3130_DOWNSTREAM; + break; + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS: + *modelName = VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PXB; + break; + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS: + *modelName = VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PXB_PCIE; + break; + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT: + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT: + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST: + break; + } +} + +int +virDomainAddressFindNewBusNr(virDomainDefPtr def) +{ +/* Try to find a nice default for busNr for a new pci-expander-bus. + * This is a bit tricky, since you need to satisfy the following: + * + * 1) There need to be enough unused bus numbers between busNr of this + * bus and busNr of the next highest bus for the guest to assign a + * unique bus number to each PCI bus that is a child of this + * bus. Each PCI controller. On top of this, the pxb device (which + * implements the pci-extender-bus) includes a pci-bridge within + * it, and that bridge also uses one bus number (so each pxb device + * requires at least 2 bus numbers). + * + * 2) There need to be enough bus numbers *below* this for all the + * child controllers of the pci-expander-bus with the next lower + * busNr (or the pci-root bus if there are no lower + * pci-expander-buses). + * + * 3) If at all possible, we want to avoid needing to change the busNr + * of a bus in the future, as that changes the guest's device ABI, + * which could potentially lead to issues with a guest OS that is + * picky about such things. + * + * Due to the impossibility of predicting what might be added to the + * config in the future, we can't make a foolproof choice, but since + * a pci-expander-bus (pxb) has slots for 32 devices, and the only + * practical use for it is to assign real devices on a particular + * NUMA node in the host, it's reasonably safe to assume it should + * never need any additional child buses (probably only a few of the + * 32 will ever be used). So for pci-expander-bus we find the lowest + * existing busNr, and set this one to the current lowest - 2 (one + * for the pxb, one for the intergrated pci-bridge), thus leaving the + * maximum possible bus numbers available for other buses plugged + * into pci-root (i.e. pci-bridges and other + * pci-expander-buses). Anyone who needs more than 32 devices + * descended from one pci-expander-bus should set the busNr manually + * in the config. + * + * There is room for more error checking here - in particular we + * can/should determine the ultimate parent (root-bus) of each PCI + * controller and determine if there is enough space for all the + * buses within the current range allotted to the bus just prior to + * this one. + */ + size_t i; + int lowestBusNr = 256; + + for (i = 0; i < def->ncontrollers; i++) { + if (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) { + int thisBusNr = def->controllers[i]->opts.pciopts.busNr; + + if (thisBusNr >= 0 && thisBusNr < lowestBusNr) + lowestBusNr = thisBusNr; + } + } + + /* If we already have a busNR = 1, then we can't auto-assign (0 is + * the pci[e]-root, and the others may have been assigned + * purposefully). + */ + if (lowestBusNr <= 2) + return -1; + + return lowestBusNr - 2; +} diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h index 0fb5f85..f4a9852 100644 --- a/src/conf/domain_addr.h +++ b/src/conf/domain_addr.h @@ -220,4 +220,23 @@ virDomainVirtioSerialAddrRelease(virDomainVirtioSerialAddrSetPtr addrs, virDomainDeviceInfoPtr info) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +int virDomainAssignVirtioSerialAddresses(virDomainDefPtr def); + +int virDomainAssignSpaprVIOAddress(virDomainDefPtr def, + virDomainDeviceInfoPtr info, + unsigned long long default_reg); + +void virDomainPrimeVirtioDeviceAddresses(virDomainDefPtr def, + virDomainDeviceAddressType type); + +virDomainPCIAddressSetPtr virDomainPCIAddressSetCreate(virDomainDefPtr def, + unsigned int nbuses, + bool dryRun); + +bool virDomainPCIBusFullyReserved(virDomainPCIAddressBusPtr bus); + +void virDomainPCIControllerSetDefaultModelName(virDomainControllerDefPtr cont); + +int virDomainAddressFindNewBusNr(virDomainDefPtr def); + #endif /* __DOMAIN_ADDR_H__ */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d29eb07..7df6b35 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -87,6 +87,9 @@ virPCIDeviceAddressParseXML; # conf/domain_addr.h +virDomainAddressFindNewBusNr; +virDomainAssignSpaprVIOAddress; +virDomainAssignVirtioSerialAddresses; virDomainCCWAddressAllocate; virDomainCCWAddressAssign; virDomainCCWAddressReleaseAddr; @@ -104,11 +107,15 @@ virDomainPCIAddressReserveAddr; virDomainPCIAddressReserveNextSlot; virDomainPCIAddressReserveSlot; virDomainPCIAddressSetAlloc; +virDomainPCIAddressSetCreate; virDomainPCIAddressSetFree; virDomainPCIAddressSetGrow; virDomainPCIAddressSlotInUse; virDomainPCIAddressValidate; +virDomainPCIBusFullyReserved; virDomainPCIControllerModelToConnectType; +virDomainPCIControllerSetDefaultModelName; +virDomainPrimeVirtioDeviceAddresses; virDomainVirtioSerialAddrAssign; virDomainVirtioSerialAddrAutoAssign; virDomainVirtioSerialAddrIsComplete; diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 1633c9b..49f3652 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -107,113 +107,6 @@ qemuDomainSetSCSIControllerModel(const virDomainDef *def, static int -qemuDomainAssignVirtioSerialAddresses(virDomainDefPtr def) -{ - int ret = -1; - size_t i; - virDomainVirtioSerialAddrSetPtr addrs = NULL; - - if (!(addrs = virDomainVirtioSerialAddrSetCreate())) - goto cleanup; - - if (virDomainVirtioSerialAddrSetAddControllers(addrs, def) < 0) - goto cleanup; - - if (virDomainDeviceInfoIterate(def, virDomainVirtioSerialAddrReserve, - addrs) < 0) - goto cleanup; - - VIR_DEBUG("Finished reserving existing ports"); - - for (i = 0; i < def->nconsoles; i++) { - virDomainChrDefPtr chr = def->consoles[i]; - if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE && - chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO && - !virDomainVirtioSerialAddrIsComplete(&chr->info) && - virDomainVirtioSerialAddrAutoAssign(def, addrs, &chr->info, true) < 0) - goto cleanup; - } - - for (i = 0; i < def->nchannels; i++) { - virDomainChrDefPtr chr = def->channels[i]; - if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL && - chr->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO && - !virDomainVirtioSerialAddrIsComplete(&chr->info) && - virDomainVirtioSerialAddrAutoAssign(def, addrs, &chr->info, false) < 0) - goto cleanup; - } - - /* we persist the addresses */ - virDomainVirtioSerialAddrSetFree(def->vioserialaddrs); - def->vioserialaddrs = addrs; - addrs = NULL; - - ret = 0; - - cleanup: - virDomainVirtioSerialAddrSetFree(addrs); - return ret; -} - - -static int -qemuDomainSpaprVIOFindByReg(virDomainDefPtr def ATTRIBUTE_UNUSED, - virDomainDeviceDefPtr device ATTRIBUTE_UNUSED, - virDomainDeviceInfoPtr info, void *opaque) -{ - virDomainDeviceInfoPtr target = opaque; - - if (info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO) - return 0; - - /* Match a dev that has a reg, is not us, and has a matching reg */ - if (info->addr.spaprvio.has_reg && info != target && - info->addr.spaprvio.reg == target->addr.spaprvio.reg) - /* Has to be < 0 so virDomainDeviceInfoIterate() will exit */ - return -1; - - return 0; -} - - -static int -qemuDomainAssignSpaprVIOAddress(virDomainDefPtr def, - virDomainDeviceInfoPtr info, - unsigned long long default_reg) -{ - bool user_reg; - int ret; - - if (info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO) - return 0; - - /* Check if the user has assigned the reg already, if so use it */ - user_reg = info->addr.spaprvio.has_reg; - if (!user_reg) { - info->addr.spaprvio.reg = default_reg; - info->addr.spaprvio.has_reg = true; - } - - ret = virDomainDeviceInfoIterate(def, qemuDomainSpaprVIOFindByReg, info); - while (ret != 0) { - if (user_reg) { - virReportError(VIR_ERR_XML_ERROR, - _("spapr-vio address %#llx already in use"), - info->addr.spaprvio.reg); - return -EEXIST; - } - - /* We assigned the reg, so try a new value */ - info->addr.spaprvio.reg += 0x1000; - ret = virDomainDeviceInfoIterate(def, qemuDomainSpaprVIOFindByReg, - info); - } - - return 0; -} - - -static int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) { @@ -227,7 +120,7 @@ qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def, if (def->nets[i]->model && STREQ(def->nets[i]->model, "spapr-vlan")) def->nets[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO; - if (qemuDomainAssignSpaprVIOAddress(def, &def->nets[i]->info, + if (virDomainAssignSpaprVIOAddress(def, &def->nets[i]->info, VIO_ADDR_NET) < 0) goto cleanup; } @@ -242,7 +135,7 @@ qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def, if (model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI && def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) def->controllers[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO; - if (qemuDomainAssignSpaprVIOAddress(def, &def->controllers[i]->info, + if (virDomainAssignSpaprVIOAddress(def, &def->controllers[i]->info, VIO_ADDR_SCSI) < 0) goto cleanup; } @@ -252,7 +145,7 @@ qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def, ARCH_IS_PPC64(def->os.arch) && STRPREFIX(def->os.machine, "pseries")) def->serials[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO; - if (qemuDomainAssignSpaprVIOAddress(def, &def->serials[i]->info, + if (virDomainAssignSpaprVIOAddress(def, &def->serials[i]->info, VIO_ADDR_SERIAL) < 0) goto cleanup; } @@ -261,7 +154,7 @@ qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def, if (ARCH_IS_PPC64(def->os.arch) && STRPREFIX(def->os.machine, "pseries")) def->nvram->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO; - if (qemuDomainAssignSpaprVIOAddress(def, &def->nvram->info, + if (virDomainAssignSpaprVIOAddress(def, &def->nvram->info, VIO_ADDR_NVRAM) < 0) goto cleanup; } @@ -275,67 +168,6 @@ qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def, } -static void -qemuDomainPrimeVirtioDeviceAddresses(virDomainDefPtr def, - virDomainDeviceAddressType type) -{ - /* - declare address-less virtio devices to be of address type 'type' - disks, networks, consoles, controllers, memballoon and rng in this - order - if type is ccw filesystem devices are declared to be of address type ccw - */ - size_t i; - - for (i = 0; i < def->ndisks; i++) { - if (def->disks[i]->bus == VIR_DOMAIN_DISK_BUS_VIRTIO && - def->disks[i]->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) - def->disks[i]->info.type = type; - } - - for (i = 0; i < def->nnets; i++) { - if (STREQ(def->nets[i]->model, "virtio") && - def->nets[i]->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { - def->nets[i]->info.type = type; - } - } - - for (i = 0; i < def->ninputs; i++) { - if (def->inputs[i]->bus == VIR_DOMAIN_DISK_BUS_VIRTIO && - def->inputs[i]->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) - def->inputs[i]->info.type = type; - } - - for (i = 0; i < def->ncontrollers; i++) { - if ((def->controllers[i]->type == - VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL || - def->controllers[i]->type == - VIR_DOMAIN_CONTROLLER_TYPE_SCSI) && - def->controllers[i]->info.type == - VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) - def->controllers[i]->info.type = type; - } - - if (def->memballoon && - def->memballoon->model == VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO && - def->memballoon->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) - def->memballoon->info.type = type; - - for (i = 0; i < def->nrngs; i++) { - if (def->rngs[i]->model == VIR_DOMAIN_RNG_MODEL_VIRTIO && - def->rngs[i]->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) - def->rngs[i]->info.type = type; - } - - if (type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { - for (i = 0; i < def->nfss; i++) { - if (def->fss[i]->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) - def->fss[i]->info.type = type; - } - } -} - - /* * Three steps populating CCW devnos * 1. Allocate empty address set @@ -353,7 +185,7 @@ qemuDomainAssignS390Addresses(virDomainDefPtr def, if (qemuDomainMachineIsS390CCW(def) && virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_CCW)) { - qemuDomainPrimeVirtioDeviceAddresses( + virDomainPrimeVirtioDeviceAddresses( def, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW); if (!(addrs = virDomainCCWAddressSetCreate())) @@ -368,7 +200,7 @@ qemuDomainAssignS390Addresses(virDomainDefPtr def, goto cleanup; } else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_S390)) { /* deal with legacy virtio-s390 */ - qemuDomainPrimeVirtioDeviceAddresses( + virDomainPrimeVirtioDeviceAddresses( def, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390); } @@ -403,211 +235,13 @@ qemuDomainAssignARMVirtioMMIOAddresses(virDomainDefPtr def, return; if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_MMIO)) { - qemuDomainPrimeVirtioDeviceAddresses( + virDomainPrimeVirtioDeviceAddresses( def, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO); } } static int -qemuDomainCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, - virDomainDeviceDefPtr device, - virDomainDeviceInfoPtr info, - void *opaque) -{ - virDomainPCIAddressSetPtr addrs = opaque; - int ret = -1; - virPCIDeviceAddressPtr addr = &info->addr.pci; - bool entireSlot; - /* flags may be changed from default below */ - virDomainPCIConnectFlags flags = (VIR_PCI_CONNECT_HOTPLUGGABLE | - VIR_PCI_CONNECT_TYPE_PCI_DEVICE); - - if (!virDeviceInfoPCIAddressPresent(info) || - ((device->type == VIR_DOMAIN_DEVICE_HOSTDEV) && - (device->data.hostdev->parent.type != VIR_DOMAIN_DEVICE_NONE))) { - /* If a hostdev has a parent, its info will be a part of the - * parent, and will have its address collected during the scan - * of the parent's device type. - */ - return 0; - } - - /* Change flags according to differing requirements of different - * devices. - */ - switch (device->type) { - case VIR_DOMAIN_DEVICE_CONTROLLER: - switch (device->data.controller->type) { - case VIR_DOMAIN_CONTROLLER_TYPE_PCI: - flags = virDomainPCIControllerModelToConnectType(device->data.controller->model); - break; - - case VIR_DOMAIN_CONTROLLER_TYPE_SATA: - /* SATA controllers aren't hot-plugged, and can be put in - * either a PCI or PCIe slot - */ - flags = (VIR_PCI_CONNECT_TYPE_PCI_DEVICE - | VIR_PCI_CONNECT_TYPE_PCIE_DEVICE); - break; - - case VIR_DOMAIN_CONTROLLER_TYPE_USB: - /* allow UHCI and EHCI controllers to be manually placed on - * the PCIe bus (but don't put them there automatically) - */ - switch (device->data.controller->model) { - case VIR_DOMAIN_CONTROLLER_MODEL_USB_EHCI: - case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_EHCI1: - case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI1: - 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 = VIR_PCI_CONNECT_TYPE_PCI_DEVICE; - 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 = (VIR_PCI_CONNECT_TYPE_PCI_DEVICE - | VIR_PCI_CONNECT_TYPE_PCIE_DEVICE); - break; - case VIR_DOMAIN_CONTROLLER_MODEL_USB_PCI_OHCI: - case VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI: - case VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX4_UHCI: - /* Allow these for PCI only */ - break; - } - } - break; - - case VIR_DOMAIN_DEVICE_SOUND: - switch (device->data.sound->model) { - case VIR_DOMAIN_SOUND_MODEL_ICH6: - case VIR_DOMAIN_SOUND_MODEL_ICH9: - flags = VIR_PCI_CONNECT_TYPE_PCI_DEVICE; - break; - } - break; - - case VIR_DOMAIN_DEVICE_VIDEO: - /* video cards aren't hot-plugged, and can be put in either a - * PCI or PCIe slot - */ - flags = (VIR_PCI_CONNECT_TYPE_PCI_DEVICE - | VIR_PCI_CONNECT_TYPE_PCIE_DEVICE); - break; - } - - /* Ignore implicit controllers on slot 0:0:1.0: - * implicit IDE controller on 0:0:1.1 (no qemu command line) - * implicit USB controller on 0:0:1.2 (-usb) - * - * If the machine does have a PCI bus, they will get reserved - * in qemuDomainAssignDevicePCISlots(). - */ - - /* These are the IDE and USB controllers in the PIIX3, hardcoded - * to bus 0 slot 1. They cannot be attached to a PCIe slot, only - * PCI. - */ - if (device->type == VIR_DOMAIN_DEVICE_CONTROLLER && addr->domain == 0 && - addr->bus == 0 && addr->slot == 1) { - virDomainControllerDefPtr cont = device->data.controller; - - if ((cont->type == VIR_DOMAIN_CONTROLLER_TYPE_IDE && cont->idx == 0 && - addr->function == 1) || - (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB && cont->idx == 0 && - (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI || - cont->model == -1) && addr->function == 2)) { - /* Note the check for nbuses > 0 - if there are no PCI - * buses, we skip this check. This is a quirk required for - * some machinetypes such as s390, which pretend to have a - * PCI bus for long enough to generate the "-usb" on the - * commandline, but that don't really care if a PCI bus - * actually exists. */ - if (addrs->nbuses > 0 && - !(addrs->buses[0].flags & VIR_PCI_CONNECT_TYPE_PCI_DEVICE)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Bus 0 must be PCI for integrated PIIX3 " - "USB or IDE controllers")); - return -1; - } else { - return 0; - } - } - } - - entireSlot = (addr->function == 0 && - addr->multi != VIR_TRISTATE_SWITCH_ON); - - if (virDomainPCIAddressReserveAddr(addrs, addr, flags, - entireSlot, true) < 0) - goto cleanup; - - ret = 0; - cleanup: - return ret; -} - -static virDomainPCIAddressSetPtr -qemuDomainPCIAddressSetCreate(virDomainDefPtr def, - unsigned int nbuses, - bool dryRun) -{ - virDomainPCIAddressSetPtr addrs; - size_t i; - - if ((addrs = virDomainPCIAddressSetAlloc(nbuses)) == NULL) - return NULL; - - addrs->nbuses = nbuses; - addrs->dryRun = dryRun; - - /* As a safety measure, set default model='pci-root' for first pci - * controller and 'pci-bridge' for all subsequent. After setting - * those defaults, then scan the config and set the actual model - * for all addrs[idx]->bus that already have a corresponding - * controller in the config. - * - */ - if (nbuses > 0) - virDomainPCIAddressBusSetModel(&addrs->buses[0], - VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT); - for (i = 1; i < nbuses; i++) { - virDomainPCIAddressBusSetModel(&addrs->buses[i], - VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE); - } - - for (i = 0; i < def->ncontrollers; i++) { - size_t idx = def->controllers[i]->idx; - - if (def->controllers[i]->type != VIR_DOMAIN_CONTROLLER_TYPE_PCI) - continue; - - if (idx >= addrs->nbuses) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Inappropriate new pci controller index %zu " - "not found in addrs"), idx); - goto error; - } - - if (virDomainPCIAddressBusSetModel(&addrs->buses[idx], - def->controllers[i]->model) < 0) - goto error; - } - - if (virDomainDeviceInfoIterate(def, qemuDomainCollectPCIAddress, addrs) < 0) - goto error; - - return addrs; - - error: - virDomainPCIAddressSetFree(addrs); - return NULL; -} - - -static int qemuDomainValidateDevicePCISlotsPIIX3(virDomainDefPtr def, virQEMUCapsPtr qemuCaps, virDomainPCIAddressSetPtr addrs) @@ -719,7 +353,7 @@ qemuDomainValidateDevicePCISlotsPIIX3(virDomainDefPtr def, _("Primary video card must have PCI address 0:0:2.0")); goto cleanup; } - /* If TYPE == PCI, then qemuDomainCollectPCIAddress() function + /* If TYPE == PCI, then virDomainCollectPCIAddress() function * has already reserved the address, so we must skip */ } } else if (addrs->nbuses && !qemuDeviceVideoUsable) { @@ -910,7 +544,7 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, _("Primary video card must have PCI address 0:0:1.0")); goto cleanup; } - /* If TYPE == PCI, then qemuDomainCollectPCIAddress() function + /* If TYPE == PCI, then virDomainCollectPCIAddress() function * has already reserved the address, so we must skip */ } } else if (addrs->nbuses && !qemuDeviceVideoUsable) { @@ -952,19 +586,6 @@ qemuDomainValidateDevicePCISlotsChipsets(virDomainDefPtr def, } -static bool -qemuDomainPCIBusFullyReserved(virDomainPCIAddressBusPtr bus) -{ - size_t i; - - for (i = bus->minSlot; i <= bus->maxSlot; i++) - if (!bus->slots[i]) - return false; - - return true; -} - - /* * This assigns static PCI slots to all configured devices. * The ordering here is chosen to match the ordering used @@ -996,7 +617,7 @@ qemuDomainPCIBusFullyReserved(virDomainPCIAddressBusPtr bus) * - Watchdog * - pci serial devices * - * Prior to this function being invoked, qemuDomainCollectPCIAddress() will have + * Prior to this function being invoked, virDomainCollectPCIAddress() will have * added all existing PCI addresses from the 'def' to 'addrs'. Thus this * function must only try to reserve addresses if info.type == NONE and * skip over info.type == PCI @@ -1330,112 +951,6 @@ qemuDomainSupportsPCI(virDomainDefPtr def, } -static void -qemuDomainPCIControllerSetDefaultModelName(virDomainControllerDefPtr cont) -{ - int *modelName = &cont->opts.pciopts.modelName; - - /* make sure it's not already set */ - if (*modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) - return; - switch ((virDomainControllerModelPCI)cont->model) { - case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE: - *modelName = VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCI_BRIDGE; - break; - case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE: - *modelName = VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_I82801B11_BRIDGE; - break; - case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT: - *modelName = VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_IOH3420; - break; - case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT: - *modelName = VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_X3130_UPSTREAM; - break; - case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT: - *modelName = VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_XIO3130_DOWNSTREAM; - break; - case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS: - *modelName = VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PXB; - break; - case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS: - *modelName = VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PXB_PCIE; - break; - case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT: - case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT: - case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST: - break; - } -} - - -static int -qemuDomainAddressFindNewBusNr(virDomainDefPtr def) -{ -/* Try to find a nice default for busNr for a new pci-expander-bus. - * This is a bit tricky, since you need to satisfy the following: - * - * 1) There need to be enough unused bus numbers between busNr of this - * bus and busNr of the next highest bus for the guest to assign a - * unique bus number to each PCI bus that is a child of this - * bus. Each PCI controller. On top of this, the pxb device (which - * implements the pci-extender-bus) includes a pci-bridge within - * it, and that bridge also uses one bus number (so each pxb device - * requires at least 2 bus numbers). - * - * 2) There need to be enough bus numbers *below* this for all the - * child controllers of the pci-expander-bus with the next lower - * busNr (or the pci-root bus if there are no lower - * pci-expander-buses). - * - * 3) If at all possible, we want to avoid needing to change the busNr - * of a bus in the future, as that changes the guest's device ABI, - * which could potentially lead to issues with a guest OS that is - * picky about such things. - * - * Due to the impossibility of predicting what might be added to the - * config in the future, we can't make a foolproof choice, but since - * a pci-expander-bus (pxb) has slots for 32 devices, and the only - * practical use for it is to assign real devices on a particular - * NUMA node in the host, it's reasonably safe to assume it should - * never need any additional child buses (probably only a few of the - * 32 will ever be used). So for pci-expander-bus we find the lowest - * existing busNr, and set this one to the current lowest - 2 (one - * for the pxb, one for the intergrated pci-bridge), thus leaving the - * maximum possible bus numbers available for other buses plugged - * into pci-root (i.e. pci-bridges and other - * pci-expander-buses). Anyone who needs more than 32 devices - * descended from one pci-expander-bus should set the busNr manually - * in the config. - * - * There is room for more error checking here - in particular we - * can/should determine the ultimate parent (root-bus) of each PCI - * controller and determine if there is enough space for all the - * buses within the current range allotted to the bus just prior to - * this one. - */ - size_t i; - int lowestBusNr = 256; - - for (i = 0; i < def->ncontrollers; i++) { - if (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) { - int thisBusNr = def->controllers[i]->opts.pciopts.busNr; - - if (thisBusNr >= 0 && thisBusNr < lowestBusNr) - lowestBusNr = thisBusNr; - } - } - - /* If we already have a busNR = 1, then we can't auto-assign (0 is - * the pci[e]-root, and the others may have been assigned - * purposefully). - */ - if (lowestBusNr <= 2) - return -1; - - return lowestBusNr - 2; -} - - static int qemuDomainAssignPCIAddresses(virDomainDefPtr def, virQEMUCapsPtr qemuCaps, @@ -1466,7 +981,7 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, virDomainDeviceInfo info; /* 1st pass to figure out how many PCI bridges we need */ - if (!(addrs = qemuDomainPCIAddressSetCreate(def, nbuses, true))) + if (!(addrs = virDomainPCIAddressSetCreate(def, nbuses, true))) goto cleanup; if (qemuDomainValidateDevicePCISlotsChipsets(def, qemuCaps, @@ -1474,7 +989,7 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, goto cleanup; for (i = 0; i < addrs->nbuses; i++) { - if (!qemuDomainPCIBusFullyReserved(&addrs->buses[i])) + if (!virDomainPCIBusFullyReserved(&addrs->buses[i])) buses_reserved = false; } @@ -1517,7 +1032,7 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, goto cleanup; } - if (!(addrs = qemuDomainPCIAddressSetCreate(def, nbuses, false))) + if (!(addrs = virDomainPCIAddressSetCreate(def, nbuses, false))) goto cleanup; if (qemuDomainSupportsPCI(def, qemuCaps)) { @@ -1544,7 +1059,7 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, * device in qemu) for any controller that doesn't yet * have it set. */ - qemuDomainPCIControllerSetDefaultModelName(cont); + virDomainPCIControllerSetDefaultModelName(cont); /* set defaults for any other auto-generated config * options for this controller that haven't been @@ -1570,7 +1085,7 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS: case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS: if (options->busNr == -1) - options->busNr = qemuDomainAddressFindNewBusNr(def); + options->busNr = virDomainAddressFindNewBusNr(def); if (options->busNr == -1) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("No free busNr lower than current " @@ -1626,7 +1141,7 @@ qemuDomainAssignAddresses(virDomainDefPtr def, virQEMUCapsPtr qemuCaps, virDomainObjPtr obj) { - if (qemuDomainAssignVirtioSerialAddresses(def) < 0) + if (virDomainAssignVirtioSerialAddresses(def) < 0) return -1; if (qemuDomainAssignSpaprVIOAddresses(def, qemuCaps) < 0) -- 2.7.4 (Apple Git-66)

pciaddrs will be moved from hypervisors' private data to domainDef, so domain_conf has to be able to free it. --- src/conf/domain_addr.c | 11 ----------- src/conf/domain_addr.h | 41 ----------------------------------------- src/conf/domain_conf.c | 10 ++++++++++ src/conf/domain_conf.h | 41 +++++++++++++++++++++++++++++++++++++++++ src/libvirt_private.syms | 2 +- 5 files changed, 52 insertions(+), 53 deletions(-) diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index 93eebf2..09cbc73 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -550,17 +550,6 @@ virDomainPCIAddressSetAlloc(unsigned int nbuses) } -void -virDomainPCIAddressSetFree(virDomainPCIAddressSetPtr addrs) -{ - if (!addrs) - return; - - VIR_FREE(addrs->buses); - VIR_FREE(addrs); -} - - int virDomainPCIAddressGetNextSlot(virDomainPCIAddressSetPtr addrs, virPCIDeviceAddressPtr next_addr, diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h index cda1e3f..e767f34 100644 --- a/src/conf/domain_addr.h +++ b/src/conf/domain_addr.h @@ -26,22 +26,8 @@ # 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, /* is hotplug needed/supported */ - - /* kinds of devices as a bitmap so they can be combined (some PCI - * controllers permit connecting multiple types of devices) - */ - VIR_PCI_CONNECT_TYPE_PCI_DEVICE = 1 << 1, - VIR_PCI_CONNECT_TYPE_PCIE_DEVICE = 1 << 2, - VIR_PCI_CONNECT_TYPE_PCIE_ROOT_PORT = 1 << 3, - VIR_PCI_CONNECT_TYPE_PCIE_SWITCH_UPSTREAM_PORT = 1 << 4, - VIR_PCI_CONNECT_TYPE_PCIE_SWITCH_DOWNSTREAM_PORT = 1 << 5, -} virDomainPCIConnectFlags; - /* a combination of all bits that describe the type of connections * allowed, e.g. PCI, PCIe, switch */ @@ -62,38 +48,11 @@ typedef enum { virDomainPCIConnectFlags virDomainPCIControllerModelToConnectType(virDomainControllerModelPCI model); -typedef struct { - virDomainControllerModelPCI model; - /* flags and min/max can be computed from model, but - * having them ready makes life easier. - */ - virDomainPCIConnectFlags flags; - size_t minSlot, maxSlot; /* usually 0,0 or 0,31, 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; - virPCIDeviceAddress 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; - char *virDomainPCIAddressAsString(virPCIDeviceAddressPtr addr) ATTRIBUTE_NONNULL(1); virDomainPCIAddressSetPtr virDomainPCIAddressSetAlloc(unsigned int nbuses); -void virDomainPCIAddressSetFree(virDomainPCIAddressSetPtr addrs); - bool virDomainPCIAddressFlagsCompatible(virPCIDeviceAddressPtr addr, const char *addrStr, virDomainPCIConnectFlags busFlags, diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 12448dd..5f31a97 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2544,6 +2544,16 @@ void virDomainCCWAddressSetFree(virDomainCCWAddressSetPtr addrs) VIR_FREE(addrs); } +void +virDomainPCIAddressSetFree(virDomainPCIAddressSetPtr addrs) +{ + if (!addrs) + return; + + VIR_FREE(addrs->buses); + VIR_FREE(addrs); +} + void virDomainDefFree(virDomainDefPtr def) { size_t i; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 28fdb98..887e061 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2111,6 +2111,46 @@ struct _virDomainCCWAddressSet { typedef struct _virDomainCCWAddressSet virDomainCCWAddressSet; typedef virDomainCCWAddressSet *virDomainCCWAddressSetPtr; +typedef enum { + VIR_PCI_CONNECT_HOTPLUGGABLE = 1 << 0, /* is hotplug needed/supported */ + + /* kinds of devices as a bitmap so they can be combined (some PCI + * controllers permit connecting multiple types of devices) + */ + VIR_PCI_CONNECT_TYPE_PCI_DEVICE = 1 << 1, + VIR_PCI_CONNECT_TYPE_PCIE_DEVICE = 1 << 2, + VIR_PCI_CONNECT_TYPE_PCIE_ROOT_PORT = 1 << 3, + VIR_PCI_CONNECT_TYPE_PCIE_SWITCH_UPSTREAM_PORT = 1 << 4, + VIR_PCI_CONNECT_TYPE_PCIE_SWITCH_DOWNSTREAM_PORT = 1 << 5, +} virDomainPCIConnectFlags; + +# define VIR_PCI_ADDRESS_SLOT_LAST 31 + +typedef struct { + virDomainControllerModelPCI model; + /* flags and min/max can be computed from model, but + * having them ready makes life easier. + */ + virDomainPCIConnectFlags flags; + size_t minSlot, maxSlot; /* usually 0,0 or 0,31, 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; + virPCIDeviceAddress 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; + typedef struct _virDomainPowerManagement virDomainPowerManagement; typedef virDomainPowerManagement *virDomainPowerManagementPtr; @@ -2554,6 +2594,7 @@ void virDomainTPMDefFree(virDomainTPMDefPtr def); void virDomainVirtioSerialControllerFree(virDomainVirtioSerialControllerPtr cont); void virDomainVirtioSerialAddrSetFree(virDomainVirtioSerialAddrSetPtr addrs); void virDomainCCWAddressSetFree(virDomainCCWAddressSetPtr addrs); +void virDomainPCIAddressSetFree(virDomainPCIAddressSetPtr addrs); typedef int (*virDomainDeviceInfoCallback)(virDomainDefPtr def, virDomainDeviceDefPtr dev, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 75c8f70..a8109d7 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -107,7 +107,6 @@ virDomainPCIAddressReserveNextSlot; virDomainPCIAddressReserveSlot; virDomainPCIAddressSetAlloc; virDomainPCIAddressSetCreate; -virDomainPCIAddressSetFree; virDomainPCIAddressSetGrow; virDomainPCIAddressSlotInUse; virDomainPCIAddressValidate; @@ -427,6 +426,7 @@ virDomainOSTypeToString; virDomainParseMemory; virDomainPausedReasonTypeFromString; virDomainPausedReasonTypeToString; +virDomainPCIAddressSetFree; virDomainPMSuspendedReasonTypeFromString; virDomainPMSuspendedReasonTypeToString; virDomainRedirdevBusTypeFromString; -- 2.7.4 (Apple Git-66)

On 06/20/2016 10:26 PM, Tomasz Flendrich wrote: In a way they are currently cached twice: once in these lists, and once in the runtime VM XML. That's what my vioserial series did, was replace the hotplug usage of the cached list with just fetching it on demand from the running VM XML, so at least at a high level these cached lists seem redundant because the info is tracked elsewhere. Hmm, that's interesting. Not sure what that is about. I tried adding this diff: diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index 794270d..7d39b69 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -792,14 +792,7 @@ virDomainCCWAddressReleaseAddr(virDomainCCWAddressSetPtr addrs, if (!addr) return -1; - if ((ret = virHashRemoveEntry(addrs->defined, addr)) == 0 && - dev->addr.ccw.cssid == addrs->next.cssid && - dev->addr.ccw.ssid == addrs->next.ssid && - dev->addr.ccw.devno < addrs->next.devno) { - addrs->next.devno = dev->addr.ccw.devno; - addrs->next.assigned = false; - } - + ret = virHashRemoveEntry(addrs->defined, addr); VIR_FREE(addr); return ret; And the test suite doesn't break, so at least it's not something completely obvious. But then again this seems to only be via hotunplug call path, and our test suite coverage for hotplug isn't that great. This logic may in fact just be an artifact of maintaining a persistent set of addresses. If we are generating an address set on demand from the runtime XML, we probably don't need to worry about 'releasing' an address from that set at all, so it may just side step whatever issue this logic was trying to address. Laine did have some patches that did touch some bits in this area, but only as a side effect. Thanks, Cole