Change in vdsm[master]: Skip setting up network if no change is found.
asegurap at redhat.com
asegurap at redhat.com
Mon Sep 17 09:11:30 UTC 2012
Antoni Segura Puimedon has posted comments on this change.
Change subject: Skip setting up network if no change is found.
......................................................................
Patch Set 1: I would prefer that you didn't submit this
(4 inline comments)
Just a couple of issues, but I like the spirit of the patch.
....................................................
File vdsm/configNetwork.py
Line 1141: configWriter.restoreBackups()
Line 1142: return define.errCode['noConPeer']['status']['code']
Line 1143:
Line 1144:
Line 1145: class NetworksOptions():
Could we have NetworkDefinitions instead?
IMHO it makes it sound clearer if we replace options for definitions throughout the patch.
Line 1146:
Line 1147: def __init__(self):
Line 1148: self.curOptions = {}
Line 1149: self.oldOptions = {}
Line 1168: self.curOptions = self.oldOptions
Line 1169: self.oldOptions = {}
Line 1170:
Line 1171:
Line 1172: networksOptions = NetworksOptions()
I would lead this variable with an underscore to indicate that it is an implementation detail, not to be relied upon externally.
Line 1173:
Line 1174:
Line 1175: def _validateNetworkSetup(networks={}, bondings={}):
Line 1176: _netinfo = netinfo.NetInfo()
Line 1270:
Line 1271: del bondings[bond]
Line 1272:
Line 1273:
Line 1274: networksParams = {}
I can't see where this is used.
Line 1275:
Line 1276:
Line 1277: def setupNetworks(networks={}, bondings={}, **options):
Line 1278: """Add/Edit/Remove configuration for networks and bondings.
Line 1330:
Line 1331: force = options.get('force', False)
Line 1332: if not utils.tobool(force):
Line 1333: logging.debug("Validating configuration")
Line 1334: _validateNetworkSetup(networks, bondings)
According to the method docstring this is right. If we are sure nobody passes networks nor bondings as lists of tuples, I am enthusiastically for this change.
Line 1335:
Line 1336: logger.debug("Applying...")
Line 1337: try:
Line 1338: # Remove edited networks and networks with 'remove' attribute
--
To view, visit http://gerrit.ovirt.org/7984
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I364b2edcd9e44112dcab8f2f27dbbbaa74224093
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu <wudxw at linux.vnet.ibm.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap at redhat.com>
More information about the vdsm-patches
mailing list