Dan Kenigsberg has posted comments on this change.
Change subject: Fix checking bonding options for missing bonds.
......................................................................
Patch Set 1: I would prefer that you didn't submit this
(4 inline comments)
....................................................
File vdsm/configNetwork.py
Line 733: try:
Line 734: for option in bondingOptions.split():
Line 735: key, value = option.split('=')
Line 736: if not os.path.exists(
Line 737: '/sys/class/net/%s/bonding/%s' % (bond, key)):
I'm not the grandest lover of the likes of "%(bonding)s", but you could have
kept using this style here (with "%(bond)s").
Line 738: raise ConfigNetworkError(ne.ERR_BAD_BONDING, '%r
is not a '
Line 739: 'valid bonding option' %
key)
Line 740: except ValueError:
Line 741: raise ConfigNetworkError(ne.ERR_BAD_BONDING, 'Error
parsing '
Line 742: 'bonding options: %r' %
bondingOptions)
Line 743:
Line 744:
Line 745: @contextmanager
Line 746: def validationBond(bonding):
please keep such things _private to this module.
Line 747: bond_created = False
Line 748: try:
Line 749: bonding = open(BONDING_MASTERS, 'r').read().split()[0]
Line 750: except IndexError:
Line 751: open(BONDING_MASTERS, 'w').write('+%s\n' % bonding)
Line 752: bond_created = True
Line 753: try:
Line 754: yield bonding
Line 755: except Exception:
why is this not done on "finally"?
Line 756: if bond_created:
Line 757: try:
Line 758: open(BONDING_MASTERS, 'w').write('-%s\n' % bonding)
Line 759: except IOError:
Line 756: if bond_created:
Line 757: try:
Line 758: open(BONDING_MASTERS, 'w').write('-%s\n' % bonding)
Line 759: except IOError:
Line 760: pass
why do you expect an IOError here?
Line 761: raise
Line 762:
Line 763:
Line 764: def validateVlanId(vlan):
--
To view, visit http://gerrit.ovirt.org/12701
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I023a5bb8a52719559bb9d4716f25e0cba8b3530b
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Antoni Segura Puimedon <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Lior Vernia <[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
