Mark Wu has posted comments on this change.
Change subject: Don't crash on libvirt network re-definition.
......................................................................
Patch Set 6: I would prefer that you didn't submit this
(2 inline comments)
sorry for reviewing it so late. it looks good to me except a couple of minor
issues.
....................................................
File vdsm/configNetwork.py
Line 407: for file_path in glob.iglob(netinfo.NET_CONF_PREF + '*'):
Line 408: with open(file_path, 'r') as conf_file:
Line 409: for line in conf_file:
Line 410: if line.startswith('BRIDGE=' + network):
Line 411: port = file_path[file_path.rindex('-') + 1:]
It seems you break vdsm variable naming convention. You used 'file_path' and
'libvirt_nets'. is it better to change to 'filePath' and 'libvirtNets' to
comply with the existing code style? @Dan, I would like to get your opinion
on it.
Line 412: logging.debug('port %s found in ifcfg for
%s', port,
Line 413: network)
Line 414: ports.append(port)
Line 415: return ports
Line 410: if line.startswith('BRIDGE=' + network):
Line 411: port = file_path[file_path.rindex('-') + 1:]
Line 412: logging.debug('port %s found in ifcfg for
%s', port,
Line 413: network)
Line 414: ports.append(port)
you should stop going through the remained lines in this cfg file, because you
already confirm it's a port of that bridge. As for the outer loop in line 407,
I think you can break too. Currently, we just use one nic or one bond as the
bridge's port connecting to physical network. So even for a broken network, it
should only have one port enslaved.
Line 415: return ports
Line 416:
Line 417: def writeConfFile(self, fileName, configuration):
Line 418: '''Backs up the previous contents of the file referenced by
fileName
--
To view, visit http://gerrit.ovirt.org/10901
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I76d95e20b7aa99280e604abdb1663c6c5c7dd32e
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Antoni Segura Puimedon <[email protected]>
Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Igor Lvovsky <[email protected]>
Gerrit-Reviewer: Mark Wu <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
