From 9ee613a0b79aeaab01460f36c881d4cb4f634c82 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Mon, 17 Apr 2023 14:35:28 -0700 Subject: [PATCH] wasi-common: deprecate fd_allocate (#6217) * wasi-common: remove allocate from WasiFile trait, always fail with NOTSUP This operation from cloudabi is linux-specific, isn't even supported across all linux filesystems, and has no support on macos or windows. Rather than ship spotty support, it has been removed from preview 2, and we are no longer supporting it in preview 1 as well. The preview 1 implementation will still check if fd is a file, and has rights, just to reject those cases with the errors expected. * wasi-tests: expect fd_allocate to always fail now. rewrite the file_allocate test to just check for failure. remove use of fd_allocate from fd_advise test, and remove test configuration setting used for excluding use of fd_allocate on macos and windows. --- .../tests/wasm_tests/runtime/mod.rs | 8 +----- .../wasi-tests/src/bin/fd_advise.rs | 10 +------ .../wasi-tests/src/bin/file_allocate.rs | 26 ++++++++----------- crates/test-programs/wasi-tests/src/config.rs | 6 ----- crates/wasi-common/cap-std-sync/src/file.rs | 4 --- crates/wasi-common/src/file.rs | 4 --- crates/wasi-common/src/snapshots/preview_1.rs | 20 +++++++++----- crates/wasi-common/tokio/src/file.rs | 3 --- 8 files changed, 26 insertions(+), 55 deletions(-) diff --git a/crates/test-programs/tests/wasm_tests/runtime/mod.rs b/crates/test-programs/tests/wasm_tests/runtime/mod.rs index 9a4fe8202a..2ea46982da 100644 --- a/crates/test-programs/tests/wasm_tests/runtime/mod.rs +++ b/crates/test-programs/tests/wasm_tests/runtime/mod.rs @@ -12,8 +12,6 @@ pub fn test_suite_environment() -> &'static [(&'static str, &'static str)] { ("ERRNO_MODE_WINDOWS", "1"), // Windows does not support dangling links or symlinks in the filesystem. ("NO_DANGLING_FILESYSTEM", "1"), - // Windows does not support fd_allocate. - ("NO_FD_ALLOCATE", "1"), // Windows does not support renaming a directory to an empty directory - // empty directory must be deleted. ("NO_RENAME_DIR_TO_EMPTY_DIR", "1"), @@ -25,10 +23,6 @@ pub fn test_suite_environment() -> &'static [(&'static str, &'static str)] { } #[cfg(target_os = "macos")] { - &[ - ("ERRNO_MODE_MACOS", "1"), - // MacOS does not support fd_allocate - ("NO_FD_ALLOCATE", "1"), - ] + &[("ERRNO_MODE_MACOS", "1")] } } diff --git a/crates/test-programs/wasi-tests/src/bin/fd_advise.rs b/crates/test-programs/wasi-tests/src/bin/fd_advise.rs index 62346fdc56..d6c37ae776 100644 --- a/crates/test-programs/wasi-tests/src/bin/fd_advise.rs +++ b/crates/test-programs/wasi-tests/src/bin/fd_advise.rs @@ -1,5 +1,5 @@ use std::{env, process}; -use wasi_tests::{open_scratch_directory, TESTCONFIG}; +use wasi_tests::open_scratch_directory; unsafe fn test_fd_advise(dir_fd: wasi::Fd) { // Create a file in the scratch directory. @@ -40,14 +40,6 @@ unsafe fn test_fd_advise(dir_fd: wasi::Fd) { let stat = wasi::fd_filestat_get(file_fd).expect("failed to fdstat 3"); assert_eq!(stat.size, 100, "file size should be 100"); - if TESTCONFIG.support_fd_allocate() { - // Use fd_allocate to expand size to 200: - wasi::fd_allocate(file_fd, 100, 100).expect("allocating size"); - - let stat = wasi::fd_filestat_get(file_fd).expect("failed to fdstat 3"); - assert_eq!(stat.size, 200, "file size should be 200"); - } - wasi::fd_close(file_fd).expect("failed to close"); wasi::path_unlink_file(dir_fd, "file").expect("failed to unlink"); } diff --git a/crates/test-programs/wasi-tests/src/bin/file_allocate.rs b/crates/test-programs/wasi-tests/src/bin/file_allocate.rs index da79315463..378caaf318 100644 --- a/crates/test-programs/wasi-tests/src/bin/file_allocate.rs +++ b/crates/test-programs/wasi-tests/src/bin/file_allocate.rs @@ -1,5 +1,5 @@ use std::{env, process}; -use wasi_tests::{open_scratch_directory, TESTCONFIG}; +use wasi_tests::open_scratch_directory; unsafe fn test_file_allocate(dir_fd: wasi::Fd) { // Create a file in the scratch directory. @@ -25,22 +25,18 @@ unsafe fn test_file_allocate(dir_fd: wasi::Fd) { let mut stat = wasi::fd_filestat_get(file_fd).expect("reading file stats"); assert_eq!(stat.size, 0, "file size should be 0"); - if TESTCONFIG.support_fd_allocate() { - // Allocate some size - wasi::fd_allocate(file_fd, 0, 100).expect("allocating size"); - stat = wasi::fd_filestat_get(file_fd).expect("reading file stats"); - assert_eq!(stat.size, 100, "file size should be 100"); + let err = wasi::fd_allocate(file_fd, 0, 100) + .err() + .expect("fd_allocate must fail"); + assert_eq!( + err, + wasi::ERRNO_NOTSUP, + "fd_allocate should fail with NOTSUP" + ); - // Allocate should not modify if less than current size - wasi::fd_allocate(file_fd, 10, 10).expect("allocating size less than current size"); - stat = wasi::fd_filestat_get(file_fd).expect("reading file stats"); - assert_eq!(stat.size, 100, "file size should remain unchanged at 100"); + stat = wasi::fd_filestat_get(file_fd).expect("reading file stats"); + assert_eq!(stat.size, 0, "file size should still be 0"); - // Allocate should modify if offset+len > current_len - wasi::fd_allocate(file_fd, 90, 20).expect("allocating size larger than current size"); - stat = wasi::fd_filestat_get(file_fd).expect("reading file stats"); - assert_eq!(stat.size, 110, "file size should increase from 100 to 110"); - } wasi::fd_close(file_fd).expect("closing a file"); wasi::path_unlink_file(dir_fd, "file").expect("removing a file"); } diff --git a/crates/test-programs/wasi-tests/src/config.rs b/crates/test-programs/wasi-tests/src/config.rs index b2d095c5c2..2c8e0c0f68 100644 --- a/crates/test-programs/wasi-tests/src/config.rs +++ b/crates/test-programs/wasi-tests/src/config.rs @@ -1,7 +1,6 @@ pub struct TestConfig { errno_mode: ErrnoMode, no_dangling_filesystem: bool, - no_fd_allocate: bool, no_rename_dir_to_empty_dir: bool, no_fdflags_sync_support: bool, } @@ -25,13 +24,11 @@ impl TestConfig { ErrnoMode::Permissive }; let no_dangling_filesystem = std::env::var("NO_DANGLING_FILESYSTEM").is_ok(); - let no_fd_allocate = std::env::var("NO_FD_ALLOCATE").is_ok(); let no_rename_dir_to_empty_dir = std::env::var("NO_RENAME_DIR_TO_EMPTY_DIR").is_ok(); let no_fdflags_sync_support = std::env::var("NO_FDFLAGS_SYNC_SUPPORT").is_ok(); TestConfig { errno_mode, no_dangling_filesystem, - no_fd_allocate, no_rename_dir_to_empty_dir, no_fdflags_sync_support, } @@ -57,9 +54,6 @@ impl TestConfig { pub fn support_dangling_filesystem(&self) -> bool { !self.no_dangling_filesystem } - pub fn support_fd_allocate(&self) -> bool { - !self.no_fd_allocate - } pub fn support_rename_dir_to_empty_dir(&self) -> bool { !self.no_rename_dir_to_empty_dir } diff --git a/crates/wasi-common/cap-std-sync/src/file.rs b/crates/wasi-common/cap-std-sync/src/file.rs index 49a86b8298..20a95e9442 100644 --- a/crates/wasi-common/cap-std-sync/src/file.rs +++ b/crates/wasi-common/cap-std-sync/src/file.rs @@ -84,10 +84,6 @@ impl WasiFile for File { self.0.advise(offset, len, convert_advice(advice))?; Ok(()) } - async fn allocate(&self, offset: u64, len: u64) -> Result<(), Error> { - self.0.allocate(offset, len)?; - Ok(()) - } async fn set_times( &self, atime: Option, diff --git a/crates/wasi-common/src/file.rs b/crates/wasi-common/src/file.rs index c799b6dcbb..04ca215bca 100644 --- a/crates/wasi-common/src/file.rs +++ b/crates/wasi-common/src/file.rs @@ -83,10 +83,6 @@ pub trait WasiFile: Send + Sync { Err(Error::badf()) } - async fn allocate(&self, _offset: u64, _len: u64) -> Result<(), Error> { - Err(Error::badf()) - } - async fn set_times( &self, _atime: Option, diff --git a/crates/wasi-common/src/snapshots/preview_1.rs b/crates/wasi-common/src/snapshots/preview_1.rs index d7b0d16f7a..1d8089b7c1 100644 --- a/crates/wasi-common/src/snapshots/preview_1.rs +++ b/crates/wasi-common/src/snapshots/preview_1.rs @@ -123,15 +123,21 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { async fn fd_allocate( &mut self, fd: types::Fd, - offset: types::Filesize, - len: types::Filesize, + _offset: types::Filesize, + _len: types::Filesize, ) -> Result<(), Error> { - self.table() + // Check if fd is a file, and has rights, just to reject those cases + // with the errors expected: + let _ = self + .table() .get_file(u32::from(fd))? - .get_cap(FileCaps::ALLOCATE)? - .allocate(offset, len) - .await?; - Ok(()) + .get_cap(FileCaps::ALLOCATE)?; + // This operation from cloudabi is linux-specific, isn't even + // supported across all linux filesystems, and has no support on macos + // or windows. Rather than ship spotty support, it has been removed + // from preview 2, and we are no longer supporting it in preview 1 as + // well. + Err(Error::not_supported()) } async fn fd_close(&mut self, fd: types::Fd) -> Result<(), Error> { diff --git a/crates/wasi-common/tokio/src/file.rs b/crates/wasi-common/tokio/src/file.rs index 114b5a2eae..2fa8ddaf86 100644 --- a/crates/wasi-common/tokio/src/file.rs +++ b/crates/wasi-common/tokio/src/file.rs @@ -127,9 +127,6 @@ macro_rules! wasi_file_impl { async fn advise(&self, offset: u64, len: u64, advice: Advice) -> Result<(), Error> { block_on_dummy_executor(move || self.0.advise(offset, len, advice)) } - async fn allocate(&self, offset: u64, len: u64) -> Result<(), Error> { - block_on_dummy_executor(move || self.0.allocate(offset, len)) - } async fn read_vectored<'a>( &self, bufs: &mut [io::IoSliceMut<'a>],