[libvirt] [PATCH v3 3/8] conf: eliminate repetitive code in virDomainPCIAddressGetNextSlot()

[Andrea Bolognani]

On Mon, 2016-12-19 at 10:23 -0500, Laine Stump wrote:
> virDomainPCIAddressGetNextSlot() starts searching from the last
> allocated address and goes to the end of all the buses, then goes back
> to the first bus and searchs from there up to the starting point (in

s/searchs/searches/

[...]
> +static int
> +virDomainPCIAddressFindUnusedFunctionOnBus(virDomainPCIAddressBusPtr bus,
> +                                           virPCIDeviceAddressPtr searchAddr,
> +                                           int function ATTRIBUTE_UNUSED,
> +                                           virDomainPCIConnectFlags flags,
> +                                           bool *found)

Having some documentation for the function, especially
what parts of @searchAddr are inspected and updated, and
how @function is used - not at all right now, but that's
going to change later in the series - would be great.


Incidentally, and not relevant to this series, passing
around partially filled-in virPCIDeviceAddress instances
*and* some of the same data on the side sucks big time.

I wonder if the whole thing could be improved by changing
the struct members to to signed and having

  #define VIR_PCI_DEVICE_ADDRESS_UNDEFINED -1

or leave them as it is and use

  #define VIR_PCI_DEVICE_ADDRESS_UNDEFINED UINT_MAX

instead. Either way, we'd have a way to tell whether each
of the components has been initialized or is in need of a
value without having to pass data out-of-band.

> +{
> +    int ret = -1;
> +    char *addrStr = NULL;
> +
> +    *found = false;
> +
> +    if (!(addrStr = virDomainPCIAddressAsString(searchAddr)))
> +        goto cleanup;
> +
> +    if (!virDomainPCIAddressFlagsCompatible(searchAddr, addrStr, bus->flags,
> +                                            flags, false, false)) {
> +        VIR_DEBUG("PCI bus %.4x:%.2x is not compatible with the device",
> +                  searchAddr->domain, searchAddr->bus);
> +    } else {
> +        while (searchAddr->slot <= bus->maxSlot) {
> +            if (bus->slot[searchAddr->slot].functions == 0) {
> +                *found = true;
> +                break;
> +            }
> +
> +            VIR_DEBUG("PCI slot %.4x:%.2x:%.2x already in use",
> +                      searchAddr->domain, searchAddr->bus, searchAddr->slot);
> +            searchAddr->slot++;
> +        }
> +    }
> +
> +    ret = 0;

Empty line here.

> + cleanup:
> +    VIR_FREE(addrStr);
> +    return ret;
> +}


ACK

-- 
Andrea Bolognani / Red Hat / Virtualization