Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

statfs in Go on Linux #3051

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
18 changes: 12 additions & 6 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -301,23 +301,26 @@ jobs:
- run: |
go version
- run: | # Build Go test apps.
./scripts/build_go_apps.sh 21
cd mirrord/layer/tests
../../../scripts/build_go_apps.sh 21
- uses: actions/setup-go@v5
with:
go-version: "1.22"
cache: false
- run: |
go version
- run: | # Build Go test apps.
./scripts/build_go_apps.sh 22
cd mirrord/layer/tests
../../../scripts/build_go_apps.sh 22
- uses: actions/setup-go@v5
with:
go-version: "1.23"
cache: false
- run: |
go version
- run: | # Build Go test apps.
./scripts/build_go_apps.sh 23
cd mirrord/layer/tests
../../../scripts/build_go_apps.sh 23
- run: |
cd mirrord/layer/tests/apps/fileops
cargo build
Expand Down Expand Up @@ -429,23 +432,26 @@ jobs:
go version
# don't use "cache" for other Gos since it will try to overwrite and have bad results.
- run: | # Build Go test apps.
./scripts/build_go_apps.sh 21
cd mirrord/layer/tests
../../../scripts/build_go_apps.sh 21
- uses: actions/setup-go@v5
with:
go-version: "1.22"
cache: false
- run: |
go version
- run: | # Build Go test apps.
./scripts/build_go_apps.sh 22
cd mirrord/layer/tests
../../../scripts/build_go_apps.sh 22
- uses: actions/setup-go@v5
with:
go-version: "1.23"
cache: false
- run: |
go version
- run: | # Build Go test apps.
./scripts/build_go_apps.sh 23
cd mirrord/layer/tests
../../../scripts/build_go_apps.sh 23
- run: |
cd mirrord/layer/tests/apps/fileops
cargo build
Expand Down
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions changelog.d/3044.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Correct statfs data for Go.
24 changes: 17 additions & 7 deletions mirrord/agent/src/file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -223,12 +223,22 @@ impl FileManager {
Some(FileResponse::Xstat(xstat_result))
}
FileRequest::XstatFs(XstatFsRequest { fd }) => {
let xstatfs_result = self.xstatfs(fd);
Some(FileResponse::XstatFs(xstatfs_result))
let xstatfs_result = self.fstatfs(fd);
// convert V2 response to old response for old client.
Some(FileResponse::XstatFs(xstatfs_result.map(Into::into)))
}
FileRequest::XstatFsV2(XstatFsRequestV2 { fd }) => {
let xstatfs_result = self.fstatfs(fd);
Some(FileResponse::XstatFsV2(xstatfs_result))
}
FileRequest::StatFs(StatFsRequest { path }) => {
let statfs_result = self.statfs(path);
Some(FileResponse::XstatFs(statfs_result))
// convert V2 response to old response for old client.
Some(FileResponse::XstatFs(statfs_result.map(Into::into)))
}
FileRequest::StatFsV2(StatFsRequestV2 { path }) => {
let statfs_result = self.statfs(path);
Some(FileResponse::XstatFsV2(statfs_result))
}

// dir operations
Expand Down Expand Up @@ -755,7 +765,7 @@ impl FileManager {
}

