Dan Kenigsberg has posted comments on this change.
Change subject: Multiple Gateways[6/2]: Swapped libvirt network & netEnt
creation
......................................................................
Patch Set 6: I would prefer that you didn't submit this
(3 inline comments)
The solution is ugly but unescapeable. Let us have the implementation prettier.
....................................................
File vdsm/sourceRoute.py
Line 32: from vdsm.utils import execCmd
Line 33: from vdsm.utils import CommandPath
Line 34:
Line 35:
Line 36: _TOUCH_BINARY = CommandPath('touch', '/bin/touch')
Using a binary is an overkill - better just open() the file for writing. I
suppose that it is my fault - I've used the "touch" term liberally.
Line 37: TRACKED_INTERFACES_FOLDER = P_VDSM_RUN + 'trackedInterfaces'
Line 38:
Line 39:
Line 40: class StaticSourceRoute(object):
Line 109:
Line 110: @staticmethod
Line 111: def removeInterfaceTracking(device):
Line 112: try:
Line 113:
os.remove(DynamicSourceRoute.getTrackingFilePath(device.name))
We have utils.rmFile for the exact purpose.
Line 114: except:
Line 115: pass
Line 116:
Line 117: @staticmethod
Line 179: self.configurator.removeSourceRoute(
Line 180: self._getRoutes(table, self.device), rules,
Line 181: self.device)
Line 182:
Line 183: def _isLibvirtInterfaceFallback(self):
Why did you move these methods? To make it easier to make changes without
anyone noticing? In such cases I prefer a gerrit comment explaining the move
and stating that there is no code change.
Line 184: """
Line 185: Checks whether the device belongs to libvirt when libvirt is
not yet
Line 186: running (network.service runs before libvirtd is started). To
do so,
Line 187: it must check if there is an autostart network that uses the
device.
--
To view, visit http://gerrit.ovirt.org/16692
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic5af431909427dc7488875f0cb14fe4361657a83
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Assaf Muller <amul...@redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegu...@redhat.com>
Gerrit-Reviewer: Assaf Muller <amul...@redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com>
Gerrit-Reviewer: Giuseppe Vallarelli <gvall...@redhat.com>
Gerrit-Reviewer: Mark Wu <wu...@linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
