search

jsalatas / plasma-pstate

Intel P-state and CPUFreq Manager Widget
GNU General Public License v2.0
274 stars 49 forks source link

System log #16

Closed AlvinZhu closed 5 years ago

AlvinZhu commented 5 years ago

Hi! I like this widget. it's great. But there are too many system logs when I run this widget:

Mar 25 09:43:03 Alvin-Laptop sudo[2121]: alvin : TTY=unknown ; PWD=/home/alvin ; USER=root ; COMMAND=/usr/share/plasma/plasmoids/gr.ictpro.jsalatas.plasma.pstate/contents/code/set_prefs.sh -read-all Mar 25 09:43:03 Alvin-Laptop sudo[2121]: pam_unix(sudo:session): session opened for user root by (uid=0) Mar 25 09:43:03 Alvin-Laptop sudo[2121]: pam_unix(sudo:session): session closed for user root Mar 25 09:43:05 Alvin-Laptop sudo[2144]: alvin : TTY=unknown ; PWD=/home/alvin ; USER=root ; COMMAND=/usr/share/plasma/plasmoids/gr.ictpro.jsalatas.plasma.pstate/contents/code/set_prefs.sh -read-all Mar 25 09:43:05 Alvin-Laptop sudo[2144]: pam_unix(sudo:session): session opened for user root by (uid=0) Mar 25 09:43:05 Alvin-Laptop sudo[2144]: pam_unix(sudo:session): session closed for user root Mar 25 09:43:07 Alvin-Laptop sudo[2167]: alvin : TTY=unknown ; PWD=/home/alvin ; USER=root ; COMMAND=/usr/share/plasma/plasmoids/gr.ictpro.jsalatas.plasma.pstate/contents/code/set_prefs.sh -read-all Mar 25 09:43:07 Alvin-Laptop sudo[2167]: pam_unix(sudo:session): session opened for user root by (uid=0) Mar 25 09:43:07 Alvin-Laptop sudo[2167]: pam_unix(sudo:session): session closed for user root Mar 25 09:43:09 Alvin-Laptop sudo[2190]: alvin : TTY=unknown ; PWD=/home/alvin ; USER=root ; COMMAND=/usr/share/plasma/plasmoids/gr.ictpro.jsalatas.plasma.pstate/contents/code/set_prefs.sh -read-all Mar 25 09:43:09 Alvin-Laptop sudo[2190]: pam_unix(sudo:session): session opened for user root by (uid=0) Mar 25 09:43:09 Alvin-Laptop sudo[2190]: pam_unix(sudo:session): session closed for user root

How can I disable these logs properly？

jsalatas commented 5 years ago

Hi!

Yeah! I noticed these too and was wondering if someone was bothered of these. I am not bothered because in my home's PC and laptop I'm not usually paying attention (it is in auth.log in my case anyway). :\

I'm afraid I don't know anyway to make these go away, so any suggestion would be highly appreciated :)

AlvinZhu commented 5 years ago

Hi, jsalatas I did some testing I removed the "sudo" brefore "set_prefs.sh -read-all" in main.qml. it works fine on my PC for just read intel_pstate and gpu freq. normal user has read permission. I have not tested it on my Dell laptop yet. I guess you add the "sudo" because “smbios-thermal-ctl” need root permission? dell_thermal_mode do not change frequently, so maybe read it every hour or just read it once.

another bug： My desktop PC, has no battery, but the widget display battery "0%".

Thank you again for your awesome widget :)

jsalatas commented 5 years ago

another bug： My desktop PC, has no battery, but the widget display battery "0%".

Is this when running with sudo, or after you removed it?

jsalatas commented 5 years ago

I guess you add the "sudo" because “smbios-thermal-ctl” need root permission? dell_thermal_mode do not change frequently, so maybe read it every hour or just read it once.

No! I won't do it that way. sorry :(

zorael commented 5 years ago

This is really debilitating, I rely on the systemd journal to see what's happening in the system and everything is drowned by these messages. :c

As a workaround I disabled smbios-thermal-ctl functionality by making set_prefs.sh only be called with sudo when making changes and not when monitoring for updates (-read-all).

--- main.qml.bak        2019-04-28 18:11:26.420209023 +0200
+++ main.qml    2019-04-28 18:09:00.539908888 +0200
@@ -193,7 +193,7 @@
         engine: 'executable'

         property bool isReady: false
-        property string commandSource: 'sudo /usr/share/plasma/plasmoids/gr.ictpro.jsalatas.plasma.pstate/contents/code/set_prefs.sh -read-all'
+        property string commandSource: '/usr/share/plasma/plasmoids/gr.ictpro.jsalatas.plasma.pstate/contents/code/set_prefs.sh -read-all'

         onNewData: {
             if (data['exit code'] > 0) {

A quick test shows it seems to be working for me on my Dell XPS 13. The systemd journal is quiet until I make a change. But then I miss out on the Dell thermal stuff.

Is there no way to make smbios-thermal-ctl -g only be called once on start up (and perhaps after resume and AC/battery connection?), and after that only when making changes? It would miss changes made by other programs but it would be better than the current state.

jsalatas commented 5 years ago

I guess the solution here is to have smbios-thermal-ctl to not require root permissions for reading the thermal mode.

I just opened an issue / feature request for this in libsmbios project https://github.com/dell/libsmbios/issues/68

AlvinZhu commented 5 years ago

Hi, @zorael you can try my fork: https://github.com/AlvinZhu/plasma-pstate It works for my XPS15.

jsalatas commented 5 years ago

Here is a solution to get rid of the system log messages with minimum intervention in the code

https://github.com/jsalatas/plasma-pstate/wiki/Too-many-messages-in-system-log

I'm still not sure if this "hack" should be incorporated in the widget, as it involves compiling source code which might not be easy/straightforward to perform in every system and by every user,

dmatej commented 5 years ago

I have Dell G5 15 5587, the solution from wiki does not work. Widget now shows only background and in auth.log I have this:

Jun  3 23:19:26 localhost sudo: pam_unix(sudo:auth): conversation failed
Jun  3 23:19:26 localhost sudo: pam_unix(sudo:auth): auth could not identify password for [dmatej]
jsalatas commented 5 years ago

I have no idea! :\ Sorry! :(

What is your distro? Could you please verify that you didn't miss anything in the wiki?

dmatej commented 5 years ago

Now I tried Alvin's fork and it seems it works! Alvin should create a pull request ;) Distro: Kubuntu 19.04, up to date. Wiki:There is only a typo in "create a new file named set_perfs.c" - it should be set_prefs.c and it seems I forgot to delete the sudo in the last part, sorry.

jsalatas commented 5 years ago

Alvin should create a pull request ;)

No! I wouldn't accept his modifications :p

Wiki:There is only a typo in "create a new file named set_perfs.c" - it should be set_prefs.c

Fixed. Thanks!

it seems I forgot to delete the sudo in the last part, sorry. No problem! Glad that it worked for you after all.

Thanks for your feedback!

arturbalabanov commented 5 years ago

@jsalatas, what if we add a config option for this? If enabled (default), it will run the command with sudo, as it does now (as to keep it backwards compatible). Otherwise, it will just run the command without sudo which as far as I understand, wouldn't cause any problems to non Dell laptops, at least for now.

It should be simple, easy to implement (I'd happily do it if needed) and would provide better user experience (not working with forks, changing the source code etc.). What do you think?

jsalatas commented 5 years ago

@arturbalabanov that would definitely work for non-dell cases (ie when thermal management is missing). I'll try to implement it during the weekend. However, you would still need some workaround in case you want the thermal management feature.