Change in vdsm[master]: Don't down bonds and nics when adding vlan or resizing
gvallare at redhat.com
gvallare at redhat.com
Wed Aug 14 19:23:03 UTC 2013
Giuseppe Vallarelli has posted comments on this change.
Change subject: Don't down bonds and nics when adding vlan or resizing
......................................................................
Patch Set 9: Code-Review-1
(5 comments)
Very minor refinements.
....................................................
File lib/vdsm/netinfo.py
Line 218: return open(BONDING_SLAVES % bonding).readline().split()
Line 219:
Line 220:
Line 221: def bondOpts(bonding):
Line 222: """ Returns a dictionary of bond mode name and a values iterable."""
bond mode and which values ? Since you added the docstring better be complete :)
Thanks I appreciate.
Line 223: opts = {}
Line 224: for path in iglob(BONDING_OPTS % bonding):
Line 225: with open(path) as optFile:
Line 226: opts[os.path.basename(path)] = optFile.read().rstrip().split(' ')
Line 278: try:
Line 279: # nics() filters out OS devices (bonds, vlans, bridges)
Line 280: # operstat() filters out down/disabled nics
Line 281: # virtio is a valid device, but doesn't support speed
Line 282: if dev in nics() and operstate(dev) == OPERSTATE_UP and \
Nitpick: instead of \ use parenthesis for a longer explanation have a look at the link below:
http://www.python.org/dev/peps/pep-0008/#maximum-line-length
Line 283: not isvirtio(dev):
Line 284: # the device may have been disabled/downed after checking
Line 285: # so we validate the return value as sysfs may return
Line 286: # special values to indicate the device is down/disabled
....................................................
File tests/functional/networkTests.py
Line 47: restoreNetConfig()
Line 48:
Line 49:
Line 50: @contextmanager
Line 51: def nonChangingOperstate(device):
why in networkTest module and not utils?
what about constantOperstate instead of nonChaningOperstate?
Line 52: def changed(dev, changes):
Line 53: status = operstate(dev)
Line 54: while not done:
Line 55: newState = operstate(dev)
....................................................
File vdsm/netmodels.py
Line 80: super(Nic, self).__init__(name, configurator, ipconfig,
Line 81: mtu=max(mtu, netinfo.getMtu(name)))
Line 82:
Line 83: def configure(self, **opts):
Line 84: if not self.vlan or \
Parenthesis instead of \ :
http://www.python.org/dev/peps/pep-0008/#maximum-line-length
Line 85: netinfo.operstate(self.name) != netinfo.OPERSTATE_UP:
Line 86: self.configurator.configureNic(self, **opts)
Line 87:
Line 88: def remove(self):
Line 180: def __repr__(self):
Line 181: return 'Bond(%s: %r)' % (self.name, self.slaves)
Line 182:
Line 183: def configure(self, **opts):
Line 184: # When the bond is up and we are not changing the configuration that
At this point I think you can turn the comment into a docstring.
Line 185: # is already applied in any way, we can skip the configuring.
Line 186: if not(self.vlan and
Line 187: self.name in netinfo.bondings() and
Line 188: netinfo.operstate(self.name) == netinfo.OPERSTATE_UP and
--
To view, visit http://gerrit.ovirt.org/17491
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id3075a2e65f06416c840c6a98689b77555b22e5d
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Assaf Muller <amuller at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Giuseppe Vallarelli <gvallare at redhat.com>
Gerrit-Reviewer: Livnat Peer <lpeer at redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw at linux.vnet.ibm.com>
Gerrit-Reviewer: Petr Ĺ ebek <psebek at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list