Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

client/connection: Add support for the socket existing in /run/incus #800

Closed
wants to merge 1 commit into from

Conversation

Conan-Kudo
Copy link
Contributor

Transient sockets are supposed to be in /run rather than /var, so make it possible to detect that automatically when used.

Transient sockets are supposed to be in /run rather than /var, so make
it possible to detect that automatically when used.

Signed-off-by: Neal Gompa <[email protected]>
@Conan-Kudo Conan-Kudo requested a review from stgraber as a code owner April 27, 2024 14:02
@stgraber
Copy link
Member

That won't work as /run/incus exists everywhere but most systems use /var/lib/incus/unix.socket

@stgraber
Copy link
Member

So the check would need to be about the exact socket path rather than just checking for the directory.

@derkuci
Copy link

derkuci commented Jun 12, 2024

It seems that putting the socket in /run/incus would require more changes than just this patch. For example, the pre-start hook only looks at /var/lib/incus (its command line is something like /proc/1078/exe callhook /var/lib/incus "default" "vm01" start) and it fails when the socket is in /run/incus.

@stgraber
Copy link
Member

Hmm, the server side is supposed to be handled through the INCUS_SOCKET environment variable being passed to incusd, the callhook logic does check for that env variable so it should work fine so long as what calls it doesn't strip the env variable somehow.

@derkuci
Copy link

derkuci commented Jun 13, 2024

I see. I'll report to Fedora that their RPM spec may miss an environment variable setting (INCUS_SOCKET=/run/incus/unix.socket).

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

Successfully merging this pull request may close these issues.

3 participants