Dan Kenigsberg has posted comments on this change. Change subject: BZ#846004 - IP on nic/bond should be preserved when VLAN added ......................................................................
Patch Set 3: I would prefer that you didn't submit this (2 inline comments) .................................................... File vdsm/configNetwork.py Line 371: cfg = """DEVICE=%s\nONBOOT=%s\n""" % (pipes.quote(name), Line 372: pipes.quote(onboot)) Line 373: cfg += conf Line 374: # Check whether we need to preserve prev ip/mask/gateway Line 375: if not ipaddr and os.path.exists(cf): it sound like a too-low level to preserve these values... Assume we have an ifcfg-eth0 with IPADDR defined. It would stay there, even if the nic is to be connected to a bridge, and no one wants it to have an IP address of its own. Line 376: ipaddr = self._getConfigValue(cf, 'IPADDR') Line 377: netmask = self._getConfigValue(cf, 'NETMASK') Line 378: gateway = self._getConfigValue(cf, 'GATEWAY') Line 379: Line 374: # Check whether we need to preserve prev ip/mask/gateway Line 375: if not ipaddr and os.path.exists(cf): Line 376: ipaddr = self._getConfigValue(cf, 'IPADDR') Line 377: netmask = self._getConfigValue(cf, 'NETMASK') Line 378: gateway = self._getConfigValue(cf, 'GATEWAY') btw, why only these params? shouldn't we maintain MTU, too? Line 379: Line 380: if ipaddr: Line 381: cfg = cfg + 'IPADDR=%s\nNETMASK=%s\n' % (pipes.quote(ipaddr), Line 382: pipes.quote(netmask)) -- To view, visit http://gerrit.ovirt.org/7259 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5a6481c64c285b8a9bd6f6cc49c1c84507c91603 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Igor Lvovsky <ilvov...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Igor Lvovsky <ilvov...@redhat.com> Gerrit-Reviewer: Livnat Peer <lp...@redhat.com> Gerrit-Reviewer: oVirt Jenkins CI Server _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
