[libvirt] [PATCH v4 08/25] conf: new function virDomainPCIAddressReserveNextAddr()

[Andrea Bolognani]

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