Repository navigation
Fix stale /etc/hosts breaking VNC on desktop-image VMs - #1091
Merged
Merged
Conversation
injectHostname rewrote /etc/hostname to each VM's own name at deploy
time but left /etc/hosts' 127.0.1.1 entry pointing at the image's baked
placeholder, so the two no longer matched. TigerVNC's vncserver script
resolves its own FQDN on startup and refuses to run once that lookup
fails ("Could not acquire fully qualified host name of this machine"),
crash-looping vncserver@1.service on every desktop-image VM.
injectHostname now reads /etc/hosts out of the image via debugfs's
non-destructive dump request and rewrites its 127.0.1.1 line to match,
in the same rm/write/sif pass as /etc/hostname.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XUUwyWgX4uLP4grY6kxZWj
![chatgpt-codex-connector[bot]](/p/https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f9d0b58b6c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
debugfs's dump request doesn't fail (nonzero exit) on a missing path -- it just prints a diagnostic and leaves the output empty, which is why the previous commit rejected an empty dump outright. But a *present* file can legitimately be zero bytes too (e.g. a minimal/custom rootfs with an empty /etc/hosts), and those images built fine before this series -- treating "empty" as "missing" wrongly aborts Prepare for them. Flagged by Codex review on #1091. Distinguish the two by debugfs's own "not found" diagnostic text instead of by content length. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XUUwyWgX4uLP4grY6kxZWj
Owner
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e7072a9554
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
debugfs's "dump" request doesn't fail on a missing path, and on a symlink (debugfs 1.47 doesn't follow them) it silently leaves a short/empty read instead -- both look identical to a legitimately empty regular file after the fact, with no reliable way to tell them apart from dump's own output. That matters here because injectHostname reads before rewriting: mistaking a symlinked /etc/hosts for an empty regular one would silently replace the symlink with a plain file, discarding whatever it pointed to. Flagged by Codex review on #1091. readDebugfsFile now confirms the path's type via "stat" up front and refuses anything but a plain regular file, rather than trying to infer failure from dump's diagnostic text. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XUUwyWgX4uLP4grY6kxZWj
Owner
Author
|
@codex review |
cdalar
enabled auto-merge
September 7, 2026 04:06
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 388e017d4f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The prior commit's "stat" type check rejected anything but a regular file -- but a minimal/custom rootfs with no /etc/hosts at all worked fine before injectHostname ever touched this file, and unlike a symlink there's nothing there to accidentally clobber by treating it as empty. That check made Prepare abort on those images instead. Flagged by Codex review on #1091. readDebugfsFile now returns (nil, nil) for a confirmed-missing path, letting the caller's write/sif commands create it fresh exactly as they've always created /etc/hostname; symlinks and other non-regular types are still rejected. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XUUwyWgX4uLP4grY6kxZWj
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
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.
injectHostname rewrote /etc/hostname to each VM's own name at deploy time but left /etc/hosts' 127.0.1.1 entry pointing at the image's baked placeholder, so the two no longer matched. TigerVNC's vncserver script resolves its own FQDN on startup and refuses to run once that lookup fails ("Could not acquire fully qualified host name of this machine"), crash-looping vncserver@1.service on every desktop-image VM.
injectHostname now reads /etc/hosts out of the image via debugfs's non-destructive dump request and rewrites its 127.0.1.1 line to match, in the same rm/write/sif pass as /etc/hostname.
Claude-Session: https://claude.ai/code/session_01XUUwyWgX4uLP4grY6kxZWj