1
0
Fork 0

fix: move ownership of Variables from Application to Profile

Currently, there is one global Variables object, as member of the global
Application. When loading profiles, variables are added to it, but never
removed. So when switching profiles, there are still variables present
from the previous profile.
With this commit, the Variables object is now owned by the Profile.
It is created by the profile loader, so that more complex laod scenarios
(multiple profiles, potentially with includes) still only use one Variables
object.

Signed-off-by: Adriaan Schmidt <adriaan.schmidt@siemens.com>
This commit is contained in:
Adriaan Schmidt 2026-06-08 12:18:47 +02:00
parent 0eb28ac3d8
commit b863272789
15 changed files with 97 additions and 78 deletions

View file

@ -30,12 +30,11 @@ class PluginBaseTestCase(unittest.TestCase):
def setUp(self):
self._plugin = DummyPlugin(monitors_repository,storage_factory,\
hardware_inventory,device_matcher,device_matcher_udev,\
plugin_instance_factory,None,None)
plugin_instance_factory,None)
self._commands_plugin = CommandsPlugin(monitors_repository,\
storage_factory,hardware_inventory,device_matcher,\
device_matcher_udev,plugin_instance_factory,None,\
profiles.variables.Variables())
device_matcher_udev,plugin_instance_factory,None)
def test_get_effective_options(self):
self.assertEqual(self._plugin._get_effective_options(\
@ -51,13 +50,13 @@ class PluginBaseTestCase(unittest.TestCase):
def test_create_instance(self):
instance = self._plugin.create_instance(\
'first_instance',0,'test','test','test','test',\
{'default_option1':'default_value2'})
{'default_option1':'default_value2'},None)
self.assertIsNotNone(instance)
def test_destroy_instance(self):
instance = self._plugin.create_instance(\
'first_instance',0,'test','test','test','test',\
{'default_option1':'default_value2'})
{'default_option1':'default_value2'},None)
instance.plugin.init_devices()
self._plugin.destroy_instance(instance)
@ -67,7 +66,7 @@ class PluginBaseTestCase(unittest.TestCase):
""" without udev regex """
instance = self._plugin.create_instance(\
'first_instance',0,'right_device*',None,'test','test',\
{'default_option1':'default_value2'})
{'default_option1':'default_value2'},None)
self.assertEqual(self._plugin._get_matching_devices(\
instance,['bad_device','right_device1','right_device2']),\
@ -76,7 +75,7 @@ class PluginBaseTestCase(unittest.TestCase):
""" with udev regex """
instance = self._plugin.create_instance(\
'second_instance',0,'right_device*','device[1-2]','test','test',\
{'default_option1':'default_value2'})
{'default_option1':'default_value2'},None)
device1 = DummyDevice('device1',{'name':'device1'})
device2 = DummyDevice('device2',{'name':'device2'})
@ -105,7 +104,7 @@ class PluginBaseTestCase(unittest.TestCase):
def test_execute_all_non_device_commands(self):
instance = self._commands_plugin.create_instance('test_instance',0,'',\
'','','',{'size':'XXL'})
'','','',{'size':'XXL'},profiles.variables.Variables())
self._commands_plugin._execute_all_non_device_commands(instance)
@ -113,7 +112,7 @@ class PluginBaseTestCase(unittest.TestCase):
def test_execute_all_device_commands(self):
instance = self._commands_plugin.create_instance('test_instance',0,'',\
'','','',{'device_setting':'010'})
'','','',{'device_setting':'010'},profiles.variables.Variables())
device1 = DummyDevice('device1',{})
device2 = DummyDevice('device2',{})
@ -134,7 +133,7 @@ class PluginBaseTestCase(unittest.TestCase):
def test_get_current_value(self):
instance = self._commands_plugin.create_instance('test_instance',0,'',\
'','','',{})
'','','',{},None)
command = [com for com in self._commands_plugin._commands.values()\
if com['name'] == 'size'][0]

View file

