Change in vdsm[master]: network: bridge inherits DHCP unique identifier from its DHC...
danken at redhat.com
danken at redhat.com
Wed Aug 19 09:32:14 UTC 2015
Dan Kenigsberg has posted comments on this change.
Change subject: network: bridge inherits DHCP unique identifier from its DHCP-enabled port
......................................................................
Patch Set 4: Code-Review-1
(2 comments)
I have minor comments to the code.
However, please handle - in a separate patch - Petr's comment. I think that we have a similar problem when we define a bridge on top of a bond on top of several nics, where one of them had dhclient running.
https://gerrit.ovirt.org/#/c/44539/4/vdsm/network/configurators/dhclient.py
File vdsm/network/configurators/dhclient.py:
Line 46: cgroup=DHCLIENT_CGROUP):
Line 47: self.iface = iface
Line 48: self.family = family
Line 49: self.default_route = default_route
Line 50: self.duid_source = None if duid_source is None else (
it is confusing to use the same name (duid_source) for both the source device name, and the source device lease file. would you please differentiate between the two?
Line 51: self.LEASE_FILE.format('' if family == 4 else '6', duid_source))
Line 52: self.pidFile = self.PID_FILE % (family, self.iface)
Line 53: if not os.path.exists(self.LEASE_DIR):
Line 54: os.mkdir(self.LEASE_DIR)
https://gerrit.ovirt.org/#/c/44539/4/vdsm/network/configurators/ifcfg.py
File vdsm/network/configurators/ifcfg.py:
Line 552: conf += 'STP=%s\n' % ('on' if bridge.stp else 'off')
Line 553: conf += 'ONBOOT=yes\n'
Line 554: if bridge.duid_source:
Line 555: conf += ('DHCLIENTARGS="-df /var/lib/dhclient/'
Line 556: 'dhclient--%s.lease"\n' % bridge.duid_source)
it is a bit odd that here you do not use the LEASE_DIR constant
Line 557:
Line 558: if 'custom' in opts and 'bridge_opts' in opts['custom']:
Line 559: opts['bridging_opts'] = opts['custom']['bridge_opts']
Line 560: self._createConfFile(conf, bridge.name, bridge.ipv4, bridge.ipv6,
--
To view, visit https://gerrit.ovirt.org/44539
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If91b46657be8a5a42a118b23155d059df379727e
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda <osvoboda at redhat.com>
Gerrit-Reviewer: Alona Kaplan <alkaplan at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Ido Barkan <ibarkan at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda <osvoboda at redhat.com>
Gerrit-Reviewer: Petr Horáček <phoracek at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list