Multiple CD-ROM / ISO Support Per VM#13101
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #13101 +/- ##
============================================
- Coverage 18.08% 18.08% -0.01%
- Complexity 16706 16722 +16
============================================
Files 6037 6041 +4
Lines 542437 542707 +270
Branches 66420 66471 +51
============================================
+ Hits 98088 98126 +38
- Misses 433333 433554 +221
- Partials 11016 11027 +11
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@blueorangutan package |
|
@Damans227 a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 17708 |
…avior for listByVmId
|
@blueorangutan package |
|
@Damans227 a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 17709 |
|
@blueorangutan package |
|
@sureshanaparti a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress. |
harikrishna-patnala
left a comment
harikrishna-patnala
left a comment
There was a problem hiding this comment.
nice work @Damans227 I've added few comments, can you please check them
| protected Boolean forced; | ||
|
|
||
| @Parameter(name = ApiConstants.ID, type = CommandType.UUID, entityType = TemplateResponse.class, | ||
| description = "The ID of the ISO to detach. Required when the Instance has more than one ISO attached.") |
There was a problem hiding this comment.
| description = "The ID of the ISO to detach. Required when the Instance has more than one ISO attached.") | |
| description = "The ID of the ISO to detach. Required when the Instance has more than one ISO attached.", since = "4.23.0") |
| "vm.cdrom.max.count", "1", | ||
| "Maximum number of CD-ROM drives per VM.", | ||
| true, | ||
| ConfigKey.Scope.Global); |
There was a problem hiding this comment.
as this is the hypervisor dependent, is it better to keep the scope as Cluster ?
If you are making this change, please update the getters of this config to read from clusterId
| private int effectiveMaxCdroms(VirtualMachine vm) { | ||
| int globalCap = VmCdromMaxCount.value(); | ||
| // hda is root on i440fx/IDE, leaving hdc/hdd available for cdroms. | ||
| int hypervisorCap = (vm.getHypervisorType() == HypervisorType.KVM) ? 2 : 1; |
There was a problem hiding this comment.
Instead of hardcoding these numbers, is there a way to fetch from the hypervisor ?
and I feel even if there is no way to fetch from hypervisor, we can still do a check if the config value is more than expected value (2 for KVM and 1 for others) and log an error message and let the operation fail.
I'm just thinking if in future if hypervisor increases the supported value we dont have do any code changes.
There was a problem hiding this comment.
Pull request overview
This PR adds support for attaching multiple ISOs (multiple CD-ROM devices) to a single VM on KVM, enabling workflows like Windows install + VirtIO drivers without ISO swapping, while keeping legacy single-ISO semantics (user_vm.iso_id, isoid) for backward compatibility.
Changes:
- Introduces
vm_iso_mapto persist additional ISO slots per VM and adds global capvm.cdrom.max.count(clamped by hypervisor). - Extends VM responses with
isos[](slot-aware) and updates UI to display/attach/detach multiple ISOs. - Updates KVM ISO attach/detach path to honor a target CD-ROM slot via
diskSeq/deviceSeq, plus adds unit test coverage for new slot/validation helpers.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| ui/src/views/compute/InstanceTab.vue | Display all attached ISOs and show slot labels in the VM details view. |
| ui/src/views/compute/AttachIso.vue | Allow multi-select attach, enforce slot cap in UI, and fan out attachIso calls. |
| ui/src/views/compute/DetachIso.vue | New multi-select detach UI and fan out detachIso calls (with optional id). |
| ui/src/config/section/compute.js | Update VM action availability logic for attach/detach ISO with multi-ISO awareness. |
| api/src/main/java/org/apache/cloudstack/api/command/user/iso/DetachIsoCmd.java | Add optional id parameter to disambiguate detach when multiple ISOs are attached. |
| api/src/main/java/org/apache/cloudstack/api/response/UserVmResponse.java | Add isos[] response field for all attached ISOs. |
| api/src/main/java/org/apache/cloudstack/api/response/AttachedIsoResponse.java | New response object representing an attached ISO + slot (deviceseq). |
| server/src/main/java/com/cloud/api/query/dao/UserVmJoinDaoImpl.java | Populate isos[] in VM responses using vm_iso_map. |
| server/src/main/java/com/cloud/template/TemplateManagerImpl.java | Persist extra ISO slots, enforce caps/duplicate attach, pre-allocate CD-ROM drives at boot, and resolve detach id. |
| plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java | Pass CD-ROM slot down to libvirt ISO attach/detach operations. |
| engine/schema/src/main/resources/META-INF/db/schema-42210to42300.sql | Add vm_iso_map table for extra ISO attachments. |
| engine/schema/src/main/resources/META-INF/cloudstack/core/spring-engine-schema-core-daos-context.xml | Wire the new VmIsoMapDaoImpl bean. |
| engine/schema/src/main/java/com/cloud/vm/VmIsoMapVO.java | New VO for vm_iso_map. |
| engine/schema/src/main/java/com/cloud/vm/dao/VmIsoMapDao.java | New DAO interface for ISO map operations. |
| engine/schema/src/main/java/com/cloud/vm/dao/VmIsoMapDaoImpl.java | DAO implementation with lookups by VM/ISO/slot. |
| engine/components-api/src/main/java/com/cloud/template/TemplateManager.java | Add vm.cdrom.max.count config and CDROM_PRIMARY_DEVICE_SEQ constant. |
| server/src/test/java/com/cloud/template/TemplateManagerImplTest.java | Add unit tests for slot allocation, detach-id resolution, and duplicate attach detection. |
| server/src/test/java/com/cloud/api/query/dao/UserVmJoinDaoImplTest.java | Stub vmIsoMapDao for new isos[] response population path. |
Comments suppressed due to low confidence (1)
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java:1221
- When detaching an ISO,
attachOrDetachISOcurrently callscleanupDiskfor all CDROM devices on the VM. With multi-ISO support, this risks disconnecting the backing storage for other still-attached ISOs when only one is being detached. Consider restricting cleanup to the CDROM matching the requesteddeviceSeq/disk label (similar to the filtering used inLibvirtComputingResource.attachOrDetachISO).
final List<DiskDef> disks = resource.getDisks(conn, vmName);
attachOrDetachDevice(conn, true, vmName, iso);
if (!isAttach) {
for (final DiskDef disk : disks) {
if (disk.getDeviceType() == DiskDef.DeviceType.CDROM) {
resource.cleanupDisk(disk);
}
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Long poolId = singleStoragePoolId(dest); | ||
| Map<Integer, Long> slotToIsoId = loadAttachedIsoSlots(vm); |
| private void enforceCdromAttachLimits(long vmId, UserVm vm, long isoId) { | ||
| Long primaryIsoId = vm.getIsoId(); | ||
| if (isIsoAlreadyAttached(vmId, primaryIsoId, isoId)) { | ||
| throw new InvalidParameterValueException("The specified ISO is already attached to this Instance."); | ||
| } | ||
| int effectiveMax = effectiveMaxCdroms(vm); | ||
| int attached = (primaryIsoId != null ? 1 : 0) + _vmIsoMapDao.listByVmId(vmId).size(); | ||
| if (attached >= effectiveMax) { | ||
| throw new InvalidParameterValueException(String.format( |
| List<AttachedIsoResponse> attachedIsos = new ArrayList<>(); | ||
| if (userVm.getIsoUuid() != null) { | ||
| attachedIsos.add(new AttachedIsoResponse(userVm.getIsoUuid(), userVm.getIsoName(), | ||
| userVm.getIsoDisplayText(), TemplateManager.CDROM_PRIMARY_DEVICE_SEQ)); | ||
| } | ||
| for (VmIsoMapVO row : vmIsoMapDao.listByVmId(userVm.getId())) { | ||
| VMTemplateVO tmpl = vmTemplateDao.findById(row.getIsoId()); | ||
| if (tmpl != null) { | ||
| attachedIsos.add(new AttachedIsoResponse(tmpl.getUuid(), tmpl.getName(), | ||
| tmpl.getDisplayText(), row.getDeviceSeq())); | ||
| } | ||
| } |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 17713 |
Description
Adds multiple CD-ROM support per VM on KVM. Today CloudStack allows only one ISO per VM, forcing a manual ISO-swap workflow to install Windows on KVM (Windows installer + VirtIO drivers). After this PR each VM can hold up to
vm.cdrom.max.countISOs simultaneously, each appearing as its own CD-ROM inside the guest.What changes:
detachIsotakes an optionalid(required when multiple ISOs are attached).listVirtualMachinesreturnsisos[]; legacyisoidstill reflects the bootable ISO.vm.cdrom.max.count(default1), clamped by the hypervisor cap (KVM/IDE = 2).vm_iso_maptable holds extra slots;user_vm.iso_idis kept as the bootable pointer for back-compat.Scope: KVM only.
Design doc: https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=421957739
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Screenshots (if appropriate):
Screen.Recording.2026-05-04.at.5.20.18.PM.mov
How Has This Been Tested?
TemplateManagerImplTestfor slot allocation, the duplicate-attach check, and the detach-id resolution helper.How did you try to break this feature and the system with this change?
user_vm.iso_idandvm_iso_mapstay in syncisoidinlistVirtualMachines) see no functional change