Dan Kenigsberg has posted comments on this change. Change subject: ethtool_opts: provide a way to apply to all slaves ......................................................................
Patch Set 1: Code-Review-1 (3 comments) http://gerrit.ovirt.org/#/c/32508/1/vdsm_hooks/ethtool_options/ethtool_options.py File vdsm_hooks/ethtool_options/ethtool_options.py: Line 49: Line 50: Line 51: def test(): Line 52: opts = {'ethtool_opts': Line 53: '--coalesce em1 rx-usecs 14 sample_interval 3 ' it's sample-interval with a dash Line 54: '--offload em2 rx on lro on tso off ' Line 55: '--change em1 speed 1000 duplex half'} Line 56: # Test with the correct nics Line 57: nics = ('em1', 'em2') Line 88: def _process_network(network, attrs): Line 89: """Applies ethtool_options to the network if necessary""" Line 90: options = attrs['custom'].get('ethtool_opts') Line 91: if options is not None: Line 92: tokens = options.split(' ') Unrelated to this patch, but I think we should split on any whitespace combination. Humans may type doublespace by mistake. Line 93: nics = _net_nics(attrs) Line 94: subcommands = list(_parse_into_subcommands(tokens)) Line 95: if any(subcmd.device == ALL_SLAVES for subcmd in subcommands): Line 96: if all(subcmd.device == ALL_SLAVES for subcmd in subcommands): Line 94: subcommands = list(_parse_into_subcommands(tokens)) Line 95: if any(subcmd.device == ALL_SLAVES for subcmd in subcommands): Line 96: if all(subcmd.device == ALL_SLAVES for subcmd in subcommands): Line 97: for nic in nics: # Set for each slave Line 98: _set_ethtool_opts(network, I believe that we must use a different command line for each subcommand - ethtool does not accept two subcommand even if their nic match. This would be enough to solve the bug. However, I love the idea of a special moniker (we may even use the astrisk symbol) to denote that these opts should apply to all network nics. Line 99: [nic if token == ALL_SLAVES else token Line 100: for token in tokens]) Line 101: else: Line 102: raise RuntimeError( -- To view, visit http://gerrit.ovirt.org/32508 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idddbc167be4ac8ba7c4b4ab10da0275b4caf2a58 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
