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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list