Draft: Bug in Variable handling

tests/unit/plugins/test_base.py

@@ -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 @@ def test_option_bool(self):
 	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 @@ def test_get_matching_devices(self):
 		""" 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 @@ def test_get_matching_devices(self):
 		""" 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,15 +104,15 @@ def test_check_commands(self):
 
 	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)
 
 		self.assertEqual(self._commands_plugin._size,'XXL')
 
 	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 @@ def test_process_assignment_modifiers(self):
 
 	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]

tests/unit/profiles/test_loader.py

@@ -53,8 +53,7 @@ def setUp(self):
 		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('*'))

tests/unit/profiles/test_merger.py

@@ -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 @@ def test_merge_without_replace(self):
 
 	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 @@ def test_merge_with_replace(self):
 
 	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")

tests/unit/profiles/test_profile.py

@@ -9,33 +9,33 @@ def _create_unit(self, name, config):
 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,15 +44,15 @@ def test_sets_options(self):
 		profile = MockProfile("test", {
 			"main": { "anything": 10 },
 			"network" : { "type": "net", "devices": "*" },
-		})
+		}, None)
 
 		self.assertIs(type(profile.options), dict)
 		self.assertEqual(profile.options["anything"], 10)
 
 	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)

tuned/daemon/application.py

@@ -39,10 +39,9 @@ def __init__(self, profile_name = None, config = None):
 		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 @@ def __init__(self, profile_name = None, config = None):
 		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)

tuned/daemon/daemon.py

@@ -196,7 +196,7 @@ def _thread_code(self):
 		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)

tuned/plugins/base.py

@@ -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 @@ def __init__(self, monitors_repository, storage_factory, hardware_inventory, dev
 		self._init_commands()
 
 		self._global_cfg = global_cfg
-		self._variables = variables
 		self._has_dynamic_options = False
 		self._devices_inited = False
 
@@ -93,14 +92,14 @@ def _option_bool(self, value):
 	# 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))
 
@@ -239,7 +238,7 @@ def _call_device_script(self, instance, script, op, devices, rollback = consts.R
 		ret = True
 		for dev in devices:
 			environ = os.environ
-			environ.update(self._variables.get_env())
+			environ.update(instance._variables.get_env())
 			arguments = [op]
 			if rollback == consts.ROLLBACK_FULL:
 				arguments.append("full_rollback")
@@ -448,13 +447,13 @@ def _storage_unset(self, instance, command, device_name=None):
 
 	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:
@@ -463,7 +462,7 @@ def _execute_all_device_commands(self, instance, devices):
 	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
@@ -472,7 +471,7 @@ def _verify_all_non_device_commands(self, instance, ignore_missing):
 	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:

tuned/plugins/instance/instance.py

@@ -4,14 +4,15 @@ 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
 		self._devices_udev_regex = devices_udev_regex
 		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 @@ def script_post(self):
 	def options(self):
 		return self._options
 
+	@property
+	def variables(self):
+		return self._variables
+
 	@property
 	def has_static_tuning(self):
 		return self._has_static_tuning

tuned/plugins/plugin_sysfs.py

@@ -51,7 +51,7 @@ def _instance_cleanup(self, instance):
 
 	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 @@ def _instance_apply_static(self, instance):
 	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)

tuned/plugins/repository.py

@@ -8,7 +8,7 @@
 
 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 @@ def __init__(self, monitor_repository, storage_factory, hardware_inventory, devi
 		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 @@ def create(self, plugin_name):
 		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
 

tuned/profiles/factory.py

@@ -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)