[libvirt] [PATCH v2 5/8] Introduce virDomainPCIMultifunctionDeviceAddressAssign
Shivaprasad bhat
shivaprasadbhat at gmail.com
Fri May 20 07:45:06 UTC 2016
On Thu, May 19, 2016 at 11:59 PM, Laine Stump <laine at laine.org> wrote:
> 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.
>
Missed to answer to this point. I am using the function numbers later with
the device_add. I need the function numbers to be passed along. So,
arriving at it is important here.
>
> 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
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20160520/23143723/attachment-0001.htm>
More information about the libvir-list
mailing list