Giuseppe Vallarelli has posted comments on this change. Change subject: netconf: Add dhcp support for iproute2 configurator ......................................................................
Patch Set 4: (2 comments) .................................................... File vdsm/netconf/iproute2.py Line 197: t = threading.Thread(target=self._dhclient, name='vdsm-dhclient-%s' Line 198: % self.iface) Line 199: t.daemon = True Line 200: t.start() Line 201: else: It wasn't clear to me till your explanation by looking a the current code. Line 202: rc, out, err = self._dhclient() Line 203: return rc Line 204: Line 205: def shutdown(self): Line 250: def ifup(self, iface): Line 251: ipwrapper.linkSet(['dev', iface.name, 'up']) Line 252: _, _, _, bootproto, async = iface.getIpConfig() Line 253: if bootproto == 'dhcp': Line 254: dhclient = DhcpClient(iface.name) Sure I will explain, when you have something like ifup(self, iface, dhcpclient) you make the dependency explicit, by doing so when we create some tests it's possible to provide not the real dhcpclient but a mock or a stub, we usually use MonkeyPatch decorators to do so, but sometimes it's not the optimal choice, I prefer the plain old composition more. Line 255: dhclient.start(async) Line 256: Line 257: def ifdown(self, iface): Line 258: ipwrapper.linkSet(['dev', iface.name, 'down']) -- To view, visit http://gerrit.ovirt.org/15492 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iea88e8693e47fa51edb89c33344332c88c5c964d Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu <[email protected]> Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]> Gerrit-Reviewer: Giuseppe Vallarelli <[email protected]> Gerrit-Reviewer: Mark Wu <[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