@ -53,8 +53,7 @@ class LoaderTestCase(unittest.TestCase):
locator = profiles.Locator([self._profiles_dir])
factory = profiles.Factory()
merger = profiles.Merger()
self._loader = profiles.Loader(locator,factory,merger,None,\
profiles.variables.Variables())
self._loader = profiles.Loader(locator,factory,merger,None)
def test_safe_name(self):
self.assertFalse(self._loader.safe_name('*'))

View file

@ -1,21 +1,23 @@
import unittest
from tuned.profiles.merger import Merger
from tuned.profiles.profile import Profile
from tuned.profiles.variables import Variables
from collections import OrderedDict
class MergerTestCase(unittest.TestCase):
def test_merge_without_replace(self):
merger = Merger()
variables = Variables()
config1 = OrderedDict([
("main", {"test_option" : "test_value1"}),
("net", { "devices": "em0", "custom": "custom_value"}),
])
profile1 = Profile('test_profile1',config1)
profile1 = Profile('test_profile1',config1,variables)
config2 = OrderedDict([
('main', {'test_option' : 'test_value2'}),
('net', { 'devices': 'em1' }),
])
profile2 = Profile("test_profile2",config2)
profile2 = Profile("test_profile2",config2,variables)
merged_profile = merger.merge([profile1, profile2])
@ -27,16 +29,17 @@ class MergerTestCase(unittest.TestCase):
def test_merge_with_replace(self):
merger = Merger()
variables = Variables()
config1 = OrderedDict([
("main", {"test_option" : "test_value1"}),
("net", { "devices": "em0", "custom": "option"}),
])
profile1 = Profile('test_profile1',config1)
profile1 = Profile('test_profile1',config1,variables)
config2 = OrderedDict([
("main", {"test_option" : "test_value2"}),
("net", { "devices": "em1", "replace": True }),
])
profile2 = Profile('test_profile2',config2)
profile2 = Profile('test_profile2',config2,variables)
merged_profile = merger.merge([profile1, profile2])
self.assertEqual(merged_profile.options["test_option"],"test_value2")
@ -46,15 +49,16 @@ class MergerTestCase(unittest.TestCase):
def test_merge_multiple_order(self):
merger = Merger()
variables = Variables()
config1 = OrderedDict([ ("main", {"test_option" : "test_value1"}),\
("net", { "devices": "em0" }) ])
profile1 = Profile('test_profile1',config1)
profile1 = Profile('test_profile1',config1,variables)
config2 = OrderedDict([ ("main", {"test_option" : "test_value2"}),\
("net", { "devices": "em1" }) ])
profile2 = Profile('test_profile2',config2)
profile2 = Profile('test_profile2',config2,variables)
config3 = OrderedDict([ ("main", {"test_option" : "test_value3"}),\
("net", { "devices": "em2" }) ])
profile3 = Profile('test_profile3',config3)
profile3 = Profile('test_profile3',config3,variables)
merged_profile = merger.merge([profile1, profile2, profile3])
self.assertEqual(merged_profile.options["test_option"],"test_value3")

View file

@ -9,33 +9,33 @@ class MockProfile(tuned.profiles.profile.Profile):
class ProfileTestCase(unittest.TestCase):
def test_init(self):
MockProfile("test", {})
MockProfile("test", {}, None)
def test_create_units(self):
profile = MockProfile("test", {
"main": { "anything": 10 },
"network" : { "type": "net", "devices": "*" },
"storage" : { "type": "disk" },
})
}, None)
self.assertIs(type(profile.units), collections.OrderedDict)
self.assertEqual(len(profile.units), 2)
self.assertListEqual(sorted([name_config for name_config in profile.units]), sorted(["network", "storage"]))
def test_create_units_empty(self):
profile = MockProfile("test", {"main":{}})
profile = MockProfile("test", {"main":{}}, None)
self.assertIs(type(profile.units), collections.OrderedDict)
self.assertEqual(len(profile.units), 0)
def test_sets_name(self):
profile1 = MockProfile("test_one", {})
profile2 = MockProfile("test_two", {})
profile1 = MockProfile("test_one", {}, None)
profile2 = MockProfile("test_two", {}, None)
self.assertEqual(profile1.name, "test_one")
self.assertEqual(profile2.name, "test_two")
def test_change_name(self):
profile = MockProfile("oldname", {})
profile = MockProfile("oldname", {}, None)
self.assertEqual(profile.name, "oldname")
profile.name = "newname"
self.assertEqual(profile.name, "newname")
@ -44,7 +44,7 @@ class ProfileTestCase(unittest.TestCase):
profile = MockProfile("test", {
"main": { "anything": 10 },
"network" : { "type": "net", "devices": "*" },
})
}, None)
self.assertIs(type(profile.options), dict)
self.assertEqual(profile.options["anything"], 10)
@ -52,7 +52,7 @@ class ProfileTestCase(unittest.TestCase):
def test_sets_options_empty(self):
profile = MockProfile("test", {
"storage" : { "type": "disk" },
})
}, None)
self.assertIs(type(profile.options), dict)
self.assertEqual(len(profile.options), 0)

