Dan Kenigsberg has posted comments on this change.
Change subject: Unified network persistence [1/3] - Save running config
......................................................................
Patch Set 22: Code-Review-1
(6 comments)
....................................................
File lib/vdsm/unifiedpersistence.py
Line 14: # You should have received a copy of the GNU General Public License
Line 15: # along with this program; if not, write to the Free Software
Line 16: # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
Line 17: #
Line 18: # Refer to the README and COPYING files for full details of the license
I'm still unhappy with the module name. It sits on the top of the vdsm package, and
may make people think that it is some kind of a vdsm-wide facility, while it is very
specific and netconf-related. How about "netconfpersistence" instead?
Line 19: #
Line 20:
Line 21: import errno
Line 22: import json
Line 39:
Line 40: def __eq__(self, other):
Line 41: return self.networks == other.networks and self.bonds == other.bonds
Line 42:
Line 43: def __repr(self):
that's a funny-looking function.
Line 44: return '%s(%s, %s)' % (self.__class__.__name__, self.networks,
Line 45: self.bonds)
Line 46:
Line 47: def _networkPath(self, network):
Line 70: fullPath = path + fileName
Line 71: networkEntities[fileName] = self._getConfigDict(fullPath)
Line 72: except OSError as ose:
Line 73: if ose.errno == errno.ENOENT:
Line 74: logging.debug('Unified persistence: No existent config
set.')
English: non-exsiting config set
Line 75: else:
Line 76: raise
Line 77:
Line 78: return networkEntities
Line 78: return networkEntities
Line 79:
Line 80: def _setConfig(self, config, path):
Line 81: dirPath = os.path.dirname(path)
Line 82: if not os.path.exists(dirPath):
Why do we need to create the directories during runtime? If we have to do that, let's
do it in a thread-safe manner
try:
os.makedirs(dirPath)
except OSError as e:
if e.errono != ENOENT:
raise
Line 83: os.makedirs(dirPath)
Line 84: with open(path, 'w') as configurationFile:
Line 85: json.dump(config, configurationFile)
Line 86:
Line 85: json.dump(config, configurationFile)
Line 86:
Line 87: def _removeConfig(self, path):
Line 88: try:
Line 89: os.unlink(path)
that's pretty much utils.rmFile().
Line 90: except OSError as ose:
Line 91: if ose.errno == errno.ENOENT:
Line 92: logging.debug('Unified persistence: Network entity at %s not
'
Line 93: 'found for removal' % path)
Line 100: if value is not None and key not in
Line 101: ('configurator', '_netinfo',
'force',
Line 102: 'implicitBonding'))
Line 103: self.networks[network] = cleanAttrs
Line 104: logging.info('Unified persistence: Adding network %s(%s)' %
I do not understand why the "Unified persistence: " prefix is useful here. we
log the module name, anyway.
Line 105: (network, cleanAttrs))
Line 106:
Line 107: def removeNetwork(self, network):
Line 108: try:
--
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: 22
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