Skip to content

<fix>[pci]: split virt mode from capability lifecycle#3568

Open
MatheMatrix wants to merge 1 commit intofeature-5.5.12-dgpufrom
sync/xinhao.huang/ZSTAC-83477@@2
Open

<fix>[pci]: split virt mode from capability lifecycle#3568
MatheMatrix wants to merge 1 commit intofeature-5.5.12-dgpufrom
sync/xinhao.huang/ZSTAC-83477@@2

Conversation

@MatheMatrix
Copy link
Owner

Add virt mode and capability persistence to the PCI
virtualization schema upgrade for the new lifecycle
model.

Resolves: ZSTAC-83477
Change-Id: I834770000000000000000000000000000000003

sync from gitlab !9426

@coderabbitai
Copy link

coderabbitai bot commented Mar 23, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

新增 PciDeviceVO.virtStatePciDeviceVO.virtMode 字段,创建 PciDeviceVirtCapabilityVO 表,并基于现有 PciDeviceVO.virtStatus 用 CASE 与多条 INSERT IGNORE ... SELECT 回填字段与派生虚拟化能力行。

Changes

Cohort / File(s) Summary
数据库迁移脚本
conf/db/upgrade/V5.5.12__schema.sql
添加列 PciDeviceVO.virtState VARCHAR(32)PciDeviceVO.virtMode VARCHAR(32);新增表 PciDeviceVirtCapabilityVOid 自增 PK、UNIQUE(pciDeviceUuid,capability)、索引 pciDeviceUuid、外键 pciDeviceUuidPciDeviceVO(uuid) ON DELETE CASCADE);使用 CASE 映射并通过多条 INSERT IGNORE ... SELECTvirtStatus 回填 virtState/virtMode 并插入能力行(SRIOVVFIO_MDEVTENSORFUSIONHAMI)。

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 分钟

Poem

🐰 我在草丛里数表格,
新列悄悄钻进旧字段,
能力一排排进库,
映射轻声把状态换装,
小胡萝卜为迁移鼓掌。

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title follows the required [scope]: format with 53 characters, well under the 72-character limit, and clearly describes the schema changes.
Description check ✅ Passed The description clearly relates to the changeset by mentioning PCI virtualization schema upgrade, virt mode, capability persistence, and the lifecycle model.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch sync/xinhao.huang/ZSTAC-83477@@2

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@conf/db/upgrade/V5.5.12__schema.sql`:
- Around line 76-83: The UPDATE on table PciDeviceVO sets virtMode based solely
on virtStatus and will overwrite already-migrated virtMode to NULL on re-run;
modify the migration to be idempotent by restricting updates to rows that still
have old virtStatus values or where virtMode IS NULL (e.g., only update when
virtMode IS NULL AND virtStatus IN (old enums) or virtStatus IN (old enums)
regardless), and also guard the script with information_schema checks for the
existence of the PciDeviceVO table/columns and any constraints referenced so the
migration safely no-ops if schema has already been changed; locate references to
PciDeviceVO, virtMode, and virtStatus in the diff to apply these conditional
guards.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: http://open.zstack.ai:20001/code-reviews/zstack-cloud.yaml (via .coderabbit.yaml)

Review profile: CHILL

Plan: Pro

Run ID: 010aec45-92fa-4666-a68c-b779e39be1ce

📥 Commits

Reviewing files that changed from the base of the PR and between f07267a and 8ed8b1f.

⛔ Files ignored due to path filters (4)
  • sdk/src/main/java/SourceClassMap.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/PciDeviceInventory.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/PciDeviceVirtMode.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/PciDeviceVirtStatus.java is excluded by !sdk/**
📒 Files selected for processing (1)
  • conf/db/upgrade/V5.5.12__schema.sql

@MatheMatrix MatheMatrix force-pushed the sync/xinhao.huang/ZSTAC-83477@@2 branch from 1741ac0 to 3dd5ebd Compare March 24, 2026 08:57
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
conf/db/upgrade/V5.5.12__schema.sql (1)

89-99: 可选:合并 virtMode 的重复分支以降低维护成本

SRIOV 的两个分支可合并,语义不变但更易读。

♻️ 可选精简改法
 UPDATE `zstack`.`PciDeviceVO`
 SET `virtMode` =
     CASE
-        WHEN `virtStatus` IN ('SRIOV_VIRTUALIZED') THEN 'SRIOV'
-        WHEN `virtStatus` = 'SRIOV_VIRTUAL' THEN 'SRIOV'
+        WHEN `virtStatus` IN ('SRIOV_VIRTUALIZED', 'SRIOV_VIRTUAL') THEN 'SRIOV'
         WHEN `virtStatus` IN ('VFIO_MDEV_VIRTUALIZED', 'VIRTUALIZED_BYPASS_ZSTACK') THEN 'VFIO_MDEV'
         WHEN `virtStatus` = 'TENSORFUSION_VIRTUALIZED' THEN 'TENSORFUSION'
         WHEN `virtStatus` = 'HAMI_VIRTUALIZED' THEN 'HAMI'
         ELSE NULL
     END
 WHERE `virtMode` IS NULL;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@conf/db/upgrade/V5.5.12__schema.sql` around lines 89 - 99, The UPDATE on
