On 08/28/2013 11:39 AM, Laine Stump wrote:
This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=983026
The netcf interface driver previously had no state driver associated with it - as a connection was opened, it would create a new netcf instance just for that connection, and close it when it was finished. the problem with this is that each connection to libvirt used up a netlink socket, and there is a per process maximum of ~1000 netlink sockets.
The solution is to create a state driver to go along with the netcf driver. The state driver will open a netcf instance, then all connections will share that same netcf instance, thus only a single netlink socket will be used no matter how many connections are mde to libvirtd.
This was rather simple to do - most of the functionality from netcfInterfaceOpen() was moved to netcfStateInitialize (which initializes a single global driverState), and netcfInterfaceOpen now just puts a pointer to driverState into the privateData. A similar change was mad eto netcfStateCleanup() vs netcfInterfaceClose(). Since all the functions already have locking around them, no other change was necessary (they now use the single global lock rather than a lock specific to their connection).
I'm torn on whether this patch qualifies as being safe enough to apply after freeze.
--- src/interface/interface_backend_netcf.c | 157 ++++++++++++++++++++++++-------- 1 file changed, 117 insertions(+), 40 deletions(-)
diff --git a/src/interface/interface_backend_netcf.c b/src/interface/interface_backend_netcf.c index f47669e..1b5c850 100644 --- a/src/interface/interface_backend_netcf.c +++ b/src/interface/interface_backend_netcf.c @@ -43,8 +43,10 @@ typedef struct { virMutex lock; struct netcf *netcf; + size_t nConnections; } virNetcfDriverState, *virNetcfDriverStatePtr;
+virNetcfDriverStatePtr driverState = NULL;
Any reason this is not static?
static void interfaceDriverLock(virNetcfDriverStatePtr driver) { @@ -56,6 +58,98 @@ static void interfaceDriverUnlock(virNetcfDriverStatePtr driver) virMutexUnlock(&driver->lock); }
+static int +netcfStateInitialize(bool privileged ATTRIBUTE_UNUSED, + virStateInhibitCallback callback ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED) +{ + if (VIR_ALLOC(driverState) < 0) + goto alloc_error; + + /* initialize non-0 stuff in driverState */ + if (virMutexInit(&driverState->lock) < 0) + { + /* what error to report? */ + goto mutex_error;
VIR_ERR_INTERNAL_ERROR is probably as good as any, since this is unlikely.
+ } + + /* open netcf */ + if (ncf_init(&driverState->netcf, NULL) != 0) + { + /* what error to report? */ + goto netcf_error;
Probably here as well.
-static virDrvOpenStatus netcfInterfaceOpen(virConnectPtr conn, - virConnectAuthPtr auth ATTRIBUTE_UNUSED, - unsigned int flags) +static virDrvOpenStatus +netcfInterfaceOpen(virConnectPtr conn, + virConnectAuthPtr auth ATTRIBUTE_UNUSED, + unsigned int flags) {
conn->interfacePrivateData = driverState; + interfaceDriverLock(driverState); + driverState->nConnections++; + interfaceDriverUnlock(driverState);
Is it worth switching things to use virObject with atomic refcounting that doesn't require a lock? But that can probably be a separate patch. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org