[node-patches] Change in ovirt-node[master]: RFC: Add tuned configuration

[fabiand at fedoraproject.org]

Fabian Deutsch has posted comments on this change.

Change subject: RFC:  Add tuned configuration
......................................................................


Patch Set 4: I would prefer that you didn't submit this

(4 inline comments)

....................................................
File src/ovirt/node/config/tuned.py
Line 29:     """
Line 30:     keys = ("OVIRT_TUNED_PROFILE",)
Line 31: 
Line 32:     def update(self, profile):
Line 33:         all_profiles = tuned.get_available_profiles()
Does "None" or "Off" needs to be added to all_profiles? Depends if get-available_profiles contains them.
Line 34:         if profile not in all_profiles:
Line 35:             raise InvalidData("Unknown tuned profile: %s" % profile)
Line 36: 
Line 37:     def transaction(self):


Line 46:         class CallTunedAdm(utils.Transaction.Element):
Line 47:             title = "Requesting profile change"
Line 48: 
Line 49:             def commit(self):
Line 50:                 tuned.set_active_profile(profile)
Does some configuration file needs to be persisted? IIUIC tuned-adm is normally persisting the changes so altering some file.
Line 51: 
Line 52:         tx = utils.Transaction("Applying tuned configuration")
Line 53:         if profile:
Line 54:             # Only add the element if there is a profile to be set


....................................................
File src/ovirt/node/setup/performance_page.py
Line 35:     def rank(self):
Line 36:         return 100
Line 37: 
Line 38:     def model(self):
Line 39:         profile = tuned.get_active_profile()
Normally the runtime profile and the configured profile shouldn't differ, but just for the sake of clean code: Wouldn't you wanto to retrieve the profile from config.tuned.Tuned.retrieve()?

Maybe a sanity check can be added here to _verify_ that

  tuned.get_active_profile() == config-tuned-Tuned()-retrieve()["profile"]
Line 40:         model = {
Line 41:             "tuned.profile": profile,
Line 42:         }
Line 43:         return model


....................................................
File src/ovirt/node/utils/tuned.py
Line 51:     else:
Line 52:         return profile.replace("\n", "").split(" ")[3]
Line 53: 
Line 54: 
Line 55: def set_active_profile(profile):
as said in othe rcommetns, please use exceptions instead of returning early.
Line 56:     """Sets the active tuned profile on the system.
Line 57: 
Line 58:     Returns:
Line 59:         A boolean based on the return of tuned-adm


--
To view, visit http://gerrit.ovirt.org/14962
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic1ed6db48bfeeed962f1bff656d71243281e1597
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-node
Gerrit-Branch: master
Gerrit-Owner: Michael Burns <mburns at redhat.com>
Gerrit-Reviewer: Fabian Deutsch <fabiand at fedoraproject.org>
Gerrit-Reviewer: Michael Burns <mburns at redhat.com>