Skip to content

fix(storage): gate default data disk until filesystem is ready - #813

Open
mjc wants to merge 1 commit into
sipeed:mainfrom
mjc:mjc/fix-data-partition-format
Open

fix(storage): gate default data disk until filesystem is ready#813
mjc wants to merge 1 commit into
sipeed:mainfrom
mjc:mjc/fix-data-partition-format

Conversation

@mjc

@mjc mjc commented Jun 5, 2026

Copy link
Copy Markdown

Summary

Prevent NanoKVM from mounting or exporting /dev/mmcblk0p3 as the default data disk until the partition setup has actually completed.

The old first-boot flow could leave /etc/kvm.disk0 behind even if formatting or mounting failed. On later boots, that stale marker could make NanoKVM treat an unformatted or partially prepared partition as usable, then expose it through USB mass storage or the storage APIs.

This change makes the default data-partition state explicit:

  • Track interrupted formatting with /etc/kvm.disk0.formatting and retry it on the next boot.
  • Create /etc/kvm.disk0 only after a filesystem is confirmed and the partition mounts successfully.
  • Remove stale ready markers when p3 is missing, pending, or an unknown non-empty partition.
  • Gate the default p3 USB export, storage mount API, and VM virtual-disk state on the same readiness checks.
  • Preserve custom /boot/usb.disk0 backing files; only the built-in default p3 path is blocked while setup is incomplete.

Fixes #76
Fixes #242

Maintainer notes

This is intentionally scoped to the default data partition lifecycle. It avoids auto-formatting unknown non-empty partitions because they may contain user data, while still recovering empty legacy partitions that were previously marked ready.

There is overlap with #814 in S03usbdev. If that PR lands first, this branch should be rebased so the same readiness gate is applied to the refactored USB gadget setup.

中文说明

修复 NanoKVM 在数据分区文件系统准备完成前,将 /dev/mmcblk0p3 当作可用磁盘的问题。

旧的首次启动流程可能在格式化或挂载失败后留下 /etc/kvm.disk0。后续启动时,NanoKVM 可能误认为数据分区已经准备完成,并通过 USB 大容量存储、存储 API 或虚拟磁盘 API 暴露未格式化或未完整初始化的分区。

这个 PR:

  • 使用 /etc/kvm.disk0.formatting 记录中断的格式化流程,并在下次启动时重试。
  • 只有在确认文件系统存在且分区成功挂载后,才创建 /etc/kvm.disk0
  • 在分区缺失、准备中、挂载失败或属于未知非空分区时清理过期标记。
  • 让 USB 大容量存储、存储挂载 API 和虚拟磁盘状态使用相同的数据分区就绪判断。
  • 保留自定义 /boot/usb.disk0 文件,不会因为默认分区尚未准备好而阻止自定义镜像。
  • 不会自动格式化未知的非空分区,以避免破坏用户数据。

Branch-added tests

  • tools/test-s01fs-data-disk.sh runs 28 isolated shell cases. The S01fs cases exercise creation and formatting of a missing p3, retry state across delayed devices and mkfs failures, marker cleanup after mount failures, reuse of valid filesystems, cautious handling of unknown or zero-filled partitions, and the distinction between the default partition and custom backing files. Its S03 cases exercise both the readiness resolver and the generated configfs mass-storage function, including pending, missing-marker, missing-device, whitespace, and custom-image paths.
  • server/service/storage/image_test.go checks default-partition path resolution, verifies that an unready p3 request is rejected before changing configfs state, and verifies that an empty request can still clear the current image while p3 is unavailable.
  • server/service/vm/virtualdisk/state_test.go checks when the virtual disk is reported configured for default and custom backing paths, then checks that command selection mounts only an unconfigured disk and otherwise unmounts it.

make test runs the shell suite plus both Go packages.

Copilot AI review requested due to automatic review settings June 5, 2026 22:10

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds a focused test harness for the S01fs init script and refactors S01fs to support safer/conditional data partition creation, formatting, and mounting with configurable paths for easier testing.

Changes:

  • Introduces tools/test-s01fs-data-disk.sh with stubbed system utilities to validate S01fs data-partition behavior.
  • Refactors kvmapp/system/init.d/S01fs to use overridable NANOKVM_* paths and adds logic to detect empty/unreadable partitions before formatting.
  • Adds marker-file management around formatting to indicate completion state.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 9 comments.

File Description
tools/test-s01fs-data-disk.sh Adds an integration-style shell test runner with stubbed commands and assertions for S01fs.
kvmapp/system/init.d/S01fs Parameterizes device/dir paths and adds partition emptiness detection + conditional mkfs/mount/marker behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread kvmapp/system/init.d/S01fs Outdated
Comment thread kvmapp/system/init.d/S01fs Outdated
Comment thread kvmapp/system/init.d/S01fs
Comment thread tools/test-s01fs-data-disk.sh
Comment thread tools/test-s01fs-data-disk.sh Outdated
Comment thread tools/test-s01fs-data-disk.sh Outdated
Comment thread tools/test-s01fs-data-disk.sh
Comment thread tools/test-s01fs-data-disk.sh
Comment thread tools/test-s01fs-data-disk.sh
@mjc
mjc marked this pull request as draft June 11, 2026 19:30
@mjc mjc changed the title fix: retry /data partition formatting when setup marker is stale fix: data partition formatting and export readiness Jun 11, 2026
@mjc
mjc force-pushed the mjc/fix-data-partition-format branch from 407882a to dfe19fe Compare June 11, 2026 21:12
@mjc mjc changed the title fix: data partition formatting and export readiness fix: only export data partition after filesystem setup Jun 19, 2026
@mjc
mjc marked this pull request as ready for review June 19, 2026 22:13
@mjc
mjc force-pushed the mjc/fix-data-partition-format branch 2 times, most recently from 03745a4 to 1f4dfc6 Compare August 6, 2026 18:45
@mjc mjc changed the title fix: only export data partition after filesystem setup fix(storage): gate default data disk until filesystem is ready Aug 6, 2026
@mjc

mjc commented Aug 6, 2026

Copy link
Copy Markdown
Author

rebased on main

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.

File Deletion Required for Full Version Device New NanoKVM - /dev/mmcblk0p3 - no filesytem

2 participants