From 7641eb86a3bacdd960951b53aeaee5f989da40a6 Mon Sep 17 00:00:00 2001 From: Facundo Poblete Date: Tue, 14 Jan 2025 01:31:05 -0300 Subject: [PATCH 01/10] Add statfs hook --- Cargo.lock | 2 +- mirrord/agent/src/file.rs | 20 +++++- mirrord/intproxy/protocol/src/lib.rs | 7 ++ mirrord/intproxy/src/proxies/files.rs | 64 +++++++++++-------- mirrord/layer/src/file/hooks.rs | 31 ++++++++- mirrord/layer/src/file/ops.rs | 12 +++- mirrord/layer/src/go/linux_x64.rs | 2 + mirrord/layer/src/go/mod.rs | 2 + mirrord/layer/tests/apps/fileops/go/main.go | 13 +++- .../apps/statfs_fstatfs/statfs_fstatfs.c | 42 ++++++++++++ mirrord/layer/tests/common/mod.rs | 48 +++++++++++++- mirrord/layer/tests/fileops.rs | 3 + mirrord/layer/tests/statfs_fstatfs.rs | 31 +++++++++ mirrord/protocol/Cargo.toml | 2 +- mirrord/protocol/src/codec.rs | 1 + mirrord/protocol/src/file.rs | 8 +++ tests/python-e2e/ops.py | 14 +++- 17 files changed, 267 insertions(+), 35 deletions(-) create mode 100644 mirrord/layer/tests/apps/statfs_fstatfs/statfs_fstatfs.c create mode 100644 mirrord/layer/tests/statfs_fstatfs.rs diff --git a/Cargo.lock b/Cargo.lock index 89528a03ca1..9f874671bf5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4479,7 +4479,7 @@ dependencies = [ [[package]] name = "mirrord-protocol" -version = "1.13.3" +version = "1.14.0" dependencies = [ "actix-codec", "bincode", diff --git a/mirrord/agent/src/file.rs b/mirrord/agent/src/file.rs index 54432a9779b..794904eb744 100644 --- a/mirrord/agent/src/file.rs +++ b/mirrord/agent/src/file.rs @@ -223,8 +223,12 @@ impl FileManager { Some(FileResponse::Xstat(xstat_result)) } FileRequest::XstatFs(XstatFsRequest { fd }) => { - let xstat_result = self.xstatfs(fd); - Some(FileResponse::XstatFs(xstat_result)) + let xstatfs_result = self.xstatfs(fd); + Some(FileResponse::XstatFs(xstatfs_result)) + } + FileRequest::StatFs(StatFsRequest { path }) => { + let statfs_result = self.statfs(path); + Some(FileResponse::XstatFs(statfs_result)) } // dir operations @@ -689,6 +693,18 @@ impl FileManager { }) } + #[tracing::instrument(level = "trace", skip(self))] + pub(crate) fn statfs(&mut self, path: PathBuf) -> RemoteResult { + let path = resolve_path(path, &self.root_path)?; + + let statfs = nix::sys::statfs::statfs(&path) + .map_err(|err| std::io::Error::from_raw_os_error(err as i32))?; + + Ok(XstatFsResponse { + metadata: statfs.into(), + }) + } + #[tracing::instrument(level = "trace", skip(self))] pub(crate) fn fdopen_dir(&mut self, fd: u64) -> RemoteResult { let path = match self diff --git a/mirrord/intproxy/protocol/src/lib.rs b/mirrord/intproxy/protocol/src/lib.rs index 623020718b6..62993b2bcfd 100644 --- a/mirrord/intproxy/protocol/src/lib.rs +++ b/mirrord/intproxy/protocol/src/lib.rs @@ -366,6 +366,13 @@ impl_request!( res_path = ProxyToLayerMessage::File => FileResponse::XstatFs, ); +impl_request!( + req = StatFsRequest, + res = RemoteResult, + req_path = LayerToProxyMessage::File => FileRequest::StatFs, + res_path = ProxyToLayerMessage::File => FileResponse::XstatFs, +); + impl_request!( req = FdOpenDirRequest, res = RemoteResult, diff --git a/mirrord/intproxy/src/proxies/files.rs b/mirrord/intproxy/src/proxies/files.rs index 79ac6695575..107197a0180 100644 --- a/mirrord/intproxy/src/proxies/files.rs +++ b/mirrord/intproxy/src/proxies/files.rs @@ -6,7 +6,7 @@ use mirrord_protocol::{ file::{ CloseDirRequest, CloseFileRequest, DirEntryInternal, ReadDirBatchRequest, ReadDirResponse, ReadFileResponse, ReadLimitedFileRequest, SeekFromInternal, MKDIR_VERSION, - READDIR_BATCH_VERSION, READLINK_VERSION, + READDIR_BATCH_VERSION, READLINK_VERSION, STATFS_VERSION, }, ClientMessage, DaemonMessage, ErrorKindInternal, FileRequest, FileResponse, RemoteIOError, ResponseError, @@ -253,6 +253,31 @@ impl FilesProxy { self.protocol_version.replace(version); } + /// Checks if the mirrord protocol version supports this [`FileRequest`]. + fn is_request_supported(&self, request: &FileRequest) -> Result<(), FileResponse> { + let protocol_version = self.protocol_version.as_ref(); + + match request { + FileRequest::ReadLink(..) + if protocol_version.is_some_and(|version| !READLINK_VERSION.matches(version)) => + { + Err(FileResponse::ReadLink(Err(ResponseError::NotImplemented))) + } + FileRequest::MakeDir(..) | FileRequest::MakeDirAt(..) + if protocol_version.is_some_and(|version| !MKDIR_VERSION.matches(version)) => + { + Err(FileResponse::MakeDir(Err(ResponseError::NotImplemented))) + } + FileRequest::StatFs(..) + if protocol_version + .is_some_and(|version: &Version| !STATFS_VERSION.matches(version)) => + { + Err(FileResponse::XstatFs(Err(ResponseError::NotImplemented))) + } + _ => Ok(()), + } + } + // #[tracing::instrument(level = Level::TRACE, skip(message_bus))] async fn file_request( &mut self, @@ -261,6 +286,18 @@ impl FilesProxy { message_id: MessageId, message_bus: &mut MessageBus, ) { + // Not supported in old `mirrord-protocol` versions. + if let Err(response) = self.is_request_supported(&request) { + message_bus + .send(ToLayer { + message_id, + layer_id, + message: ProxyToLayerMessage::File(response), + }) + .await; + return; + } + match request { // Should trigger remote close only when the fd is closed in all layer instances. FileRequest::Close(close) => { @@ -454,31 +491,6 @@ impl FilesProxy { } }, - // Not supported in old `mirrord-protocol` versions. - req @ FileRequest::ReadLink(..) => { - let supported = self - .protocol_version - .as_ref() - .is_some_and(|version| READLINK_VERSION.matches(version)); - - if supported { - self.request_queue.push_back(message_id, layer_id); - message_bus - .send(ProxyMessage::ToAgent(ClientMessage::FileRequest(req))) - .await; - } else { - message_bus - .send(ToLayer { - message_id, - message: ProxyToLayerMessage::File(FileResponse::ReadLink(Err( - ResponseError::NotImplemented, - ))), - layer_id, - }) - .await; - } - } - // Should only be sent from intproxy, not from the layer. FileRequest::ReadDirBatch(..) => { unreachable!("ReadDirBatch request is never sent from the layer"); diff --git a/mirrord/layer/src/file/hooks.rs b/mirrord/layer/src/file/hooks.rs index 4de1e73577f..a0762080fb2 100644 --- a/mirrord/layer/src/file/hooks.rs +++ b/mirrord/layer/src/file/hooks.rs @@ -904,8 +904,9 @@ unsafe extern "C" fn fstatat_detour( }) } +/// Hook for `libc::fstatfs`. #[hook_guard_fn] -unsafe extern "C" fn fstatfs_detour(fd: c_int, out_stat: *mut statfs) -> c_int { +pub(crate) unsafe extern "C" fn fstatfs_detour(fd: c_int, out_stat: *mut statfs) -> c_int { if out_stat.is_null() { return HookError::BadPointer.into(); } @@ -919,6 +920,25 @@ unsafe extern "C" fn fstatfs_detour(fd: c_int, out_stat: *mut statfs) -> c_int { .unwrap_or_bypass_with(|_| FN_FSTATFS(fd, out_stat)) } +/// Hook for `libc::statfs`. +#[hook_guard_fn] +pub(crate) unsafe extern "C" fn statfs_detour( + raw_path: *const c_char, + out_stat: *mut statfs, +) -> c_int { + if out_stat.is_null() { + return HookError::BadPointer.into(); + } + + crate::file::ops::statfs(raw_path.checked_into()) + .map(|res| { + let res = res.metadata; + fill_statfs(out_stat, &res); + 0 + }) + .unwrap_or_bypass_with(|_| FN_STATFS(raw_path, out_stat)) +} + unsafe fn realpath_logic( source_path: *const c_char, output_path: *mut c_char, @@ -1286,6 +1306,8 @@ pub(crate) unsafe fn enable_file_hooks(hook_manager: &mut HookManager) { FnFstatfs, FN_FSTATFS ); + replace!(hook_manager, "statfs", statfs_detour, FnStatfs, FN_STATFS); + replace!( hook_manager, "fdopendir", @@ -1368,6 +1390,13 @@ pub(crate) unsafe fn enable_file_hooks(hook_manager: &mut HookManager) { FnFstatfs, FN_FSTATFS ); + replace!( + hook_manager, + "statfs$INODE64", + statfs_detour, + FnStatfs, + FN_STATFS + ); replace!( hook_manager, "fdopendir$INODE64", diff --git a/mirrord/layer/src/file/ops.rs b/mirrord/layer/src/file/ops.rs index ca9dd10f951..d314e09b968 100644 --- a/mirrord/layer/src/file/ops.rs +++ b/mirrord/layer/src/file/ops.rs @@ -9,7 +9,7 @@ use mirrord_protocol::{ file::{ MakeDirAtRequest, MakeDirRequest, OpenFileRequest, OpenFileResponse, OpenOptionsInternal, ReadFileResponse, ReadLinkFileRequest, ReadLinkFileResponse, SeekFileResponse, - WriteFileResponse, XstatFsResponse, XstatResponse, + StatFsRequest, WriteFileResponse, XstatFsResponse, XstatResponse, }, ResponseError, }; @@ -671,6 +671,16 @@ pub(crate) fn xstatfs(fd: RawFd) -> Detour { Detour::Success(response) } +#[mirrord_layer_macro::instrument(level = "trace")] +pub(crate) fn statfs(path: Detour) -> Detour { + let path = path?; + let lstatfs = StatFsRequest { path }; + + let response = common::make_proxy_request_with_response(lstatfs)??; + + Detour::Success(response) +} + #[cfg(target_os = "linux")] #[mirrord_layer_macro::instrument(level = "trace")] pub(crate) fn getdents64(fd: RawFd, buffer_size: u64) -> Detour { diff --git a/mirrord/layer/src/go/linux_x64.rs b/mirrord/layer/src/go/linux_x64.rs index 5acaceb13b2..e9922a4a337 100644 --- a/mirrord/layer/src/go/linux_x64.rs +++ b/mirrord/layer/src/go/linux_x64.rs @@ -340,6 +340,8 @@ unsafe extern "C" fn c_abi_syscall_handler( faccessat_detour(param1 as _, param2 as _, param3 as _, 0) as i64 } libc::SYS_fstat => fstat_detour(param1 as _, param2 as _) as i64, + libc::SYS_statfs => statfs_detour(param1 as _, param2 as _) as i64, + libc::SYS_fstatfs => fstatfs_detour(param1 as _, param2 as _) as i64, libc::SYS_getdents64 => getdents64_detour(param1 as _, param2 as _, param3 as _) as i64, #[cfg(all(target_os = "linux", not(target_arch = "aarch64")))] libc::SYS_mkdir => mkdir_detour(param1 as _, param2 as _) as i64, diff --git a/mirrord/layer/src/go/mod.rs b/mirrord/layer/src/go/mod.rs index 003eed8692c..7f542c6e868 100644 --- a/mirrord/layer/src/go/mod.rs +++ b/mirrord/layer/src/go/mod.rs @@ -101,6 +101,8 @@ unsafe extern "C" fn c_abi_syscall6_handler( .into() } libc::SYS_fstat => fstat_detour(param1 as _, param2 as _) as i64, + libc::SYS_statfs => statfs_detour(param1 as _, param2 as _) as i64, + libc::SYS_fstatfs => fstatfs_detour(param1 as _, param2 as _) as i64, libc::SYS_fsync => fsync_detour(param1 as _) as i64, libc::SYS_fdatasync => fsync_detour(param1 as _) as i64, libc::SYS_openat => { diff --git a/mirrord/layer/tests/apps/fileops/go/main.go b/mirrord/layer/tests/apps/fileops/go/main.go index 5973e013d5e..69db336fb3e 100644 --- a/mirrord/layer/tests/apps/fileops/go/main.go +++ b/mirrord/layer/tests/apps/fileops/go/main.go @@ -7,10 +7,21 @@ import ( func main() { tempFile := "/tmp/test_file.txt" - syscall.Open(tempFile, syscall.O_CREAT|syscall.O_WRONLY, 0644) + fd, _ := syscall.Open(tempFile, syscall.O_CREAT|syscall.O_WRONLY, 0644) var stat syscall.Stat_t err := syscall.Stat(tempFile, &stat) if err != nil { panic(err) } + + var statfs syscall.Statfs_t + err = syscall.Statfs(tempFile, &statfs) + if err != nil { + panic(err) + } + + err = syscall.Fstatfs(fd, &statfs) + if err != nil { + panic(err) + } } diff --git a/mirrord/layer/tests/apps/statfs_fstatfs/statfs_fstatfs.c b/mirrord/layer/tests/apps/statfs_fstatfs/statfs_fstatfs.c new file mode 100644 index 00000000000..dc937c830f7 --- /dev/null +++ b/mirrord/layer/tests/apps/statfs_fstatfs/statfs_fstatfs.c @@ -0,0 +1,42 @@ +#include +#include +#include +#include +#include +#include + +#if defined(__APPLE__) && defined(__MACH__) +#include +#include +#else +#include +#endif + +/// Test `statfs / fstatfs`. +/// +/// Gets information about a mounted filesystem +/// +int main() +{ + char *tmp_test_path = "/statfs_fstatfs_test_path"; + int fd = mkdir(tmp_test_path, 0777); + + struct statfs statfs_buf; + if (statfs(tmp_test_path, &statfs_buf) == -1) + { + perror("statfs failed"); + close(fd); + return EXIT_FAILURE; + } + + struct statfs fstatfs_buf; + if (fstatfs(fd, &fstatfs_buf) == -1) + { + perror("fstatfs failed"); + close(fd); + return EXIT_FAILURE; + } + + close(fd); + return 0; +} diff --git a/mirrord/layer/tests/common/mod.rs b/mirrord/layer/tests/common/mod.rs index 22945790e4b..8cc205d93d3 100644 --- a/mirrord/layer/tests/common/mod.rs +++ b/mirrord/layer/tests/common/mod.rs @@ -17,7 +17,7 @@ use mirrord_intproxy::{agent_conn::AgentConnection, IntProxy}; use mirrord_protocol::{ file::{ AccessFileRequest, AccessFileResponse, OpenFileRequest, OpenOptionsInternal, - ReadFileRequest, SeekFromInternal, XstatRequest, XstatResponse, + ReadFileRequest, SeekFromInternal, XstatFsResponse, XstatRequest, XstatResponse, }, tcp::{DaemonTcp, LayerTcp, NewTcpConnection, TcpClose, TcpData}, ClientMessage, DaemonCodec, DaemonMessage, FileRequest, FileResponse, @@ -487,6 +487,48 @@ impl TestIntProxy { .unwrap(); } + /// Makes a [`FileRequest::Statefs`] and answers it. + pub async fn expect_statfs(&mut self, expected_path: &str) { + // Expecting `statfs` call with path. + assert_matches!( + self.recv().await, + ClientMessage::FileRequest(FileRequest::StatFs( + mirrord_protocol::file::StatFsRequest { path } + )) if path.to_str().unwrap() == expected_path + ); + + // Answer `statfs`. + self.codec + .send(DaemonMessage::File(FileResponse::XstatFs(Ok( + XstatFsResponse { + metadata: Default::default(), + }, + )))) + .await + .unwrap(); + } + + /// Makes a [`FileRequest::Xstatefs`] and answers it. + pub async fn expect_fstatfs(&mut self, expected_fd: Option) { + // Expecting `fstatfs` call with path. + assert_matches!( + self.recv().await, + ClientMessage::FileRequest(FileRequest::XstatFs( + mirrord_protocol::file::XstatFsRequest { fd } + )) if expected_fd.is_none() || expected_fd.unwrap() == fd + ); + + // Answer `fstatfs`. + self.codec + .send(DaemonMessage::File(FileResponse::XstatFs(Ok( + XstatFsResponse { + metadata: Default::default(), + }, + )))) + .await + .unwrap(); + } + /// Verify that the passed message (not the next message from self.codec!) is a file read. /// Return buffer size. pub async fn expect_message_file_read(message: ClientMessage, expected_fd: u64) -> u64 { @@ -763,6 +805,7 @@ pub enum Application { Fork, ReadLink, MakeDir, + StatfsFstatfs, OpenFile, CIssue2055, CIssue2178, @@ -819,6 +862,7 @@ impl Application { Application::Fork => String::from("tests/apps/fork/out.c_test_app"), Application::ReadLink => String::from("tests/apps/readlink/out.c_test_app"), Application::MakeDir => String::from("tests/apps/mkdir/out.c_test_app"), + Application::StatfsFstatfs => String::from("tests/apps/statfs_fstatfs/out.c_test_app"), Application::Realpath => String::from("tests/apps/realpath/out.c_test_app"), Application::NodeHTTP | Application::NodeIssue2283 | Application::NodeIssue2807 => { String::from("node") @@ -1057,6 +1101,7 @@ impl Application { | Application::Fork | Application::ReadLink | Application::MakeDir + | Application::StatfsFstatfs | Application::Realpath | Application::RustFileOps | Application::RustIssue1123 @@ -1135,6 +1180,7 @@ impl Application { | Application::Fork | Application::ReadLink | Application::MakeDir + | Application::StatfsFstatfs | Application::Realpath | Application::Go21Issue834 | Application::Go22Issue834 diff --git a/mirrord/layer/tests/fileops.rs b/mirrord/layer/tests/fileops.rs index 5a9ee96f726..f8598cc2396 100644 --- a/mirrord/layer/tests/fileops.rs +++ b/mirrord/layer/tests/fileops.rs @@ -345,6 +345,9 @@ async fn go_stat( )))) .await; + intproxy.expect_statfs("/tmp/test_file.txt").await; + intproxy.expect_fstatfs(Some(fd)).await; + test_process.wait_assert_success().await; test_process.assert_no_error_in_stderr().await; } diff --git a/mirrord/layer/tests/statfs_fstatfs.rs b/mirrord/layer/tests/statfs_fstatfs.rs new file mode 100644 index 00000000000..90b5472c186 --- /dev/null +++ b/mirrord/layer/tests/statfs_fstatfs.rs @@ -0,0 +1,31 @@ +#![feature(assert_matches)] +use std::{path::Path, time::Duration}; + +use rstest::rstest; + +mod common; +pub use common::*; + +/// Test for the [`libc::statfs`] and [`libc::fstatfs`] functions. +#[rstest] +#[tokio::test] +#[timeout(Duration::from_secs(60))] +async fn mkdir(dylib_path: &Path) { + let application = Application::StatfsFstatfs; + + let (mut test_process, mut intproxy) = application + .start_process_with_layer(dylib_path, Default::default(), None) + .await; + + println!("waiting for file request."); + intproxy.expect_statfs("/statfs_fstatfs_test_path").await; + + println!("waiting for file request."); + intproxy.expect_fstatfs(None).await; + + assert_eq!(intproxy.try_recv().await, None); + + test_process.wait_assert_success().await; + test_process.assert_no_error_in_stderr().await; + test_process.assert_no_error_in_stdout().await; +} diff --git a/mirrord/protocol/Cargo.toml b/mirrord/protocol/Cargo.toml index 7d787e1e5c6..fd33bba8e6b 100644 --- a/mirrord/protocol/Cargo.toml +++ b/mirrord/protocol/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mirrord-protocol" -version = "1.13.3" +version = "1.14.0" authors.workspace = true description.workspace = true documentation.workspace = true diff --git a/mirrord/protocol/src/codec.rs b/mirrord/protocol/src/codec.rs index 4071018fe6a..fdbf69a54f4 100644 --- a/mirrord/protocol/src/codec.rs +++ b/mirrord/protocol/src/codec.rs @@ -71,6 +71,7 @@ pub enum FileRequest { Access(AccessFileRequest), Xstat(XstatRequest), XstatFs(XstatFsRequest), + StatFs(StatFsRequest), FdOpenDir(FdOpenDirRequest), ReadDir(ReadDirRequest), CloseDir(CloseDirRequest), diff --git a/mirrord/protocol/src/file.rs b/mirrord/protocol/src/file.rs index c0b4cfe2f18..8de89c7b3cb 100644 --- a/mirrord/protocol/src/file.rs +++ b/mirrord/protocol/src/file.rs @@ -28,6 +28,9 @@ pub static MKDIR_VERSION: LazyLock = pub static OPEN_LOCAL_VERSION: LazyLock = LazyLock::new(|| ">=1.13.3".parse().expect("Bad Identifier")); +pub static STATFS_VERSION: LazyLock = + LazyLock::new(|| ">=1.14.0".parse().expect("Bad Identifier")); + /// Internal version of Metadata across operating system (macOS, Linux) /// Only mutual attributes #[derive(Encode, Decode, Debug, PartialEq, Clone, Copy, Eq, Default)] @@ -390,6 +393,11 @@ pub struct XstatFsRequest { pub fd: u64, } +#[derive(Encode, Decode, Debug, PartialEq, Eq, Clone)] +pub struct StatFsRequest { + pub path: PathBuf, +} + #[derive(Encode, Decode, Debug, PartialEq, Eq, Clone)] pub struct XstatResponse { pub metadata: MetadataInternal, diff --git a/tests/python-e2e/ops.py b/tests/python-e2e/ops.py index 9f94425921e..50606d4f2ac 100644 --- a/tests/python-e2e/ops.py +++ b/tests/python-e2e/ops.py @@ -87,7 +87,20 @@ def test_mkdir_errors(self): os.mkdir("test_mkdir_error_already_exists", dir_fd=dir) os.close(dir) + + def test_statfs_and_fstatvfs_sucess(self): + """ + Test statfs / fstatfs + """ + file_path, _ = self._create_new_tmp_file() + statvfs_result = os.statvfs(file_path) + self.assertIsNotNone(statvfs_result) + + fd = os.open(file_path, os.O_RDONLY) + fstatvfs_result = os.fstatvfs(fd) + self.assertIsNotNone(fstatvfs_result) + def _create_new_tmp_file(self): """ Creates a new file in /tmp and returns the path and name of the file. @@ -97,6 +110,5 @@ def _create_new_tmp_file(self): w_file.write(TEXT) return file_path, file_name - if __name__ == "__main__": unittest.main() From 49914a4c3b96871e1a4a2fbb5dbe0d1ee7de6573 Mon Sep 17 00:00:00 2001 From: Facundo Poblete Date: Tue, 14 Jan 2025 01:32:33 -0300 Subject: [PATCH 02/10] + changelog.d --- changelog.d/statfs.added.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/statfs.added.md diff --git a/changelog.d/statfs.added.md b/changelog.d/statfs.added.md new file mode 100644 index 00000000000..b1cea16a410 --- /dev/null +++ b/changelog.d/statfs.added.md @@ -0,0 +1 @@ +Add statfs support \ No newline at end of file From 806cae5a3b67209f3ca2f604138d29902ae90492 Mon Sep 17 00:00:00 2001 From: Facundo Poblete Date: Tue, 14 Jan 2025 02:13:39 -0300 Subject: [PATCH 03/10] Fix test statfs_fstatfs.rs --- mirrord/layer/tests/statfs_fstatfs.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/mirrord/layer/tests/statfs_fstatfs.rs b/mirrord/layer/tests/statfs_fstatfs.rs index 90b5472c186..e7354f75c8d 100644 --- a/mirrord/layer/tests/statfs_fstatfs.rs +++ b/mirrord/layer/tests/statfs_fstatfs.rs @@ -17,6 +17,11 @@ async fn mkdir(dylib_path: &Path) { .start_process_with_layer(dylib_path, Default::default(), None) .await; + println!("waiting for file request."); + intproxy + .expect_make_dir("/statfs_fstatfs_test_path", 0o777) + .await; + println!("waiting for file request."); intproxy.expect_statfs("/statfs_fstatfs_test_path").await; From a0c5ee387d9b99843ddf851d953fda49f7895789 Mon Sep 17 00:00:00 2001 From: Facundo Poblete Date: Tue, 14 Jan 2025 02:39:56 -0300 Subject: [PATCH 04/10] Fix statfs_fstatfs.c --- .../tests/apps/statfs_fstatfs/statfs_fstatfs.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/mirrord/layer/tests/apps/statfs_fstatfs/statfs_fstatfs.c b/mirrord/layer/tests/apps/statfs_fstatfs/statfs_fstatfs.c index dc937c830f7..52e7a2f4904 100644 --- a/mirrord/layer/tests/apps/statfs_fstatfs/statfs_fstatfs.c +++ b/mirrord/layer/tests/apps/statfs_fstatfs/statfs_fstatfs.c @@ -19,16 +19,25 @@ int main() { char *tmp_test_path = "/statfs_fstatfs_test_path"; - int fd = mkdir(tmp_test_path, 0777); + mkdir(tmp_test_path, 0777); + // statfs struct statfs statfs_buf; if (statfs(tmp_test_path, &statfs_buf) == -1) { perror("statfs failed"); - close(fd); return EXIT_FAILURE; } + // fstatfs + int fd = open(tmp_test_path, O_RDONLY); + + if (fd == -1) + { + perror("Error opening tmp_test_path"); + return 1; + } + struct statfs fstatfs_buf; if (fstatfs(fd, &fstatfs_buf) == -1) { From a8d54d6efba48d2029ec9d26caa39dbab61aae41 Mon Sep 17 00:00:00 2001 From: Facundo Poblete Date: Tue, 14 Jan 2025 02:41:10 -0300 Subject: [PATCH 05/10] Fix statfs_fstatfs.c --- mirrord/layer/tests/apps/statfs_fstatfs/statfs_fstatfs.c | 1 + 1 file changed, 1 insertion(+) diff --git a/mirrord/layer/tests/apps/statfs_fstatfs/statfs_fstatfs.c b/mirrord/layer/tests/apps/statfs_fstatfs/statfs_fstatfs.c index 52e7a2f4904..6c474cce4e3 100644 --- a/mirrord/layer/tests/apps/statfs_fstatfs/statfs_fstatfs.c +++ b/mirrord/layer/tests/apps/statfs_fstatfs/statfs_fstatfs.c @@ -4,6 +4,7 @@ #include #include #include +#include #if defined(__APPLE__) && defined(__MACH__) #include From df28b10776d88fc5fe5f9f994df53edaf3026791 Mon Sep 17 00:00:00 2001 From: Facundo Poblete Date: Tue, 14 Jan 2025 03:05:45 -0300 Subject: [PATCH 06/10] Fix statfs_fstatfs.rs --- mirrord/layer/tests/common/mod.rs | 4 ++-- mirrord/layer/tests/fileops.rs | 2 +- mirrord/layer/tests/statfs_fstatfs.rs | 8 +++++++- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/mirrord/layer/tests/common/mod.rs b/mirrord/layer/tests/common/mod.rs index 8cc205d93d3..14e8d9acda5 100644 --- a/mirrord/layer/tests/common/mod.rs +++ b/mirrord/layer/tests/common/mod.rs @@ -509,13 +509,13 @@ impl TestIntProxy { } /// Makes a [`FileRequest::Xstatefs`] and answers it. - pub async fn expect_fstatfs(&mut self, expected_fd: Option) { + pub async fn expect_fstatfs(&mut self, expected_fd: u64) { // Expecting `fstatfs` call with path. assert_matches!( self.recv().await, ClientMessage::FileRequest(FileRequest::XstatFs( mirrord_protocol::file::XstatFsRequest { fd } - )) if expected_fd.is_none() || expected_fd.unwrap() == fd + )) if expected_fd == fd ); // Answer `fstatfs`. diff --git a/mirrord/layer/tests/fileops.rs b/mirrord/layer/tests/fileops.rs index f8598cc2396..c2f726b93a5 100644 --- a/mirrord/layer/tests/fileops.rs +++ b/mirrord/layer/tests/fileops.rs @@ -346,7 +346,7 @@ async fn go_stat( .await; intproxy.expect_statfs("/tmp/test_file.txt").await; - intproxy.expect_fstatfs(Some(fd)).await; + intproxy.expect_fstatfs(fd).await; test_process.wait_assert_success().await; test_process.assert_no_error_in_stderr().await; diff --git a/mirrord/layer/tests/statfs_fstatfs.rs b/mirrord/layer/tests/statfs_fstatfs.rs index e7354f75c8d..3bdc525e32d 100644 --- a/mirrord/layer/tests/statfs_fstatfs.rs +++ b/mirrord/layer/tests/statfs_fstatfs.rs @@ -22,11 +22,17 @@ async fn mkdir(dylib_path: &Path) { .expect_make_dir("/statfs_fstatfs_test_path", 0o777) .await; + println!("waiting for file request."); + let fd: u64 = 1; + intproxy + .expect_file_open_for_reading("/statfs_fstatfs_test_path", fd) + .await; + println!("waiting for file request."); intproxy.expect_statfs("/statfs_fstatfs_test_path").await; println!("waiting for file request."); - intproxy.expect_fstatfs(None).await; + intproxy.expect_fstatfs(fd).await; assert_eq!(intproxy.try_recv().await, None); From 96daf15f2d8e0c2e0326a28c99d68bedb8246b4b Mon Sep 17 00:00:00 2001 From: Facundo Poblete Date: Tue, 14 Jan 2025 08:26:39 -0300 Subject: [PATCH 07/10] Fix statfs_fstatfs.rs --- mirrord/layer/tests/statfs_fstatfs.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/mirrord/layer/tests/statfs_fstatfs.rs b/mirrord/layer/tests/statfs_fstatfs.rs index 3bdc525e32d..a3fa72315e2 100644 --- a/mirrord/layer/tests/statfs_fstatfs.rs +++ b/mirrord/layer/tests/statfs_fstatfs.rs @@ -22,15 +22,15 @@ async fn mkdir(dylib_path: &Path) { .expect_make_dir("/statfs_fstatfs_test_path", 0o777) .await; + println!("waiting for file request."); + intproxy.expect_statfs("/statfs_fstatfs_test_path").await; + println!("waiting for file request."); let fd: u64 = 1; intproxy .expect_file_open_for_reading("/statfs_fstatfs_test_path", fd) .await; - println!("waiting for file request."); - intproxy.expect_statfs("/statfs_fstatfs_test_path").await; - println!("waiting for file request."); intproxy.expect_fstatfs(fd).await; From a99ef263dd5a4fa016440f04cd647a6afe2e2456 Mon Sep 17 00:00:00 2001 From: Facundo Poblete Date: Tue, 14 Jan 2025 08:40:42 -0300 Subject: [PATCH 08/10] Fix statfs_fstatfs.rs --- mirrord/layer/tests/statfs_fstatfs.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/mirrord/layer/tests/statfs_fstatfs.rs b/mirrord/layer/tests/statfs_fstatfs.rs index a3fa72315e2..38f48c8495f 100644 --- a/mirrord/layer/tests/statfs_fstatfs.rs +++ b/mirrord/layer/tests/statfs_fstatfs.rs @@ -17,23 +17,26 @@ async fn mkdir(dylib_path: &Path) { .start_process_with_layer(dylib_path, Default::default(), None) .await; - println!("waiting for file request."); + println!("waiting for file request (mkdir)."); intproxy .expect_make_dir("/statfs_fstatfs_test_path", 0o777) .await; - println!("waiting for file request."); + println!("waiting for file request (statfs)."); intproxy.expect_statfs("/statfs_fstatfs_test_path").await; - println!("waiting for file request."); + println!("waiting for file request (open)."); let fd: u64 = 1; intproxy .expect_file_open_for_reading("/statfs_fstatfs_test_path", fd) .await; - println!("waiting for file request."); + println!("waiting for file request (fstatfs)."); intproxy.expect_fstatfs(fd).await; + println!("waiting for file request (close)."); + intproxy.expect_file_close(fd).await; + assert_eq!(intproxy.try_recv().await, None); test_process.wait_assert_success().await; From ec47380606b55cd1d31e8bc709d0a1035e5ad29a Mon Sep 17 00:00:00 2001 From: Facundo Date: Tue, 21 Jan 2025 12:48:28 -0300 Subject: [PATCH 09/10] Update file.rs --- mirrord/protocol/src/file.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mirrord/protocol/src/file.rs b/mirrord/protocol/src/file.rs index 8de89c7b3cb..a0850e29892 100644 --- a/mirrord/protocol/src/file.rs +++ b/mirrord/protocol/src/file.rs @@ -29,7 +29,7 @@ pub static OPEN_LOCAL_VERSION: LazyLock = LazyLock::new(|| ">=1.13.3".parse().expect("Bad Identifier")); pub static STATFS_VERSION: LazyLock = - LazyLock::new(|| ">=1.14.0".parse().expect("Bad Identifier")); + LazyLock::new(|| ">=1.15.0".parse().expect("Bad Identifier")); /// Internal version of Metadata across operating system (macOS, Linux) /// Only mutual attributes From 2c274ac5996363bc99d8be3469429e6fdabbba59 Mon Sep 17 00:00:00 2001 From: Facundo Poblete Date: Wed, 22 Jan 2025 14:59:34 -0300 Subject: [PATCH 10/10] PR comments --- mirrord/intproxy/src/proxies/files.rs | 8 ++++---- mirrord/protocol/src/codec.rs | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/mirrord/intproxy/src/proxies/files.rs b/mirrord/intproxy/src/proxies/files.rs index b4975754676..55c1b2f98f0 100644 --- a/mirrord/intproxy/src/proxies/files.rs +++ b/mirrord/intproxy/src/proxies/files.rs @@ -259,24 +259,24 @@ impl FilesProxy { match request { FileRequest::ReadLink(..) - if protocol_version.is_some_and(|version| !READLINK_VERSION.matches(version)) => + if protocol_version.is_none_or(|version| !READLINK_VERSION.matches(version)) => { Err(FileResponse::ReadLink(Err(ResponseError::NotImplemented))) } FileRequest::MakeDir(..) | FileRequest::MakeDirAt(..) - if protocol_version.is_some_and(|version| !MKDIR_VERSION.matches(version)) => + if protocol_version.is_none_or(|version| !MKDIR_VERSION.matches(version)) => { Err(FileResponse::MakeDir(Err(ResponseError::NotImplemented))) } FileRequest::RemoveDir(..) | FileRequest::Unlink(..) | FileRequest::UnlinkAt(..) if protocol_version - .is_some_and(|version: &Version| !RMDIR_VERSION.matches(version)) => + .is_none_or(|version: &Version| !RMDIR_VERSION.matches(version)) => { Err(FileResponse::RemoveDir(Err(ResponseError::NotImplemented))) } FileRequest::StatFs(..) if protocol_version - .is_some_and(|version: &Version| !STATFS_VERSION.matches(version)) => + .is_none_or(|version: &Version| !STATFS_VERSION.matches(version)) => { Err(FileResponse::XstatFs(Err(ResponseError::NotImplemented))) } diff --git a/mirrord/protocol/src/codec.rs b/mirrord/protocol/src/codec.rs index b91c67e4d37..9c25ef004bd 100644 --- a/mirrord/protocol/src/codec.rs +++ b/mirrord/protocol/src/codec.rs @@ -77,7 +77,6 @@ pub enum FileRequest { Access(AccessFileRequest), Xstat(XstatRequest), XstatFs(XstatFsRequest), - StatFs(StatFsRequest), FdOpenDir(FdOpenDirRequest), ReadDir(ReadDirRequest), CloseDir(CloseDirRequest), @@ -95,6 +94,7 @@ pub enum FileRequest { RemoveDir(RemoveDirRequest), Unlink(UnlinkRequest), UnlinkAt(UnlinkAtRequest), + StatFs(StatFsRequest), } /// Minimal mirrord-protocol version that allows `ClientMessage::ReadyForLogs` message.