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

net: Allow creating vsocks in listen mode. #246

Merged
merged 1 commit into from
Jan 14, 2025

Conversation

WhatAmISupposedToPutHere
Copy link
Contributor

Previously all vsocks were expected to be connected from the guest. This patch adds support for sockets that are listened to by the guest and are connected from the host.

Copy link
Member

@tylerfanelli tylerfanelli left a comment

Choose a reason for hiding this comment

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

Thanks! Any readily-available testing methods?

@WhatAmISupposedToPutHere
Copy link
Contributor Author

Build the linked PR from muvm, and launch a second command in the same vm. The second one will use this feature to send the launch request.

Copy link
Contributor

@slp slp left a comment

Choose a reason for hiding this comment

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

This looks great, thanks. I only requested a minor change to force the library users to provide a healthy path for the unix socket.

@mtjhrc
Copy link
Collaborator

mtjhrc commented Jan 8, 2025

Thanks, I'll try to test this later, but for now I realized something:

If I am not mistaken, this only starts listening on the socket once the vsock device is activated, so if you want to connect to the socket from host, you would have to wait until the guest boot process initializes the vsock device.
Is this intended or useful for your use case? If so, it should at least be documented, because the users of libkrun could easily run into a race condition where they sometimes try to connect to the socket before the guest boots. An alternative could be to call listen on the socket before krun_add_vsock_port2 returns.

@WhatAmISupposedToPutHere
Copy link
Contributor Author

Yes, the socket file appearing only after the vm is ready to process incoming connection is desired, that way the vm can wait for the file to appear before sending the requests.

Previously all vsocks were expected to be connected from the guest.
This patch adds support for sockets that are listened to by the guest
and are connected from the host.

Signed-off-by: Sasha Finkelstein <[email protected]>
@slp
Copy link
Contributor

slp commented Jan 13, 2025

@mtjhrc Do you want to take a second look or can we merge it already?

Copy link
Collaborator

@mtjhrc mtjhrc left a comment

Choose a reason for hiding this comment

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

Sorry for the delay. I managed to test this just by adding a listening vsock in chroot_vm and using it in the guest.
Now I understand how it's meant to be used. Still I am unsure if we generally want to bind the sockets at the device initialization stage, but if It works for you this is fine for now, and we could always add another API later.

So for me this is good to merge! Thanks

@slp slp merged commit cb0d6c2 into containers:main Jan 14, 2025
5 checks passed
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.

4 participants