Skip to content

Replace Eventfd with EventNotifier/EventConsumer #308

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

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

uran0sH
Copy link

@uran0sH uran0sH commented Jul 9, 2025

Summary of the PR

Eventfd is Linux-specific. To support more platforms, we replace it with
the EventNotifier/EventConsumer abstractions.
EventSender and EventReceiver are wrappers that encapsulate eventfd functionality
Use pipefd to replace eventfd in the test.

Requirements

Before submitting your PR, please make sure you addressed the following
requirements:

  • [ x ] All commits in this PR have Signed-Off-By trailers (with
    git commit -s), and the commit message has max 60 characters for the
    summary and max 75 characters for each description line.
  • [ x ] All added/changed functionality has a corresponding unit/integration
    test.
  • [ x ] All added/changed public-facing functionality has entries in the "Upcoming
    Release" section of CHANGELOG.md (if no such section exists, please create one).
  • [ x ] Any newly added unsafe code is properly documented.

@uran0sH
Copy link
Author

uran0sH commented Jul 9, 2025

@stefano-garzarella stefano-garzarella marked this pull request as draft July 9, 2025 16:34
@stefano-garzarella
Copy link
Member

@uran0sH marked as draft, since we need a new vmm-sys-util release before merging this.

@stefano-garzarella
Copy link
Member

@uran0sH please rebase and check CI (come failures need the new rust version we are updating in the CI, but others are related to format, etc.)

@stefano-garzarella
Copy link
Member

@uran0sH we just merged #309 so you should be able to use the new API requiring 1.87

Update to introduce EventNotifier and EventConsumer.

Signed-off-by: Wenyu Huang <[email protected]>
@stefano-garzarella
Copy link
Member

@uran0sH please check the CI, there some clippy warning to fix, but also some tests blocked for more than 60s. Should I restart the CI or it's an issue we need to fix?

@uran0sH
Copy link
Author

uran0sH commented Jul 21, 2025

@uran0sH please check the CI, there some clippy warning to fix, but also some tests blocked for more than 60s. Should I restart the CI or it's an issue we need to fix?

I think it's an issue

Eventfd is Linux-specific. To support more platforms, we replace it with
the EventNotifier/EventConsumer abstractions.
EventSender and EventReceiver are wrappers that encapsulate eventfd functionality
Use pipefd to replace eventfd in the test.

Signed-off-by: Wenyu Huang <[email protected]>
@uran0sH
Copy link
Author

uran0sH commented Jul 22, 2025

@uran0sH please check the CI, there some clippy warning to fix, but also some tests blocked for more than 60s. Should I restart the CI or it's an issue we need to fix?

I think it's an issue

Fix this issue by changing this line to into_raw_fd() (https://github.com/rust-vmm/vhost/pull/308/files#diff-e1c9dc7802234bc118de8314ec9cacc98066eea325a6f14ff5887ece3466d5a3R96). Because when we use pipe, using as_raw_fd will cause the consumer's fd to be closed, like this (https://github.com/rust-vmm/vhost/pull/308/files#diff-c9eba2d5821720c1f7e164d0d01d85b14040d8f3200fac89607fdd1af09d5491R111). So we need to take the ownership of this fd.

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