server: Avoid device id conflicts between data volumes, the CD-ROM and the config drive ISO by weizhouapache · Pull Request #14073 · apache/cloudstack

weizhouapache · 2026-09-07T14:09:29Z

Description

This PR fixes #13484 (the fix for main, which also covers multiple ISOs per Instance (#14321), is in #14407).

On KVM, a data volume could get the same device id as the CD-ROM or the config drive ISO. When both devices end up with the same sd* target name (SATA CD-ROMs on UEFI/q35 Instances plus SCSI or SATA data disks), libvirt rejects the domain XML with target 'sde' duplicated for disk sources .... The config drive ISO is attached with the device id 4, but the allocators of volume device ids only reserved the device id 3 (the CD-ROM), so the third data volume got the device id 4.

The same conflict could happen when attaching a volume to a stopped Instance (it is accepted, and the Instance cannot start any more), when restoring volumes/Instances from backup, and when importing Instances (unmanaged Instances and VMware to KVM), which numbered the data disks 1, 2, 3, 4, ...

Device ids of an Instance

Device	Device id
Root volume	`0`
CD-ROM (ISO)	`3`
Config drive ISO (if the Instance uses config drive)	`4`
Data volumes	`1`, `2`, then the next device ids which are not used by the CD-ROM and the config drive. The maximum number of data volumes is not reduced by the reserved device ids.

Example (default maximum of 6 data volumes):

Config drive	Reserved device ids	Data volumes
no	`3`	`1`, `2`, `4`, `5`, `6`, `7`
yes	`3`, `4`	`1`, `2`, `5`, `6`, `7`, `8`

An Instance uses config drive if the Dhcp, Dns or UserData service of any of its networks is provided by ConfigDrive (checked on the network providers, so it also works for Instances which have never been started), or if it has the config drive location detail.

