Re: [libvirt] [PATCH v3 3/8] conf: eliminate repetitive code in virDomainPCIAddressGetNextSlot()
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