Fix watchdogd disarm spam#3623
Fix watchdogd disarm spam#3623essess-ss wants to merge 2 commits intou-root:mainu-root/u-root:mainfrom essess-ss:fix-watchdogd-disarm-spamessess-ss/u-root:fix-watchdogd-disarm-spamCopy head branch name to clipboard
Conversation
599f1d1 to
83af213
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3623 +/- ##
==========================================
+ Coverage 60.92% 60.95% +0.03%
==========================================
Files 659 659
Lines 46507 46510 +3
==========================================
+ Hits 28333 28349 +16
+ Misses 18174 18161 -13
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
- Stop petting loop and clear CurrentWd on disarm. - Make StartPetting idempotent. - Auto-start petting on arm. - Fix potential nil pointer dereference on SetTimeout/SetPreTimeout failure paths in ArmWatchdog. - Add unit tests for daemon start/stop and petting loop using a mock device file. Signed-off-by: Sahil Sharma <sharmasah@google.com>
0125745 to
3d50fcd
Compare
| } | ||
| if d.CurrentOpts.Timeout != timeoutIgnore { | ||
| if err := wd.SetTimeout(d.CurrentOpts.Timeout); err != nil { | ||
| d.CurrentWd.Close() |
There was a problem hiding this comment.
Wow, this is crazy that it happened this way.
| if err := wd.SetTimeout(d.CurrentOpts.Timeout); err != nil { | ||
| d.CurrentWd.Close() | ||
| wd.Close() | ||
| log.Printf("Failed to set timeout: %v", err) |
There was a problem hiding this comment.
Pretty much every print in this package should be removed. This kind of print in a package is asking for trouble -- as you saw. I'm going to guess this was a command that got turned into package?
| d.CurrentWd.Close() | ||
| wd.Close() | ||
| log.Printf("Failed to set pretimeout: %v", err) | ||
| return OpResultError |
There was a problem hiding this comment.
we've got errors package now, and fmt.Errorf, let's use them?
return fmt.Errorf("failed to set pretimeout: %w", err)
same for all the other returns.
| log.Printf("No armed Watchdog") | ||
| return OpResultOk | ||
| } | ||
| if r := d.StopPetting(); r != OpResultOk { |
There was a problem hiding this comment.
it seems very odd that these functions return runes instead of error -- what's the purpose of that? the convention would be
if err := d.StopPetting(); err != nil
What's the pupose of having them return runes? Or is this something you inherited :-)
There was a problem hiding this comment.
ah I see where it came from. But it should be fixed, if you are willilng.
pkg/watchdogd: fix disarm spam and potential panics
Why?
Because of the below spams once the watchdog is disarmed and waiting for host os kexec:
watchdogd: 2026/06/03 16:36:10 New op received: D
watchdogd: 2026/06/03 16:36:10 Watchdog disarming request went through (Watchdog will not be disabled if CONFIG_WATCHDOG_NOWAYOUT is enabled)
watchdogd: 2026/06/03 16:36:11 Failed to keep alive: write /dev/watchdog: file already closed
watchdogd: 2026/06/03 16:36:16 Failed to keep alive: write /dev/watchdog: file already closed
watchdogd: 2026/06/03 16:36:21 Failed to keep alive: write /dev/watchdog: file already closed
watchdogd: 2026/06/03 16:36:26 Failed to keep alive: write /dev/watchdog: file already closed
watchdogd: 2026/06/03 16:36:31 Failed to keep alive: write /dev/watchdog: file already closed
watchdogd: 2026/06/03 16:36:36 Failed to keep alive: write /dev/watchdog: file already closed
watchdogd: 2026/06/03 16:36:41 Failed to keep alive: write /dev/watchdog: file already closed
watchdogd: 2026/06/03 16:36:46 Failed to keep alive: write /dev/watchdog: file already closed
watchdogd: 2026/06/03 16:36:51 Failed to keep alive: write /dev/watchdog: file already closed