table PciDeviceVO sets virtMode based on virtStatus and currently has two
separate WHEN branches for SRIOV; simplify by merging those two SRIOV branches
into a single WHEN clause (e.g., WHEN `virtStatus` IN
('SRIOV_VIRTUALIZED','SRIOV_VIRTUAL') THEN 'SRIOV') in the CASE expression that
assigns `virtMode` (the UPDATE ... SET `virtMode` ... WHERE `virtMode` IS NULL
block) to reduce duplication and improve readability.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@conf/db/upgrade/V5.5.12__schema.sql`:
- Around line 89-99: The UPDATE on table PciDeviceVO sets virtMode based on
virtStatus and currently has two separate WHEN branches for SRIOV; simplify by
merging those two SRIOV branches into a single WHEN clause (e.g., WHEN
`virtStatus` IN ('SRIOV_VIRTUALIZED','SRIOV_VIRTUAL') THEN 'SRIOV') in the CASE
expression that assigns `virtMode` (the UPDATE ... SET `virtMode` ... WHERE
`virtMode` IS NULL block) to reduce duplication and improve readability.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: http://open.zstack.ai:20001/code-reviews/zstack-cloud.yaml (via .coderabbit.yaml)

Review profile: CHILL

Plan: Pro

Run ID: 75f5bb8b-66c6-4f31-9e49-ec38c414f029

📥 Commits

Reviewing files that changed from the base of the PR and between 1741ac0 and 3dd5ebd.

⛔ Files ignored due to path filters (3)
  • sdk/src/main/java/org/zstack/sdk/PciDeviceInventory.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/PciDeviceVirtMode.java is excluded by !sdk/**
  • sdk/src/main/java/org/zstack/sdk/PciDeviceVirtState.java is excluded by !sdk/**
📒 Files selected for processing (1)
  • conf/db/upgrade/V5.5.12__schema.sql

@MatheMatrix MatheMatrix force-pushed the sync/xinhao.huang/ZSTAC-83477@@2 branch 4 times, most recently from 676bf36 to 1cc34b2 Compare March 26, 2026 03:27
…scheduling

DBImpact

Introduce pci virtualization metadata with virtState, virtMode and persisted virtCapabilities.
Backfill existing data in V5.5.12 and expose the new fields through SDK inventories.
Normalize capability/mode derivation during PCI sync, keep upgrade retry idempotent, and avoid stale or inconsistent virtMode capability states.
Use the new metadata in dGPU capability filtering/allocation flows and add coverage for PCI sync and dGPU cases.

Resolves: ZSTAC-83477

Change-Id: I71736e666272737978676a74656d767a6a786c72
@MatheMatrix MatheMatrix force-pushed the sync/xinhao.huang/ZSTAC-83477@@2 branch from 1cc34b2 to 1a54a97 Compare March 26, 2026 05:17
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.

1 participant