Assaf Muller has posted comments on this change.
Change subject: Unified network persistence [1/3] - Save running config
......................................................................
Patch Set 21: Code-Review-1
(2 comments)
Inline comments. -1 just for visibility.
....................................................
File lib/vdsm/unifiedpersistence.py
Line 92: logging.debug('Unified persistence: Network entity at %s not
'
Line 93: 'found for removal' % path)
Line 94: else:
Line 95: raise
Line 96:
You deleted getNetwork and getNetworks but they're used in later patches.
Line 97: def setNetwork(self, network, attributes):
Line 98: # Clean netAttrs from fields that should not be serialized
Line 99: cleanAttrs = dict((key, value) for key, value in attributes.iteritems()
Line 100: if value is not None and key not in
Line 144: def clear(self):
Line 145: self.networks = {}
Line 146: self.bonds = {}
Line 147: self._clearDisk()
Line 148:
If this method is called save, then clear should probably be named delete? Since these two
methods are opposite and the two opposing words commonly used are save and delete, not
save and clear.
Line 149: def save(self):
Line 150: self._clearDisk()
Line 151: for bond, attrs in self.bonds.iteritems():
Line 152: self._setConfig(attrs, self._bondingPath(bond))
--
To view, visit
http://gerrit.ovirt.org/16699
To unsubscribe, visit
http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I7137a96f84abd2c5e532c6c37737e36ef17567a9
Gerrit-PatchSet: 21
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Assaf Muller <amuller(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Giuseppe Vallarelli <gvallare(a)redhat.com>
Gerrit-Reviewer: Livnat Peer <lpeer(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Petr Šebek <psebek(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes