** Description changed: [Impact] The fix for bug #1882272, currently applied to Impish and Jammy via a debian sync, has a collateral effect that it may leave the fancontrol service started after a resume from suspend even if it was disabled and/or stopped. The fix wraps the restart call in a is-active check, and only issues the restart if the fancontrol service was running before (at suspend time). [Test Plan] The goal is to compare the state of the fancontrol service before and after suspend. It should be the same. There are two hardware requirements to fully test this SRU: - a machine capable of suspend/resume, like a laptop - a machine with hardware fan control compatible with fancontrol/lm-sensors A valid /etc/fancontrol configuration file must be generated, either manually or via pwmconfig(8). Without this file, the fancontrol service won't start. I didn't have any of those when preparing the fix, so I used a similarly configured systemd service to emulate what would happen (rsync in this case, which also depends on a config file to start). With the above setup, this is the test plan. For the following combinations, check that the desired result is achieved when resuming from suspend: At suspend time -> When resuming: Service is enabled and active --> is restarted Service is disabled and active --> is restarted Service is enabled and inactive --> is not restarted Service is disabled and inactive --> is not restarted [Where problems could occur] + One could argue that this is changing behavior, but I think in the right direction. - * Think about what the upload changes in the software. Imagine the change is - wrong or breaks something else: how would this show up? + I might have missed some scenario where a restart of fancontrol is + *always* desired, regardless of its previous state. - * It is assumed that any SRU candidate patch is well-tested before - upload and has a low overall risk of regression, but it's important - to make the effort to think about what ''could'' happen in the - event of a regression. + If the script in /lib/systemd/system-sleep fails, there doesn't seem to + be any impact on the resume event. An initial iteration of my fix was + calling systemctl is-active incorrectly, and all that happened was a log + entry about that. - * This must '''never''' be "None" or "Low", or entirely an argument as to why - your upload is low risk. - - * This both shows the SRU team that the risks have been considered, - and provides guidance to testers in regression-testing the SRU. + If the fancontrol daemon was started outside of systemd, then "systemctl + is-active" won't see it, and it won't be restarted after resume. That is + already an edge case without the is-active check, as "systemctl restart" + might also fail to start fancontrol again if one copy is already + running. [Other Info] * Anything else you think is useful to include * Anticipate questions from users, SRU, +1 maintenance, security teams and the Technical Board * and address these questions in advance [Original Description] The fix for bug #1882272 issues a "systemctl restart fancontrol" when resuming from suspend. This ignores the fact that fancontrol could have been disabled, or could not even have been runing at suspend time. I suggest to wrap the restart with a "systemctl is-active fancontrol.service" call, and only issue the restart if fancontrol was already running when the machine was suspended.
-- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/1967432 Title: After resume from suspend, fancontrol can be running even if it was disabled before To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/lm-sensors/+bug/1967432/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
