Persistence of dynamic tuning changes#758
Open
adriaan42 wants to merge 2 commits into
Open
Conversation
adriaan42
force-pushed
the
adriaan/persistence
branch
3 times, most recently
from
d6c2b69 to
f0bdfb8
Compare
Move the calculation if the active state of an instance from _add_devices_nocheck() to _add_devices_process(), to make it symmetric with device removal. Signed-off-by: Adriaan Schmidt <adriaan.schmidt@siemens.com>
Currently, any changes made to the tuning via the `instance_*` dbus calls
are lost when tuning is stopped by the service, or when the TuneD service
itself is stopped/restarted, or when the service crashes.
This commit:
* implements a sync of Plugin Instances and Profile Units that is currently missing.
This way, dynamic instances and device assignments are persistent across stop/start
dbus calls to TuneD.
* calculates a hash of the current profile after loading it from disk
(after processing all includes, so we have a "flat" representation)
* creates snapshots of the current profile whenever instances or assigned
devices change. the snapshot includes the hash of the profile as
it was initially loaded. for each instance it stores the devices that
are currently attached.
* restores a snapshot found at startup, if the hashes match (i.e. there
have been no profile switches and no changes to the profile or any of
its includes on disk)
snapshots are restored in case of
- daemon restarts (systemctl restart/stop/start)
- daemon crashes
snapshots are NOT restored in case of
- reboots (snapshots are stored in /var/run)
- profile changes (snapshots are explicitly deleted when switching profiles,
even when "switching" to the same/current profile)
Signed-off-by: Adriaan Schmidt <adriaan.schmidt@siemens.com>
adriaan42
force-pushed
the
adriaan/persistence
branch
from
f0bdfb8 to
709b1a4
Compare
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR introduces a profile snapshot mechanism to persist and restore dynamic instance configuration. Profile and Unit classes become serializable with hash validation, the loader manages snapshot lifecycle on disk, plugins support device restoration, the daemon orchestrates unit synchronization with snapshots, and the controller triggers sync at device operation boundaries. ChangesProfile Snapshot and Instance Synchronization
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
tuned/daemon/daemon.py (1)
185-185: ⚡ Quick winVerify access to private
_pluginattribute.The code accesses
instance._plugin.nameat line 185, using the private attribute_plugin(underscore prefix). From the Instance class context snippet, there's a public propertypluginthat should be used instead.♻️ Proposed fix to use public API
config = { "priority": instance.priority, - "type": instance._plugin.name, + "type": instance.plugin.name, "enabled": instance.active,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tuned/daemon/daemon.py` at line 185, The code is accessing the private attribute instance._plugin.name; replace this with the public API by using instance.plugin.name instead. Locate occurrences that reference instance._plugin (specifically the one producing the "type" field) and change them to use the Instance.property plugin (e.g., instance.plugin.name), ensuring any None/absent-plugin checks use the public property as well.tuned/profiles/unit.py (1)
54-55: ⚡ Quick winValidate space-joining for list values.
The code joins list elements with spaces (
" ".join(str(v) for v in value)). If any element contains internal spaces, the round-trip through the profile loader will fail—parsing will split on all spaces, treating a single element with spaces as multiple elements.From the comment mentioning "absolute paths," this appears to be safe for the current use case (file paths typically don't contain spaces in tuned profiles). However, if this method is later used for other list types, it could break silently.
💡 Consider documenting the space constraint
`@staticmethod` def _snapshot_value(value): - """serialize an option value into a form that round-trips through the profile loader""" + """serialize an option value into a form that round-trips through the profile loader + + Note: list elements must not contain spaces, as they are joined with spaces for serialization. + """ # some options (e.g. the script plugin's "script") are stored as lists of # absolute paths; emit them space-joined so they are not rendered as a list repr if isinstance(value, list):🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tuned/profiles/unit.py` around lines 54 - 55, The current list serialization uses " ".join(str(v) for v in value) which breaks round-trips when elements contain spaces; replace this ad-hoc join with a robust serializer (e.g., JSON) so lists round-trip safely: change the writer in tuned/profiles/unit.py to emit a JSON-encoded string for list values (use json.dumps on value) and update the corresponding profile loader/consumer to parse with json.loads, or alternatively document the space-only constraint clearly if you must keep the space-join; refer to the list handling expression (" ".join(str(v) for v in value)) and ensure the paired reader/parser is updated to match the new serialization.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tuned/plugins/base.py`:
- Around line 193-195: The active flag is currently set using only
instance.assigned_devices; update the logic around the assignment to consider
both instance.processed_devices and instance.assigned_devices (e.g., active =
len(processed_devices) + len(assigned_devices) > 0 or check either list
non-empty) so instance.active mirrors the hotplug handlers' behavior; modify the
assignment of instance.active (and the following log warning) in the same block
in base.py to use both collections when deciding activity.
- Around line 167-177: The restore_devices method currently skips devices not in
_free_devices silently; update restore_devices to record any skipped/missing
devices, log a warning for each missing device (use the existing log object)
referencing the instance.name and device, and return or expose the list of
failed devices so callers can detect partial restores; modify the function that
contains restore_devices to append missing devices to a local failed_devices
list, call log.warning("Failed to restore device %s for instance %s: not in free
pool", device, instance.name) for each, and after the loop return the
failed_devices (or raise a specific exception if the caller semantics require
immediate failure) while keeping the existing behavior of removing and assigning
devices for those present in _free_devices and updating
instance.assigned_devices and _assigned_devices.
In `@tuned/profiles/loader.py`:
- Around line 69-73: The create_snapshot function currently writes snapshot data
to consts.PROFILE_SNAPSHOT_FILE without handling I/O errors; wrap the open/write
in a try/except catching OSError (or IOError) and handle failures by logging a
clear error with the exception details (use log.error and include
consts.PROFILE_SNAPSHOT_FILE and the exception), optionally ensure the parent
directory exists (os.makedirs(..., exist_ok=True)) before opening, and decide to
either return gracefully or re-raise after logging so the snapshot workflow
doesn't crash silently; refer to create_snapshot, profile.snapshot, and
consts.PROFILE_SNAPSHOT_FILE to locate where to apply this change.
- Around line 85-99: snapshot_hash is being compared to profile._base_hash
without validating either value; update the restore branch in loader.py to
explicitly check that both snapshot_hash and profile._base_hash are not None (or
non-empty) before doing the equality check, and if profile._base_hash is None
log an error/warning and skip restoring (and remove consts.PROFILE_SNAPSHOT_FILE
as currently done for mismatches); reference the symbols snapshot_hash,
profile._base_hash, calculate_hash() (which should be ensured to have run or its
failure handled), and consts.PROFILE_SNAPSHOT_FILE to locate where to add the
pre-check and the early-skip behavior.
In `@tuned/profiles/profile.py`:
- Line 14: The __init__ signature currently uses a mutable default config={}
which can cause shared-state bugs; change the parameter to config=None in the
Profile.__init__ and inside the method set config = {} if config is None (or
copy it if you need to avoid aliasing), then use that local dict for further
logic—update references to the parameter within __init__ accordingly to avoid
any shared-mutation between instances.
---
Nitpick comments:
In `@tuned/daemon/daemon.py`:
- Line 185: The code is accessing the private attribute instance._plugin.name;
replace this with the public API by using instance.plugin.name instead. Locate
occurrences that reference instance._plugin (specifically the one producing the
"type" field) and change them to use the Instance.property plugin (e.g.,
instance.plugin.name), ensuring any None/absent-plugin checks use the public
property as well.
In `@tuned/profiles/unit.py`:
- Around line 54-55: The current list serialization uses " ".join(str(v) for v
in value) which breaks round-trips when elements contain spaces; replace this
ad-hoc join with a robust serializer (e.g., JSON) so lists round-trip safely:
change the writer in tuned/profiles/unit.py to emit a JSON-encoded string for
list values (use json.dumps on value) and update the corresponding profile
loader/consumer to parse with json.loads, or alternatively document the
space-only constraint clearly if you must keep the space-join; refer to the list
handling expression (" ".join(str(v) for v in value)) and ensure the paired
reader/parser is updated to match the new serialization.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 9c6b2b8b-8ae3-4dc8-9eb3-0c0fb5c70b34
📒 Files selected for processing (10)
tests/unit/profiles/test_profile.pytuned/consts.pytuned/daemon/controller.pytuned/daemon/daemon.pytuned/plugins/base.pytuned/plugins/hotplug.pytuned/profiles/loader.pytuned/profiles/profile.pytuned/profiles/unit.pytuned/units/manager.py
Comment on lines
+167
to
+177
| def restore_devices(self, instance, devices): | ||
| if not self._devices_supported: | ||
| return | ||
|
|
||
| log.debug("Restoring devices of instance %s: %s" % (instance.name, " ".join(devices))) | ||
| for device in devices: | ||
| if device not in self._free_devices: | ||
| continue | ||
| self._free_devices.remove(device) | ||
| instance.assigned_devices.add(device) | ||
| self._assigned_devices.add(device) |
There was a problem hiding this comment.
Log or handle devices that cannot be restored.
The method silently skips devices not in _free_devices (line 173-174). If a device is missing from the free pool (e.g., already assigned to another instance or removed from the system), the restore operation partially fails without notification. This could leave instances in an inconsistent state compared to the snapshot.
📋 Proposed fix to log missing devices
def restore_devices(self, instance, devices):
if not self._devices_supported:
return
log.debug("Restoring devices of instance %s: %s" % (instance.name, " ".join(devices)))
for device in devices:
if device not in self._free_devices:
+ log.warning("Cannot restore device '%s' to instance '%s': device not in free pool" % (device, instance.name))
continue
self._free_devices.remove(device)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def restore_devices(self, instance, devices): | |
| if not self._devices_supported: | |
| return | |
| log.debug("Restoring devices of instance %s: %s" % (instance.name, " ".join(devices))) | |
| for device in devices: | |
| if device not in self._free_devices: | |
| continue | |
| self._free_devices.remove(device) | |
| instance.assigned_devices.add(device) | |
| self._assigned_devices.add(device) | |
| def restore_devices(self, instance, devices): | |
| if not self._devices_supported: | |
| return | |
| log.debug("Restoring devices of instance %s: %s" % (instance.name, " ".join(devices))) | |
| for device in devices: | |
| if device not in self._free_devices: | |
| log.warning("Cannot restore device '%s' to instance '%s': device not in free pool" % (device, instance.name)) | |
| continue | |
| self._free_devices.remove(device) | |
| instance.assigned_devices.add(device) | |
| self._assigned_devices.add(device) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tuned/plugins/base.py` around lines 167 - 177, The restore_devices method
currently skips devices not in _free_devices silently; update restore_devices to
record any skipped/missing devices, log a warning for each missing device (use
the existing log object) referencing the instance.name and device, and return or
expose the list of failed devices so callers can detect partial restores; modify
the function that contains restore_devices to append missing devices to a local
failed_devices list, call log.warning("Failed to restore device %s for instance
%s: not in free pool", device, instance.name) for each, and after the loop
return the failed_devices (or raise a specific exception if the caller semantics
require immediate failure) while keeping the existing behavior of removing and
assigning devices for those present in _free_devices and updating
instance.assigned_devices and _assigned_devices.
Comment on lines
+193
to
+195
| instance.active = len(instance.assigned_devices) > 0 | ||
| if not instance.active: | ||
| log.warning("instance %s: no matching devices available" % instance.name) |
There was a problem hiding this comment.
Include processed devices when determining instance activity.
The active state at line 193 is based only on assigned_devices, but the hotplug plugin's device add/remove handlers (shown in relevant snippet from hotplug.py:29-60) set instance.active based on both processed_devices and assigned_devices. After device restoration, processed_devices may be non-empty (if devices were previously processed), so the active state calculation should include both sets for consistency.
🐛 Proposed fix
- instance.active = len(instance.assigned_devices) > 0
+ instance.active = len(instance.assigned_devices) + len(instance.processed_devices) > 0
if not instance.active:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| instance.active = len(instance.assigned_devices) > 0 | |
| if not instance.active: | |
| log.warning("instance %s: no matching devices available" % instance.name) | |
| instance.active = len(instance.assigned_devices) + len(instance.processed_devices) > 0 | |
| if not instance.active: | |
| log.warning("instance %s: no matching devices available" % instance.name) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tuned/plugins/base.py` around lines 193 - 195, The active flag is currently
set using only instance.assigned_devices; update the logic around the assignment
to consider both instance.processed_devices and instance.assigned_devices (e.g.,
active = len(processed_devices) + len(assigned_devices) > 0 or check either list
non-empty) so instance.active mirrors the hotplug handlers' behavior; modify the
assignment of instance.active (and the following log warning) in the same block
in base.py to use both collections when deciding activity.
Comment on lines
+69
to
+73
| def create_snapshot(self, profile, instances): | ||
| snapshot = profile.snapshot(instances) | ||
| log.debug("Storing profile snapshot in %s:\n%s" % (consts.PROFILE_SNAPSHOT_FILE, snapshot)) | ||
| with open(consts.PROFILE_SNAPSHOT_FILE, "w") as f: | ||
| f.write(snapshot) |
There was a problem hiding this comment.
Add error handling for file write operation.
The open() call at line 72 can fail due to permission issues, disk full, or missing parent directory. Although /run/tuned/ typically exists in running systems, failures here would crash the snapshot workflow without a clear error message.
🛡️ Proposed fix to add error handling
def create_snapshot(self, profile, instances):
snapshot = profile.snapshot(instances)
log.debug("Storing profile snapshot in %s:\n%s" % (consts.PROFILE_SNAPSHOT_FILE, snapshot))
- with open(consts.PROFILE_SNAPSHOT_FILE, "w") as f:
- f.write(snapshot)
+ try:
+ with open(consts.PROFILE_SNAPSHOT_FILE, "w") as f:
+ f.write(snapshot)
+ except (OSError, IOError) as e:
+ log.error("Failed to write profile snapshot: %s" % e)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def create_snapshot(self, profile, instances): | |
| snapshot = profile.snapshot(instances) | |
| log.debug("Storing profile snapshot in %s:\n%s" % (consts.PROFILE_SNAPSHOT_FILE, snapshot)) | |
| with open(consts.PROFILE_SNAPSHOT_FILE, "w") as f: | |
| f.write(snapshot) | |
| def create_snapshot(self, profile, instances): | |
| snapshot = profile.snapshot(instances) | |
| log.debug("Storing profile snapshot in %s:\n%s" % (consts.PROFILE_SNAPSHOT_FILE, snapshot)) | |
| try: | |
| with open(consts.PROFILE_SNAPSHOT_FILE, "w") as f: | |
| f.write(snapshot) | |
| except (OSError, IOError) as e: | |
| log.error("Failed to write profile snapshot: %s" % e) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tuned/profiles/loader.py` around lines 69 - 73, The create_snapshot function
currently writes snapshot data to consts.PROFILE_SNAPSHOT_FILE without handling
I/O errors; wrap the open/write in a try/except catching OSError (or IOError)
and handle failures by logging a clear error with the exception details (use
log.error and include consts.PROFILE_SNAPSHOT_FILE and the exception),
optionally ensure the parent directory exists (os.makedirs(..., exist_ok=True))
before opening, and decide to either return gracefully or re-raise after logging
so the snapshot workflow doesn't crash silently; refer to create_snapshot,
profile.snapshot, and consts.PROFILE_SNAPSHOT_FILE to locate where to apply this
change.
Comment on lines
+85
to
+99
| if snapshot_hash == profile._base_hash: | ||
| snapshot = self._profile_factory.create("restore", config) | ||
| snapshot.name = profile.name | ||
| # the snapshot is created directly (not via the merger), | ||
| # so extract its [variables] section manually | ||
| if consts.PLUGIN_VARIABLES_UNIT_NAME in snapshot.units: | ||
| snapshot.variables.update(snapshot.units[consts.PLUGIN_VARIABLES_UNIT_NAME].options) | ||
| del snapshot.units[consts.PLUGIN_VARIABLES_UNIT_NAME] | ||
| self._variables.add_from_cfg(snapshot.variables) | ||
| self._expand_vars_in_devices(snapshot) | ||
| self._expand_vars_in_regexes(snapshot) | ||
| log.info("Restored profile snapshot: %s" % snapshot.name) | ||
| else: | ||
| log.debug("Snapshot hash '%s' does not match current base hash '%s'. Not restoring." % (snapshot_hash, profile._base_hash)) | ||
| os.remove(consts.PROFILE_SNAPSHOT_FILE) |
There was a problem hiding this comment.
Validate hash values before comparison.
At line 85, snapshot_hash and profile._base_hash are compared without checking if either is None. If profile._base_hash is None (e.g., if calculate_hash() was never called or failed), the comparison snapshot_hash == profile._base_hash could incorrectly match when snapshot_hash is also None, potentially restoring a snapshot against the wrong profile.
From the code flow, calculate_hash() is called at line 57 after loading, so profile._base_hash should be set. However, if an exception occurs during hashing, _base_hash could remain None.
🛡️ Proposed fix to add validation
try:
config = self._load_config_data(consts.PROFILE_SNAPSHOT_FILE)
snapshot_hash = config.get("main", {}).get("profile_base_hash", None)
- if snapshot_hash == profile._base_hash:
+ if snapshot_hash is not None and profile._base_hash is not None and snapshot_hash == profile._base_hash:
snapshot = self._profile_factory.create("restore", config)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if snapshot_hash == profile._base_hash: | |
| snapshot = self._profile_factory.create("restore", config) | |
| snapshot.name = profile.name | |
| # the snapshot is created directly (not via the merger), | |
| # so extract its [variables] section manually | |
| if consts.PLUGIN_VARIABLES_UNIT_NAME in snapshot.units: | |
| snapshot.variables.update(snapshot.units[consts.PLUGIN_VARIABLES_UNIT_NAME].options) | |
| del snapshot.units[consts.PLUGIN_VARIABLES_UNIT_NAME] | |
| self._variables.add_from_cfg(snapshot.variables) | |
| self._expand_vars_in_devices(snapshot) | |
| self._expand_vars_in_regexes(snapshot) | |
| log.info("Restored profile snapshot: %s" % snapshot.name) | |
| else: | |
| log.debug("Snapshot hash '%s' does not match current base hash '%s'. Not restoring." % (snapshot_hash, profile._base_hash)) | |
| os.remove(consts.PROFILE_SNAPSHOT_FILE) | |
| if snapshot_hash is not None and profile._base_hash is not None and snapshot_hash == profile._base_hash: | |
| snapshot = self._profile_factory.create("restore", config) | |
| snapshot.name = profile.name | |
| # the snapshot is created directly (not via the merger), | |
| # so extract its [variables] section manually | |
| if consts.PLUGIN_VARIABLES_UNIT_NAME in snapshot.units: | |
| snapshot.variables.update(snapshot.units[consts.PLUGIN_VARIABLES_UNIT_NAME].options) | |
| del snapshot.units[consts.PLUGIN_VARIABLES_UNIT_NAME] | |
| self._variables.add_from_cfg(snapshot.variables) | |
| self._expand_vars_in_devices(snapshot) | |
| self._expand_vars_in_regexes(snapshot) | |
| log.info("Restored profile snapshot: %s" % snapshot.name) | |
| else: | |
| log.debug("Snapshot hash '%s' does not match current base hash '%s'. Not restoring." % (snapshot_hash, profile._base_hash)) | |
| os.remove(consts.PROFILE_SNAPSHOT_FILE) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tuned/profiles/loader.py` around lines 85 - 99, snapshot_hash is being
compared to profile._base_hash without validating either value; update the
restore branch in loader.py to explicitly check that both snapshot_hash and
profile._base_hash are not None (or non-empty) before doing the equality check,
and if profile._base_hash is None log an error/warning and skip restoring (and
remove consts.PROFILE_SNAPSHOT_FILE as currently done for mismatches); reference
the symbols snapshot_hash, profile._base_hash, calculate_hash() (which should be
ensured to have run or its failure handled), and consts.PROFILE_SNAPSHOT_FILE to
locate where to add the pre-check and the early-skip behavior.
| __slots__ = ["_name", "_options", "_variables", "_units"] | ||
| __slots__ = ["_name", "_options", "_variables", "_units", "_base_hash"] | ||
|
|
||
| def __init__(self, name=None, config={}): |
There was a problem hiding this comment.
Replace mutable default argument with None.
The default mutable argument config={} is shared across all calls. If any caller mutates this dict (e.g., via config.setdefault(...)), the mutations persist and affect subsequent calls. Although the current implementation does not appear to mutate config, this pattern creates a latent defect that can cause hard-to-debug state pollution.
As per coding guidelines, mutable default arguments should be replaced with None and initialized within the function.
🐛 Proposed fix
- def __init__(self, name=None, config={}):
+ def __init__(self, name=None, config=None):
+ if config is None:
+ config = {}
self._name = name📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def __init__(self, name=None, config={}): | |
| def __init__(self, name=None, config=None): | |
| if config is None: | |
| config = {} | |
| self._name = name |
🧰 Tools
🪛 Ruff (0.15.15)
[warning] 14-14: Do not use mutable data structures for argument defaults
Replace with None; initialize within function
(B006)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tuned/profiles/profile.py` at line 14, The __init__ signature currently uses
a mutable default config={} which can cause shared-state bugs; change the
parameter to config=None in the Profile.__init__ and inside the method set
config = {} if config is None (or copy it if you need to avoid aliasing), then
use that local dict for further logic—update references to the parameter within
__init__ accordingly to avoid any shared-mutation between instances.
Source: Linters/SAST tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Currently, any changes made to the tuning via the
instance_*dbus calls are lost when tuning is stopped by the service, or when the TuneD service itself is stopped/restarted, or when the service crashes.This PR:
implements a sync of Plugin Instances and Profile Units that is currently missing. This way, dynamic instances and device assignments are persistent across stop/start dbus calls to TuneD.
calculates a hash of the current profile after loading it from disk (after processing all includes, so we have a "flat" representation)
creates snapshots of the current profile whenever instances or assigned devices change. the snapshot includes the hash of the profile as it was initially loaded. for each instance it stores the devices that are currently attached.
restores a snapshot found at startup, if the hashes match (i.e. there have been no profile switches and no changes to the profile or any of its includes on disk)
snapshots are restored in case of
snapshots are NOT restored in case of
even when "switching" to the same/current profile)