fix(avahi): start stopped daemon on update#2661
Merged
Merged
Conversation
🔧 PR Test Plugin AvailableA test plugin has been generated for this PR that includes the modified files. Version: 📥 Installation Instructions:Install via Unraid Web UI:
Alternative: Direct Download
|
Purpose of the change: - Let rc.avahidaemon update recover Avahi when the daemon exited during a network transition. Before this change: - The update action only did work when Avahi was already running. - If Avahi failed to restart while no suitable network interface was available, later update calls could not start it again. Why that was a problem: - A transient network outage could leave Avahi stopped even after networking recovered. - Manual restart worked, but the normal service update path could not recover the daemon. What the new change accomplishes: - When computed bindings differ from the current Avahi configuration, update restarts Avahi if it is running or starts it if it is stopped. - Existing restart-on-bind-mismatch behavior remains intact when Avahi is running. How it works: - avahid_update runs the existing check and bind mismatch comparison first. - If the bind list changed and Avahi is running, it uses the existing restart path. - If the bind list changed and Avahi is stopped, it delegates to avahid_start.
elibosley
force-pushed
the
codex/avahi-update-start-stopped
branch
from
dd85954 to
de8897b
Compare
Contributor
WalkthroughThe avahid_update function now performs conditional config updates when ChangesAvahi Daemon Update Logic
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@etc/rc.d/rc.avahidaemon`:
- Around line 123-129: The current guard (if check && [[ "$(this
allow-interfaces)" != "$BIND" ]]) prevents avahid_start from running when the
daemon is stopped but the allow-interfaces list equals BIND; change the logic so
the outer check only validates "check" (and any BIND sanity you need) and move
the allow-interfaces != BIND test to only gate the avahid_restart path: call
avahid_restart when avahid_running and allow-interfaces differs from BIND, and
call avahid_start when avahid_running is false (regardless of the
allow-interfaces == BIND condition) so stopped-daemon recovery is attempted
independently; reference avahid_running, avahid_restart, avahid_start, check,
BIND and this allow-interfaces in your changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 8ee16fe6-91db-473a-815b-84b1facc8799
📒 Files selected for processing (1)
etc/rc.d/rc.avahidaemon
Comment on lines
+123
to
+129
| if check && [[ "$(this allow-interfaces)" != "$BIND" ]]; then | ||
| if avahid_running; then | ||
| log "Updating $DAEMON..." | ||
| avahid_restart # note we need restart here, not reload in order to update interfaces | ||
| else | ||
| avahid_start # handle case where avahi-daemon exited because no suitable interfaces | ||
| fi |
Contributor
There was a problem hiding this comment.
Stopped-daemon recovery is still blocked when bind list matches config.
At Line 123, both restart and start paths are gated by allow-interfaces != BIND. If Avahi is stopped and interfaces recover to the same bind list (common after transient loss), update remains a no-op and Avahi stays down. The mismatch check should gate only the running restart path; the stopped recovery path should be independent (with check/BIND validation).
Suggested fix
avahid_update(){
- if check && [[ "$(this allow-interfaces)" != "$BIND" ]]; then
- if avahid_running; then
- log "Updating $DAEMON..."
- avahid_restart # note we need restart here, not reload in order to update interfaces
- else
- avahid_start # handle case where avahi-daemon exited because no suitable interfaces
- fi
- fi
+ if avahid_running; then
+ if check && [[ "$(this allow-interfaces)" != "$BIND" ]]; then
+ log "Updating $DAEMON..."
+ avahid_restart # note we need restart here, not reload in order to update interfaces
+ fi
+ else
+ # recover daemon when network is back, even if config already matches computed bind list
+ if check && [[ -n $BIND ]]; then
+ log "Recovering $DAEMON..."
+ avahid_start
+ fi
+ fi
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@etc/rc.d/rc.avahidaemon` around lines 123 - 129, The current guard (if check
&& [[ "$(this allow-interfaces)" != "$BIND" ]]) prevents avahid_start from
running when the daemon is stopped but the allow-interfaces list equals BIND;
change the logic so the outer check only validates "check" (and any BIND sanity
you need) and move the allow-interfaces != BIND test to only gate the
avahid_restart path: call avahid_restart when avahid_running and
allow-interfaces differs from BIND, and call avahid_start when avahid_running is
false (regardless of the allow-interfaces == BIND condition) so stopped-daemon
recovery is attempted independently; reference avahid_running, avahid_restart,
avahid_start, check, BIND and this allow-interfaces in your changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Make
rc.avahidaemon updaterecover Avahi when the daemon is stopped and the computed bind list differs from the current Avahi configuration.What Changed
avahid_updatestill starts with the existingcheckandallow-interfacescomparison.avahid_start.Why
If Avahi restarts while no suitable network interface is available, it can exit and remain stopped. In the observed failure, the early restart happened while the active interface list had shrunk; when networking later recovered, the computed bind list differed from the saved Avahi
allow-interfacesvalue, but the oldupdatepath could not recover because it was guarded byavahid_running.This alternate fix keeps the patch minimal and focuses on recovery: when a later service update sees the bind list needs to change, it can start Avahi instead of doing nothing.
Relationship To Other PR
Alternative to #2660.
Verification
bash -n etc/rc.d/rc.avahidaemongit diff --checkReview Notes
if avahid_running; then.Summary by CodeRabbit