** 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. - [Test Plan] + Since a suspend is needed, a laptop may be the only way to test this. + "systemctl suspend" might work in a VM, but it probably depends on the + hardware that the VM is emulating. YMMV. - * detailed instructions how to reproduce the bug + Additionally, the fancontrol service will not run if there is no config + file, and pwmconfig will only generate one if it detects that fan + controls are available in the hardware. This again inhibits a bit the + testing from a VM. - * these should allow someone who is not familiar with the affected - package to reproduce the bug and verify that the updated package fixes - the problem. + For the following combinations, check that the desired result is + achieved when resuming from suspend: - * if other testing is appropriate to perform before landing this update, - this should also be described here. + 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 + + For VM testing, my suggestion is to: + a) create a dummy config file (echo "# comment" | sudo tee /etc/fancontrol) + b) monitor "sudo journalctl -u fancontrol -f" and look for restart attempts + c) use "systemctl suspend". It's kind of a fake suspend, but seems enough to trigger the right events. + [Where problems could occur] * Think about what the upload changes in the software. Imagine the change is wrong or breaks something else: how would this show up? * 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. * 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. [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.
** 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. Since a suspend is needed, a laptop may be the only way to test this. "systemctl suspend" might work in a VM, but it probably depends on the hardware that the VM is emulating. YMMV. Additionally, the fancontrol service will not run if there is no config file, and pwmconfig will only generate one if it detects that fan controls are available in the hardware. This again inhibits a bit the - testing from a VM. + testing from a VM, as the service will never really be considered + "active". 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 - - For VM testing, my suggestion is to: - a) create a dummy config file (echo "# comment" | sudo tee /etc/fancontrol) - b) monitor "sudo journalctl -u fancontrol -f" and look for restart attempts - c) use "systemctl suspend". It's kind of a fake suspend, but seems enough to trigger the right events. [Where problems could occur] * Think about what the upload changes in the software. Imagine the change is wrong or breaks something else: how would this show up? * 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. * 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. [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