View file

@ -39,10 +39,9 @@ class Application(object):
device_matcher = hardware.DeviceMatcher()
device_matcher_udev = hardware.DeviceMatcherUdev()
plugin_instance_factory = plugins.instance.Factory()
self.variables = profiles.variables.Variables()
plugins_repository = plugins.Repository(monitors_repository, storage_factory, hardware_inventory,\
device_matcher, device_matcher_udev, plugin_instance_factory, self.config, self.variables)
device_matcher, device_matcher_udev, plugin_instance_factory, self.config)
def_instance_priority = int(self.config.get(consts.CFG_DEFAULT_INSTANCE_PRIORITY, consts.CFG_DEF_DEFAULT_INSTANCE_PRIORITY))
unit_manager = units.Manager(
plugins_repository, monitors_repository,
@ -51,7 +50,7 @@ class Application(object):
profile_factory = profiles.Factory()
profile_merger = profiles.Merger()
profile_locator = profiles.Locator(self.config.get_list(consts.CFG_PROFILE_DIRS, consts.CFG_DEF_PROFILE_DIRS))
profile_loader = profiles.Loader(profile_locator, profile_factory, profile_merger, self.config, self.variables)
profile_loader = profiles.Loader(profile_locator, profile_factory, profile_merger, self.config)
self._daemon = daemon.Daemon(unit_manager, profile_loader, profile_name, self.config, self)
self._controller = controller.Controller(self._daemon, self.config)

View file

@ -198,7 +198,7 @@ class Daemon(object):
if self._profile is None:
raise TunedException("Cannot start the daemon without setting a profile.")
self._unit_manager.create(self._profile.units)
self._unit_manager.create(self._profile.units, self._profile.variables)
self._save_active_profile(" ".join(self._active_profiles),
self._manual)
self._save_post_loaded_profile(self._post_loaded_profile)

View file

@ -19,7 +19,7 @@ class Plugin(object):
Intentionally a lot of logic is included in the plugin to increase plugin flexibility.
"""
def __init__(self, monitors_repository, storage_factory, hardware_inventory, device_matcher, device_matcher_udev, instance_factory, global_cfg, variables):
def __init__(self, monitors_repository, storage_factory, hardware_inventory, device_matcher, device_matcher_udev, instance_factory, global_cfg):
"""Plugin constructor."""
self._storage = storage_factory.create(self.__class__.__name__)
@ -33,7 +33,6 @@ class Plugin(object):
self._init_commands()
self._global_cfg = global_cfg
self._variables = variables
self._has_dynamic_options = False
self._devices_inited = False
@ -93,14 +92,14 @@ class Plugin(object):
# Interface for manipulation with instances of the plugin.
#
def create_instance(self, name, priority, devices_expression, devices_udev_regex, script_pre, script_post, options):
def create_instance(self, name, priority, devices_expression, devices_udev_regex, script_pre, script_post, options, variables):
"""Create new instance of the plugin and seize the devices."""
if name in self._instances:
raise Exception("Plugin instance with name '%s' already exists." % name)
effective_options = self._get_effective_options(options)
instance = self._instance_factory.create(self, name, priority, devices_expression, devices_udev_regex, \
script_pre, script_post, effective_options)
script_pre, script_post, effective_options, variables)
self._instances[name] = instance
self._instances = collections.OrderedDict(sorted(self._instances.items(), key=lambda x: x[1].priority))
@ -249,8 +248,8 @@ class Plugin(object):
dir_name = os.path.dirname(script)
ret = True
for dev in devices:
environ = os.environ
environ.update(self._variables.get_env())
environ = os.environ.copy()
environ.update(instance._variables.get_env())
arguments = [op]
if rollback == consts.ROLLBACK_FULL:
arguments.append("full_rollback")
@ -459,13 +458,13 @@ class Plugin(object):
def _execute_all_non_device_commands(self, instance):
for command in [command for command in list(self._commands.values()) if not command["per_device"]]:
new_value = self._variables.expand(instance.options.get(command["name"], None))
new_value = instance._variables.expand(instance.options.get(command["name"], None))
if new_value is not None:
self._execute_non_device_command(instance, command, new_value)
def _execute_all_device_commands(self, instance, devices):
for command in [command for command in list(self._commands.values()) if command["per_device"]]:
new_value = self._variables.expand(instance.options.get(command["name"], None))
new_value = instance._variables.expand(instance.options.get(command["name"], None))
if new_value is None:
continue
for device in devices:
@ -474,7 +473,7 @@ class Plugin(object):
def _verify_all_non_device_commands(self, instance, ignore_missing):
ret = True
for command in [command for command in list(self._commands.values()) if not command["per_device"]]:
new_value = self._variables.expand(instance.options.get(command["name"], None))
new_value = instance._variables.expand(instance.options.get(command["name"], None))
if new_value is not None:
if self._verify_non_device_command(instance, command, new_value, ignore_missing) == False:
ret = False
@ -483,7 +482,7 @@ class Plugin(object):
def _verify_all_device_commands(self, instance, devices, ignore_missing):
ret = True
for command in [command for command in list(self._commands.values()) if command["per_device"]]:
new_value = self._variables.expand(instance.options.get(command["name"], None))
new_value = instance._variables.expand(instance.options.get(command["name"], None))
if new_value is None:
continue
for device in devices:

View file

@ -4,7 +4,7 @@ class Instance(object):
"""
"""
def __init__(self, plugin, name, priority, devices_expression, devices_udev_regex, script_pre, script_post, options):
def __init__(self, plugin, name, priority, devices_expression, devices_udev_regex, script_pre, script_post, options, variables):
self._plugin = plugin
self._name = name
self._devices_expression = devices_expression
@ -12,6 +12,7 @@ class Instance(object):
self._script_pre = script_pre
self._script_post = script_post
self._options = options
self._variables = variables
self._active = True
self._priority = priority
@ -71,6 +72,10 @@ class Instance(object):
def options(self):
return self._options
@property
def variables(self):
return self._variables
@property
def has_static_tuning(self):
return self._has_static_tuning

View file

@ -51,7 +51,7 @@ class SysfsPlugin(base.Plugin):
def _instance_apply_static(self, instance):
for key, value in list(instance._sysfs.items()):
v = self._variables.expand(value)
v = instance.variables.expand(value)
for f in glob.iglob(key):
if self._check_sysfs(f):
instance._sysfs_original[f] = self._read_sysfs(f)
@ -62,7 +62,7 @@ class SysfsPlugin(base.Plugin):
def _instance_verify_static(self, instance, ignore_missing, devices):
ret = True
for key, value in list(instance._sysfs.items()):
v = self._variables.expand(value)
v = instance.variables.expand(value)
for f in glob.iglob(key):
if self._check_sysfs(f):
curr_val = self._read_sysfs(f)

View file

@ -8,7 +8,7 @@ __all__ = ["Repository"]
class Repository(ClassLoader):
def __init__(self, monitor_repository, storage_factory, hardware_inventory, device_matcher, device_matcher_udev, plugin_instance_factory, global_cfg, variables):
def __init__(self, monitor_repository, storage_factory, hardware_inventory, device_matcher, device_matcher_udev, plugin_instance_factory, global_cfg):
super(Repository, self).__init__()
self._plugins = set()
self._monitor_repository = monitor_repository
@ -18,7 +18,6 @@ class Repository(ClassLoader):
self._device_matcher_udev = device_matcher_udev
self._plugin_instance_factory = plugin_instance_factory
self._global_cfg = global_cfg
self._variables = variables
@property
def plugins(self):
@ -33,7 +32,7 @@ class Repository(ClassLoader):
log.debug("creating plugin %s" % plugin_name)
plugin_cls = self.load_class(plugin_name)
plugin_instance = plugin_cls(self._monitor_repository, self._storage_factory, self._hardware_inventory, self._device_matcher,\
self._device_matcher_udev, self._plugin_instance_factory, self._global_cfg, self._variables)
self._device_matcher_udev, self._plugin_instance_factory, self._global_cfg)
self._plugins.add(plugin_instance)
return plugin_instance

View file

@ -1,5 +1,5 @@
import tuned.profiles.profile
class Factory(object):
def create(self, name, config):
return tuned.profiles.profile.Profile(name, config)
def create(self, name, config, variables):
return tuned.profiles.profile.Profile(name, config, variables)

View file

@ -1,5 +1,5 @@
import tuned.profiles.profile
import tuned.profiles.variables
from tuned.profiles.variables import Variables
from tuned.utils.config_parser import ConfigParser, Error
import tuned.consts as consts
import os.path
@ -15,17 +15,16 @@ class Loader(object):
Profiles loader.
"""
__slots__ = ["_profile_locator", "_profile_merger", "_profile_factory", "_global_config", "_variables"]
__slots__ = ["_profile_locator", "_profile_merger", "_profile_factory", "_global_config"]
def __init__(self, profile_locator, profile_factory, profile_merger, global_config, variables):
def __init__(self, profile_locator, profile_factory, profile_merger, global_config):
self._profile_locator = profile_locator
self._profile_factory = profile_factory
self._profile_merger = profile_merger
self._global_config = global_config
self._variables = variables
def _create_profile(self, profile_name, config):
return tuned.profiles.profile.Profile(profile_name, config)
def _create_profile(self, profile_name, config, variables):
return tuned.profiles.profile.Profile(profile_name, config, variables)
@classmethod
def safe_name(cls, profile_name):
@ -49,11 +48,11 @@ class Loader(object):
log.info("loading profile: %s" % profile_names[0])
profiles = []
processed_files = []
self._load_profile(profile_names, profiles, processed_files)
self._load_profile(profile_names, profiles, processed_files, Variables())
final_profile = self._profile_merger.merge(profiles)
final_profile.name = " ".join(profile_names)
self._variables.add_from_cfg(final_profile.variables)
final_profile.variables.add_from_cfg(final_profile.variable_cfg)
# FIXME hack, do all variable expansions in one place
self._expand_vars_in_devices(final_profile)
self._expand_vars_in_regexes(final_profile)
@ -61,14 +60,14 @@ class Loader(object):
def _expand_vars_in_devices(self, profile):
for unit in profile.units:
profile.units[unit].devices = self._variables.expand(profile.units[unit].devices)
profile.units[unit].devices = profile.variables.expand(profile.units[unit].devices)
def _expand_vars_in_regexes(self, profile):
for unit in profile.units:
profile.units[unit].cpuinfo_regex = self._variables.expand(profile.units[unit].cpuinfo_regex)
profile.units[unit].uname_regex = self._variables.expand(profile.units[unit].uname_regex)
profile.units[unit].cpuinfo_regex = profile.variables.expand(profile.units[unit].cpuinfo_regex)
profile.units[unit].uname_regex = profile.variables.expand(profile.units[unit].uname_regex)
def _load_profile(self, profile_names, profiles, processed_files):
def _load_profile(self, profile_names, profiles, processed_files, variables):
for name in profile_names:
filename = self._profile_locator.get_config(name, processed_files)
if filename == "":
@ -78,10 +77,10 @@ class Loader(object):
processed_files.append(filename)
config = self._load_config_data(filename)
profile = self._profile_factory.create(name, config)
profile = self._profile_factory.create(name, config, variables)
if "include" in profile.options:
include_names = re.split(r"\s*[,;]\s*", self._variables.expand(profile.options.pop("include")))
self._load_profile(include_names, profiles, processed_files)
include_names = re.split(r"\s*[,;]\s*", profile.variables.expand(profile.options.pop("include")))
self._load_profile(include_names, profiles, processed_files, variables)
profiles.append(profile)

View file

@ -1,6 +1,7 @@
import tuned.consts as consts
from functools import reduce
from tuned.profiles.profile import Profile
from tuned.profiles.variables import Variables
class Merger(object):
"""
@ -15,7 +16,11 @@ class Merger(object):
Merge multiple configurations into one. If there are multiple units of the same type, option 'devices'
is set for each unit with respect to eliminating any duplicate devices.
"""
merged_config = reduce(self._merge_two, configs, Profile())
# All loaded profiles share the same Variables object, owned by the
# profile. Reuse it for the merged profile so that the merged result
# keeps the variables that were collected during loading.
variables = configs[0].variables if len(configs) > 0 else Variables()
merged_config = reduce(self._merge_two, configs, Profile(None, {}, variables))
return merged_config
def _merge_two(self, profile_a, profile_b):
@ -32,13 +37,13 @@ class Merger(object):
for unit_name, unit in list(profile_b.units.items()):
if unit.type == consts.PLUGIN_VARIABLES_UNIT_NAME:
if unit.replace:
profile_a.variables.clear()
overwritten_variables = set(profile_a.variables.keys()) & set(unit.options.keys())
profile_a.variables.update(unit.options)
profile_a.variable_cfg.clear()
overwritten_variables = set(profile_a.variable_cfg.keys()) & set(unit.options.keys())
profile_a.variable_cfg.update(unit.options)
if unit.prepend:
for variable in reversed(unit.options):
if variable not in overwritten_variables:
profile_a.variables.move_to_end(variable, last=False)
profile_a.variable_cfg.move_to_end(variable, last=False)
elif unit.replace or unit_name not in profile_a.units:
profile_a.units[unit_name] = unit
else:

View file

@ -7,11 +7,12 @@ class Profile(object):
Representation of a tuning profile.
"""
__slots__ = ["_name", "_options", "_variables", "_units"]
__slots__ = ["_name", "_options", "_units", "_variables", "_variable_cfg"]
def __init__(self, name=None, config={}):
def __init__(self, name, config, variables):
self._name = name
self._variables = collections.OrderedDict()
self._variables = variables
self._variable_cfg = collections.OrderedDict()
self._init_options(config)
self._init_units(config)
@ -41,10 +42,6 @@ class Profile(object):
def name(self, value):
self._name = value
@property
def variables(self):
return self._variables
@property
def units(self):
"""
@ -58,3 +55,17 @@ class Profile(object):
Profile global options.
"""
return self._options
@property
def variables(self):
"""
Profile variables (Variables object).
"""
return self._variables
@property
def variable_cfg(self):
"""
Ordered variable configuration collected while merging profiles.
"""
return self._variable_cfg

View file

@ -60,7 +60,7 @@ class Manager(object):
return re.search(unit.uname_regex, uname_string,
re.MULTILINE) is not None
def create(self, instances_config):
def create(self, instances_config, variables):
instance_info_list = []
for instance_name, instance_info in list(instances_config.items()):
if not instance_info.enabled:
@ -105,7 +105,7 @@ class Manager(object):
log.debug("creating '%s' (%s)" % (instance_info.name, instance_info.type))
new_instance = plugin.create_instance(instance_info.name, instance_info.priority, \
instance_info.devices, instance_info.devices_udev_regex, \
instance_info.script_pre, instance_info.script_post, instance_info.options)
instance_info.script_pre, instance_info.script_post, instance_info.options, variables)
instances.append(new_instance)
for instance in instances:
instance.plugin.init_devices()