#[tracing::instrument(level = "trace", skip(self))]
pub(crate) fn xstatfs(&mut self, fd: u64) -> RemoteResult<XstatFsResponse> {
pub(crate) fn fstatfs(&mut self, fd: u64) -> RemoteResult<XstatFsResponseV2> {
let target = self
.open_files
.get(&fd)
Expand All @@ -768,19 +778,19 @@ impl FileManager {
.map_err(|err| std::io::Error::from_raw_os_error(err as i32))?,
};

Ok(XstatFsResponse {
Ok(XstatFsResponseV2 {
metadata: statfs.into(),
})
}

#[tracing::instrument(level = "trace", skip(self))]
pub(crate) fn statfs(&mut self, path: PathBuf) -> RemoteResult<XstatFsResponse> {
pub(crate) fn statfs(&mut self, path: PathBuf) -> RemoteResult<XstatFsResponseV2> {
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 {
Ok(XstatFsResponseV2 {
metadata: statfs.into(),
})
}
Expand Down
14 changes: 14 additions & 0 deletions mirrord/intproxy/protocol/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -387,13 +387,27 @@ impl_request!(
res_path = ProxyToLayerMessage::File => FileResponse::XstatFs,
);

impl_request!(
req = XstatFsRequestV2,
res = RemoteResult<XstatFsResponseV2>,
req_path = LayerToProxyMessage::File => FileRequest::XstatFsV2,
res_path = ProxyToLayerMessage::File => FileResponse::XstatFsV2,
);

impl_request!(
req = StatFsRequest,
res = RemoteResult<XstatFsResponse>,
req_path = LayerToProxyMessage::File => FileRequest::StatFs,
res_path = ProxyToLayerMessage::File => FileResponse::XstatFs,
);

impl_request!(
req = StatFsRequestV2,
res = RemoteResult<XstatFsResponseV2>,
req_path = LayerToProxyMessage::File => FileRequest::StatFsV2,
res_path = ProxyToLayerMessage::File => FileResponse::XstatFsV2,
);

impl_request!(
req = FdOpenDirRequest,
res = RemoteResult<OpenDirResponse>,
Expand Down
45 changes: 43 additions & 2 deletions mirrord/intproxy/src/proxies/files.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use mirrord_protocol::{
file::{
CloseDirRequest, CloseFileRequest, DirEntryInternal, ReadDirBatchRequest, ReadDirResponse,
ReadFileResponse, ReadLimitedFileRequest, SeekFromInternal, MKDIR_VERSION,
READDIR_BATCH_VERSION, READLINK_VERSION, RMDIR_VERSION, STATFS_VERSION,
READDIR_BATCH_VERSION, READLINK_VERSION, RMDIR_VERSION, STATFS_V2_VERSION, STATFS_VERSION,
},
ClientMessage, DaemonMessage, ErrorKindInternal, FileRequest, FileResponse, RemoteIOError,
ResponseError,
Expand Down Expand Up @@ -274,7 +274,7 @@ impl FilesProxy {
{
Err(FileResponse::RemoveDir(Err(ResponseError::NotImplemented)))
}
FileRequest::StatFs(..)
FileRequest::StatFs(..) | FileRequest::StatFsV2(..)
if protocol_version
.is_none_or(|version: &Version| !STATFS_VERSION.matches(version)) =>
{
Expand Down Expand Up @@ -539,6 +539,32 @@ impl FilesProxy {
.send(ClientMessage::FileRequest(FileRequest::Seek(seek)))
.await;
}
FileRequest::StatFsV2(statfs_v2)
if self
.protocol_version
.as_ref()
.is_none_or(|version| !STATFS_V2_VERSION.matches(version)) =>
{
self.request_queue.push_back(message_id, layer_id);
message_bus
.send(ClientMessage::FileRequest(FileRequest::StatFs(
statfs_v2.into(),
)))
.await;
}
FileRequest::XstatFsV2(xstatfs_v2)
if self
.protocol_version
.as_ref()
.is_none_or(|version| !STATFS_V2_VERSION.matches(version)) =>
{
self.request_queue.push_back(message_id, layer_id);
message_bus
.send(ClientMessage::FileRequest(FileRequest::XstatFs(
xstatfs_v2.into(),
)))
.await;
}

// Doesn't require any special logic.
other => {
Expand Down Expand Up @@ -742,6 +768,21 @@ impl FilesProxy {
})
.await;
}
// Convert to XstatFsV2 so that the layer doesn't ever need to deal with the old type.
FileResponse::XstatFs(res) => {
let (message_id, layer_id) = self.request_queue.pop_front().ok_or_else(|| {
UnexpectedAgentMessage(DaemonMessage::File(FileResponse::XstatFs(res.clone())))
})?;
message_bus
.send(ToLayer {
message_id,
layer_id,
message: ProxyToLayerMessage::File(FileResponse::XstatFsV2(
res.map(Into::into),
)),
})
.await;
}

// Doesn't require any special logic.
other => {
Expand Down
2 changes: 1 addition & 1 deletion mirrord/layer/src/file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use libc::{c_int, O_ACCMODE, O_APPEND, O_CREAT, O_RDONLY, O_RDWR, O_TRUNC, O_WRO
use mirrord_protocol::file::{
AccessFileRequest, CloseFileRequest, FdOpenDirRequest, OpenDirResponse, OpenOptionsInternal,
OpenRelativeFileRequest, ReadFileRequest, ReadLimitedFileRequest, SeekFileRequest,
WriteFileRequest, WriteLimitedFileRequest, XstatFsRequest, XstatRequest,
WriteFileRequest, WriteLimitedFileRequest, XstatRequest,
};
/// File operations on remote pod.
///
Expand Down
95 changes: 89 additions & 6 deletions mirrord/layer/src/file/hooks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ use libc::{
use libc::{dirent64, stat64, statx, EBADF, ENOENT, ENOTDIR};
use mirrord_layer_macro::{hook_fn, hook_guard_fn};
use mirrord_protocol::file::{
FsMetadataInternal, MetadataInternal, ReadFileResponse, ReadLinkFileResponse, WriteFileResponse,
FsMetadataInternalV2, MetadataInternal, ReadFileResponse, ReadLinkFileResponse,
WriteFileResponse,
};
#[cfg(target_os = "linux")]
use mirrord_protocol::ResponseError::{NotDirectory, NotFound};
Expand Down Expand Up @@ -736,19 +737,49 @@ unsafe extern "C" fn fill_stat(out_stat: *mut stat64, metadata: &MetadataInterna
}

/// Fills the `statfs` struct with the metadata
unsafe extern "C" fn fill_statfs(out_stat: *mut statfs, metadata: &FsMetadataInternal) {
unsafe extern "C" fn fill_statfs(out_stat: *mut statfs, metadata: &FsMetadataInternalV2) {
// Acording to linux documentation "Fields that are undefined for a particular file system are
// set to 0."
out_stat.write_bytes(0, 1);
let out = &mut *out_stat;
out.f_type = best_effort_cast(metadata.filesystem_type);
out.f_bsize = best_effort_cast(metadata.block_size);
out.f_blocks = metadata.blocks;
out.f_bfree = metadata.blocks_free;
out.f_bavail = metadata.blocks_available;
out.f_files = metadata.files;
out.f_ffree = metadata.files_free;
#[cfg(target_os = "linux")]
{
// SAFETY: fsid_t has C repr and holds just an array with two i32s.
out.f_fsid = std::mem::transmute::<[i32; 2], libc::fsid_t>(metadata.filesystem_id);
out.f_namelen = metadata.name_len;
out.f_frsize = metadata.fragment_size;
}
}

/// Fills the `statfs` struct with the metadata
#[cfg(target_os = "linux")]
unsafe extern "C" fn fill_statfs64(out_stat: *mut libc::statfs64, metadata: &FsMetadataInternalV2) {
// Acording to linux documentation "Fields that are undefined for a particular file system are
// set to 0."
out_stat.write_bytes(0, 1);
let out = &mut *out_stat;
// on macOS the types might be different, so we try to cast and do our best..
out.f_type = best_effort_cast(metadata.filesystem_type);
out.f_bsize = best_effort_cast(metadata.block_size);
out.f_blocks = metadata.blocks;
out.f_bfree = metadata.blocks_free;
out.f_bavail = metadata.blocks_available;
out.f_files = metadata.files;
out.f_ffree = metadata.files_free;
#[cfg(target_os = "linux")]
{
// SAFETY: fsid_t has C repr and holds just an array with two i32s.
out.f_fsid = std::mem::transmute::<[i32; 2], libc::fsid_t>(metadata.filesystem_id);
out.f_namelen = metadata.name_len;
out.f_frsize = metadata.fragment_size;
out.f_flags = metadata.flags;
}
}

fn stat_logic<const FOLLOW_SYMLINK: bool>(
Expand Down Expand Up @@ -917,7 +948,27 @@ unsafe extern "C" fn fstatfs_detour(fd: c_int, out_stat: *mut statfs) -> c_int {
fill_statfs(out_stat, &res);
0
})
.unwrap_or_bypass_with(|_| FN_FSTATFS(fd, out_stat))
.unwrap_or_bypass_with(|_| crate::file::hooks::FN_FSTATFS(fd, out_stat))
}

/// Hook for `libc::fstatfs64`.
#[cfg(target_os = "linux")]
#[hook_guard_fn]
pub(crate) unsafe extern "C" fn fstatfs64_detour(
fd: c_int,
out_stat: *mut libc::statfs64,
) -> c_int {
if out_stat.is_null() {
return HookError::BadPointer.into();
}

xstatfs(fd)
.map(|res| {
let res = res.metadata;
fill_statfs64(out_stat, &res);
0
})
.unwrap_or_bypass_with(|_| FN_FSTATFS64(fd, out_stat))
}

/// Hook for `libc::statfs`.
Expand All @@ -936,6 +987,26 @@ unsafe extern "C" fn statfs_detour(raw_path: *const c_char, out_stat: *mut statf
.unwrap_or_bypass_with(|_| FN_STATFS(raw_path, out_stat))
}

/// Hook for `libc::statfs`.
#[cfg(target_os = "linux")]
#[hook_guard_fn]
pub(crate) unsafe extern "C" fn statfs64_detour(
raw_path: *const c_char,
out_stat: *mut libc::statfs64,
) -> 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_statfs64(out_stat, &res);
0
})
.unwrap_or_bypass_with(|_| crate::file::hooks::FN_STATFS64(raw_path, out_stat))
}

unsafe fn realpath_logic(
source_path: *const c_char,
output_path: *mut c_char,
Expand Down Expand Up @@ -1293,6 +1364,20 @@ pub(crate) unsafe fn enable_file_hooks(hook_manager: &mut HookManager) {
#[cfg(target_os = "linux")]
{
replace!(hook_manager, "statx", statx_detour, FnStatx, FN_STATX);
replace!(
hook_manager,
"fstatfs64",
fstatfs64_detour,
FnFstatfs64,
FN_FSTATFS64
);
replace!(
hook_manager,
"statfs64",
statfs64_detour,
FnStatfs64,
FN_STATFS64
);
}

#[cfg(not(all(target_os = "macos", target_arch = "x86_64")))]
Expand Down Expand Up @@ -1342,7 +1427,6 @@ pub(crate) unsafe fn enable_file_hooks(hook_manager: &mut HookManager) {
FnFstatat,
FN_FSTATAT
);

replace!(
hook_manager,
"fstatfs",
Expand All @@ -1351,7 +1435,6 @@ pub(crate) unsafe fn enable_file_hooks(hook_manager: &mut HookManager) {
FN_FSTATFS
);
replace!(hook_manager, "statfs", statfs_detour, FnStatfs, FN_STATFS);

replace!(
hook_manager,
"fdopendir",
Expand Down
Loading
Loading