Dan Kenigsberg has posted comments on this change. Change subject: net: Use VLAN ID as integer across VDSM ......................................................................
Patch Set 1: Code-Review-1 (3 comments) https://gerrit.ovirt.org/#/c/51802/1/lib/vdsm/netconfpersistence.py File lib/vdsm/netconfpersistence.py: Line 107: class Config(BaseConfig): Line 108: def __init__(self, savePath): Line 109: self.networksPath = os.path.join(savePath, 'nets', '') Line 110: self.bondingsPath = os.path.join(savePath, 'bonds', '') Line 111: super(Config, self).__init__(self._getConfigs(self.networksPath), I suspect we have to handle the upgrade case: what happens if the persisted config has str(vlan)? we should translate it here to int. P.S, does this work with MTU? Line 112: self._getConfigs(self.bondingsPath)) Line 113: Line 114: def delete(self): Line 115: self.networks = {} Line 222: '' here, vlan may be either an int, or the empty string. this is not very pretty. I think that vlan_suff = str(network.get('vlan', '')) would be nicer. https://gerrit.ovirt.org/#/c/51802/1/lib/vdsm/network/api.py File lib/vdsm/network/api.py: Line 594: networkAttrs['vlan'] wouldn't this explode for non-vlan networks? -- To view, visit https://gerrit.ovirt.org/51802 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia3a0133560a6946c3438161fb1696f002f2cdb21 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Edward Haas <edwa...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
