[libvirt] [PATCH v2 5/8] Introduce virDomainPCIMultifunctionDeviceAddressAssign

Laine Stump laine at laine.org
Thu May 19 18:29:36 UTC 2016


On 05/18/2016 05:32 PM, Shivaprasad G Bhat wrote:
> This patch assigns multifunction pci addresses to devices in the devlist.
> The pciaddrs passed in the argument is not altered so that the actual call to
> reserve the address using virDomainPCIAddressEnsureAddr() passes. The function
> focuses on user given address validation and also the auto-assign of addresses.
> The address auto-assignment works well for PPC64 and x86-i440fx machines.

Since you know that after hotplugging these devices into this slot, you 
won't be able to add any new devices to it, I think it's unnecessary to 
keep track of exactly which functions of the slot are occupied and which 
aren't. Effectively, they *all* are.

So instead of copy-pasting the huge chunk of code and allocating one 
function at a time, how about just marking the entire slot in use at a 
higher level rather than trying to mark individual functions? (unless 
you're looking at these individual bits later for something *other* than 
just deciding which ones to free.

Note that you'll need to determine the CONNECT_TYPE at the higher level 
too, and be sure to choose PCI_DEVICE or PCIE_DEVICE appropriately (and 
if there is any attempt to mix the two, I would say you should refuse to 
auto-assign an address (but allow it if they manually specify the address).


>
> The q35 machines needs some level of logic here to get the correct next valid
> slot so that the hotplug go through fine.

Can you explain that? There should be no difference.


>
> Signed-off-by: Shivaprasad G Bhat <sbhat at linux.vnet.ibm.com>
> ---
>   src/conf/domain_addr.c   |  199 ++++++++++++++++++++++++++++++++++++++++++++++
>   src/conf/domain_addr.h   |    4 +
>   src/libvirt_private.syms |    1
>   3 files changed, 204 insertions(+)
>
> diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
> index 7ea9e4d..7c52f84 100644
> --- a/src/conf/domain_addr.c
> +++ b/src/conf/domain_addr.c
> @@ -454,6 +454,205 @@ virDomainPCIAddressReserveSlot(virDomainPCIAddressSetPtr addrs,
>       return virDomainPCIAddressReserveAddr(addrs, addr, flags, true, false);
>   }
>   
> +static int
> +virDomainPCIAddressGetNextFunctionAddr(uint8_t *slot,
> +                                       virPCIDeviceAddressPtr addr)
> +{
> +    size_t i = 0;
> +
> +    for (i = 0; i < 8; i++) {
> +        if (!(*slot & 1 << i)) {
> +            addr->function = i;
> +            break;
> +        }
> +    }
> +
> +    return 0;
> +}
> +
> +static int
> +virDomainPCIAddressAssignFunctionAddr(virDomainPCIAddressSetPtr addrs,
> +                                      uint8_t *slot,
> +                                      virPCIDeviceAddressPtr addr,
> +                                      virDomainPCIConnectFlags flags,
> +                                      bool fromConfig)
> +{
> +    int ret = -1;
> +    char *addrStr = NULL;
> +    virErrorNumber errType = (fromConfig
> +                              ? VIR_ERR_XML_ERROR : VIR_ERR_INTERNAL_ERROR);
> +
> +    if (!(addrStr = virDomainPCIAddressAsString(addr)))
> +        goto cleanup;
> +
> +    /* Check that the requested bus exists, is the correct type, and we
> +     * are asking for a valid slot and function
> +     */
> +    if (!virDomainPCIAddressValidate(addrs, addr, addrStr, flags, fromConfig))
> +        goto cleanup;
> +
> +    if (*slot & (1 << addr->function)) {
> +        virReportError(errType,
> +                       _("Attempted double use of PCI Address %s"),
> +                       addrStr);
> +        goto cleanup;
> +    }
> +    *slot |= (1 << addr->function);
> +    if (addr->function == 0)
> +        addr->multi = VIR_TRISTATE_SWITCH_ON;
> +    VIR_DEBUG("Reserving PCI address %s", addrStr);
> +
> +    ret = 0;
> + cleanup:
> +    VIR_FREE(addrStr);
> +    return ret;
> +}
> +
> +
> +static int
> +virDomainPCIAddressAssignSlotNextFunction(virDomainPCIAddressSetPtr addrs,
> +                                          uint8_t *slot,
> +                                          virDomainDeviceInfoPtr dev,
> +                                          virDomainPCIConnectFlags flags)
> +{
> +    if (virDomainPCIAddressGetNextFunctionAddr(slot, &dev->addr.pci) < 0)
> +        return -1;
> +
> +    if (virDomainPCIAddressAssignFunctionAddr(addrs, slot, &dev->addr.pci, flags, false) < 0)
> +        return -1;
> +
> +    dev->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
> +
> +    return 0;
> +}
> +
> +
> +static int
> +virDomainPCIAddressAssignSlotAddr(virDomainPCIAddressSetPtr addrs,
> +                                  uint8_t *slot,
> +                                  virDomainDeviceInfoPtr dev)
> +{
> +    int ret = -1;
> +    char *addrStr = NULL;
> +    virDomainPCIConnectFlags flags = (VIR_PCI_CONNECT_HOTPLUGGABLE |
> +                                      VIR_PCI_CONNECT_TYPE_PCI_DEVICE);
> +
> +    if (!(addrStr = virDomainPCIAddressAsString(&dev->addr.pci)))
> +        goto cleanup;
> +
> +    if (dev->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
> +        if (((dev->addr.pci.function == 0) && (dev->addr.pci.multi == VIR_TRISTATE_SWITCH_ON)) ||
> +            dev->addr.pci.function != 0) {
> +
> +            if (!virDomainPCIAddressValidate(addrs, &dev->addr.pci,
> +                                             addrStr, flags, true))
> +                goto cleanup;
> +
> +            ret = virDomainPCIAddressAssignFunctionAddr(addrs, slot, &dev->addr.pci, flags, true);
> +        } else {
> +            virReportError(VIR_ERR_XML_ERROR,
> +                       _("Not a multifunction device address %s"), addrStr);
> +        }
> +    } else {
> +        ret = virDomainPCIAddressAssignSlotNextFunction(addrs, slot, dev, flags);
> +    }
> +
> + cleanup:
> +    VIR_FREE(addrStr);
> +    return ret;
> +}
> +
> +int
> +virDomainPCIMultifunctionDeviceAddressAssign(virDomainPCIAddressSetPtr addrs, virDomainDeviceDefListPtr devlist)
> +{
> +    size_t i;
> +    virPCIDeviceAddress addr;
> +    virDomainPCIAddressBusPtr bus;
> +    uint8_t slot;
> +    virDomainDeviceInfoPtr info = NULL, privinfo = NULL;
> +    virDomainPCIConnectFlags flags = VIR_PCI_CONNECT_TYPES_MASK;
> +
> +    if (virDomainPCIAddressGetNextSlot(addrs, &addr, flags) < 0)
> +        return -1;
> +
> +    bus = &addrs->buses[addr.bus];
> +    slot = bus->slots[addr.slot];
> +
> +    for (i = 0; i < devlist->count; i++) {
> +        virDomainDeviceDefPtr dev = devlist->devs[devlist->count - i - 1];
> +        switch ((virDomainDeviceType) dev->type) {
> +        case VIR_DOMAIN_DEVICE_DISK:
> +            info = &dev->data.disk->info;
> +            break;
> +        case VIR_DOMAIN_DEVICE_NET:
> +            info = &dev->data.net->info;
> +            break;
> +        case VIR_DOMAIN_DEVICE_RNG:
> +            info = &dev->data.rng->info;
> +            break;
> +        case VIR_DOMAIN_DEVICE_HOSTDEV:
> +            info = dev->data.hostdev->info;
> +            break;
> +        case VIR_DOMAIN_DEVICE_CONTROLLER:
> +            info = &dev->data.controller->info;
> +            break;
> +        case VIR_DOMAIN_DEVICE_CHR:
> +            info = &dev->data.chr->info;
> +            break;
> +        case VIR_DOMAIN_DEVICE_FS:
> +        case VIR_DOMAIN_DEVICE_INPUT:
> +        case VIR_DOMAIN_DEVICE_SOUND:
> +        case VIR_DOMAIN_DEVICE_VIDEO:
> +        case VIR_DOMAIN_DEVICE_WATCHDOG:
> +        case VIR_DOMAIN_DEVICE_HUB:
> +        case VIR_DOMAIN_DEVICE_SMARTCARD:
> +        case VIR_DOMAIN_DEVICE_MEMBALLOON:
> +        case VIR_DOMAIN_DEVICE_NVRAM:
> +        case VIR_DOMAIN_DEVICE_SHMEM:
> +        case VIR_DOMAIN_DEVICE_LEASE:
> +        case VIR_DOMAIN_DEVICE_REDIRDEV:
> +        case VIR_DOMAIN_DEVICE_MEMORY:
> +        case VIR_DOMAIN_DEVICE_NONE:
> +        case VIR_DOMAIN_DEVICE_TPM:
> +        case VIR_DOMAIN_DEVICE_PANIC:
> +        case VIR_DOMAIN_DEVICE_GRAPHICS:
> +        case VIR_DOMAIN_DEVICE_LAST:
> +            break;
> +        }
> +
> +        if (i == 0 && info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
> +           /* User has given an address in xml */
> +            bus = &addrs->buses[info->addr.pci.bus];
> +            slot = bus->slots[info->addr.pci.slot];
> +        }
> +
> +        if (privinfo) {
> +            if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
> +                if (privinfo->addr.pci.bus != info->addr.pci.bus ||
> +                    privinfo->addr.pci.slot != info->addr.pci.slot) {
> +                    virReportError(VIR_ERR_XML_ERROR, "%s",
> +                               _("Multifunction PCI device "
> +                                 "cant be split across different guest PCI slots"));
> +                    return -1;
> +                }
> +            }
> +        }
> +
> +        if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
> +            info->addr.pci.bus = addr.bus;
> +            info->addr.pci.slot = addr.slot;
> +            info->addr.pci.domain = addr.domain;
> +        }
> +
> +        if (virDomainPCIAddressAssignSlotAddr(addrs, &slot, info) < 0)
> +            return -1;
> +        privinfo = info;
> +    }
> +
> +    return 0;
> +}
> +
> +
>   int
>   virDomainPCIAddressEnsureAddr(virDomainPCIAddressSetPtr addrs,
>                                 virDomainDeviceInfoPtr dev)
> diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h
> index f3eda89..9eb6b9d 100644
> --- a/src/conf/domain_addr.h
> +++ b/src/conf/domain_addr.h
> @@ -157,6 +157,10 @@ int virDomainPCIAddressReserveNextSlot(virDomainPCIAddressSetPtr addrs,
>                                          virDomainPCIConnectFlags flags)
>       ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
>   
> +int
> +virDomainPCIMultifunctionDeviceAddressAssign(virDomainPCIAddressSetPtr addrs,
> +                                             virDomainDeviceDefListPtr devlist);
> +
>   struct _virDomainCCWAddressSet {
>       virHashTablePtr defined;
>       virDomainDeviceCCWAddress next;
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index d6a60b5..d72dc63 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -109,6 +109,7 @@ virDomainPCIAddressSetGrow;
>   virDomainPCIAddressSlotInUse;
>   virDomainPCIAddressValidate;
>   virDomainPCIControllerModelToConnectType;
> +virDomainPCIMultifunctionDeviceAddressAssign;
>   virDomainPCIMultifunctionDeviceDefParseNode;
>   virDomainVirtioSerialAddrAssign;
>   virDomainVirtioSerialAddrAutoAssign;
>
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>




More information about the libvir-list mailing list