Dan Kenigsberg has posted comments on this change. Change subject: added support for mirror promisc mode ......................................................................
Patch Set 8: I would prefer that you didn't submit this (4 inline comments) .................................................... File vdsm/supervdsmServer.py Line 186: Release captured mirror network traffic from network bridge this function simple unsets the promisc mode which setMirrorPromisc() has set? then unsetMirrorPromisc would be a clearer name. Line 197: raise Exception(e) raising a specific exception is better style. and I agree with Saggi that this handling of TC command line would look better in a function of its own. .................................................... File vdsm/tc.py Line 2: # Copyright 2009-2011 Red Hat, Inc. actually, this is new in 2012 Line 40: if hasattr(nic, 'promisc'): a nic is connected only to a single host network. so why promisc has a list of them? It should've been True/False. anyway, I do not understand the motivation of this class, and I am not sure it warrants its own module. -- To view, visit http://gerrit.ovirt.org/956 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I90f2f39c326528e76b10c68b1a101bd3ed7a20ec Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Saggi Mizrahi <[email protected]> Gerrit-Reviewer: Shahar Havivi <[email protected]> _______________________________________________ vdsm-patches mailing list [email protected] https://fedorahosted.org/mailman/listinfo/vdsm-patches
