On Wed, Sep 11, 2013 at 10:43 AM, Michal Privoznik <mprivozn@xxxxxxxxxx> wrote: > On 11.09.2013 17:37, Doug Goldstein wrote: >> On Wed, Sep 11, 2013 at 10:06 AM, Laine Stump <laine@xxxxxxxxx> 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 opens a netcf instance, then all >>> connections 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 - a new virObjectLockable class is >>> created for the single driverState object, which is created in >>> netcfStateInitialize and contains the single netcf handle; instead of >>> creating a new object for each client connection, netcfInterfaceOpen >>> now just increments the driverState object's reference count and puts >>> a pointer to it into the connection's privateData. Similarly, >>> netcfInterfaceClose() just un-refs the driverState object (as does >>> netcfStateCleanup()), and virNetcfInterfaceDriverStateDispose() >>> handles closing the netcf instance. Since all the functions already >>> have locking around them, the static lock functions used by all >>> functions just needed to be changed to call virObjectLock() and >>> virObjectUnlock() instead of directly calling the virMutex* functions. >>> --- >>> Changes from V1: >>> >>> * make driverState a static. >>> >>> * switch to using a virObjectLockable for driverState, at >>> Eric's suggestion. >>> >>> * add a simple error message if ncf_init() fails. >>> >>> Again, I've tried this with a small number of simultaneous connections >>> (including virt-manager), but I don't have a ready-made stress test. >>> >>> >>> src/interface/interface_backend_netcf.c | 173 +++++++++++++++++++++++--------- >>> 1 file changed, 125 insertions(+), 48 deletions(-) >>> >>> diff --git a/src/interface/interface_backend_netcf.c b/src/interface/interface_backend_netcf.c >>> index f47669e..627c225 100644 >>> --- a/src/interface/interface_backend_netcf.c >>> +++ b/src/interface/interface_backend_netcf.c >>> @@ -41,19 +41,119 @@ >>> /* Main driver state */ >>> typedef struct >>> { >>> - virMutex lock; >>> + virObjectLockable parent; >>> struct netcf *netcf; >>> } virNetcfDriverState, *virNetcfDriverStatePtr; >>> >>> +static virClassPtr virNetcfDriverStateClass; >>> +static void virNetcfDriverStateDispose(void *obj); >>> >>> -static void interfaceDriverLock(virNetcfDriverStatePtr driver) >>> +static int >>> +virNetcfDriverStateOnceInit(void) >>> +{ >>> + if (!(virNetcfDriverStateClass = virClassNew(virClassForObjectLockable(), >>> + "virNetcfDriverState", >>> + sizeof(virNetcfDriverState), >>> + virNetcfDriverStateDispose))) >>> + return -1; >>> + return 0; >> >> Bad spacing? >> >>> +} >>> + >>> +VIR_ONCE_GLOBAL_INIT(virNetcfDriverState) >>> + >>> +static virNetcfDriverStatePtr driverState = NULL; > >> >> So my understanding of libvirt's object and locking code isn't too >> good so I hope someone else will review. But I don't see >> virNetcfDriverStateOnceInit() called anywhere and I don't see >> virNetcfDriverStateInitialize() defined anywhere? Are those suppose to >> be one in the same or is there some magical macro expansion going on >> somewhere that I just don't know about. >> > > The VIR_ONCE_GLOBAL_INIT(virDummy) macro creates a function called > virDummyInitialize() which when called for the 1st time calls > virDummyOnceInit(). Any subsequent call to virDummyInitialize() results > in immediate return. > > Michal > Thanks Michal. ACK this patch. And ignore the spacing comment. Gmail ate some white space. -- Doug Goldstein -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list