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

examples/module: allow module on hostfs #2972

Merged
merged 1 commit into from
Jan 23, 2025

Conversation

yf13
Copy link
Contributor

@yf13 yf13 commented Jan 23, 2025

Summary

This allows loading kernel modules from hostfs file system.

Impact

None

Testing

  • local checks with FLAT mode NuttX on qemu-armv7a
  • CI checks

This allows loading the kernel module from a hostfs file system.

Signed-off-by: Yanfeng Liu <p-liuyanfeng9@xiaomi.com>
@nuttxpr
Copy link

nuttxpr commented Jan 23, 2025

[Experimental Bot, please feedback here]

No, this PR does not fully meet the NuttX requirements. While it provides a summary, it lacks crucial details. Here's a breakdown of the missing information:

  • Summary: Needs more detail. Why is loading kernel modules from hostfs necessary? What problem does it solve or what functionality does it enable? Which part of the kernel is changed (e.g., module loader, VFS)? How does this change work technically? Are there any related NuttX issues?

  • Impact: "None" is highly unlikely. At a minimum, this impacts the build system (since hostfs needs to be enabled), potentially impacts hardware (if hostfs driver is specific), and definitely impacts documentation (this is a new feature and needs to be documented). Security and compatibility implications should also be considered. This section needs thorough review and realistic answers.

  • Testing: While local testing is mentioned, the logs are missing. CI checks are insufficient without demonstrating the actual change in behavior. The target configuration (board:config) is vague. More specific information on the target and build host is needed (e.g., "Build Host: Linux (Ubuntu 20.04), x86_64, GCC 9.4.0. Target: qemu-armv7a, sim:nsh"). Critically, the logs showing the module loading from hostfs are absent. Before/after logs should clearly demonstrate the new functionality working.

In short, the PR needs significantly more detail in every section to meet the requirements. The current description is too superficial to allow for proper review.

@xiaoxiang781216 xiaoxiang781216 merged commit 048b3e6 into apache:master Jan 23, 2025
37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants