Skip to content

[slayerfs]: fix redis bugs from ivanbeethoven#609

Open
marshawcoco wants to merge 1 commit intork8s-dev:mainfrom
marshawcoco:fix/ivanbeethoven-changes-cleanup
Open

[slayerfs]: fix redis bugs from ivanbeethoven#609
marshawcoco wants to merge 1 commit intork8s-dev:mainfrom
marshawcoco:fix/ivanbeethoven-changes-cleanup

Conversation

@marshawcoco
Copy link
Copy Markdown
Contributor

Testing

  • cargo fmt
  • cargo clippy

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c0dca310d0

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +216 to +220
self.truncate_inode(dst_ino as i64, 0)
.await
.map_err(Into::<Errno>::into)?;
self.copy_file_range_inodes(src_ino, 0, dst_ino as i64, 0, src_attr.size)
.await
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Guard against self-clone before truncating destination

Detect and reject FICLONE when src_fd resolves to the same inode as dst_ino before calling truncate_inode. In the current flow, truncate_inode(dst_ino, 0) runs first, so ioctl(fd, FICLONE, fd) (or two fds to the same file) truncates the source and destination together, then copies zero bytes and finally extends back to the old size, which silently corrupts the file with zero-filled data.

Useful? React with 👍 / 👎.

Comment on lines +198 to +201
for start in starts {
let candidate = &host_path[start..];
if let Ok(ino) = self.lookup_path_to_ino(candidate).await {
return Ok(ino);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Resolve source inode without suffix-based path probing

Avoid selecting the source inode by iterating every slash-aligned suffix of /proc/<pid>/fd/<fd> and returning the first path that exists. This can bind to the wrong file when mountpoint path components also exist inside the mounted filesystem (for example, probing can match /mnt/fs/... before the intended /...), causing clone operations to copy data from an unrelated inode.

Useful? React with 👍 / 👎.

@yyjeqhc
Copy link
Copy Markdown
Collaborator

yyjeqhc commented Apr 28, 2026

Please check — some files should not have been committed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants