Change in vdsm[master]: netlink: Introduce socket pool

nsoffer at redhat.com nsoffer at redhat.com
Tue Feb 18 15:40:50 UTC 2014


Nir Soffer has posted comments on this change.

Change subject: netlink: Introduce socket pool
......................................................................


Patch Set 3:

(2 comments)

Nice except the unneeded possibly not thread safe lazy initialization.

But I think the biggest issue is that this does not guarantee available socket to the sampling code, if for some reason there are multiple network calls that take all the available sockets.

So while the pool is nicely written, it may not be the solution we actually need.

http://gerrit.ovirt.org/#/c/24603/3/lib/vdsm/netlink.py
File lib/vdsm/netlink.py:

Line 132:                 _pool_lock.release()
Line 133:         else:
Line 134:             with _pool_lock:
Line 135:                 pass
Line 136:     return _pool
This lazy creation of the pool is the same code use to lazy-populate the pool before, but less nice, since now you have to deal with non-existing pool. So if we compare both options, the old version which hide the details in the pool was better.

But I don't see a need for this feature. Why should be create the sockets in a lazy manner, dealing with thread safety issues when we can create all of them on startup, when there is only one thread running?
Line 137: 
Line 138: 
Line 139: def _open_socket():
Line 140:     """Returns an open netlink socket."""


Line 150: 
Line 151: 
Line 152: def _close_socket(sock):
Line 153:     """Closes and frees the resources of the passed netlink socket. It does
Line 154:     not raise"""
This obviously raise AssertionError :-)
Line 155:     assert sock is not None
Line 156:     _nl_handle_destroy(sock)
Line 157: 
Line 158: 


-- 
To view, visit http://gerrit.ovirt.org/24603
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I657ac3d3e0c2661ce73bdef9aa807ead888a42eb
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list