error out when run_vminitd.sock path length is too long #89
Conversation
On macOS, UNIX socket path is limited to 104 characters (including \0). To circumvent this limitation, vmInstance.Start() tries to compute the relative path to the run_vminitd.sock. However, if the bundle path points to a symlinked directory, this relative path may contain many double-dots and the full path, exceeding the limit. This is the case if containerd state dir is in /tmp (which is symlinked to /private/tmp on macOS). In that case, nothing fails explicitly. It appears as if the VM failed to start, whereas the log show that vminitd executed successfully. The only error logged is: Timeout while waiting for VM to start. Detect if the socket path is too long before trying to start the VM, and return an explicit error. Signed-off-by: Albin Kerouanton <albin.kerouanton@docker.com>
There was a problem hiding this comment.
Pull request overview
This PR addresses a macOS-specific issue where UNIX socket paths exceeding 104 characters cause VM startup failures without clear error messages. The change adds explicit validation to detect overly long socket paths before attempting to start the VM, providing users with a clear error message instead of a generic timeout.
Changes:
- Added pre-emptive validation for socket path length on macOS
- Added explanatory comments about the macOS socket path limitation and failure symptoms
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // didn't start properly. There's no easy way to figure this out as the | ||
| // only log is: "Timeout while waiting for VM to start". Thus, return an | ||
| // error preventively here. | ||
| if runtime.GOOS == "darwin" && len(socketPath) >= 104 { |
There was a problem hiding this comment.
The condition checks if length is >= 104, but since the 104-character limit includes the null terminator (\0), the check should be > 103 or >= 104 with the actual path plus null terminator. The current check may reject valid 103-character paths. Consider changing to len(socketPath) > 103 to allow exactly 103 characters of actual path data.
| if runtime.GOOS == "darwin" && len(socketPath) >= 104 { | |
| if runtime.GOOS == "darwin" && len(socketPath) > 103 { |
There was a problem hiding this comment.
I think copilot got confused by your comment:
is limited to 104 characters (including \0).
The limit is 104 bytes (so 103 chars + NUL byte), so the code is correct.
| // only log is: "Timeout while waiting for VM to start". Thus, return an | ||
| // error preventively here. | ||
| if runtime.GOOS == "darwin" && len(socketPath) >= 104 { | ||
| return fmt.Errorf("socket path is too long: %s", socketPath) |
There was a problem hiding this comment.
The error message should provide actionable guidance. Consider enhancing it to explain the limitation and suggest a solution, such as: socket path exceeds macOS limit of 103 characters (%d): %s. Consider using a shorter state directory path or avoiding symlinked directories in /tmp
| return fmt.Errorf("socket path is too long: %s", socketPath) | |
| return fmt.Errorf("socket path exceeds macOS limit of 103 characters (%d): %s. Consider using a shorter state directory path or avoiding symlinked directories in /tmp", len(socketPath), socketPath) |
|
LGTM on this change, did something else change to cause the calculation to differ or use a different cwd? |
|
We should probably just ditch the state directory in the vm instance and use cwd. If we need to support multiple VMs in the future, we can add an identifier which just gets added in the .sock file name |
3 participants
On macOS, UNIX socket path is limited to 104 characters (including \0). To circumvent this limitation, vmInstance.Start() tries to compute the relative path to the run_vminitd.sock. However, if the bundle path points to a symlinked directory, this relative path may contain many double-dots and the full path, exceeding the limit. This is the case if containerd state dir is in /tmp (which is symlinked to /private/tmp on macOS).
In that case, nothing fails explicitly. It appears as if the VM failed to start, whereas the log show that vminitd executed successfully. The only error logged is: Timeout while waiting for VM to start.
Detect if the socket path is too long before trying to start the VM, and return an explicit error.