A generic comment on the patch email format. The subject seems
corrupted, it should be of the following format:
[PATCH] ...
Also the subject contains quite a long string, I believe the last part of it
should have been part of the patch description rather than subject.
Last thing is that you put 'From: ...' into patch description, which
should not be there.
More comments on the patch code changes inline.
Tue, Apr 14, 2020 at 08:40:37PM CEST, pgagne(a)redhat.com wrote:
From: Perry Gagne <pgagne(a)redhat.com>
Signed-off-by: Perry Gagne <pgagne(a)redhat.com>
---
lnst/Devices/TeamDevice.py | 49 ++++++++++++++++-----------------
lnst/Recipes/ENRT/TeamRecipe.py | 11 ++++----
2 files changed, 29 insertions(+), 31 deletions(-)
diff --git a/lnst/Devices/TeamDevice.py b/lnst/Devices/TeamDevice.py
index 4f6eee5..3a60125 100644
--- a/lnst/Devices/TeamDevice.py
+++ b/lnst/Devices/TeamDevice.py
@@ -9,44 +9,43 @@ published by the Free Software Foundation; see COPYING for details.
__author__ = """
olichtne(a)redhat.com (Ondrej Lichtner)
"""
-
-import re
+import json
from lnst.Common.ExecCmd import exec_cmd
-from lnst.Common.Utils import bool_it
+from lnst.Common.DeviceError import DeviceError, DeviceConfigError
from lnst.Devices.MasterDevice import MasterDevice
-def prepare_json_str(json_str):
- if not json_str:
- return "{}"
- json_str = json_str.replace('"', '\\"')
- json_str = re.sub('\s+', ' ', json_str)
- return json_str
-#TODO rework with pyroute2? don't know how json and teamd works with that...
Please do not remove the TODO unless you resolved it or you're sure this
is not doable. Maybe you already discussed this with Ondrej, in that
case ignore my comment.
class TeamDevice(MasterDevice):
_name_template = "t_team"
def __init__(self, ifmanager, *args, **kwargs):
+ self._config = {}
+ self._dbus = False
super(TeamDevice, self).__init__(ifmanager, *args, **kwargs)
- self._config = kwargs.get("config", None)
- self._dbus = not bool_it(kwargs.get("disable_dbus", False))
-
@property
def config(self):
return self._config
+ @config.setter
+ def config(self, v: dict):
+ if not isinstance(v, dict):
+ raise DeviceConfigError("team device config must be dict")
+ self._config = v
+
@property
def dbus(self):
return self._dbus
- def _create(self):
- teamd_config = prepare_json_str(self.config)
+ @dbus.setter
+ def dbus(self, v: bool):
+ if not isinstance(v, bool):
+ raise DeviceConfigError("team dbus setting must be bool")
+ self._dbus = v
- exec_cmd("teamd -r -d -c \"%s\" -t %s %s" %\
- (teamd_config,
- self.name,
- " -D" if self.dbus else ""))
+ def _create(self):
+ teamd_json = json.dumps(self.config)
+ exec_cmd(f"teamd -r -d -c '{teamd_json}' -t {self.name}
{'-D' if self.dbus else ''}")
retry = 0
while self._nl_msg is None and retry < 5:
@@ -56,11 +55,11 @@ class TeamDevice(MasterDevice):
def destroy(self):
exec_cmd("teamd -k -t %s" % self.name)
- def slave_add(self, dev, port_config=None):
- exec_cmd("teamdctl %s %s port config update %s \"%s\"" %\
- (" -D" if self.dbus else "",
- self.name,
- dev.name,
- prepare_json_str(port_config)))
+ def slave_add(self, dev, port_config={}):
+ if not isinstance(port_config, dict):
+ raise DeviceConfigError(f"team link {dev.name} port config must be
dict")
+
+ port_json = json.dumps(port_config)
+ exec_cmd(f"teamdctl {'-D' if self.dbus else ''} {self.name}
port config update {dev.name} '{port_json}'")
dev.master = self
diff --git a/lnst/Recipes/ENRT/TeamRecipe.py b/lnst/Recipes/ENRT/TeamRecipe.py
index e083b90..ccad5e7 100644
--- a/lnst/Recipes/ENRT/TeamRecipe.py
+++ b/lnst/Recipes/ENRT/TeamRecipe.py
@@ -9,8 +9,9 @@ from lnst.Recipes.ENRT.ConfigMixins.CommonHWSubConfigMixin import (
from lnst.RecipeCommon.Ping.PingEndpoints import PingEndpoints
from lnst.Devices import TeamDevice
+
class TeamRecipe(CommonHWSubConfigMixin, OffloadSubConfigMixin,
- BaseEnrtRecipe):
+ BaseEnrtRecipe):
You've added another newline before the class definition, likely
unnecessary.
Also, you (or your IDE) reindented the list of derived from classes.
Again, unnecessary change.
host1 = HostReq()
host1.eth0 = DeviceReq(label="tnet",
driver=RecipeParam("driver"))
host1.eth1 = DeviceReq(label="tnet",
driver=RecipeParam("driver"))
@@ -30,10 +31,8 @@ class TeamRecipe(CommonHWSubConfigMixin, OffloadSubConfigMixin,
def test_wide_configuration(self):
host1, host2 = self.matched.host1, self.matched.host2
- #The config argument needs to be used with a team device normally
- #(e.g to specify the runner mode), but it is not used here due to
- #a bug in the TeamDevice module
- host1.team0 = TeamDevice()
+ teamd_config = {'runner': {'name': self.params.runner_name}}
+ host1.team0 = TeamDevice(config=teamd_config)
configuration = super().test_wide_configuration()
configuration.test_wide_devices = [host1.team0, host2.eth0]
@@ -90,7 +89,7 @@ class TeamRecipe(CommonHWSubConfigMixin, OffloadSubConfigMixin,
def generate_perf_endpoints(self, config):
return [(self.matched.host1.team0, self.matched.host2.eth0),
- (self.matched.host2.eth0, self.matched.host1.team0)]
+ (self.matched.host2.eth0, self.matched.host1.team0)]
Once again, an unnecessary indentation change.
@property
def offload_nics(self):
--
2.21.1
_______________________________________________
LNST-developers mailing list -- lnst-developers(a)lists.fedorahosted.org
To unsubscribe send an email to lnst-developers-leave(a)lists.fedorahosted.org
Fedora Code of Conduct:
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines:
https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives:
https://lists.fedorahosted.org/archives/list/lnst-developers@lists.fedora...