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

Describe the twin-socket feature in the spec #775

Merged
merged 8 commits into from
Sep 15, 2023

Conversation

mnissler-rivos
Copy link
Contributor

No description provided.

docs/vfio-user.rst Outdated Show resolved Hide resolved
docs/vfio-user.rst Outdated Show resolved Hide resolved
Copy link
Member

@tmakatos tmakatos left a comment

Choose a reason for hiding this comment

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

LGTM

docs/vfio-user.rst Outdated Show resolved Hide resolved
@tmakatos tmakatos requested a review from jlevon August 31, 2023 09:26
@mnissler-rivos
Copy link
Contributor Author

Btw. we might want to keep this open while the implementation MR is still in flight in case we end up deciding to make changes.

@jlevon
Copy link
Collaborator

jlevon commented Sep 6, 2023

Sorry for the slow reply. On reflection I think I'd prefer two tweaks:

  1. s/enable/supported/
  2. put supported in the reply capabilities as well as the fd/fd index

My thinking here is that it's then much more like a negotiation of capabilities:

  1. client: I support twin socket, please enable it if you do too
  2. server: so do I, and here's the fd you should use

thoughts?

Copy link
Collaborator

@jlevon jlevon left a comment

Choose a reason for hiding this comment

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

per coment

@mnissler-rivos
Copy link
Contributor Author

Sorry for the slow reply. On reflection I think I'd prefer two tweaks:

1. s/enable/supported/

2. put supported in the reply capabilities as well as the fd/fd index

My thinking here is that it's then much more like a negotiation of capabilities:

1. client: I support twin socket, please enable it if you do too

2. server: so do I, and here's the fd you should use

thoughts?

Works for me. And I'm assuming clients will bail and close the connection on inconsistent replies (such as supported being true, but fd_index absent).

@jlevon
Copy link
Collaborator

jlevon commented Sep 6, 2023

And I'm assuming clients will bail and close the connection on inconsistent replies (such as supported being true, but fd_index absent).

yes

@jlevon jlevon merged commit 478ddb5 into nutanix:master Sep 15, 2023
8 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.

3 participants