✨ feat: add pending write/delete support and dummy write FUSE ops (experimental)

[ChanTsune]

No description provided.

[gemini-code-assist]

Hello @ChanTsune, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

Summary of Changes

Hello! Gemini here, providing a summary of this pull request. This PR introduces initial, experimental support for write and delete operations within the FUSE filesystem implementation. The core idea is to track these changes in memory as 'pending' operations before potentially persisting them later. Dummy implementations for the create, write, and unlink FUSE operations have been added. These dummy implementations currently log the operation and update the in-memory pending state but do not yet modify the underlying archive file. This is a foundational step towards making the FUSE mount writable.

Highlights

• Pending Changes Tracking: A new PendingChanges struct is introduced to keep track of files that have been created, written to, or marked for deletion during the FUSE session.
• Dummy Write FUSE Operations: Experimental, non-persisting implementations for the FUSE create, write, and unlink operations have been added. These methods log their calls and update the in-memory PendingChanges state.
• In-Memory State: Changes are currently only tracked in memory within the PnaFS struct's new pending field. There is no persistence to the archive file in this PR.

Changelog

• src/file_manager.rs
  • Added an empty() method to the LoadedEntry struct (Lines 20-26) to easily create a new, empty file entry.
• src/filesystem.rs
  • Defined a new PendingChanges struct to hold maps/lists of created, written, and deleted file inodes and data (Lines 13-18).
  • Added a pending field of type PendingChanges to the PnaFS struct (Line 22).
  • Initialized the pending field with Default::default() in the PnaFS::new constructor (Line 29).
  • Implemented a dummy create FUSE operation that logs the call, generates a placeholder inode, creates a dummy File entry, and adds it to the pending.created map (Lines 178-215).
  • Implemented a dummy write FUSE operation that logs the call, retrieves or creates an entry in the pending.written map for the given inode, resizes the buffer if necessary, copies the provided data into the buffer at the specified offset, and reports the number of bytes written (Lines 217-239).
  • Implemented a dummy unlink FUSE operation that logs the call, attempts to find the file by name within the parent's children (using the manager), and if found, adds its inode to the pending.deleted list (Lines 241-252).

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

---

Filesystem mount,
Read-only, now takes count,
Writes wait in mind.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request introduces experimental support for pending writes/deletes and dummy FUSE write operations (create, write, unlink). This is a significant step towards enabling write capabilities for the PNA filesystem.

The overall structure for managing pending changes seems like a good starting point. The code is generally clear and follows Rust idioms.

Given the experimental nature, the current dummy implementations are understandable. My review focuses on potential issues that might arise as these features are further developed and hardened, particularly around correctness and robustness.

It would be beneficial to consider adding tests for these new write operations in the future, similar to how read operations are tested, to ensure their behavior as they evolve from dummy implementations.

Summary of Findings

• Inode Generation Strategy in create: The inode generation in the create FUSE operation uses arbitrary increments/fallbacks, which could lead to collisions. Addressed in a review comment.
• unlink Handling of Pending Creations: The unlink FUSE operation may not correctly handle files that have been created but not yet persisted (i.e., exist only in pending.created). Addressed in a review comment.
• Inconsistent Comment Language (Low Severity): Comments in src/filesystem.rs (lines 16, 177, 243) are in Japanese, while the rest of the codebase primarily uses English. For consistency and broader maintainability, using a single language for comments is recommended. (Not commented inline due to review settings).
• Hardcoded UID/GID in create (Low Severity): The create operation hardcodes uid: 0, gid: 0 (lines 205-206 in src/filesystem.rs). For a production system, this should ideally reflect the UID/GID of the requesting user or be configurable. (Not commented inline due to review settings, acceptable for current experimental/dummy status).

Merge Readiness

This pull request introduces valuable experimental features. However, there are a couple of medium-severity issues identified concerning inode generation in create and the handling of pending files in unlink. It's recommended that these points be discussed and ideally addressed, or at least a clear plan for their resolution be established, before merging to ensure a more robust foundation for future development.

As a reviewer, I am not authorized to approve pull requests. Please ensure further review and approval from authorized maintainers before merging.

[gemini-code-assist]

The current inode generation logic has a couple of aspects to consider for future robustness:

1. Redundant Call: The original line calls self.manager.get_file(parent) twice if the parent exists. This can be made more efficient.
2. Arbitrary Inode Assignment: The inode assignment logic (e.g., parent_ino + 10000 or fallback to 99999) seems arbitrary. This could potentially lead to inode collisions with those managed by FileManager::next_inode() or across multiple concurrent create operations, especially as this feature moves beyond a dummy implementation. This could affect filesystem integrity.

For a more robust system, have you considered centralizing inode generation? Perhaps FileManager could be responsible for allocating all new inodes, including those for pending creations, or a different unique ID strategy could be employed to prevent collisions.

The suggestion below addresses the redundant call. The primary concern for discussion is the overall inode assignment strategy.

Suggested change

	let ino = self.manager.get_file(parent).map(|_| self.manager.get_file(parent).unwrap().attr.ino + 10000).unwrap_or(99999);
	let ino = self.manager.get_file(parent).map(|pf| pf.attr.ino + 10000).unwrap_or(99999);

[gemini-code-assist]

The unlink function currently attempts to find the file to be deleted by querying self.manager.get_children(parent). This is fine for files already persisted and known to the FileManager.

However, if a file is newly created (and thus exists only in self.pending.created) and then an unlink operation is performed on it before it's persisted to the main archive structure, will this unlink correctly identify and process it? It seems it might only find files already managed by self.manager.

To handle this scenario, should unlink also check self.pending.created? If a file is found there:

• It could be removed from self.pending.created.
• Its inode could be added to self.pending.deleted to ensure it's not accidentally processed later if there's any intermediate state.

Consider a sequence like: create("newfile") -> unlink("newfile"). The unlink needs to effectively cancel out the pending creation.

How do you envision handling unlinking of files that are in a pending-creation state?