[libvirt] [PATCHv2] netcf driver: use a single netcf handle for all connections

[Michal Privoznik]

On 11.09.2013 17:37, Doug Goldstein wrote:
> On Wed, Sep 11, 2013 at 10:06 AM, Laine Stump <laine at laine.org> 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