[io][conformance] Return NodeInfo::Vmofile for VMO files in rust vfs
This fixes a small bug in the Rust VFS's Describe method and OnOpen
event for VMO files. It now correctly returns NodeInfo::Vmofile for
these, instead of NodeInfo::File, which it was previously doing.
Bug: 68968
Change-Id: I5a8a086be0c693a22e6eaccd614ba4ecb5286873
Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/483004
Reviewed-by: Chris Suter <csuter@google.com>
Commit-Queue: Cameron Silvestrini <csilvestrini@google.com>
diff --git a/src/lib/storage/vfs/rust/src/directory/mutable/simple/tests.rs b/src/lib/storage/vfs/rust/src/directory/mutable/simple/tests.rs
index 924f650..7780bdf 100644
--- a/src/lib/storage/vfs/rust/src/directory/mutable/simple/tests.rs
+++ b/src/lib/storage/vfs/rust/src/directory/mutable/simple/tests.rs
@@ -17,6 +17,7 @@
assert_unlink, assert_unlink_err, assert_watch, assert_watcher_one_message_watched_events,
assert_write, open_as_directory_assert_err, open_as_file_assert_err,
open_get_directory_proxy_assert_ok, open_get_file_proxy_assert_ok, open_get_proxy_assert,
+ open_get_vmo_file_proxy_assert_ok,
};
use crate::{
@@ -543,9 +544,9 @@
// `open_as_file_assert_content!` as the later will immediately close the connection,
// and if no connections exist the VMO is recycled and the assertion is invoked to
// check if the VMO content matches the final expected state.
- let file = open_get_file_proxy_assert_ok!(&proxy, flags, "file");
+ let file = open_get_vmo_file_proxy_assert_ok!(&proxy, flags, "file");
- open_as_file_assert_content!(&proxy, flags, "file", "Initial");
+ open_as_vmo_file_assert_content!(&proxy, flags, "file", "Initial");
let root_token = assert_get_token!(&proxy);
assert_link!(&proxy, "file", root_token, "same-file");
@@ -562,11 +563,11 @@
assert_read_dirents!(proxy, 1000, expected.into_vec());
}
- let same_file = open_get_file_proxy_assert_ok!(&proxy, flags, "same-file");
+ let same_file = open_get_vmo_file_proxy_assert_ok!(&proxy, flags, "same-file");
assert_write!(file, "------- twice");
assert_write!(same_file, "Updated");
- open_as_file_assert_content!(&proxy, flags, "file", "Updated twice");
+ open_as_vmo_file_assert_content!(&proxy, flags, "file", "Updated twice");
drop(watcher_client);
assert_close!(file);
@@ -612,9 +613,9 @@
// `open_as_file_assert_content!` as the later will immediately close the connection,
// and if no connections exist the VMO is recycled and the assertion is invoked to
// check if the VMO content matches the final expected state.
- let passwd = open_get_file_proxy_assert_ok!(&etc, flags, "passwd");
+ let passwd = open_get_vmo_file_proxy_assert_ok!(&etc, flags, "passwd");
- open_as_file_assert_content!(&etc, flags, "passwd", "secret");
+ open_as_vmo_file_assert_content!(&etc, flags, "passwd", "secret");
let tmp_token = assert_get_token!(&tmp);
assert_link!(&etc, "passwd", tmp_token, "linked-passwd");
@@ -638,11 +639,11 @@
assert_read_dirents!(tmp, 1000, expected.into_vec());
}
- let linked_passwd = open_get_file_proxy_assert_ok!(&tmp, flags, "linked-passwd");
+ let linked_passwd = open_get_vmo_file_proxy_assert_ok!(&tmp, flags, "linked-passwd");
assert_write!(passwd, "-------- twice");
assert_write!(linked_passwd, "modified");
- open_as_file_assert_content!(&etc, flags, "passwd", "modified twice");
+ open_as_vmo_file_assert_content!(&etc, flags, "passwd", "modified twice");
drop(tmp_watcher_client);
assert_close!(passwd);
@@ -740,9 +741,9 @@
// `open_as_file_assert_content!` as the later will immediately close the connection,
// and if no connections exist the VMO is recycled and the assertion is invoked to
// check if the VMO content matches the final expected state.
- let passwd = open_get_file_proxy_assert_ok!(&etc, flags, "passwd");
+ let passwd = open_get_vmo_file_proxy_assert_ok!(&etc, flags, "passwd");
- open_as_file_assert_content!(&etc, flags, "passwd", "secret");
+ open_as_vmo_file_assert_content!(&etc, flags, "passwd", "secret");
let tmp_token = assert_get_token!(&tmp);
assert_link!(&etc, "passwd", tmp_token, "linked-passwd");
@@ -766,11 +767,11 @@
assert_read_dirents!(tmp, 1000, expected.into_vec());
}
- let linked_passwd = open_get_file_proxy_assert_ok!(&tmp, flags, "linked-passwd");
+ let linked_passwd = open_get_vmo_file_proxy_assert_ok!(&tmp, flags, "linked-passwd");
assert_write!(passwd, "-------- twice");
assert_write!(linked_passwd, "modified");
- open_as_file_assert_content!(&etc, flags, "passwd", "modified twice");
+ open_as_vmo_file_assert_content!(&etc, flags, "passwd", "modified twice");
drop(tmp_watcher_client);
assert_close!(passwd);
diff --git a/src/lib/storage/vfs/rust/src/directory/test_utils.rs b/src/lib/storage/vfs/rust/src/directory/test_utils.rs
index c2ff7f2..d0aa0fd 100644
--- a/src/lib/storage/vfs/rust/src/directory/test_utils.rs
+++ b/src/lib/storage/vfs/rust/src/directory/test_utils.rs
@@ -120,6 +120,17 @@
}};
}
+/// Opens the specified path as a VMO file and checks its content. Also see all the `assert_*`
+/// macros in `../test_utils.rs`.
+#[macro_export]
+macro_rules! open_as_vmo_file_assert_content {
+ ($proxy:expr, $flags:expr, $path:expr, $expected_content:expr) => {{
+ let file = open_get_vmo_file_proxy_assert_ok!($proxy, $flags, $path);
+ assert_read!(file, $expected_content);
+ assert_close!(file);
+ }};
+}
+
#[macro_export]
macro_rules! assert_watch {
($proxy:expr, $mask:expr) => {{
diff --git a/src/lib/storage/vfs/rust/src/file/vmo/asynchronous/tests.rs b/src/lib/storage/vfs/rust/src/file/vmo/asynchronous/tests.rs
index 4986ff7..791c00e 100644
--- a/src/lib/storage/vfs/rust/src/file/vmo/asynchronous/tests.rs
+++ b/src/lib/storage/vfs/rust/src/file/vmo/asynchronous/tests.rs
@@ -13,7 +13,7 @@
assert_read_fidl_err_closed, assert_seek, assert_seek_err, assert_truncate,
assert_truncate_err, assert_vmo_content, assert_write, assert_write_at, assert_write_at_err,
assert_write_err, assert_write_fidl_err_closed, clone_as_file_assert_err,
- clone_get_file_proxy_assert_ok, clone_get_proxy_assert, report_invalid_vmo_content,
+ clone_get_proxy_assert, clone_get_vmo_file_proxy_assert_ok, report_invalid_vmo_content,
};
use crate::{
@@ -33,7 +33,7 @@
use {
fidl::endpoints::create_proxy,
fidl_fuchsia_io::{
- FileEvent, FileMarker, FileObject, NodeAttributes, NodeInfo, INO_UNKNOWN, MODE_TYPE_FILE,
+ FileEvent, FileMarker, NodeAttributes, NodeInfo, Vmofile, INO_UNKNOWN, MODE_TYPE_FILE,
OPEN_FLAG_DESCRIBE, OPEN_FLAG_NODE_REFERENCE, OPEN_FLAG_POSIX, OPEN_FLAG_TRUNCATE,
OPEN_RIGHT_READABLE, OPEN_RIGHT_WRITABLE, VMO_FLAG_EXACT, VMO_FLAG_PRIVATE, VMO_FLAG_READ,
VMO_FLAG_WRITE,
@@ -111,10 +111,8 @@
assert_event!(proxy, FileEvent::OnOpen_ { s, info }, {
assert_eq!(s, ZX_OK);
- assert_eq!(
- info,
- Some(Box::new(NodeInfo::File(FileObject { event: None, stream: None })))
- );
+ let info = *info.expect("Empty NodeInfo");
+ assert!(matches!(info, NodeInfo::Vmofile(Vmofile { vmo: _, offset: 0, length: 14 })));
});
});
}
@@ -256,10 +254,11 @@
assert_event!(proxy, FileEvent::OnOpen_ { s, info }, {
assert_eq!(s, ZX_OK);
- assert_eq!(
+ let info = *info.expect("Empty NodeInfo");
+ assert!(matches!(
info,
- Some(Box::new(NodeInfo::File(FileObject { event: None, stream: None })))
- );
+ NodeInfo::Vmofile(Vmofile { vmo: _, offset: 0, length: 10 })
+ ));
});
assert_read!(proxy, "Have value");
@@ -744,7 +743,7 @@
assert_seek!(first_proxy, 0, Start);
assert_write!(first_proxy, "As updated");
- let second_proxy = clone_get_file_proxy_assert_ok!(
+ let second_proxy = clone_get_vmo_file_proxy_assert_ok!(
&first_proxy,
OPEN_RIGHT_READABLE | OPEN_FLAG_DESCRIBE
);
@@ -771,7 +770,7 @@
assert_seek!(first_proxy, 0, Start);
assert_write!(first_proxy, "As updated");
- let second_proxy = clone_get_file_proxy_assert_ok!(
+ let second_proxy = clone_get_vmo_file_proxy_assert_ok!(
&first_proxy,
CLONE_FLAG_SAME_RIGHTS | OPEN_FLAG_DESCRIBE
);
@@ -917,7 +916,8 @@
assert_write_fidl_err_closed!(second_proxy, "Write attempt");
// We now try without OPEN_RIGHT_READABLE, as we might still be able to Seek.
- let third_proxy = clone_get_file_proxy_assert_ok!(&first_proxy, OPEN_FLAG_DESCRIBE);
+ let third_proxy =
+ clone_get_vmo_file_proxy_assert_ok!(&first_proxy, OPEN_FLAG_DESCRIBE);
assert_seek_err!(third_proxy, 0, Current, Status::BAD_HANDLE, 0);
diff --git a/src/lib/storage/vfs/rust/src/file/vmo/connection/io1.rs b/src/lib/storage/vfs/rust/src/file/vmo/connection/io1.rs
index b2fefaa..e56c3b4 100644
--- a/src/lib/storage/vfs/rust/src/file/vmo/connection/io1.rs
+++ b/src/lib/storage/vfs/rust/src/file/vmo/connection/io1.rs
@@ -21,8 +21,8 @@
anyhow::Error,
fidl::endpoints::{RequestStream, ServerEnd},
fidl_fuchsia_io::{
- FileObject, FileRequest, FileRequestStream, NodeAttributes, NodeInfo, NodeMarker,
- SeekOrigin, INO_UNKNOWN, MODE_TYPE_FILE, OPEN_FLAG_DESCRIBE, OPEN_FLAG_NODE_REFERENCE,
+ FileRequest, FileRequestStream, NodeAttributes, NodeInfo, NodeMarker, SeekOrigin, Vmofile,
+ INO_UNKNOWN, MODE_TYPE_FILE, OPEN_FLAG_DESCRIBE, OPEN_FLAG_NODE_REFERENCE,
OPEN_FLAG_TRUNCATE, OPEN_RIGHT_READABLE, OPEN_RIGHT_WRITABLE, VMO_FLAG_EXACT,
VMO_FLAG_EXEC, VMO_FLAG_PRIVATE, VMO_FLAG_READ, VMO_FLAG_WRITE,
},
@@ -199,15 +199,7 @@
}
};
- if flags & OPEN_FLAG_DESCRIBE != 0 {
- let mut info = NodeInfo::File(FileObject { event: None, stream: None });
- match control_handle.send_on_open_(Status::OK.into_raw(), Some(&mut info)) {
- Ok(()) => (),
- Err(_) => return,
- }
- }
-
- let handle_requests = FileConnection {
+ let mut connection = FileConnection {
scope: scope.clone(),
file,
requests,
@@ -215,9 +207,26 @@
readable,
writable,
seek: 0,
+ };
+
+ if flags & OPEN_FLAG_DESCRIBE != 0 {
+ match connection.get_node_info().await {
+ Ok(mut info) => {
+ let send_result =
+ control_handle.send_on_open_(Status::OK.into_raw(), Some(&mut info));
+ if send_result.is_err() {
+ return;
+ }
+ }
+ Err(status) => {
+ debug_assert!(status != Status::OK);
+ control_handle.shutdown_with_epitaph(status);
+ return;
+ }
+ }
}
- .handle_requests();
- handle_requests.await;
+
+ connection.handle_requests().await;
}
async fn ensure_vmo<'state_guard>(
@@ -366,6 +375,19 @@
}
}
+ /// Returns `NodeInfo` for the VMO file.
+ async fn get_node_info(&mut self) -> Result<NodeInfo, Status> {
+ let vmofile = update_initialized_state! {
+ match &*self.file.state().await;
+ error: "get_node_info" => Err(Status::INTERNAL);
+ { vmo, size, .. } => {
+ let vmo = vmo.duplicate_handle(Rights::SAME_RIGHTS).unwrap();
+ Ok(Vmofile {vmo, offset: 0, length: *size})
+ }
+ }?;
+ Ok(NodeInfo::Vmofile(vmofile))
+ }
+
/// Handle a [`FileRequest`]. This function is responsible for handing all the file operations
/// that operate on the connection-specific buffer.
async fn handle_request(&mut self, req: FileRequest) -> Result<ConnectionState, Error> {
@@ -381,10 +403,13 @@
let _ = self.handle_close(|status| responder.send(status.into_raw())).await;
return Ok(ConnectionState::Closed);
}
- FileRequest::Describe { responder } => {
- let mut info = NodeInfo::File(FileObject { event: None, stream: None });
- responder.send(&mut info)?;
- }
+ FileRequest::Describe { responder } => match self.get_node_info().await {
+ Ok(mut info) => responder.send(&mut info)?,
+ Err(status) => {
+ debug_assert!(status != Status::OK);
+ responder.control_handle().shutdown_with_epitaph(status);
+ }
+ },
FileRequest::Sync { responder } => {
// VMOs are always in sync.
responder.send(ZX_OK)?;
diff --git a/src/lib/storage/vfs/rust/src/test_utils/assertions.rs b/src/lib/storage/vfs/rust/src/test_utils/assertions.rs
index 4c447cc..d119ac3 100644
--- a/src/lib/storage/vfs/rust/src/test_utils/assertions.rs
+++ b/src/lib/storage/vfs/rust/src/test_utils/assertions.rs
@@ -10,10 +10,10 @@
crate::directory::test_utils::DirentsSameInodeBuilder,
fidl_fuchsia_io::{
DirectoryEvent, DirectoryMarker, DirectoryObject, FileEvent, FileMarker, FileObject,
- NodeInfo, SeekOrigin, Service, DIRENT_TYPE_BLOCK_DEVICE, DIRENT_TYPE_DIRECTORY,
- DIRENT_TYPE_FILE, DIRENT_TYPE_SERVICE, DIRENT_TYPE_SOCKET, DIRENT_TYPE_UNKNOWN,
- INO_UNKNOWN, OPEN_FLAG_DESCRIBE, OPEN_RIGHT_READABLE, WATCH_EVENT_ADDED,
- WATCH_EVENT_EXISTING, WATCH_EVENT_IDLE, WATCH_EVENT_REMOVED,
+ NodeInfo, SeekOrigin, Service, Vmofile, DIRENT_TYPE_BLOCK_DEVICE,
+ DIRENT_TYPE_DIRECTORY, DIRENT_TYPE_FILE, DIRENT_TYPE_SERVICE, DIRENT_TYPE_SOCKET,
+ DIRENT_TYPE_UNKNOWN, INO_UNKNOWN, OPEN_FLAG_DESCRIBE, OPEN_RIGHT_READABLE,
+ WATCH_EVENT_ADDED, WATCH_EVENT_EXISTING, WATCH_EVENT_IDLE, WATCH_EVENT_REMOVED,
},
fuchsia_zircon::{MessageBuf, Status},
futures::stream::StreamExt,
@@ -370,6 +370,25 @@
// See comment at the top of the file for why this is a macro.
#[macro_export]
+macro_rules! open_get_vmo_file_proxy_assert_ok {
+ ($proxy:expr, $flags:expr, $path:expr) => {{
+ use $crate::test_utils::assertions::reexport::{
+ FileEvent, FileMarker, NodeInfo, Status, Vmofile,
+ };
+
+ open_get_proxy_assert!($proxy, $flags, $path, FileMarker, FileEvent::OnOpen_ { s, info }, {
+ assert_eq!(Status::from_raw(s), Status::OK);
+ let info = *info.expect("Empty NodeInfo");
+ assert!(
+ matches!(info, NodeInfo::Vmofile(Vmofile { .. })),
+ format!("Expected Vmofile but got {:?}", info)
+ );
+ })
+ }};
+}
+
+// See comment at the top of the file for why this is a macro.
+#[macro_export]
macro_rules! open_as_file_assert_err {
($proxy:expr, $flags:expr, $path:expr, $expected_status:expr) => {{
use $crate::test_utils::assertions::reexport::{FileEvent, FileMarker, Status};
@@ -461,6 +480,25 @@
// See comment at the top of the file for why this is a macro.
#[macro_export]
+macro_rules! clone_get_vmo_file_proxy_assert_ok {
+ ($proxy:expr, $flags:expr) => {{
+ use $crate::test_utils::assertions::reexport::{
+ FileEvent, FileMarker, NodeInfo, Status, Vmofile,
+ };
+
+ clone_get_proxy_assert!($proxy, $flags, FileMarker, FileEvent::OnOpen_ { s, info }, {
+ assert_eq!(Status::from_raw(s), Status::OK);
+ let info = *info.expect("Empty NodeInfo");
+ assert!(
+ matches!(info, NodeInfo::Vmofile(Vmofile { .. })),
+ format!("Expected Vmofile but got {:?}", info)
+ );
+ })
+ }};
+}
+
+// See comment at the top of the file for why this is a macro.
+#[macro_export]
macro_rules! clone_as_file_assert_err {
($proxy:expr, $flags:expr, $expected_status:expr) => {{
use $crate::test_utils::assertions::reexport::{FileEvent, FileMarker, Status};
