Skip to content

Clean vsock sockets after krunkit exit#104

Open
unsuman wants to merge 1 commit into
containers:mainfrom
unsuman:fix/clean-vsock-socket-after-exit
Open

Clean vsock sockets after krunkit exit#104
unsuman wants to merge 1 commit into
containers:mainfrom
unsuman:fix/clean-vsock-socket-after-exit

Conversation

@unsuman
Copy link
Copy Markdown

@unsuman unsuman commented May 11, 2026

Fixes #101

Note: AI was used to assist with the development of this PR

Fixes containers#101

Signed-off-by: Ansuman Sahoo <anshumansahoo500@gmail.com>
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a cleanup mechanism for vsock socket files by implementing a VsockCleanupGuard that handles both normal exits and SIGINT signals via the ctrlc crate. The changes include a new cleanup module and a vsock_socket_paths method in KrunContext to identify files for removal. The reviewer suggested refactoring the timesync socket path generation into a helper method to eliminate code duplication and improve maintainability.

Comment thread src/context.rs
Comment on lines +220 to +235
/// Collect all vsock socket paths that should be cleaned up on exit.
pub fn vsock_socket_paths(&self) -> Vec<PathBuf> {
let mut paths = Vec::new();
for device in &self.args.devices {
if let VirtioDeviceConfig::Vsock(vsock) = device {
paths.push(vsock.socket_url.clone());
}
}
if self.args.timesync.is_some() {
paths.push(PathBuf::from(format!(
"/tmp/krunkit_timesync_{}.sock",
std::process::id()
)));
}
paths
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The logic for generating the timesync socket path is duplicated here and in the TryFrom<Args> for KrunContext implementation (lines 205-208). Refactoring this into a helper method improves maintainability and ensures consistency across the codebase. Please also update the usage in the TryFrom implementation to use this new method.

    fn timesync_socket_path() -> PathBuf {
        PathBuf::from(format!(
            "/tmp/krunkit_timesync_{}.sock",
            std::process::id()
        ))
    }

    /// Collect all vsock socket paths that should be cleaned up on exit.
    pub fn vsock_socket_paths(&self) -> Vec<PathBuf> {
        let mut paths = Vec::new();
        for device in &self.args.devices {
            if let VirtioDeviceConfig::Vsock(vsock) = device {
                paths.push(vsock.socket_url.clone());
            }
        }
        if self.args.timesync.is_some() {
            paths.push(Self::timesync_socket_path());
        }
        paths
    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unix sockets from virtio-vsock devices not cleaned up

1 participant