fix[vm]: reject Legacy-BIOS vm on ZBS-vhost root volume (ZSTAC-86547)#4476
Open
ZStack-Robot wants to merge 1 commit into
Open
fix[vm]: reject Legacy-BIOS vm on ZBS-vhost root volume (ZSTAC-86547)#4476ZStack-Robot wants to merge 1 commit into
ZStack-Robot wants to merge 1 commit into
Conversation
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (11)
Warning
|
| Layer / File(s) | Summary |
|---|---|
错误码常量定义 utils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java |
新增 ORG_ZSTACK_COMPUTE_VM_10335、ORG_ZSTACK_COMPUTE_VM_10336 两个错误码字符串常量。 |
拦截器校验逻辑实现 compute/src/main/java/org/zstack/compute/vm/VmInstanceApiInterceptor.java |
新增 ExternalPrimaryStorageVO 相关导入；在启动路径新增 validateZbsVhostLegacyBootOnStart，创建路径新增 validateZbsVhostLegacyBootOnCreate 及辅助方法 isZbsVhostPrimaryStorage、bootModeFromSystemTags、isUefiBootMode，用于检测ZBS-vhost + 非UEFI bootMode场景并抛出拦截异常。 |
集成测试用例 test/src/test/groovy/.../ZbsVhostLegacyBootInterceptCase.groovy, test/src/test/groovy/.../ZbsVhostVolumeCase.groovy |
新增测试类构建ZBS-vhost测试环境，验证legacy根卷创建被拒绝、UEFI根卷不被拒绝；为已有用例补充 bootMode::UEFI 系统标签。 |
Estimated code review effort: 3 (Moderate) | ~25 minutes
Sequence Diagram(s)
sequenceDiagram
participant Client
participant VmInstanceApiInterceptor
participant PrimaryStorageDB
Client->>VmInstanceApiInterceptor: APICreateVmInstanceMsg / APIStartVmInstanceMsg
VmInstanceApiInterceptor->>PrimaryStorageDB: 查询root volume所在primary storage
PrimaryStorageDB-->>VmInstanceApiInterceptor: 返回ExternalPrimaryStorageVO(identity=zbs)
VmInstanceApiInterceptor->>VmInstanceApiInterceptor: bootModeFromSystemTags解析bootMode
alt 非UEFI且为ZBS-vhost
VmInstanceApiInterceptor-->>Client: 抛出拦截异常(ORG_ZSTACK_COMPUTE_VM_10335)
else UEFI或非ZBS-vhost
VmInstanceApiInterceptor-->>Client: 校验通过
end
Poem
兔子敲键盘，UEFI要设好，
ZBS-vhost路上，Legacy莫乱跑，
一条错误码，拦住旧引导，
测试来把关，代码更周到！🐇✨
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | 标题简洁且准确概括了本次修复：拒绝 Legacy-BIOS VM 使用 ZBS-vhost 根卷。 |
| Description check | ✅ Passed | 描述与变更内容一致，清楚说明了问题、修复方案、限制和测试。 |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
✨ Finishing Touches
📝 Generate docstrings
- Create stacked PR
- Commit on current branch
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
sync/jin.ma/fix/ZSTAC-86547
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
compute/src/main/java/org/zstack/compute/vm/VmInstanceApiInterceptor.java (1)
619-642: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value两处拦截逻辑高度相似，可提取公共校验方法
validateZbsVhostLegacyBootOnStart与validateZbsVhostLegacyBootOnCreate均遵循「取 psUuid → 判断 ZBS-vhost → 判断 bootMode → 抛出拦截异常」的相同流程，仅错误码与错误信息文案略有差异。可以抽取一个私有辅助方法（接收 psUuid、bootMode、errorCode 三个参数）以消除重复逻辑。Also applies to: 1283-1294
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@compute/src/main/java/org/zstack/compute/vm/VmInstanceApiInterceptor.java` around lines 619 - 642, The start-time and create-time ZBS-vhost legacy boot checks in validateZbsVhostLegacyBootOnStart and validateZbsVhostLegacyBootOnCreate duplicate the same psUuid lookup, ZBS-vhost detection, bootMode check, and exception flow. Extract that shared logic into a private helper that takes the primary storage UUID, boot mode, and error code/message parameters, and have both methods delegate to it while keeping their specific validation source and error text intact.test/src/test/groovy/org/zstack/test/integration/storage/primary/addon/zbs/ZbsVhostLegacyBootInterceptCase.groovy (1)
143-156: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win断言偏弱，无法确认 UEFI 创建真正成功
当前断言
result.error == null || !contains(ORG_ZSTACK_COMPUTE_VM_10335)只要失败原因不是该拦截错误码即视为通过，无法验证 UEFI + ZBS-vhost 根卷创建确实成功。若创建流程因其他原因失败（例如 stub/环境配置问题），该测试仍会通过，掩盖了真实回归。建议直接断言result.error == null，必要时补充资源清理（destroyVmInstance/expungeVmInstance）以与用例末尾环境保持一致。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/src/test/groovy/org/zstack/test/integration/storage/primary/addon/zbs/ZbsVhostLegacyBootInterceptCase.groovy` around lines 143 - 156, The test assertion in testUefiRootOnZbsVhostNotRejectedByInterceptor is too weak because it passes on any failure that is not ORG_ZSTACK_COMPUTE_VM_10335. Tighten the check by asserting that CreateVmInstanceAction.call() returns no error at all, so the UEFI + ZBS-vhost root-volume creation is verified as truly successful. If the test leaves a VM behind, add cleanup using the existing VM lifecycle helpers such as destroyVmInstance or expungeVmInstance to keep the test environment consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@compute/src/main/java/org/zstack/compute/vm/VmInstanceApiInterceptor.java`:
- Around line 619-642: The start-time and create-time ZBS-vhost legacy boot
checks in validateZbsVhostLegacyBootOnStart and
validateZbsVhostLegacyBootOnCreate duplicate the same psUuid lookup, ZBS-vhost
detection, bootMode check, and exception flow. Extract that shared logic into a
private helper that takes the primary storage UUID, boot mode, and error
code/message parameters, and have both methods delegate to it while keeping
their specific validation source and error text intact.
In
`@test/src/test/groovy/org/zstack/test/integration/storage/primary/addon/zbs/ZbsVhostLegacyBootInterceptCase.groovy`:
- Around line 143-156: The test assertion in
testUefiRootOnZbsVhostNotRejectedByInterceptor is too weak because it passes on
any failure that is not ORG_ZSTACK_COMPUTE_VM_10335. Tighten the check by
asserting that CreateVmInstanceAction.call() returns no error at all, so the
UEFI + ZBS-vhost root-volume creation is verified as truly successful. If the
test leaves a VM behind, add cleanup using the existing VM lifecycle helpers
such as destroyVmInstance or expungeVmInstance to keep the test environment
consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b7e7259f-6e0e-4e78-8662-fd025a3252e5
📒 Files selected for processing (4)
compute/src/main/java/org/zstack/compute/vm/VmInstanceApiInterceptor.javatest/src/test/groovy/org/zstack/test/integration/storage/primary/addon/zbs/ZbsVhostLegacyBootInterceptCase.groovytest/src/test/groovy/org/zstack/test/integration/storage/primary/addon/zbs/ZbsVhostVolumeCase.groovyutils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java
MatheMatrix
force-pushed
the
sync/jin.ma/fix/ZSTAC-86547
branch
from
c9567cd to
eb1af7c
Compare
Owner
|
Comment from jin.ma: 已按评审意见重做：废弃 |
MatheMatrix
force-pushed
the
sync/jin.ma/fix/ZSTAC-86547
branch
2 times, most recently
from
754d731 to
8972503
Compare
A vm created with a ZBS-vhost root volume reached Running but SeaBIOS reported "No bootable device", because the ZBS-vhost bdev exposes a 4096-byte logical block size on which traditional 512-sector BIOS/MBR (Legacy, non-UEFI) images cannot boot. Replace the hardcoded VmInstanceApiInterceptor guard with a layered allocator filter reusing the existing requiredFeatures pipeline: - StorageCapabilities gains minLogicalSectorSize (default 512); the ZBS controller reports 4096 when its default protocol is Vhost. - Add PrimaryStorageFeature.LEGACY_BOOT. VmAllocatePrimaryStorageFlow detects a Legacy-boot image (via image bootMode system tag) and sets the feature on the allocate msg. - ExternalPrimaryStorageFactory filters out candidates whose reported minLogicalSectorSize exceeds 512 when LEGACY_BOOT is required, so a Legacy image can no longer land on a ZBS-vhost primary storage. Resolves: ZSTAC-86547 Change-Id: If64d639597498884a084108ad76f664b923e158f
MatheMatrix
force-pushed
the
sync/jin.ma/fix/ZSTAC-86547
branch
from
8972503 to
e2d78b8
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
问题（ZSTAC-86547）
用 ZBS-vhost 根盘创建的 VM 状态 Running，但控制台 SeaBIOS 报
No bootable device，无法进入系统。根因：ZBS-vhost 的 bdev 暴露 4096 字节 logical block size，传统 512-sector BIOS/MBR（Legacy，非 UEFI）镜像在 logical=4096 的启动路径下无法引导。这是 Jira 分析里给出的两条修复方向中的兜底拦截方案（另一条根治方向是 agent 侧 bdev_zbs 实现 512e，工作量大，本 MR 先止血）。
修复
在
VmInstanceApiInterceptor两处拦截 Legacy + ZBS-vhost 根盘的 VM：APICreateVmInstanceMsg）：当显式指定根盘主存储为 ZBS-vhost（identity=zbs 且输出协议 Vhost）且 bootMode 非 UEFI 时，抛ApiMessageInterceptionException（error codeORG_ZSTACK_COMPUTE_VM_10335）提前拦截。APIStartVmInstanceMsg）：VM 已实例化、根盘主存储已确定时，通过 vmUuid → rootVolume → primaryStorage 判定，同样拦截（error code10336）。bootMode 解析口径：VM 的
bootMode::systemTag，无 tag 即视为 Legacy（与KVMHost默认口径一致）。局限（重要）
创建拦截仅覆盖用户显式指定根盘主存储的场景（
primaryStorageUuidForRootVolume非空）。未指定时根盘 PS 由 allocator 调度期决定，创建拦截阶段无法可靠判断——这类场景由启动拦截兜底（此时 PS 已确定）。测试
ZbsVhostLegacyBootInterceptCase：Legacy + ZBS-vhost 根盘创建被拒（断言 error code10335）；UEFI tag 放行不被拦。本地实跑Tests run: 1, Failures: 0。ZbsVhostVolumeCase.testVhostVmStartActivationChain建的是 Legacy vhost 根盘 VM 并期望 Running，会被本拦截误伤——已将其 bootMode 改为 UEFI（该用例本意是验证 vhost 启动链，与 bootMode 无关）。本地实跑Tests run: 1, Failures: 0。Resolves: ZSTAC-86547
sync from gitlab !10437