Dan Kenigsberg has posted comments on this change. Change subject: net: Drop default bonding options ......................................................................
Patch Set 3: Code-Review-1 (2 comments) https://gerrit.ovirt.org/#/c/42594/3/vdsm/network/configurators/ifcfg.py File vdsm/network/configurators/ifcfg.py: Line 794: """Restore bond options to the default options of the desired mode. First Line 795: we change the bond mode to the desired mode (if needed) to avoid Line 796: 'Operation not permitted' errors and then reset the non-default options Line 797: """ Line 798: desired_options = desired_options or '' I don't understand the need of this line. desired_options is never None or {} or set(). How do I know - you call .split() on the next line. if it evaluates to False, it is already the empty string. Line 799: desired_options = dict(p.split('=', 1) for p in desired_options.split()) Line 800: current_opts = netinfo.bondOpts(bond_name) Line 801: current_mode = current_opts['mode'] Line 802: desired_mode = (_get_mode_from_desired_options(desired_options) https://gerrit.ovirt.org/#/c/42594/3/vdsm/network/models.py File vdsm/network/models.py: Line 245: Line 246: self.configurator.configureBond(self, **opts) Line 247: Line 248: def areOptionsApplied(self): Line 249: if not self.options: is this really true? if we are asked to have no options, but the existing bond has some non-default ones, we should reconfigure it. Line 250: return True Line 251: confOpts = [option.split('=', 1) for option in self.options.split(' ')] Line 252: activeOpts = netinfo.bondOpts(self.name, Line 253: (name for name, value in confOpts)) -- To view, visit https://gerrit.ovirt.org/42594 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9e4a8d832891f62d1fcc412606b014fa4f874062 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ido Barkan <ibar...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Ido Barkan <ibar...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
