Migrate Firecracker to use rust-vmm/event-manager

[acatangiu]

Reason for This PR

Use upstream https://github.com/rust-vmm components.

Fixes #2154

Description of Changes

This patch series migrates Firecracker from using local polly crate, to using rust-vmm/event-manager crate.

Note: The project doesn't build with only part of this patch-set.

License Acceptance

By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache 2.0 license.

PR Checklist

[Author TODO: Meet these criteria.]
[Reviewer TODO: Verify that these criteria are met. Request changes if not]

• All commits in this PR are signed (git commit -s).
• The reason for this PR is clearly provided (issue no. or explanation).
• The description of changes is clear and encompassing.
• Any required documentation changes (code and docs) are included in this PR.
• Any newly added unsafe code is properly documented.
• Any API changes are reflected in firecracker/swagger.yaml.
• Any user-facing changes are mentioned in CHANGELOG.md.
• All added/changed functionality is tested.

[sandreim]

PR looks good to me! I would also run the net/block/vsock perf tests, but I don't expect any performance impact from this change as polly and eventmanager are closely following the same pattern but with a different interface/implementation.

Lastly, I would look to replace the epoll primitive usage in the VsockMuxer: impl VsockEpollListener for VsockMuxer . We can also do this in a subsequent PR if it turns to be a lot of work.

[acatangiu]

I would look to replace the epoll primitive usage in the VsockMuxer: impl VsockEpollListener for VsockMuxer . We can also do this in a subsequent PR if it turns to be a lot of work.

Let's do that in a separate PR. It involves considerable Vsock refactoring and is somewhat orthogonal to this PR.

[serban300]

The PR looks good functionally, but could we keep the unit tests as close to how they were before ? I'm referring to simulating events through the event manager instead of calling the handlers directly. Just to make sure that the entire path works correctly. I understand that the event manager logic is tested inside the crate, but even so I think we might be missing use cases if we simplify the tests.

[acatangiu]

could we keep the unit tests as close to how they were before ? I'm referring to simulating events through the event manager instead of calling the handlers directly. Just to make sure that the entire path works correctly.

The rust-vmm/event-manager interface doesn't allow us to simulate events as before..

The process() function part of the public interface/trait takes an internal-only parameter of type EventOps.

EventOps has no public builder interface nor public members so it cannot be created outside the event-manager crate.

[serban300]

could we keep the unit tests as close to how they were before ? I'm referring to simulating events through the event manager instead of calling the handlers directly. Just to make sure that the entire path works correctly.

The rust-vmm/event-manager interface doesn't allow us to simulate events as before..

The process() function part of the public interface/trait takes an internal-only parameter of type EventOps.

EventOps has no public builder interface nor public members so it cannot be created outside the event-manager crate.

This is somewhat of a step back since it doesn't allow us to test a part of the code. Do you know if there are any plans on offering support for simulating events ? Is waiting an option ?

[sandreim]

could we keep the unit tests as close to how they were before ? I'm referring to simulating events through the event manager instead of calling the handlers directly. Just to make sure that the entire path works correctly.

The rust-vmm/event-manager interface doesn't allow us to simulate events as before..
The process() function part of the public interface/trait takes an internal-only parameter of type EventOps.
EventOps has no public builder interface nor public members so it cannot be created outside the event-manager crate.

This is somewhat of a step back since it doesn't allow us to test a part of the code. Do you know if there are any plans on offering support for simulating events ? Is waiting an option ?

I agree with your concerns @serban300 regarding the limited testing capabilities that we get with this switch at the moment. However, I would not gate short term on this support if we have identical coverage in tests.

I think we should create an event-manager issue and discuss/contribute extended support for test configurations.

[acatangiu]

Opened rust-vmm/event-manager#67 for adding some way to unit-test process() implementations.