Antoni Segura Puimedon has posted comments on this change. Change subject: netlink: event monitor ......................................................................
Patch Set 35: Code-Review-1 (5 comments) http://gerrit.ovirt.org/#/c/32626/35/lib/vdsm/netlink/__init__.py File lib/vdsm/netlink/__init__.py: Line 160: [0] * (15 - len(groups_codes)) Try without filling the array, I think it will probably work. http://gerrit.ovirt.org/#/c/32626/35/lib/vdsm/netlink/monitor.py File lib/vdsm/netlink/monitor.py: Line 173: return this return should not be here. Line 176: nl_obje In line 205 you are rightfully passing a closure of _object input as the nl_object parameter. This name for the parameter is wrong, since what it needs is not an nl_object but a object processing callback. Find a nicer name ;-) Line 212: while not self._stop.is_set(): Line 213: if self._timeout: Line 214: if (time.time() - self._timeStart > self._timeout): Line 215: self._stop.set() Line 216: self._queue.put_nowait(TimeoutError) TimeoutError() so we pass an instance of the exception. Line 217: continue Line 218: fd = _nl_socket_get_fd(self.sock) Line 219: if select.select([fd], [], [], 0.1) != ([], [], []): Line 220: _nl_recvmsgs_default(self.sock) Line 223: _close_socket(self.sock) Line 224: Line 225: _nl_object_get_msgtype = _int_proto(('nl_object_get_msgtype', LIBNL)) Line 226: _nl_nlmsgtype2str = CFUNCTYPE(c_char_p, c_int, c_char_p, c_size_t)( Line 227: ('nl_nlmsgtype2str', LIBNL)) You're not using these above anymore. Remove what is not used. -- To view, visit http://gerrit.ovirt.org/32626 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I23ea72986564c5a115e36be0e7cf679c28c8ea96 Gerrit-PatchSet: 35 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček <[email protected]> Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
