7:27 a.m.
Dan Kenigsberg has uploaded a new change for review.
Change subject: confNet: ifcfg's VLAN should be set only on vlan cfg
......................................................................
confNet: ifcfg's VLAN should be set only on vlan cfg
If a misguided script calls addNetwork with explicit VLAN option, it
sohuld not be blindly copied to the top-most device.
We set VLAN=yes on the ifcfg script for the vlan device, and that's
that.
Change-Id: Ia2c9d6c86e73104424ca8ac901bf09e4b98a47ab
Bug-Id: BZ#847733
Signed-off-by: Dan Kenigsberg <danken(a)redhat.com>
---
M vdsm/configNetwork.py
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/11/7411/1
diff --git a/vdsm/configNetwork.py b/vdsm/configNetwork.py
index 3f99a66..2ef0a8a 100755
--- a/vdsm/configNetwork.py
+++ b/vdsm/configNetwork.py
@@ -384,7 +384,7 @@
if mtu:
cfg = cfg + 'MTU=%d\n' % mtu
cfg += 'NM_CONTROLLED=no\n'
- BLACKLIST = ['TYPE', 'NAME', 'DEVICE',
'bondingOptions',
+ BLACKLIST = ['TYPE', 'NAME', 'DEVICE', 'VLAN',
'bondingOptions',
'force', 'blockingdhcp',
'connectivityCheck', 'connectivityTimeout',
'implicitBonding']
--
To view, visit http://gerrit.ovirt.org/7411
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia2c9d6c86e73104424ca8ac901bf09e4b98a47ab
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
2:40 p.m.
Dan Kenigsberg has posted comments on this change.
Change subject: confNet: ifcfg's VLAN should be set only on vlan cfg
......................................................................
Patch Set 1:
You have raised a valid issue here. The original intention was to keep vdsm's API as a
stupid pass-through on top of initscripts. E.g. if someone ever wanted to set IPV6ADDR,
they could do it with no changes to vdsm. However, the code has rotten. The pass-through
should have stopped at the top-level device (bridge), but currently "options" is
duplicated to all ifcfg files, which is mostly useless.
We need to clear this mess, one way or another. Going towards a stricter, well-defined,
API is what we need, anyway. Particularly, if we are to re-implement network config over
netcf/NM
--
To view, visit http://gerrit.ovirt.org/7411
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia2c9d6c86e73104424ca8ac901bf09e4b98a47ab
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
9:46 p.m.
Mark Wu has posted comments on this change.
Change subject: confNet: ifcfg's VLAN should be set only on vlan cfg
......................................................................
Patch Set 1: I would prefer that you didn't submit this
After looking at the code, I found this problem could be cause by any wrong option
specified by the misguided script. Why do we only blacklist "VLAN"? How about
covert the BLACKLIST into WHITELIST? Actually, I am wondering if there's any option
need passthrough vdsm and be written into the the conf file. If no, we should remove the
parameter 'kwargs' from _createConfFile or even addNic, addVlan, addBonding and
addBridge.
--
To view, visit http://gerrit.ovirt.org/7411
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia2c9d6c86e73104424ca8ac901bf09e4b98a47ab
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
8:24 a.m.
Igor Lvovsky has posted comments on this change.
Change subject: confNet: ifcfg's VLAN should be set only on vlan cfg
......................................................................
Patch Set 1: Looks good to me, approved
--
To view, visit http://gerrit.ovirt.org/7411
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia2c9d6c86e73104424ca8ac901bf09e4b98a47ab
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
8:14 a.m.
Antoni Segura Puimedon has posted comments on this change.
Change subject: confNet: ifcfg's VLAN should be set only on vlan cfg
......................................................................
Patch Set 1: (1 inline comment)
small typo fix in the commit message.
....................................................
Commit Message
Line 6:
Line 7: confNet: ifcfg's VLAN should be set only on vlan cfg
Line 8:
Line 9: If a misguided script calls addNetwork with explicit VLAN option, it
Line 10: sohuld not be blindly copied to the top-most device.
s/sohuld/should/
Line 11: We set VLAN=yes on the ifcfg script for the vlan device, and that's
Line 12: that.
Line 13:
Line 14: Change-Id: Ia2c9d6c86e73104424ca8ac901bf09e4b98a47ab
--
To view, visit http://gerrit.ovirt.org/7411
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia2c9d6c86e73104424ca8ac901bf09e4b98a47ab
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
7:47 a.m.
oVirt Jenkins CI Server has posted comments on this change.
Change subject: confNet: ifcfg's VLAN should be set only on vlan cfg
......................................................................
Patch Set 1:
Build Successful
http://jenkins.ovirt.info/job/patch_vdsm_unit_tests/590/ : SUCCESS
--
To view, visit http://gerrit.ovirt.org/7411
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia2c9d6c86e73104424ca8ac901bf09e4b98a47ab
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
12:32 a.m.
Dan Kenigsberg has abandoned this change.
Change subject: confNet: ifcfg's VLAN should be set only on vlan cfg
......................................................................
Abandoned
Completely out of date
--
To view, visit http://gerrit.ovirt.org/7411
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: abandon
Gerrit-Change-Id: Ia2c9d6c86e73104424ca8ac901bf09e4b98a47ab
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
8:22 a.m.
Antoni Segura Puimedon has posted comments on this change.
Change subject: confNet: ifcfg's VLAN should be set only on vlan cfg
......................................................................
Patch Set 3: Code-Review+1
--
To view, visit http://gerrit.ovirt.org/7411
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia2c9d6c86e73104424ca8ac901bf09e4b98a47ab
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Moti Asayag <masayag(a)redhat.com>
Gerrit-Reviewer: Ondřej Svoboda <osvoboda(a)redhat.com>
Gerrit-HasComments: No
9:58 a.m.
Dan Kenigsberg has submitted this change and it was merged.
Change subject: confNet: ifcfg's VLAN should be set only on vlan cfg
......................................................................
confNet: ifcfg's VLAN should be set only on vlan cfg
If a misguided script calls addNetwork with explicit VLAN option, it
should not be blindly copied to the top-most device.
We set VLAN=yes on the ifcfg script for the vlan device, and that's
that.
This is a very limited hack of a wider problem: unrecognized network
options are copied to the bridge's ifcfg file.
Bug-Url: https://bugzilla.redhat.com/1051150
Change-Id: Ia2c9d6c86e73104424ca8ac901bf09e4b98a47ab
Signed-off-by: Dan Kenigsberg <danken(a)redhat.com>
Reviewed-on: http://gerrit.ovirt.org/7411
Reviewed-by: Antoni Segura Puimedon <asegurap(a)redhat.com>
---
M vdsm/netconf/ifcfg.py
1 file changed, 1 insertion(+), 1 deletion(-)
Approvals:
Antoni Segura Puimedon: Looks good to me, but someone else must approve
Dan Kenigsberg: Verified; Looks good to me, approved
--
To view, visit http://gerrit.ovirt.org/7411
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ia2c9d6c86e73104424ca8ac901bf09e4b98a47ab
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Moti Asayag <masayag(a)redhat.com>
Gerrit-Reviewer: Ondřej Svoboda <osvoboda(a)redhat.com>
9:58 a.m.
Dan Kenigsberg has posted comments on this change.
Change subject: confNet: ifcfg's VLAN should be set only on vlan cfg
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.ovirt.org/7411
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia2c9d6c86e73104424ca8ac901bf09e4b98a47ab
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Moti Asayag <masayag(a)redhat.com>
Gerrit-Reviewer: Ondřej Svoboda <osvoboda(a)redhat.com>
Gerrit-HasComments: No
7 a.m.
oVirt Jenkins CI Server has posted comments on this change.
Change subject: confNet: ifcfg's VLAN should be set only on vlan cfg
......................................................................
Patch Set 3: Verified-1
Build Failed
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7338/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6436/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7220/ : SUCCESS
--
To view, visit http://gerrit.ovirt.org/7411
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia2c9d6c86e73104424ca8ac901bf09e4b98a47ab
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Moti Asayag <masayag(a)redhat.com>
Gerrit-Reviewer: Ondřej Svoboda <osvoboda(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
2:51 p.m.
Moti Asayag has posted comments on this change.
Change subject: confNet: ifcfg's VLAN should be set only on vlan cfg
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.ovirt.org/#/c/7411/2//COMMIT_MSG
Commit Message:
Line 6:
Line 7: confNet: ifcfg's VLAN should be set only on vlan cfg
Line 8:
Line 9: If a misguided script calls addNetwork with explicit VLAN option, it
Line 10: sohuld not be blindly copied to the top-most device.
s/sohuld/should
Line 11: We set VLAN=yes on the ifcfg script for the vlan device, and that's
Line 12: that.
Line 13:
Line 14: This is a very limitted hack of a wider problem: unrecongnized network
--
To view, visit http://gerrit.ovirt.org/7411
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia2c9d6c86e73104424ca8ac901bf09e4b98a47ab
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Moti Asayag <masayag(a)redhat.com>
Gerrit-Reviewer: Ondřej Svoboda <osvoboda(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
5:05 a.m.
Dan Kenigsberg has posted comments on this change.
Change subject: confNet: ifcfg's VLAN should be set only on vlan cfg
......................................................................
Patch Set 2: Verified+1
--
To view, visit http://gerrit.ovirt.org/7411
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia2c9d6c86e73104424ca8ac901bf09e4b98a47ab
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
5:02 p.m.
oVirt Jenkins CI Server has posted comments on this change.
Change subject: confNet: ifcfg's VLAN should be set only on vlan cfg
......................................................................
Patch Set 2:
Build Successful
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7318/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6416/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7200/ : SUCCESS
--
To view, visit http://gerrit.ovirt.org/7411
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia2c9d6c86e73104424ca8ac901bf09e4b98a47ab
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
4:49 p.m.
Dan Kenigsberg has restored this change.
Change subject: confNet: ifcfg's VLAN should be set only on vlan cfg
......................................................................
Restored
it's very partial, but let us restore it.
--
To view, visit http://gerrit.ovirt.org/7411
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: restore
Gerrit-Change-Id: Ia2c9d6c86e73104424ca8ac901bf09e4b98a47ab
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
3745
days inactive
4292
days old
vdsm-patches@lists.fedorahosted.org
14 comments
6 participants
participants (6)
-
asegurap＠redhat.com -
Dan Kenigsberg -
Igor Lvovsky -
Jenkins CI RO -
masayag＠redhat.com -
wudxw＠linux.vnet.ibm.com