Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,8 @@ public interface MaxDataVolumeNumberExtensionPoint {
String getHypervisorTypeForMaxDataVolumeNumberExtension();

int getMaxDataVolumeNumber();

default int getMaxDataVolumeNumberForRunningVm() {
return getMaxDataVolumeNumber();
}
}
5 changes: 5 additions & 0 deletions plugin/kvm/src/main/java/org/zstack/kvm/KVMConstant.java
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,11 @@ public interface KVMConstant {

Integer DEFAULT_MAX_NIC_QUEUE_NUMBER = 12;

// Maximum data volumes for hot-plug (running VM) due to PCI bus slot constraints.
// i440fx provides 32 PCI slots; ~12 are reserved by system devices and libvirt PCI bridge,
// leaving ~20 slots for hot-plugged virtio-blk data volumes (SharedBlock storage scenario).
int KVM_MAX_DATA_VOLUME_NUMBER_HOT_PLUG = 20;

Comment on lines +163 to +167
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

KVM_MAX_DATA_VOLUME_NUMBER_HOT_PLUG 可能有一位偏差,无法拦截第 20 块热插场景

当前上限设为 20,而 VolumeBase.vmAttachVolumeValidator 的判断是 vmDataVolumeUsage + 1 > maxDataVolumeNum。这会放行第 20 次挂载请求,和“第 20 次触发 libvirt PCI 槽位不足”的问题描述不一致,可能导致修复不生效。建议将上限调整为 19(或同步调整判断语义)。

🛠 建议修复
-    // leaving ~20 slots for hot-plugged virtio-blk data volumes (SharedBlock storage scenario).
-    int KVM_MAX_DATA_VOLUME_NUMBER_HOT_PLUG = 20;
+    // leaving ~19 slots for hot-plugged virtio-blk data volumes (SharedBlock storage scenario).
+    int KVM_MAX_DATA_VOLUME_NUMBER_HOT_PLUG = 19;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Maximum data volumes for hot-plug (running VM) due to PCI bus slot constraints.
// i440fx provides 32 PCI slots; ~12 are reserved by system devices and libvirt PCI bridge,
// leaving ~20 slots for hot-plugged virtio-blk data volumes (SharedBlock storage scenario).
int KVM_MAX_DATA_VOLUME_NUMBER_HOT_PLUG = 20;
// Maximum data volumes for hot-plug (running VM) due to PCI bus slot constraints.
// i440fx provides 32 PCI slots; ~12 are reserved by system devices and libvirt PCI bridge,
// leaving ~19 slots for hot-plugged virtio-blk data volumes (SharedBlock storage scenario).
int KVM_MAX_DATA_VOLUME_NUMBER_HOT_PLUG = 19;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@plugin/kvm/src/main/java/org/zstack/kvm/KVMConstant.java` around lines 163 -
167, The constant KVM_MAX_DATA_VOLUME_NUMBER_HOT_PLUG is off by one versus the
validator logic in VolumeBase.vmAttachVolumeValidator (which uses the check
vmDataVolumeUsage + 1 > maxDataVolumeNum), so adjust the limit to prevent
allowing the 20th hot-plug: change KVM_MAX_DATA_VOLUME_NUMBER_HOT_PLUG from 20
to 19 (or alternatively change the validator's comparison semantics), ensure
references to KVM_MAX_DATA_VOLUME_NUMBER_HOT_PLUG remain consistent, and run
relevant unit/integration tests around vmAttachVolumeValidator and hot-plug
flows to confirm the 20th attach is rejected.

String CONNECT_HOST_PRIMARYSTORAGE_ERROR = "psError";

String VIRTUALIZER_QEMU_KVM = "qemu-kvm";
Expand Down
5 changes: 5 additions & 0 deletions plugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -1098,6 +1098,11 @@ public int getMaxDataVolumeNumber() {
return maxDataVolumeNum;
}

@Override
public int getMaxDataVolumeNumberForRunningVm() {
return Math.min(maxDataVolumeNum, KVMConstant.KVM_MAX_DATA_VOLUME_NUMBER_HOT_PLUG);
}

@Override
@AsyncThread
public void managementNodeReady() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3272,7 +3272,10 @@ static void vmAttachVolumeValidator(VmInstanceInventory vmInv, String volumeUuid

PluginRegistry pluginRgty = Platform.getComponentLoader().getComponent(PluginRegistry.class);
MaxDataVolumeNumberExtensionPoint ext = pluginRgty.getExtensionFromMap(hypervisorType, MaxDataVolumeNumberExtensionPoint.class);
int maxDataVolumeNum = ext == null ? VolumeConstant.DEFAULT_MAX_DATA_VOLUME_NUMBER : ext.getMaxDataVolumeNumber();
boolean isRunning = VmInstanceState.Running.toString().equals(vmInv.getState())
|| VmInstanceState.Paused.toString().equals(vmInv.getState());
int maxDataVolumeNum = ext == null ? VolumeConstant.DEFAULT_MAX_DATA_VOLUME_NUMBER
: (isRunning ? ext.getMaxDataVolumeNumberForRunningVm() : ext.getMaxDataVolumeNumber());

long vmDataVolumeUsage = Q.New(VolumeVO.class)
.eq(VolumeVO_.type, VolumeType.Data)
Expand Down