Re: [libvirt] [PATCH 2/3] network: consolidated info log for all network allocate/free operations

There are three functions that deal with allocating and freeing devices from a networks netdev/pci device pool: network(Allocate|Notify|Release)ActualDevice(). These functions also maintain a counter of the number of domains currently using a network (regardless of whether or not that network uses a device pool). Each of these functions had multiple log messages (output using VIR_DEBUG) that were in slightly different formats and gave varying amounts of information. This patch creates a single function to log the pertinent information in a consistent manner for all three of these functions. Along with assuring that all the functions produce a consistent form of output (and making it simpler to change), it adds the MAC address of the domain interface involved in the operation, making it possible to verify which interface of which domain the operation is being done for (assuming that all MAC addresses are unique, of course). All of these messages are raised from DEBUG to INFO, since they don't happen that often (once per interface per domain/libvirtd start or domain stop), and can be very informative and helpful - eliminating the need to log debug level messages makes it much easier to sort these out. --- src/network/bridge_driver.c | 80 ++++++++++++++++++++++----------------------- 1 file changed, 39 insertions(+), 41 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index dcd38ed..c305f8e 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -3863,6 +3863,40 @@ int networkRegister(void) * driver, we will need to present these functions via a second * "backend" function table. */

+static void +networkLogAllocation(virNetworkDefPtr netdef, + virDomainNetType actualType, + virNetworkForwardIfDefPtr dev, + virDomainNetDefPtr iface, + bool inUse) +{ + char macStr[VIR_MAC_STRING_BUFLEN]; + const char *verb = inUse ? "using" : "releasing"; + + if (!dev) { + VIR_INFO("MAC %s %s network %s (%d connections)", + virMacAddrFormat(&iface->mac, macStr), verb, + netdef->name, netdef->connections); + } else { + /* mark the allocation */ + dev->connections++;

+ if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) { + VIR_INFO("MAC %s %s network %s (%d connections) " + "physical device %04x:%02x:%02x.%x (%d connections)", + virMacAddrFormat(&iface->mac, macStr), verb, + netdef->name, netdef->connections, + dev->device.pci.domain, dev->device.pci.bus, + dev->device.pci.slot, dev->device.pci.function, + dev->connections); + } else { + VIR_INFO("MAC %s %s network %s (%d connections) " + "physical device %s (%d connections)", + virMacAddrFormat(&iface->mac, macStr), verb, + netdef->name, netdef->connections, + dev->device.dev, dev->connections); + } + } +} /* networkAllocateActualDevice: * @dom: domain definition that @iface belongs to @@ -4236,23 +4270,8 @@ networkAllocateActualDevice(virDomainDefPtr dom, if (netdef) { netdef->connections++; - VIR_DEBUG("Using network %s, %d connections", - netdef->name, netdef->connections); - - if (dev) { - /* mark the allocation */ + if (dev) dev->connections++;

- if (actualType != VIR_DOMAIN_NET_TYPE_HOSTDEV) { - VIR_DEBUG("Using physical device %s, %d connections", - dev->device.dev, dev->connections); - } else { - VIR_DEBUG("Using physical device %04x:%02x:%02x.%x, connections %d", - dev->device.pci.domain, dev->device.pci.bus, - dev->device.pci.slot, dev->device.pci.function, - dev->connections); - } - } - /* finally we can call the 'plugged' hook script if any */ if (networkRunHook(network, dom, iface, VIR_HOOK_NETWORK_OP_IFACE_PLUGGED, @@ -4263,6 +4282,7 @@ networkAllocateActualDevice(virDomainDefPtr dom, dev->connections--; goto error; } + networkLogAllocation(netdef, actualType, dev, iface, true);

} ret = 0; @@ -4388,10 +4408,6 @@ networkNotifyActualDevice(virDomainDefPtr dom, netdef->name, actualDev); goto error; } - - VIR_DEBUG("Using physical device %s, connections %d", - dev->device.dev, dev->connections + 1); - } else /* if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) */ { virDomainHostdevDefPtr hostdev; @@ -4441,20 +4457,12 @@ networkNotifyActualDevice(virDomainDefPtr dom, dev->device.pci.slot, dev->device.pci.function); goto error; } - - VIR_DEBUG("Using physical device %04x:%02x:%02x.%x, connections %d", - dev->device.pci.domain, dev->device.pci.bus, - dev->device.pci.slot, dev->device.pci.function, - dev->connections); } success: netdef->connections++; if (dev) dev->connections++; - VIR_DEBUG("Using network %s, %d connections", - netdef->name, netdef->connections); - /* finally we can call the 'plugged' hook script if any */ if (networkRunHook(network, dom, iface, VIR_HOOK_NETWORK_OP_IFACE_PLUGGED, VIR_HOOK_SUBOP_BEGIN) < 0) { @@ -4464,6 +4472,7 @@ networkNotifyActualDevice(virDomainDefPtr dom, netdef->connections--; goto error; } + networkLogAllocation(netdef, actualType, dev, iface, true);

ret = 0; cleanup: @@ -4475,6 +4484,7 @@ networkNotifyActualDevice(virDomainDefPtr dom, } + /* networkReleaseActualDevice: * @dom: domain definition that @iface belongs to * @iface: a domain's NetDef (interface definition) @@ -4559,10 +4569,6 @@ networkReleaseActualDevice(virDomainDefPtr dom, netdef->name, actualDev); goto error; } - - VIR_DEBUG("Releasing physical device %s, connections %d", - dev->device.dev, dev->connections - 1); - } else /* if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) */ { virDomainHostdevDefPtr hostdev; @@ -4594,11 +4600,6 @@ networkReleaseActualDevice(virDomainDefPtr dom, hostdev->source.subsys.u.pci.addr.function); goto error; } - - VIR_DEBUG("Releasing physical device %04x:%02x:%02x.%x, connections %d", - dev->device.pci.domain, dev->device.pci.bus, - dev->device.pci.slot, dev->device.pci.function, - dev->connections - 1); } success: @@ -4606,13 +4607,10 @@ networkReleaseActualDevice(virDomainDefPtr dom, netdef->connections--; if (dev) dev->connections--; - - VIR_DEBUG("Releasing network %s, %d connections", - netdef->name, netdef->connections); - /* finally we can call the 'unplugged' hook script if any */ networkRunHook(network, dom, iface, VIR_HOOK_NETWORK_OP_IFACE_UNPLUGGED, VIR_HOOK_SUBOP_BEGIN);

+ networkLogAllocation(netdef, actualType, dev, iface, false);

} ret = 0; cleanup: