<fix>[vm]: filter zbs for encrypted auto ps#4464
Open
zstack-robot-2 wants to merge 1 commit into
Open
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 25 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (16)
Walkthrough新增 VmAllocatePrimaryStorageExtensionPoint 扩展点接口，接入 VmAllocateHostAndPrimaryStorageFlow 与 PrimaryStorageMainAllocatorFlow 的候选过滤链路；新增 encryptedVolumeAutoAllocation 标记字段贯穿多个消息/规格类；新增 EncryptedVolumePrimaryStorageAllocatorExtension 实现按类型剔除不支持加密卷的候选主存储，并补充单元测试。 Changes主存储分配候选过滤扩展点
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Allocator as PrimaryStorageMainAllocatorFlow/VmAllocateHostAndPrimaryStorageFlow
participant Extension as EncryptedVolumePrimaryStorageAllocatorExtension
participant DB as ExternalPrimaryStorageVO
Allocator->>Extension: filterPrimaryStorageCandidates(spec, candidates)
Extension->>Extension: 判断是否需要加密卷自动过滤
Extension->>DB: 查询 identity=expon, protocol=vhost 的主存储 UUID
DB-->>Extension: 返回不支持的 UUID 列表
Extension->>Extension: removeIf 剔除不支持的候选
Extension-->>Allocator: 返回过滤后的候选列表
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (5)
test/src/test/java/org/zstack/test/unittest/storage/encrypt/EncryptedVolumePrimaryStorageAllocatorExtensionTest.java (1)
15-36: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value建议补充
filterPrimaryStorageCandidates与shouldAllocatePrimaryStorageForTemplateDataVolume的用例。当前仅覆盖
beforeAllocatePrimaryStorage。另外两个 override 方法涉及 DB 查询分支与短路逻辑，是本次过滤行为的核心路径，建议补充测试（可考虑 mockQ）以覆盖候选过滤与模板数据卷分配判定。🤖 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/java/org/zstack/test/unittest/storage/encrypt/EncryptedVolumePrimaryStorageAllocatorExtensionTest.java` around lines 15 - 36, Current tests only cover beforeAllocatePrimaryStorage, but the core filtering logic also lives in filterPrimaryStorageCandidates and shouldAllocatePrimaryStorageForTemplateDataVolume. Add test cases for these two methods in EncryptedVolumePrimaryStorageAllocatorExtensionTest, including the DB-query branch in filterPrimaryStorageCandidates and the short-circuit behavior in shouldAllocatePrimaryStorageForTemplateDataVolume; use mocking for Q where needed so both candidate filtering and template data volume allocation decisions are verified.compute/src/main/java/org/zstack/compute/vm/VmAllocatePrimaryStorageFlow.java (1)
258-264: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff扩展点调用循环在多个 Flow 中重复。
相同的
pluginRgty.getExtensionList(VmAllocatePrimaryStorageExtensionPoint.class)遍历回调逻辑在VmAllocatePrimaryStorageFlow、VmInstantiateOtherDiskFlow（beforeAllocatePrimaryStorage）以及VmAllocateHostAndPrimaryStorageFlow（filterPrimaryStorageCandidates）中重复出现。可考虑参照 ZStack 既有的*ExtensionPointEmitter模式抽出统一的 emitter，减少重复并集中管理契约。🤖 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/VmAllocatePrimaryStorageFlow.java` around lines 258 - 264, The extension-point iteration logic for VmAllocatePrimaryStorageExtensionPoint is duplicated across multiple flows, so extract it into a shared emitter following the existing *ExtensionPointEmitter pattern. Create a centralized emitter for the pluginRgty.getExtensionList(VmAllocatePrimaryStorageExtensionPoint.class) callback sequence and use it from VmAllocatePrimaryStorageFlow.beforeAllocatePrimaryStorage, VmInstantiateOtherDiskFlow.beforeAllocatePrimaryStorage, and VmAllocateHostAndPrimaryStorageFlow.filterPrimaryStorageCandidates to keep the contract in one place and remove repeated loops.header/src/main/java/org/zstack/header/vm/VmAllocatePrimaryStorageExtensionPoint.java (1)
8-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win建议在扩展点接口上明确
spec/disk的可空契约。当前多个调用方会传入 null：
VmInstantiateOtherDiskFlow传入spec=null，而VmAllocatePrimaryStorageFlow/VmAllocateHostAndPrimaryStorageFlow在自动分配分支可能传入disk=null（getRootDisk()可能为 null）。默认实现不解引用这些参数因而安全，但后续新增的实现容易在未察觉的情况下触发 NPE。建议在 Javadoc 中说明各参数何时可能为 null，提示实现方做空值防护。🤖 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 `@header/src/main/java/org/zstack/header/vm/VmAllocatePrimaryStorageExtensionPoint.java` around lines 8 - 18, Clarify the nullable contract for the VmAllocatePrimaryStorageExtensionPoint methods so implementers know that spec and disk may be null in some call paths. Update the interface Javadoc around beforeAllocatePrimaryStorage, filterPrimaryStorageCandidates, and shouldAllocatePrimaryStorageForTemplateDataVolume to state when VmInstanceSpec or DiskAO can be absent, referencing the extension point methods directly so future implementations add null checks and avoid NPEs.storage/src/main/java/org/zstack/storage/volume/InstantiateVolumeForNewCreatedVmExtension.java (2)
307-315: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value可用 Stream
anyMatch替代显式循环。该判断逻辑适合用 Stream 表达。As per coding guidelines：“使用 Java Stream 或 Lambda 表达式代替冗长的循环与条件判断”。
♻️ 建议的写法
- private boolean shouldAllocatePrimaryStorageForTemplateDataVolume(VmInstanceSpec spec, DiskAO diskAO, String defaultPsUuid) { - for (VmAllocatePrimaryStorageExtensionPoint ext : - pluginRgty.getExtensionList(VmAllocatePrimaryStorageExtensionPoint.class)) { - if (ext.shouldAllocatePrimaryStorageForTemplateDataVolume(spec, diskAO, defaultPsUuid)) { - return true; - } - } - return false; - } + private boolean shouldAllocatePrimaryStorageForTemplateDataVolume(VmInstanceSpec spec, DiskAO diskAO, String defaultPsUuid) { + return pluginRgty.getExtensionList(VmAllocatePrimaryStorageExtensionPoint.class).stream() + .anyMatch(ext -> ext.shouldAllocatePrimaryStorageForTemplateDataVolume(spec, diskAO, defaultPsUuid)); + }🤖 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 `@storage/src/main/java/org/zstack/storage/volume/InstantiateVolumeForNewCreatedVmExtension.java` around lines 307 - 315, Replace the explicit loop in shouldAllocatePrimaryStorageForTemplateDataVolume with a Stream anyMatch chain over pluginRgty.getExtensionList(VmAllocatePrimaryStorageExtensionPoint.class), using the same predicate ext.shouldAllocatePrimaryStorageForTemplateDataVolume(spec, diskAO, defaultPsUuid) to preserve behavior while following the Stream/Lambda guideline.Source: Path instructions
276-278: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value建议简化嵌套三元表达式以提升可读性。
psUuid使用了嵌套三元，语义不直观。可改用早返回或普通条件赋值表达清楚意图。As per coding guidelines：“if 条件表达不宜过长或过于复杂，必要时可以将条件抽成 boolean 变量描述”，并尽量减少难以阅读的分支结构。♻️ 建议的写法
- boolean allocatePrimaryStorage = shouldAllocatePrimaryStorageForTemplateDataVolume(spec, diskAO, defaultPsUuid); - String psUuid = diskAO.getPrimaryStorageUuid() != null ? diskAO.getPrimaryStorageUuid() : - allocatePrimaryStorage ? null : defaultPsUuid; + boolean allocatePrimaryStorage = shouldAllocatePrimaryStorageForTemplateDataVolume(spec, diskAO, defaultPsUuid); + String psUuid; + if (diskAO.getPrimaryStorageUuid() != null) { + psUuid = diskAO.getPrimaryStorageUuid(); + } else { + psUuid = allocatePrimaryStorage ? null : defaultPsUuid; + }🤖 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 `@storage/src/main/java/org/zstack/storage/volume/InstantiateVolumeForNewCreatedVmExtension.java` around lines 276 - 278, The psUuid assignment in InstantiateVolumeForNewCreatedVmExtension uses a nested ternary that is hard to read; replace it with clearer conditional logic, such as an explicit if/else or early-return style, while preserving the same behavior based on diskAO.getPrimaryStorageUuid(), allocatePrimaryStorage, and defaultPsUuid. Keep the boolean from shouldAllocatePrimaryStorageForTemplateDataVolume() as the descriptive condition and use it to make the storage selection flow obvious.Source: Path instructions
🤖 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/VmAllocatePrimaryStorageFlow.java`:
- Around line 258-264: The extension-point iteration logic for
VmAllocatePrimaryStorageExtensionPoint is duplicated across multiple flows, so
extract it into a shared emitter following the existing *ExtensionPointEmitter
pattern. Create a centralized emitter for the
pluginRgty.getExtensionList(VmAllocatePrimaryStorageExtensionPoint.class)
callback sequence and use it from
VmAllocatePrimaryStorageFlow.beforeAllocatePrimaryStorage,
VmInstantiateOtherDiskFlow.beforeAllocatePrimaryStorage, and
VmAllocateHostAndPrimaryStorageFlow.filterPrimaryStorageCandidates to keep the
contract in one place and remove repeated loops.
In
`@header/src/main/java/org/zstack/header/vm/VmAllocatePrimaryStorageExtensionPoint.java`:
- Around line 8-18: Clarify the nullable contract for the
VmAllocatePrimaryStorageExtensionPoint methods so implementers know that spec
and disk may be null in some call paths. Update the interface Javadoc around
beforeAllocatePrimaryStorage, filterPrimaryStorageCandidates, and
shouldAllocatePrimaryStorageForTemplateDataVolume to state when VmInstanceSpec
or DiskAO can be absent, referencing the extension point methods directly so
future implementations add null checks and avoid NPEs.
In
`@storage/src/main/java/org/zstack/storage/volume/InstantiateVolumeForNewCreatedVmExtension.java`:
- Around line 307-315: Replace the explicit loop in
shouldAllocatePrimaryStorageForTemplateDataVolume with a Stream anyMatch chain
over pluginRgty.getExtensionList(VmAllocatePrimaryStorageExtensionPoint.class),
using the same predicate
ext.shouldAllocatePrimaryStorageForTemplateDataVolume(spec, diskAO,
defaultPsUuid) to preserve behavior while following the Stream/Lambda guideline.
- Around line 276-278: The psUuid assignment in
InstantiateVolumeForNewCreatedVmExtension uses a nested ternary that is hard to
read; replace it with clearer conditional logic, such as an explicit if/else or
early-return style, while preserving the same behavior based on
diskAO.getPrimaryStorageUuid(), allocatePrimaryStorage, and defaultPsUuid. Keep
the boolean from shouldAllocatePrimaryStorageForTemplateDataVolume() as the
descriptive condition and use it to make the storage selection flow obvious.
In
`@test/src/test/java/org/zstack/test/unittest/storage/encrypt/EncryptedVolumePrimaryStorageAllocatorExtensionTest.java`:
- Around line 15-36: Current tests only cover beforeAllocatePrimaryStorage, but
the core filtering logic also lives in filterPrimaryStorageCandidates and
shouldAllocatePrimaryStorageForTemplateDataVolume. Add test cases for these two
methods in EncryptedVolumePrimaryStorageAllocatorExtensionTest, including the
DB-query branch in filterPrimaryStorageCandidates and the short-circuit behavior
in shouldAllocatePrimaryStorageForTemplateDataVolume; use mocking for Q where
needed so both candidate filtering and template data volume allocation decisions
are verified.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: ecc85355-2f22-4d1d-8987-4cf43115be05
⛔ Files ignored due to path filters (1)
conf/springConfigXml/VolumeManager.xmlis excluded by!**/*.xml
📒 Files selected for processing (7)
compute/src/main/java/org/zstack/compute/vm/VmAllocateHostAndPrimaryStorageFlow.javacompute/src/main/java/org/zstack/compute/vm/VmAllocatePrimaryStorageFlow.javacompute/src/main/java/org/zstack/compute/vm/VmInstantiateOtherDiskFlow.javaheader/src/main/java/org/zstack/header/vm/VmAllocatePrimaryStorageExtensionPoint.javastorage/src/main/java/org/zstack/storage/encrypt/EncryptedVolumePrimaryStorageAllocatorExtension.javastorage/src/main/java/org/zstack/storage/volume/InstantiateVolumeForNewCreatedVmExtension.javatest/src/test/java/org/zstack/test/unittest/storage/encrypt/EncryptedVolumePrimaryStorageAllocatorExtensionTest.java
MatheMatrix
force-pushed
the
sync/zstackio/codex/filter-zhps-encrypted-auto-ps
branch
7 times, most recently
from
29fed71 to
478e59b
Compare
Owner
|
Comment from yaohua.wu: Review: MR !10426 — ZSV-12607Background (preserved across rounds)
🔴 Critical
🟡 Warning
🟢 Suggestion
Coverage
Verdict: REVISION_REQUIRED2 条 Critical 都是修复不完整路径，可能让 🤖 Robot Reviewer |
Collaborator
|
Comment from 周众: 已修复：attach 未实例化加密盘补充 encryptedVolumeAutoAllocation；模板数据盘加密且自动选择时不再继承 defaultPsUuid，改为先通过 allocator dry-run 选择非 ZHPS，再继续原创建流程。过滤逻辑仍集中在 PrimaryStorageMainAllocatorFlow 的 filterPrimaryStorageCandidates 扩展点。 |
MatheMatrix
force-pushed
the
sync/zstackio/codex/filter-zhps-encrypted-auto-ps
branch
5 times, most recently
from
2baeca6 to
0de0ff1
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
compute/src/main/java/org/zstack/compute/vm/VmAllocatePrimaryStorageFlow.java (1)
149-149: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win加密自动分配判断逻辑在多个文件中重复。
Boolean.TRUE.equals(x.getEncrypted()) && x.getPrimaryStorageUuid() == null这一表达式在本文件（根卷/数据卷）、VmAllocatePrimaryStorageForAttachingDiskFlow、VmInstantiateOtherDiskFlow（两处）共出现 5 次。本 PR 的迭代历史本身就说明了这种重复的风险——review 过程中曾遗漏了附加磁盘与模板数据卷两处，需要额外补丁修复。建议抽取为统一的静态工具方法（例如EncryptedVolumeAllocationUtils.isEncryptedAutoAllocation(Boolean encrypted, String primaryStorageUuid)），避免未来新增分配入口再次遗漏。♻️ 建议的重构方向
+public class EncryptedVolumeAllocationUtils { + public static boolean isEncryptedAutoAllocation(Boolean encrypted, String primaryStorageUuid) { + return Boolean.TRUE.equals(encrypted) && primaryStorageUuid == null; + } +}- rmsg.setEncryptedVolumeAutoAllocation(disk != null && Boolean.TRUE.equals(disk.getEncrypted())); + rmsg.setEncryptedVolumeAutoAllocation(disk != null + && EncryptedVolumeAllocationUtils.isEncryptedAutoAllocation(disk.getEncrypted(), disk.getPrimaryStorageUuid()));As per path instructions（
## 6. 设计原则和模块划分中的"单一职责原则"），应避免重复逻辑分散在多处、造成后续维护和一致性风险。Also applies to: 184-186
🤖 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/VmAllocatePrimaryStorageFlow.java` at line 149, The encrypted auto-allocation check is duplicated across multiple VM primary-storage flows, so refactor it into one shared helper and reuse it everywhere. Extract the repeated Boolean.TRUE.equals(encrypted) && primaryStorageUuid == null logic into a static utility such as EncryptedVolumeAllocationUtils.isEncryptedAutoAllocation, then update VmAllocatePrimaryStorageFlow, VmAllocatePrimaryStorageForAttachingDiskFlow, and VmInstantiateOtherDiskFlow to call that helper instead of inlining the condition. Keep the existing behavior identical while centralizing the rule to prevent future misses in new allocation paths.Source: Path instructions
test/src/test/java/org/zstack/test/unittest/storage/encrypt/EncryptedVolumePrimaryStorageAllocatorExtensionTest.java (1)
16-92: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win缺少"加密根卷 + 自动分配主存储"场景的用例。
现有 4 个测试仅覆盖了
PrimaryStorageAllocationSpec直接标记及VmInstanceSpec中非根盘（DiskAO.nonRootDisk()+ 模板盘）两类场景，EncryptedVolumePrimaryStorageAllocatorExtension.isEncryptedAutoAllocatedRootVolume（根卷加密且getCandidatePrimaryStorageUuidsForRootVolume()为空）这条分支未被任何用例触发。考虑到根卷是加密自动分配最常见的入口之一，建议补充一个设置spec.getRootDisk()为加密盘、且未指定候选主存储的用例，以覆盖该分支并验证getCandidatePrimaryStorageUuidsForRootVolume()默认值的空安全性。✅ 建议补充的测试用例示例
`@Test` public void testFilterZhpsForEncryptedRootDiskWithAutoPrimaryStorage() { VmInstanceSpec spec = new VmInstanceSpec(); DiskAO rootDisk = DiskAO.rootDisk(); // 视实际工厂方法调整 rootDisk.setEncrypted(true); spec.setRootDisk(rootDisk); List<PrimaryStorageVO> candidates = new ArrayList<>(Arrays.asList(primaryStorage("normal-ps-uuid"), primaryStorage(ZHPS_PS_UUID))); extension.filterPrimaryStorageCandidates(spec, candidates); Assert.assertEquals(1, candidates.size()); Assert.assertEquals("normal-ps-uuid", candidates.get(0).getUuid()); }🤖 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/java/org/zstack/test/unittest/storage/encrypt/EncryptedVolumePrimaryStorageAllocatorExtensionTest.java` around lines 16 - 92, Add a test for the missing “encrypted root volume + auto primary storage” path in EncryptedVolumePrimaryStorageAllocatorExtensionTest. Introduce a case using VmInstanceSpec with an encrypted root disk via DiskAO.rootDisk() (or the equivalent root-disk factory in this test) and no explicit candidate primary storage, then call filterPrimaryStorageCandidates and assert ZHPS_PS_UUID is filtered out. This should directly exercise EncryptedVolumePrimaryStorageAllocatorExtension.isEncryptedAutoAllocatedRootVolume and verify getCandidatePrimaryStorageUuidsForRootVolume() is safely handled when unset.
🤖 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.
Inline comments:
In
`@storage/src/main/java/org/zstack/storage/encrypt/EncryptedVolumePrimaryStorageAllocatorExtension.java`:
- Around line 56-65: `hasEncryptedAutoAllocatedVolume(VmInstanceSpec)` is using
the VM-wide `candidatePrimaryStorageUuidsForDataVolume` field inside a per-disk
check, which can incorrectly skip ZBS filtering in multi-disk cases. Refactor
the logic in `hasEncryptedAutoAllocatedVolume` so each deprecated disk is
evaluated independently: keep checking `isEncrypted(disk)` and
`disk.getPrimaryStorageUuid() == null`, but move the candidate-primary-storage
condition to disk-specific handling instead of relying on
`spec.getCandidatePrimaryStorageUuidsForDataVolume()` globally. Ensure the
helper still correctly detects auto-allocated encrypted root volumes via
`isEncryptedAutoAllocatedRootVolume` and only lets disks through when that
specific disk’s primary-storage selection warrants it.
---
Nitpick comments:
In
`@compute/src/main/java/org/zstack/compute/vm/VmAllocatePrimaryStorageFlow.java`:
- Line 149: The encrypted auto-allocation check is duplicated across multiple VM
primary-storage flows, so refactor it into one shared helper and reuse it
everywhere. Extract the repeated Boolean.TRUE.equals(encrypted) &&
primaryStorageUuid == null logic into a static utility such as
EncryptedVolumeAllocationUtils.isEncryptedAutoAllocation, then update
VmAllocatePrimaryStorageFlow, VmAllocatePrimaryStorageForAttachingDiskFlow, and
VmInstantiateOtherDiskFlow to call that helper instead of inlining the
condition. Keep the existing behavior identical while centralizing the rule to
prevent future misses in new allocation paths.
In
`@test/src/test/java/org/zstack/test/unittest/storage/encrypt/EncryptedVolumePrimaryStorageAllocatorExtensionTest.java`:
- Around line 16-92: Add a test for the missing “encrypted root volume + auto
primary storage” path in EncryptedVolumePrimaryStorageAllocatorExtensionTest.
Introduce a case using VmInstanceSpec with an encrypted root disk via
DiskAO.rootDisk() (or the equivalent root-disk factory in this test) and no
explicit candidate primary storage, then call filterPrimaryStorageCandidates and
assert ZHPS_PS_UUID is filtered out. This should directly exercise
EncryptedVolumePrimaryStorageAllocatorExtension.isEncryptedAutoAllocatedRootVolume
and verify getCandidatePrimaryStorageUuidsForRootVolume() is safely handled when
unset.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 052ff466-0332-4f2a-b612-3063562652a8
⛔ Files ignored due to path filters (1)
conf/springConfigXml/VolumeManager.xmlis excluded by!**/*.xml
📒 Files selected for processing (11)
compute/src/main/java/org/zstack/compute/vm/VmAllocateHostAndPrimaryStorageFlow.javacompute/src/main/java/org/zstack/compute/vm/VmAllocatePrimaryStorageFlow.javacompute/src/main/java/org/zstack/compute/vm/VmAllocatePrimaryStorageForAttachingDiskFlow.javacompute/src/main/java/org/zstack/compute/vm/VmInstantiateOtherDiskFlow.javaheader/src/main/java/org/zstack/header/storage/primary/AllocatePrimaryStorageMsg.javaheader/src/main/java/org/zstack/header/storage/primary/PrimaryStorageAllocationSpec.javaheader/src/main/java/org/zstack/header/vm/VmAllocatePrimaryStorageExtensionPoint.javastorage/src/main/java/org/zstack/storage/encrypt/EncryptedVolumePrimaryStorageAllocatorExtension.javastorage/src/main/java/org/zstack/storage/primary/PrimaryStorageMainAllocatorFlow.javastorage/src/main/java/org/zstack/storage/primary/PrimaryStorageManagerImpl.javatest/src/test/java/org/zstack/test/unittest/storage/encrypt/EncryptedVolumePrimaryStorageAllocatorExtensionTest.java
MatheMatrix
force-pushed
the
sync/zstackio/codex/filter-zhps-encrypted-auto-ps
branch
2 times, most recently
from
dd70533 to
0af5fe5
Compare
Owner
|
Comment from yaohua.wu: Review: MR !10426 — ZSV-12607Background
🔴 Critical
Coverage
Verdict: REVISION_REQUIRED当前实现会在混合 root/data 场景中过度过滤 ZHPS，导致合法的非加密卷放置被拒绝，需要按卷粒度修正后再合入。 🤖 Robot Reviewer |
MatheMatrix
force-pushed
the
sync/zstackio/codex/filter-zhps-encrypted-auto-ps
branch
10 times, most recently
from
2a98582 to
62968b9
Compare
Filter zhps in the allocator extension point. Only applies to encrypted auto primary storage allocation. Test: git diff --check Test: cbok zsv compile --no-deploy passed Resolves: ZSV-12607 Change-Id: If4078641c7a42c67552373fcc9d7581cf4f97b1a
MatheMatrix
force-pushed
the
sync/zstackio/codex/filter-zhps-encrypted-auto-ps
branch
from
62968b9 to
6146b1c
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.
Summary
Filter ZBS primary storage from encrypted VM disk automatic primary storage allocation.
Changes
Testing
sync from gitlab !10426