From 1c3ba38583765fbe9bf6646f80ac50fa5665c5c1 Mon Sep 17 00:00:00 2001 From: Frederick Mayle Date: Mon, 29 Aug 2022 21:53:42 +0000 Subject: [PATCH] base: don't export platform specific Event types TEST=cargo build && ./tools/dev_container ./tools/run_tests --target=host --build-target=mingw64 --build-only BUG=b:231344063 Change-Id: I125f4b200abdc6758bae93d98c590c2139fe915b Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3864025 Auto-Submit: Frederick Mayle Commit-Queue: Frederick Mayle Reviewed-by: Alexandre Courbot --- base/src/event.rs | 9 ++++++++- base/src/lib.rs | 2 +- base/src/sys/unix/eventfd.rs | 2 +- base/src/sys/unix/mod.rs | 6 +----- base/src/sys/windows/event.rs | 8 +++++++- base/src/sys/windows/named_pipes.rs | 2 +- common/cros_asyncv2/src/event.rs | 11 +++++------ common/cros_asyncv2/src/unix/event.rs | 7 +++---- common/cros_asyncv2/src/unix/io_driver/uring.rs | 7 +++---- common/cros_asyncv2/src/unix/seqpacket.rs | 9 ++++----- devices/src/virtio/vsock/vsock.rs | 8 ++++---- 11 files changed, 38 insertions(+), 33 deletions(-) diff --git a/base/src/event.rs b/base/src/event.rs index c434f71282..d998740cb1 100644 --- a/base/src/event.rs +++ b/base/src/event.rs @@ -10,6 +10,7 @@ use serde::Serialize; use crate::descriptor::AsRawDescriptor; use crate::descriptor::FromRawDescriptor; use crate::descriptor::IntoRawDescriptor; +use crate::descriptor::SafeDescriptor; use crate::platform::Event as PlatformEvent; use crate::RawDescriptor; use crate::Result; @@ -28,7 +29,7 @@ pub enum EventReadResult { // TODO(b:231344063) Move/update documentation. #[derive(Debug, PartialEq, Eq, Serialize, Deserialize)] #[serde(transparent)] -pub struct Event(pub PlatformEvent); +pub struct Event(pub(crate) PlatformEvent); impl Event { pub fn new() -> Result { PlatformEvent::new().map(Event) @@ -68,3 +69,9 @@ impl IntoRawDescriptor for Event { self.0.into_raw_descriptor() } } + +impl From for SafeDescriptor { + fn from(evt: Event) -> Self { + Self::from(evt.0) + } +} diff --git a/base/src/lib.rs b/base/src/lib.rs index a12a373ca8..e8685443bc 100644 --- a/base/src/lib.rs +++ b/base/src/lib.rs @@ -85,7 +85,7 @@ cfg_if::cfg_if! { pub use platform::{ block_signal, clear_signal, get_blocked_signals, new_pipe_full, register_rt_signal_handler, signal, unblock_signal, Killable, SIGRTMIN, - AcpiNotifyEvent, NetlinkGenericSocket, SignalFd, Terminal, EventFd, + AcpiNotifyEvent, NetlinkGenericSocket, SignalFd, Terminal, }; pub use platform::{ diff --git a/base/src/sys/unix/eventfd.rs b/base/src/sys/unix/eventfd.rs index 6ac9826541..9c2ba5581e 100644 --- a/base/src/sys/unix/eventfd.rs +++ b/base/src/sys/unix/eventfd.rs @@ -32,7 +32,7 @@ use crate::EventReadResult; /// and out of the KVM API. They can also be polled like any other file descriptor. #[derive(Debug, PartialEq, Eq, Serialize, Deserialize)] #[serde(transparent)] -pub struct EventFd { +pub(crate) struct EventFd { event_handle: SafeDescriptor, } diff --git a/base/src/sys/unix/mod.rs b/base/src/sys/unix/mod.rs index 9a2a91de63..d6da903108 100644 --- a/base/src/sys/unix/mod.rs +++ b/base/src/sys/unix/mod.rs @@ -66,11 +66,7 @@ use std::time::Duration; pub use acpi_event::*; pub use capabilities::drop_capabilities; pub use descriptor::*; -// EventFd is deprecated. Use Event instead. EventFd will be removed as soon as rest of the current -// users migrate. -// TODO(b:231344063): Remove EventFd. -pub use eventfd::EventFd as Event; -pub use eventfd::EventFd; +pub(crate) use eventfd::EventFd as Event; pub use file_flags::*; pub use file_traits::AsRawFds; pub use file_traits::FileAllocate; diff --git a/base/src/sys/windows/event.rs b/base/src/sys/windows/event.rs index 711655aa1a..369abe8d75 100644 --- a/base/src/sys/windows/event.rs +++ b/base/src/sys/windows/event.rs @@ -45,7 +45,7 @@ use crate::EventReadResult; /// events. #[derive(Debug, PartialEq, Eq, Serialize, Deserialize)] #[serde(transparent)] -pub struct Event { +pub(crate) struct Event { event_handle: SafeDescriptor, } @@ -242,6 +242,12 @@ impl IntoRawDescriptor for Event { } } +impl From for SafeDescriptor { + fn from(evt: Event) -> Self { + evt.event_handle + } +} + // Event is safe for send & Sync despite containing a raw handle to its // file mapping object. As long as the instance to Event stays alive, this // pointer will be a valid handle. diff --git a/base/src/sys/windows/named_pipes.rs b/base/src/sys/windows/named_pipes.rs index 6361bd4639..db5de297a3 100644 --- a/base/src/sys/windows/named_pipes.rs +++ b/base/src/sys/windows/named_pipes.rs @@ -52,12 +52,12 @@ use winapi::um::winbase::PIPE_TYPE_MESSAGE; use winapi::um::winbase::PIPE_WAIT; use winapi::um::winbase::SECURITY_IDENTIFICATION; -use super::Event; use super::RawDescriptor; use crate::descriptor::AsRawDescriptor; use crate::descriptor::FromRawDescriptor; use crate::descriptor::IntoRawDescriptor; use crate::descriptor::SafeDescriptor; +use crate::Event; /// The default buffer size for all named pipes in the system. If this size is too small, writers /// on named pipes that expect not to block *can* block until the reading side empties the buffer. diff --git a/common/cros_asyncv2/src/event.rs b/common/cros_asyncv2/src/event.rs index fc6f7d756f..a0e3f19397 100644 --- a/common/cros_asyncv2/src/event.rs +++ b/common/cros_asyncv2/src/event.rs @@ -41,10 +41,10 @@ impl Event { } #[cfg(unix)] -impl TryFrom for Event { +impl TryFrom for Event { type Error = anyhow::Error; - fn try_from(evt: base::EventFd) -> anyhow::Result { + fn try_from(evt: base::Event) -> anyhow::Result { sys::Event::try_from(evt).map(|inner| Event { inner }) } } @@ -73,15 +73,14 @@ mod tests { #[test] #[cfg(unix)] fn next_val_reads_value() { - use base::EventFd; async fn go(event: Event) -> u64 { event.next_val().await.unwrap() } - let eventfd = EventFd::new().unwrap(); - eventfd.write(0xaa).unwrap(); + let sync_event = base::Event::new().unwrap(); + sync_event.write(0xaa).unwrap(); let ex = Executor::new(); - let val = ex.run_until(go(eventfd.try_into().unwrap())).unwrap(); + let val = ex.run_until(go(sync_event.try_into().unwrap())).unwrap(); assert_eq!(val, 0xaa); } diff --git a/common/cros_asyncv2/src/unix/event.rs b/common/cros_asyncv2/src/unix/event.rs index 8725db02ba..9f294856c6 100644 --- a/common/cros_asyncv2/src/unix/event.rs +++ b/common/cros_asyncv2/src/unix/event.rs @@ -9,7 +9,6 @@ use std::sync::Arc; use anyhow::ensure; use anyhow::Context; -use base::EventFd; use base::SafeDescriptor; use super::io_driver; @@ -21,7 +20,7 @@ pub struct Event { impl Event { pub fn new() -> anyhow::Result { - EventFd::new() + base::Event::new() .map_err(io::Error::from) .context("failed to create eventfd") .and_then(Event::try_from) @@ -60,10 +59,10 @@ impl Event { } } -impl TryFrom for Event { +impl TryFrom for Event { type Error = anyhow::Error; - fn try_from(evt: EventFd) -> anyhow::Result { + fn try_from(evt: base::Event) -> anyhow::Result { io_driver::prepare(&evt)?; Ok(Event { fd: Arc::new(SafeDescriptor::from(evt)), diff --git a/common/cros_asyncv2/src/unix/io_driver/uring.rs b/common/cros_asyncv2/src/unix/io_driver/uring.rs index 49246b5148..620b923eef 100644 --- a/common/cros_asyncv2/src/unix/io_driver/uring.rs +++ b/common/cros_asyncv2/src/unix/io_driver/uring.rs @@ -31,7 +31,6 @@ use anyhow::Context; use base::error; use base::warn; use base::AsRawDescriptor; -use base::EventFd; use base::FromRawDescriptor; use base::LayoutAllocation; use base::SafeDescriptor; @@ -419,7 +418,7 @@ impl State { fn submit_waker(&mut self) -> anyhow::Result<()> { let entry = opcode::Read::new( - Fd(self.waker.0.as_raw_fd()), + Fd(self.waker.0.as_raw_descriptor()), ptr::null_mut(), size_of::() as u32, ) @@ -605,10 +604,10 @@ fn unpack_buffer_id(bid: u16) -> (usize, usize) { (alloc_idx, buffer_idx) } -pub struct Waker(EventFd); +pub struct Waker(base::Event); impl Waker { fn new() -> anyhow::Result { - EventFd::new() + base::Event::new() .map(Waker) .map_err(|e| anyhow!(io::Error::from(e))) } diff --git a/common/cros_asyncv2/src/unix/seqpacket.rs b/common/cros_asyncv2/src/unix/seqpacket.rs index fae8ac6748..d988c82ff6 100644 --- a/common/cros_asyncv2/src/unix/seqpacket.rs +++ b/common/cros_asyncv2/src/unix/seqpacket.rs @@ -439,7 +439,6 @@ mod test { use std::time::Instant; use base::AsRawDescriptor; - use base::EventFd; use super::*; use crate::with_deadline; @@ -504,7 +503,7 @@ mod test { .run_until(async { let (s1, s2) = SeqPacket::pair().expect("failed to create socket pair"); - let evt = EventFd::new().expect("failed to create eventfd"); + let evt = base::Event::new().expect("failed to create eventfd"); let write_count = s1 .send_with_fds(&[], &[evt.as_raw_descriptor()]) .await @@ -540,7 +539,7 @@ mod test { .run_until(async { let (s1, s2) = SeqPacket::pair().expect("failed to create socket pair"); - let evt = EventFd::new().expect("failed to create eventfd"); + let evt = base::Event::new().expect("failed to create eventfd"); let (res, _) = s1 .send_iobuf_with_fds(OwnedIoBuf::new(vec![]), &[evt.as_raw_descriptor()]) .await; @@ -575,7 +574,7 @@ mod test { .run_until(async { let (s1, s2) = SeqPacket::pair().expect("failed to create socket pair"); - let evt = EventFd::new().expect("failed to create eventfd"); + let evt = base::Event::new().expect("failed to create eventfd"); let write_count = s1 .send_with_fds(&[237], &[evt.as_raw_descriptor()]) .await @@ -614,7 +613,7 @@ mod test { .run_until(async { let (s1, s2) = SeqPacket::pair().expect("failed to create socket pair"); - let evt = EventFd::new().expect("failed to create eventfd"); + let evt = base::Event::new().expect("failed to create eventfd"); let (res, _) = s1 .send_iobuf_with_fds(OwnedIoBuf::new(vec![237]), &[evt.as_raw_descriptor()]) .await; diff --git a/devices/src/virtio/vsock/vsock.rs b/devices/src/virtio/vsock/vsock.rs index b2cceb4892..0e44426a2f 100644 --- a/devices/src/virtio/vsock/vsock.rs +++ b/devices/src/virtio/vsock/vsock.rs @@ -27,6 +27,7 @@ use base::warn; use base::AsRawDescriptor; use base::Error as SysError; use base::Event; +use base::EventExt; use cros_async::select2; use cros_async::select6; use cros_async::AsyncError; @@ -379,8 +380,7 @@ impl Worker { .map_err(|e| { error!("Could not clone h_event."); VsockError::CloneDescriptor(e) - }) - .map(base::Event)?; + })?; let evt_async = EventAsync::new(h_evt, ex).map_err(|e| { error!("Could not create EventAsync."); VsockError::CreateEventAsync(e) @@ -419,7 +419,7 @@ impl Worker { { if port.host == CONNECTION_EVENT_PORT_NUM { // New connection event. Setup futures again. - if let Err(e) = self.connection_event.0.reset() { + if let Err(e) = self.connection_event.reset() { error!("vsock: port: {}: could not reset connection_event.", port); return Err(VsockError::ResetEventObject(e)); } @@ -734,7 +734,7 @@ impl Worker { // always be negligible, but will sometimes be non-zero in cases where // traffic is high on the NamedPipe, especially a duplex pipe. if let Ok(cloned_event) = write_completed_event.try_clone() { - if let Ok(async_event) = EventAsync::new(Event(cloned_event), ex) { + if let Ok(async_event) = EventAsync::new(cloned_event, ex) { let _ = async_event.next_val().await; } else { error!(