Only one ISO can be attached to an Instance in this version, so there is a single CD-ROM (device id 3). The device ids of the CD-ROMs are obtained from getVmCdromCount(), which returns 1, so that the logic can be merged forward to main, where it is calculated from the Instance (#14407).

Changes

VolumeApiService.getReservedDeviceIds(vm) returns the device ids reserved for the CD-ROM and the config drive ISO (if the Instance uses config drive). It is used when allocating device ids in:
- attach volume (auto-allocated device ids, and validation of the deviceid parameter, whose error message now lists the reserved ids). This also covers Instances which are stopped.
- restore of volumes and Instances from backup (KVMGuru). When a whole Instance is restored, its networks may not be available any more, so the device id of the config drive is always kept free.
- import of Instances (unmanaged Instances, VMware to KVM). The NICs are not imported yet at that point, so the device id of the config drive is always kept free.
ConfigDriveNetworkElement.CONFIGDRIVEDISKSEQ is public, so it can be used by the allocators.

Notes

The volumes which already use the device id 4 on an Instance using config drive (e.g. created before this fix) are not changed: such an Instance still fails to start until the volume is detached.

Types of changes

Breaking change (fix or feature that would cause existing functionality to change)
New feature (non-breaking change which adds functionality)
Bug fix (non-breaking change which fixes an issue)
Enhancement (improves an existing feature and functionality)
Cleanup (Code refactoring and cleanup, that may add test cases)
Build/CI
Test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Major
Minor

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

Unit tests (VolumeApiServiceImplTest, KVMGuruTest, UnmanagedVMsManagerImplTest) cover the reserved device ids with and without config drive (including Instances which have never been started, detected from the network providers), the device id allocation for attach volume, restore and import, that the maximum number of data volumes can still be attached, and the error message of an invalid deviceid.

This branch has not been run on a 4.22 environment since the last change. The same logic was tested manually on main (see #14407), with Instances on BIOS and UEFI/q35, with and without config drive, attaching volumes to running and stopped Instances, and importing an unmanaged Instance.

How did you try to break this feature and the system with this change?

Instances which have never been started, with config drive networks, with volumes attached while they are stopped
Instances which do not use config drive (the device id 4 can still be used by a data volume)
the maximum number of data volumes of an Instance using config drive

weizhouapache · 2026-09-07T14:13:30Z

Tested with

Ubuntu 24.04 cloud image
machine type is q35
UEFI=SECURE
rootdiskcontroller = scsi
datadiskcontroller = scsi

Note: still got error org.libvirt.LibvirtException: internal error: No more available PCI slots if disk controllers are virtio

codecov · 2026-09-07T14:16:49Z

Codecov Report

❌ Patch coverage is 90.24390% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 18.04%. Comparing base (c7667d8) to head (a28af1a).

Files with missing lines	Patch %	Lines
...er/src/main/java/com/cloud/hypervisor/KVMGuru.java	70.00%	3 Missing ⚠️
...n/java/com/cloud/template/TemplateManagerImpl.java	94.11%	1 Missing and 1 partial ⚠️
.../apache/cloudstack/vm/UnmanagedVMsManagerImpl.java	86.66%	2 Missing ⚠️
...n/java/com/cloud/storage/VolumeApiServiceImpl.java	95.45%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               4.22   #14073      +/-   ##
============================================
+ Coverage     18.03%   18.04%   +0.01%     
- Complexity    16286    16321      +35     
============================================
  Files          5937     5937              
  Lines        536771   536825      +54     
  Branches      65782    65791       +9     
============================================
+ Hits          96787    96886      +99     
+ Misses       428932   428882      -50     
- Partials      11052    11057       +5

Flag	Coverage Δ
uitests	`4.04% <ø> (ø)`
unittests	`19.12% <90.24%> (+0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot

🟡 Changes recommended

Reserving an additional device-id for ConfigDrive without expanding the computed max device-id range can reduce the maximum attachable data volumes and cause avoidable allocation failures.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR addresses KVM/libvirt disk target collisions when ConfigDrive is enabled by ensuring the device-id allocators avoid using the ConfigDrive ISO’s reserved disk sequence (4), preventing duplicate target devices on Q35/UEFI machine types.

Changes:

Detect ConfigDrive presence via VmDetailConstants.CONFIG_DRIVE_LOCATION and skip device id CONFIGDRIVEDISKSEQ during device-id allocation.
Expose CONFIGDRIVEDISKSEQ publicly so allocators can reference the reserved slot.
Update KVM import/restore volume attach path to use the updated allocator logic.

File summaries

File	Description
server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java	Skip ConfigDrive’s reserved device id during device-id validation/allocation for volume attach.
server/src/main/java/com/cloud/network/element/ConfigDriveNetworkElement.java	Make ConfigDrive disk sequence constant publicly accessible.
server/src/main/java/com/cloud/hypervisor/KVMGuru.java	Skip ConfigDrive’s reserved device id when selecting the next available device id during VM restore/import flows.

Review details

Files reviewed: 3/3 changed files
Comments generated: 3
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

DaanHoogland

clgtm

Damans227 · 2026-10-06T12:00:28Z

        int maxDevices = getMaxDataVolumesSupported(vm) + 2; // add 2 to consider devices root volume and cdrom
        int maxDeviceId = maxDevices - 1;
        List<VolumeVO> vols = _volsDao.findByInstance(vm.getId());
+        boolean vmHasConfigDrive = vmInstanceDetailsDao.findDetail(vm.getId(), VmDetailConstants.CONFIG_DRIVE_LOCATION) != null;


what happens for a vm deployed with startvm=false and then given three data disks? the config drive detail only shows up on first start, so the third disk still lands on 4

good point. I did not consider this case.
maybe we need to determine if configdrive ISO is needed by the network/offering providers.

makes sense, checking if the default network uses config drive for user data would work even before the first start

that covers it

weizhouapache · 2026-10-07T11:40:47Z

need re-work to consider

multiple CD-ROMs
vm import

moving to draft

cc @DaanHoogland

Damans227 · 2026-10-13T17:55:18Z

+     * {@link TemplateManager#CDROM_PRIMARY_DEVICE_SEQ}.
+     */
+    protected int getVmCdromCount(VirtualMachine vm) {
+        return 1;


on main a second cd drive sits on 4 too, so should the config drive move past the last cd slot when this merges forward?

@Damans227
yes, there will be another PR target to main, which calculate the count of CD-ROMs from vm

that covers it, #14407 does it on main

vishesh92

clgtm. didn't test.

…olumes on KVM ConfigDriveNetworkElement hardcodes disk sequence 4 (CONFIGDRIVEDISKSEQ) for the config drive ISO, but the volume device-id allocators (VolumeApiServiceImpl.getDeviceId, KVMGuru.getNextAvailableDeviceId) only look at persisted VolumeVO rows and have no knowledge that slot 4 is reserved by the config drive. When a VM using ConfigDrive already has data disks on device ids 1 and 2, attaching a 3rd data disk gets assigned device id 4 as well, producing two disks with the same libvirt target device (e.g. "sde") once both end up on the same bus namespace, as happens with Q35/UEFI machine types. Both allocators now skip CONFIGDRIVEDISKSEQ when the VM has a config drive (detected via the existing CONFIG_DRIVE_LOCATION vm detail), the same way they already permanently reserve device id 3 for the CD-ROM slot.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Data volumes could be assigned the same device id as the CD-ROM or the config drive ISO, which end up with the same libvirt target name (e.g. sde) when they are on the same bus namespace (SATA with UEFI/Q35). Add VolumeApiService.getReservedDeviceIds(), which returns the device ids of the CD-ROM devices and of the config drive ISO when the Instance uses config drive. The config drive is detected from the network providers (Dhcp, Dns, UserData), so Instances which have never been started are covered as well as the ones with the config drive location detail. The reserved ids are skipped when allocating device ids in: - attach volume - restore of volumes and Instances from backup - import of Instances (unmanaged Instances and VMware to KVM) The max number of data volumes is preserved when device ids are reserved, and the error message of an invalid deviceId lists the reserved ids.

Move the calculation of the device ids reserved for the CD-ROM and the config drive ISO to TemplateManager, as getCdromDeviceIds(). An Instance has one CD-ROM, and the config drive ISO is one more CD-ROM (device id 4) when any network of the Instance uses config drive. VolumeApiService#getReservedDeviceIds() delegates to it, and a variant with the usage of config drive is added for the cases where the networks of the Instance are not known yet (VM import and backup restore).

Copilot

🟡 Changes recommended

Initial deployment still assigns data-disk IDs without skipping reserved CD-ROM and config-drive IDs.

1 open finding

Initial deployment bypasses reserved disk ID allocation · New

3 resolved since last review

The max device-id range is computed as maxDataVolumesSupported + 2 (root + CD-ROM), but when a VM…
The config-drive reservation here removes another device id (in addition to the CD-ROM’s reserved…
Modifier order for constants in this codebase is consistently public static final ...; `public…

🧠 Review effort: Lite

+     * @param vm the Instance
+     * @return the device ids of the CD-ROM devices, including the one of the config drive ISO if the Instance uses config drive
+     */
+    Set<Long> getCdromDeviceIds(VirtualMachine vm);


weizhouapache · 2026-10-16T11:49:44Z

I deployed two Ubuntu 24 VMs on kvm1. t-cd is on the existing L2-configdrive network. t-nocd is on a new l2-noconfigdrive network (VLAN 2500). I hot-attached 5 data volumes to each, then stopped each VM, attached a volume while it was stopped, and started it again.

VM	Device ids on hot-attach	Device id on stopped attach	Domain after restart
`t-cd` (config drive)	1, 2, 5, 6, 7	8	CD-ROM `hdc` empty, config drive ISO on `hdd`, disks `vdb`, `vdc`, `vdf`–`vdi`, no duplicate target
`t-nocd` (no config drive)	1, 2, 4, 5, 6	7	CD-ROM `hdc` empty only, `vde` (id 4) is a data disk, no clash

With config drive, ids 3 and 4 are reserved for the CD-ROM and the config drive ISO. Without it, only id 3 is reserved, so id 4 is available to a data disk.

Damans227

thanks @weizhouapache, fix looks right. LGTM

boring-cyborg Bot added component:networking component:storage labels Sep 7, 2026

weizhouapache requested a lite review from Copilot September 7, 2026 14:13

weizhouapache added this to the 4.22.2 milestone Sep 7, 2026

Copilot started reviewing on behalf of weizhouapache September 7, 2026 14:22

View session

Copilot AI reviewed Sep 7, 2026

View reviewed changes

Comment thread server/src/main/java/com/cloud/hypervisor/KVMGuru.java Outdated

Comment thread server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java Outdated

Comment thread server/src/main/java/com/cloud/network/element/ConfigDriveNetworkElement.java Outdated

weizhouapache linked an issue Sep 7, 2026 that may be closed by this pull request

ConfigDrive: duplicated target for disk sources with Q35 machine type #13484

Open

DaanHoogland added the needs-testing label Sep 30, 2026

DaanHoogland approved these changes Oct 6, 2026

View reviewed changes

Damans227 reviewed Oct 6, 2026

View reviewed changes

Comment thread server/src/main/java/com/cloud/network/element/ConfigDriveNetworkElement.java

weizhouapache mentioned this pull request Oct 6, 2026

KVM: Data disks and pre-allocated CD-ROMs can get conflicting target device names (multiple-ISO support, UEFI) #14321

Open

weizhouapache marked this pull request as draft October 7, 2026 11:39

DaanHoogland self-requested a review October 12, 2026 12:49

KamilBartok mentioned this pull request Oct 13, 2026

ConfigDrive: duplicated target for disk sources with Q35 machine type #13484

Open

boring-cyborg Bot added the component:api label Oct 13, 2026

Damans227 reviewed Oct 13, 2026

View reviewed changes

vishesh92 approved these changes Oct 15, 2026

View reviewed changes

weizhouapache mentioned this pull request Oct 15, 2026

server,kvm: Avoid device id conflicts between data volumes, CD-ROMs and the config drive ISO #14407

Open

2 of 14 tasks

weizhouapache changed the title ~~server: Avoid device id collision between config drive ISO and data volumes on KVM~~ server: Avoid device id conflicts between data volumes, the CD-ROM and the config drive ISO Oct 15, 2026

weizhouapache and others added 4 commits October 16, 2026 10:36

Change CONFIGDRIVEDISKSEQ to static final

87da208

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

weizhouapache force-pushed the 4.22-fix-13484 branch from 4eab500 to a28af1a Compare October 16, 2026 08:38

weizhouapache requested review from vishesh92 and a lite review from Copilot October 16, 2026 10:47

weizhouapache requested a review from Damans227 October 16, 2026 10:47

Copilot started reviewing on behalf of weizhouapache October 16, 2026 10:48

View session

Copilot AI reviewed Oct 16, 2026

View reviewed changes

Comment thread engine/components-api/src/main/java/com/cloud/template/TemplateManager.java

* @param vm the Instance

* @return the device ids of the CD-ROM devices, including the one of the config drive ISO if the Instance uses config drive

*/

Set<Long> getCdromDeviceIds(VirtualMachine vm);

weizhouapache marked this pull request as ready for review October 16, 2026 11:50

Damans227 approved these changes Oct 16, 2026

View reviewed changes

Conversation

weizhouapache commented Sep 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Device ids of an Instance

Changes

Notes

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

Uh oh!

weizhouapache commented Sep 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Sep 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

DaanHoogland left a comment

Choose a reason for hiding this comment

Uh oh!

Damans227 Oct 6, 2026

Choose a reason for hiding this comment

Uh oh!

weizhouapache Oct 6, 2026

Choose a reason for hiding this comment

Uh oh!

Damans227 Oct 6, 2026

Choose a reason for hiding this comment

Uh oh!

Damans227 Oct 16, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

weizhouapache commented Oct 7, 2026

Uh oh!

Damans227 Oct 13, 2026

Choose a reason for hiding this comment

Uh oh!

weizhouapache Oct 13, 2026

Choose a reason for hiding this comment

Uh oh!

Damans227 Oct 16, 2026

Choose a reason for hiding this comment

Uh oh!

vishesh92 left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

weizhouapache commented Oct 16, 2026

Uh oh!

Damans227 left a comment

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

weizhouapache commented Sep 7, 2026 •

edited

Loading

weizhouapache commented Sep 7, 2026 •

edited

Loading

codecov Bot commented Sep 7, 2026 •

edited

Loading