search

rust-vmm / vmm-sys-util

Helpers and utilities used by multiple rust-vmm components and VMMs
BSD 3-Clause "New" or "Revised" License
79 stars 64 forks source link

Add epoll wrapper #79

Closed lauralt closed 4 years ago

lauralt commented 4 years ago

Added small wrapper over libc::epoll that can be used for the basic operations regarding the epoll API.

andreeaflorescu commented 4 years ago

@sameo @jiangliu can you also take a quick look over this PR? The plan would be to eventually get rid of poll.rs module and use this safe wrapper instead. I assume right now this is not possible because probably you are using EpollContext. But if we switch to the vmm-epoll (see PR: https://github.com/rust-vmm/vmm-epoll/pull/2) then EpollContext and all the other wrappers in poll.rs would not be needed any more. Alternatively, we could keep the existing functionality in poll.rs and just remove the dependency on the epoll crate. WDYT?

sameo commented 4 years ago

@sameo @jiangliu can you also take a quick look over this PR? The plan would be to eventually get rid of poll.rs module and use this safe wrapper instead. I assume right now this is not possible because probably you are using EpollContext. But if we switch to the vmm-epoll (see PR: rust-vmm/vmm-epoll#2) then EpollContext and all the other wrappers in poll.rs would not be needed any more. Alternatively, we could keep the existing functionality in poll.rs and just remove the dependency on the epoll crate. WDYT?

@andreeaflorescu I'm ok with this PR. Is the plan to have vmm-epoll use that module and basically move https://github.com/rust-vmm/vmm-epoll/pull/2/files#diff-1a41e4dcb6cec5befecca71500e4d036 here? If so, could we enhance this PR with the missing bits from the vmm-epoll/src/epoll.rs file? Although we would not be consuming this module directly from Cloud-Hypervisor, this PR LGTM.

sameo commented 4 years ago

@sameo @jiangliu can you also take a quick look over this PR? The plan would be to eventually get rid of poll.rs module and use this safe wrapper instead. I assume right now this is not possible because probably you are using EpollContext. But if we switch to the vmm-epoll (see PR: rust-vmm/vmm-epoll#2) then EpollContext and all the other wrappers in poll.rs would not be needed any more. Alternatively, we could keep the existing functionality in poll.rs and just remove the dependency on the epoll crate. WDYT?

@andreeaflorescu I'm ok with this PR. Is the plan to have vmm-epoll use that module and basically move https://github.com/rust-vmm/vmm-epoll/pull/2/files#diff-1a41e4dcb6cec5befecca71500e4d036 here? If so, could we enhance this PR with the missing bits from the vmm-epoll/src/epoll.rs file?

Nevermind, it is complete. Once we remove the chatty New() example, I'll add my approval.

andreeaflorescu commented 4 years ago

@sameo @jiangliu can you also take a quick look over this PR? The plan would be to eventually get rid of poll.rs module and use this safe wrapper instead. I assume right now this is not possible because probably you are using EpollContext. But if we switch to the vmm-epoll (see PR: rust-vmm/vmm-epoll#2) then EpollContext and all the other wrappers in poll.rs would not be needed any more. Alternatively, we could keep the existing functionality in poll.rs and just remove the dependency on the epoll crate. WDYT?

@andreeaflorescu I'm ok with this PR. Is the plan to have vmm-epoll use that module and basically move https://github.com/rust-vmm/vmm-epoll/pull/2/files#diff-1a41e4dcb6cec5befecca71500e4d036 here? If so, could we enhance this PR with the missing bits from the vmm-epoll/src/epoll.rs file? Although we would not be consuming this module directly from Cloud-Hypervisor, this PR LGTM.

@sameo yap, the plan is to use it from vmm-epoll. If CH still wants to use poll.rs, I think we should improve it and remove the dependency on the external crate epoll. If long term you also plan to switch to vmm-epoll, then there is no point in investing that effort in poll.rs any more.