[WIP] Feat/virtual file sys

[juntao]

Hello, I am a code review bot on flows.network. Here are my reviews of code commits in this PR.

---

Overall Summary:

• The provided summaries contain potential issues and errors as well as important findings for each set of changes in the pull request.
• The potential problems identified include compatibility issues, unintended side effects, missing implementations, incorrect error codes, naming confusion, and incomplete or inconsistent changes.
• The most important findings include the lack of documentation and comments, extensive changes without clear context or purpose, lack of tests and documentation updates, and the need for further discussion and clarification.
• Suggestions for improvement include providing more context, completing the changes, adding tests and documentation, clarifying behavior and requirements, and addressing the potential issues and errors.

It is recommended to thoroughly review and address the potential problems and important findings, and to provide more context, tests, and documentation before merging the changes.

Details

Commit d6b9bda2161e238d81390d653fe63255fa0f1e38

Key changes:

• The serialize feature has been removed from the async-wasi crate.
• The io_state field of the WasiCtx struct has been removed.
• The serialize module and its associated code have been removed.
• The SerialWasiCtx, SerialSocketType, SerialConnectState, and SerialWasiSocketState structs, and their associated implementations, have been removed.

Potential problems:

• The removal of the serialize feature may cause compatibility issues with other parts of the codebase that rely on this feature. It is important to ensure that all dependencies and uses of the serialize feature have been updated accordingly.
• The removal of the io_state field may affect the functionality of the WasiCtx struct. It is important to review and validate that the removal of this field does not introduce any unintended side effects or break the existing functionality.

Commit 198fcf110435ce4c3c46a130012b68e6f4c828bc

Summary of key changes:

• The object_pool.rs file has been deleted.
• The slab crate has been added as a dependency in the Cargo.toml file.
• The object_pool module has been removed from lib.rs.
• The push, remove, empty_chunk, cleanup_stores, first_none, value_and_chunk, and get methods of the ObjectPool struct have been removed.

Potential problems:

• The object_pool.rs file has been deleted without any replacement code. This may break functionality if other parts of the codebase relied on the ObjectPool struct or its methods.
• It is unclear if the changes made in this patch are part of a larger refactoring effort or if they are intended to be standalone changes. The pull request title suggests that this is a work in progress, so further changes or additions may be expected.

Commit 901c15c8ec94bbf21e53a341b3f6d3854f0cb028

Key changes:

• Added a new file impls.rs under crates/async-wasi/src/snapshots/common/vfs/ directory.
• Added a new file virtual_sys.rs under crates/async-wasi/src/snapshots/common/vfs/ directory.
• Modified the Cargo.toml file in crates/async-wasi crate to add a new dependency on the log crate.
• Made various changes to the existing files.

Potential problems:

• The code changes are not well-documented with comments or commit messages, which makes it difficult to understand the intention behind each change.
• The code changes are quite extensive, with many insertions and deletions, making it hard to review the precise changes made in each file.
• There are commented-out code blocks in the MemoryDir and MemoryFile structs, which should be removed before merging the code.
• The mark_remove method in the MemoryDir struct is not implemented, even though it is mentioned in the trait.
• The fd_filestat_set_size method in the MemoryFile struct returns Errno::__WASI_ERRNO_NOTSUP when trying to set the file size, which is not a valid WASI error code.

Commit 971cb10fb9035ff894666b5d7089a7c167500284

Key changes in the patch include:

1. Changes to the AsyncWasiSocketInner struct, adding the accept function as an async method that uses tokio::time::timeout for a non-blocking behavior.
2. Changes to the AsyncWasiSocket::accept function, implementing a non-blocking behavior with a timeout using tokio::time::timeout.
3. Changes to the AsyncWasiSocket::recv_vectored_with_flags function, adding support for non-blocking behavior using AsyncFd.
4. Other miscellaneous changes to imports, error handling, and timeouts.

Potential problems in the patch include:

1. It seems that the author has mixed blocking and non-blocking behavior, as some functions are made async while others aren't. This inconsistency could lead to unexpected behavior or confusion.
2. The timeout values used in the non-blocking behaviors (e.g., the tokio::time::timeout durations) are quite low (50 milliseconds) and might not be suitable for all scenarios. It would be better to allow configuration of these values.
3. Error handling might not be complete, as some errors are simply logged or ignored, which could lead to hidden issues or bugs.
4. The patch could benefit from more explanatory comments to improve code readability and maintainability.

Commit 9b20217bf01c77c16c2dfaaf3f42e73de3140353

Key changes:

• Added implementation for Drop trait for MemoryDir and MemoryFile in the impls.rs file.
• Added a new function path_link_file in the WasiFileSys trait and implemented it in virtual_sys.rs file.
• Added parameters old_dir and new_dir to the path_rename function in the WasiFileSys trait and implemented it in virtual_sys.rs file.

Potential problems:

