On Fri, 2016-10-14 at 15:54 -0400, Laine Stump wrote:
There is an existing virDomainPCIAddressReserveNextSlot() which will reserve all functions of the next available PCI slot. One place in the qemu PCI address assignment code requires reserving a *single* function of the next available PCI slot. This patch modifies and renames virDomainPCIAddressReserveNextSlot() so that it can fulfill both the original purpose and the need to reserve a single function. (This is being done so that the abovementioned code in qemu can have its "kind of open coded" solution replaced with a call to this new function). --- Changes: Fixed indentation and comments, and removed unnecessary setting of lastaddr.function and lastaddr.multi. src/conf/domain_addr.c | 49 +++++++++++++++++++++++++++++++++++++++++++----- src/conf/domain_addr.h | 7 +++++++ src/libvirt_private.syms | 1 + 3 files changed, 52 insertions(+), 5 deletions(-) diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index 080d882..1710220 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -691,29 +691,68 @@ virDomainPCIAddressGetNextSlot(virDomainPCIAddressSetPtr addrs, return 0; } + +/** + * virDomainPCIAddressReserveNextAddr: + * + * find the next *completely unreserved* slot with compatible + * connection @flags, mark either one function or the entire + * slot as in-use (according to @function and @reserveEntireSlot), + * and set @dev->addr.pci with this newly reserved address.
This looks much much better...
+ * @addrs: a set of PCI addresses. + * + * @dev: virDomainDeviceInfo that should get the new address. + * + * @flags: CONNECT_TYPE flags for the device that needs an address. + * + * @function: which function on the slot to mark as reserved + * (if @reserveEntireSlot is false) + * + * @reserveEntireSlot: true to reserve all functions on the new slot, + * false to reserve just @function
... but these are still weird. Please take a look at include/libvirt/libvirt-domain.h to see what I mean. Interestingly, in that file the arguments are documented *before* describing the function itself. We should probably stick to that order here as well.
+ *
Extra line here, didn't notice it the first time around.
+ * + * returns 0 on success, or -1 on failure. + */ int -virDomainPCIAddressReserveNextSlot(virDomainPCIAddressSetPtr addrs, +virDomainPCIAddressReserveNextAddr(virDomainPCIAddressSetPtr addrs,
ACK after you tweak indentation for the comment :) -- Andrea Bolognani / Red Hat / Virtualization