From: Ondrej Lichtner olichtne@redhat.com
For now throwing an exception when trying to read/write coalescing settings to a device that doesn't support them is still the best option.
However, there's a need to easily distinguish and ignore this error in operations that try to access all properties of a Device class instance, e.g. enable_readonly_cache. In such cases the tester probably doesn't care that coalescing isn't supported and has no option to skip it for the cache generation operation leading to the recipe failing with an exception.
Handling device capabilities dynamically is a larger feature that might be hard to implement for now so instead a special DeviceFeatureNotSupported exception class is added that can be safely ignored when performing these "bulk"/"iterative" operations.
Signed-off-by: Ondrej Lichtner olichtne@redhat.com --- lnst/Common/DeviceError.py | 3 +++ lnst/Devices/Device.py | 11 +++++++---- lnst/Devices/RemoteDevice.py | 8 +++++++- 3 files changed, 17 insertions(+), 5 deletions(-)
diff --git a/lnst/Common/DeviceError.py b/lnst/Common/DeviceError.py index c89541e..dc17205 100644 --- a/lnst/Common/DeviceError.py +++ b/lnst/Common/DeviceError.py @@ -32,3 +32,6 @@ class DeviceReadOnly(DeviceError):
class DeviceConfigValueError(DeviceConfigError): pass + +class DeviceFeatureNotSupported(DeviceError): + pass diff --git a/lnst/Devices/Device.py b/lnst/Devices/Device.py index 3984776..e69a297 100644 --- a/lnst/Devices/Device.py +++ b/lnst/Devices/Device.py @@ -23,6 +23,7 @@ from lnst.Common.ExecCmd import exec_cmd from lnst.Common.DeviceError import DeviceError, DeviceDeleted, DeviceDisabled from lnst.Common.DeviceError import DeviceConfigError, DeviceConfigValueError +from lnst.Common.DeviceError import DeviceFeatureNotSupported from lnst.Common.IpAddress import ipaddress from lnst.Common.HWAddress import hwaddress
@@ -624,8 +625,9 @@ def _read_adaptive_coalescing(self): try: res = re.search(regex, res).groups() except AttributeError: - raise DeviceError("No values for coalescence of %s." % - self.name) + raise DeviceFeatureNotSupported( + "No values for coalescence of %s." % self.name + ) return list(res)
def _write_adaptive_coalescing(self, rx_val, tx_val): @@ -635,8 +637,9 @@ def _write_adaptive_coalescing(self, rx_val, tx_val): exec_cmd("ethtool -C %s adaptive-rx %s adaptive-tx %s" % (self.name, rx_val, tx_val)) except: - raise DeviceConfigError("Not allowed to modify coalescence " - "settings for %s." % self.name) + raise DeviceFeatureNotSupported( + "Not allowed to modify coalescence settings for %s." % self.name + )
def restore_coalescing(self): rx_val = self._cleanup_data["adaptive_rx_coalescing"] diff --git a/lnst/Devices/RemoteDevice.py b/lnst/Devices/RemoteDevice.py index 17bc52b..55912ea 100644 --- a/lnst/Devices/RemoteDevice.py +++ b/lnst/Devices/RemoteDevice.py @@ -11,9 +11,11 @@ olichtne@redhat.com (Ondrej Lichtner) """
+import logging from copy import deepcopy from lnst.Devices.Device import Device from lnst.Common.DeviceError import DeviceDeleted, DeviceReadOnly +from lnst.Common.DeviceError import DeviceFeatureNotSupported
def remotedev_decorator(cls): def func(*args, **kwargs): @@ -150,7 +152,11 @@ def __iter__(self): attr = getattr(self._dev_cls, x)
if not callable(attr): - yield (x, getattr(self, x)) + try: + yield (x, getattr(self, x)) + except DeviceFeatureNotSupported as e: + logging.debug(str(e)) + continue
def _match_update_data(self, data): return False
From: Ondrej Lichtner olichtne@redhat.com
Signed-off-by: Ondrej Lichtner olichtne@redhat.com --- lnst/Devices/RemoteDevice.py | 3 --- 1 file changed, 3 deletions(-)
diff --git a/lnst/Devices/RemoteDevice.py b/lnst/Devices/RemoteDevice.py index 55912ea..652e193 100644 --- a/lnst/Devices/RemoteDevice.py +++ b/lnst/Devices/RemoteDevice.py @@ -90,9 +90,6 @@ def __id_set(self, value):
_id = property(__id_get, __id_set)
- def _get_dev_cls(self): - return self._dev_cls - @property def host(self): return self._machine._initns
From: Ondrej Lichtner olichtne@redhat.com
For pretty printing the object in automated logs.
Signed-off-by: Ondrej Lichtner olichtne@redhat.com --- lnst/Common/HWAddress.py | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/lnst/Common/HWAddress.py b/lnst/Common/HWAddress.py index b35550d..0271285 100644 --- a/lnst/Common/HWAddress.py +++ b/lnst/Common/HWAddress.py @@ -53,6 +53,9 @@ def _parse_addr(self, addr): res.append(val) return res
+ def __repr__(self): + return "{}({})".format(self.__class__.__name__, str(self)) + def hwaddress(addr): """Factory method to create a _HWAddress object""" if isinstance(addr, HWAddress):
From: Ondrej Lichtner olichtne@redhat.com
Instread of throwing an exception we should simply return a float infinity value so that it propagates all the way to the user.
Same exception could also be raised by the BaseFlowMeasurement "report" methods while calculating the percentage of the standard deviation out of the average value.
A helper method was utilized to calculate the percentage for all of the measured values.
Signed-off-by: Ondrej Lichtner olichtne@redhat.com --- .../Perf/Measurements/BaseFlowMeasurement.py | 11 +++++++++-- lnst/RecipeCommon/Perf/Results.py | 5 ++++- 2 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/lnst/RecipeCommon/Perf/Measurements/BaseFlowMeasurement.py b/lnst/RecipeCommon/Perf/Measurements/BaseFlowMeasurement.py index 14f0661..9890ed2 100644 --- a/lnst/RecipeCommon/Perf/Measurements/BaseFlowMeasurement.py +++ b/lnst/RecipeCommon/Perf/Measurements/BaseFlowMeasurement.py @@ -198,7 +198,7 @@ def _report_flow_results(cls, recipe, flow_results): desc.append("Generator measured throughput: {tput:.2f} +-{deviation:.2f}({percentage:.2f}%) {unit} per second." .format(tput=generator.average, deviation=generator.std_deviation, - percentage=(generator.std_deviation/generator.average) * 100, + percentage=cls._deviation_percentage(generator), unit=generator.unit)) desc.append("Generator process CPU data: {cpu:.2f} +-{cpu_deviation:.2f} {cpu_unit} per second." .format(cpu=generator_cpu.average, @@ -207,7 +207,7 @@ def _report_flow_results(cls, recipe, flow_results): desc.append("Receiver measured throughput: {tput:.2f} +-{deviation:.2f}({percentage:.2}%) {unit} per second." .format(tput=receiver.average, deviation=receiver.std_deviation, - percentage=(receiver.std_deviation/receiver.average) * 100, + percentage=cls._deviation_percentage(receiver), unit=receiver.unit)) desc.append("Receiver process CPU data: {cpu:.2f} +-{cpu_deviation:.2f} {cpu_unit} per second." .format(cpu=receiver_cpu.average, @@ -238,3 +238,10 @@ def _aggregate_flows(self, old_flow, new_flow): new_result.add_results(old_flow) new_result.add_results(new_flow) return new_result + + @classmethod + def _deviation_percentage(cls, result): + try: + return (result.std_deviation/result.average) * 100 + except ZeroDivisionError: + return float('inf') if result.std_deviation >= 0 else float("-inf") diff --git a/lnst/RecipeCommon/Perf/Results.py b/lnst/RecipeCommon/Perf/Results.py index 1b44cde..4519fb8 100644 --- a/lnst/RecipeCommon/Perf/Results.py +++ b/lnst/RecipeCommon/Perf/Results.py @@ -4,7 +4,10 @@ class PerfStatMixin(object): @property def average(self): - return float(self.value) / self.duration + try: + return float(self.value) / self.duration + except ZeroDivisionError: + return float('inf') if self.value >= 0 else float('-inf')
@property def std_deviation(self):
Tue, Oct 01, 2019 at 10:47:01AM CEST, olichtne@redhat.com wrote:
From: Ondrej Lichtner olichtne@redhat.com
For now throwing an exception when trying to read/write coalescing settings to a device that doesn't support them is still the best option.
However, there's a need to easily distinguish and ignore this error in operations that try to access all properties of a Device class instance, e.g. enable_readonly_cache. In such cases the tester probably doesn't care that coalescing isn't supported and has no option to skip it for the cache generation operation leading to the recipe failing with an exception.
Handling device capabilities dynamically is a larger feature that might be hard to implement for now so instead a special DeviceFeatureNotSupported exception class is added that can be safely ignored when performing these "bulk"/"iterative" operations.
Signed-off-by: Ondrej Lichtner olichtne@redhat.com
Ack to series.
Acked-by: Jan Tluka jtluka@redhat.com
On Tue, Oct 01, 2019 at 12:08:14PM +0200, Jan Tluka wrote:
Tue, Oct 01, 2019 at 10:47:01AM CEST, olichtne@redhat.com wrote:
From: Ondrej Lichtner olichtne@redhat.com
For now throwing an exception when trying to read/write coalescing settings to a device that doesn't support them is still the best option.
However, there's a need to easily distinguish and ignore this error in operations that try to access all properties of a Device class instance, e.g. enable_readonly_cache. In such cases the tester probably doesn't care that coalescing isn't supported and has no option to skip it for the cache generation operation leading to the recipe failing with an exception.
Handling device capabilities dynamically is a larger feature that might be hard to implement for now so instead a special DeviceFeatureNotSupported exception class is added that can be safely ignored when performing these "bulk"/"iterative" operations.
Signed-off-by: Ondrej Lichtner olichtne@redhat.com
Ack to series.
Acked-by: Jan Tluka jtluka@redhat.com
pushed, thanks for review.
-Ondrej
lnst-developers@lists.fedorahosted.org
-
Jan Tluka -
olichtne＠redhat.com -
Ondrej Lichtner