• In the path_link_file function, the old_dir and new_dir parameters are not currently used.
• In the path_unlink_file function, the code has a mix of file_ino and file variable names which can be confusing.

Additional notes:

• The patch does not contain any information about the overall context or purpose of the changes.
• The patch includes some commented out code which should be removed.
• There are no unit tests accompanying the changes.

Commit 0007434386942e5af6a7c093a9d1edac35d6f96a

Key changes in the patch:

1. In vfs/impls.rs, the code has been updated to remove the usage of if let Some(_) = ... and instead use if self.paths.remove(path).is_some().
2. In vfs/virtual_sys.rs, a Default implementation has been added for the WasiVirtualSys struct.
3. In vfs/virtual_sys.rs, the code to handle the OFlags has been refactored to improve readability.
4. In vfs/virtual_sys.rs, the code to open directories and files has been updated to remove the explicit return statements and use implicit returns.
5. In vfs/virtual_sys.rs, the code to remove directories has been updated to remove the explicit return statement and use an implicit return.
6. In env.rs, the code to handle fd_renumber has been updated to return Err(Errno::__WASI_ERRNO_NOTSUP).

Potential problems:

• It seems that the patch is a work in progress ([WIP] is mentioned in the title). This means that the changes proposed in the patch may not be complete and might require further work.
• The patch does not provide any description or context for the changes, making it difficult to understand the motivation behind the changes.
• The patch does not include any tests or documentation updates for the changes, which could affect the overall quality of the codebase.
• The code in env.rs for fd_renumber is currently returning an error indicating that the operation is not supported. It may be necessary to further investigate and determine if this behavior aligns with the requirements.

Overall, it is recommended to provide more context, complete the proposed changes, and consider adding tests and documentation. Additionally, further discussion and clarification are needed regarding the fd_renumber behavior in env.rs.

Commit 70a5f07b113eba8aedbaca67b31feef8856ef56f

Key changes:

• Added a new CI workflow for continuous integration.
• The workflow is triggered by pushes and pull requests, but ignores certain file paths.
• The workflow consists of multiple jobs, each targeting a different operating system.
• The jobs include steps to install dependencies, build and test the Rust SDK, and perform code formatting and linting.

Potential problems:

1. The patch includes a WIP (Work in Progress) tag in the title, indicating that the work is not yet complete. This should be resolved before merging the changes.
2. The patch introduces a new CI workflow, but it's unclear if the existing workflows are still needed or should be modified. It would be helpful to provide more information or context about the purpose of this new workflow.
3. The patch modifies the .github/workflows directory by adding a new file. It's important to ensure that this file follows the existing naming and formatting conventions used in the repository.
4. The patch includes changes to the GitHub Actions workflow files, but it doesn't mention any impact on the rest of the codebase. It's important to ensure that these changes don't introduce any regressions or break existing functionality.
5. The patch updates the Rust toolchain for different jobs, but doesn't provide a clear explanation for the need to use different versions of Rust. This should be clarified or documented to ensure consistent behavior across platforms.
6. The patch includes steps to download and install dependencies, such as WasmEdge and LLVM, but it doesn't specify the exact versions or sources being used. It's important to ensure that the dependencies are obtained from trusted and reliable sources.
7. The patch includes commands to run code formatting and linting tools (Rustfmt and Clippy), but it doesn't describe what actions should be taken if any issues are flagged. It would be helpful to define a policy or guide on handling linting errors or warnings.

These are the most important findings and potential areas for improvement in the given patch. It's recommended to address these concerns and seek further clarification or feedback from the author before merging the changes.

Commit c908e9e24d7fbb50031619862ad81b87255ef6b7

Key Changes:

• The async-wasi version has been updated from "0.1.0" to "0.2.0-alpha".

Potential Problems:

• There is no description in the PR or the commit message to explain why the async-wasi version is being updated.
• The new version "0.2.0-alpha" suggests that it is not a stable release. It is important to consider the potential impact of using an alpha version on the stability and functionality of the software.

Suggestions for Improvement:

• Add a description or reasoning in the PR or commit message to explain why the async-wasi version is being updated.
• Consider the implications of using an alpha version of the async-wasi crate and whether it is necessary for the current development needs. If possible, it may be advisable to use a stable version to minimize potential risks.

Commit 621698eda7d429a5ecde74c761a1ec3c671816bf

Key changes in the patch:

• Updated the trigger conditions for the CI workflow in the .github/workflows/ci-build.yml file.
• Added the branches filter to the trigger conditions for both the push and pull_request events.
• Included a specific branch filter bump/** for both events.

Potential problems:

• The patch looks good and the changes seem to be intentional.
• However, it's important to verify if the bump/** branch filter is accurate and matches the intended branching strategy for the project.
• The changes only affect the trigger conditions for the CI workflow, so no additional problems are apparent at the moment.

[apepkuss]

@L-jasmine Please check the failed CI workflows. It is required to guarantee that the Continuous integration is passed.