From d3668e2329079614b9d86a81ed0a5d5ce0696d79 Mon Sep 17 00:00:00 2001 From: Daman Arora Date: Mon, 4 May 2026 11:29:31 -0400 Subject: [PATCH 01/27] pre-allocate a second empty cdrom slot at boot (hardcoded) --- .../main/java/com/cloud/template/TemplateManagerImpl.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java index 3aaebc691309..23bc9e32a19f 100755 --- a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java +++ b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java @@ -718,6 +718,11 @@ public void prepareIsoForVmProfile(VirtualMachineProfile profile, DeployDestinat DiskTO disk = new DiskTO(iso, 3L, null, Volume.Type.ISO); profile.addDisk(disk); } + // FR283: pre-allocate an additional empty cdrom slot at diskSeq=4 (hdd). + // Hardcoded second slot for now; commit 2 makes this configurable. + TemplateObjectTO extraIso = new TemplateObjectTO(); + extraIso.setFormat(ImageFormat.ISO); + profile.addDisk(new DiskTO(extraIso, 4L, null, Volume.Type.ISO)); } private void prepareTemplateInOneStoragePool(final VMTemplateVO template, final StoragePoolVO pool) { From 90b415f56d5f895f7e6b475f45c88258b68e01a5 Mon Sep 17 00:00:00 2001 From: Daman Arora Date: Mon, 4 May 2026 11:44:42 -0400 Subject: [PATCH 02/27] drive cdrom slot count via vm.cdrom.max.count ConfigKey --- .../java/com/cloud/template/TemplateManager.java | 7 +++++++ .../com/cloud/template/TemplateManagerImpl.java | 15 +++++++++------ 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/engine/components-api/src/main/java/com/cloud/template/TemplateManager.java b/engine/components-api/src/main/java/com/cloud/template/TemplateManager.java index f1891c774edd..593fcca3e4ae 100644 --- a/engine/components-api/src/main/java/com/cloud/template/TemplateManager.java +++ b/engine/components-api/src/main/java/com/cloud/template/TemplateManager.java @@ -64,6 +64,13 @@ public interface TemplateManager { true, ConfigKey.Scope.Global); + ConfigKey VmCdromMaxCount = new ConfigKey("Advanced", + Integer.class, + "vm.cdrom.max.count", "1", + "Maximum number of CD-ROM drives per VM.", + true, + ConfigKey.Scope.Global); + static final String VMWARE_TOOLS_ISO = "vmware-tools.iso"; static final String XS_TOOLS_ISO = "xs-tools.iso"; diff --git a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java index 23bc9e32a19f..418c53f72f45 100755 --- a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java +++ b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java @@ -718,11 +718,13 @@ public void prepareIsoForVmProfile(VirtualMachineProfile profile, DeployDestinat DiskTO disk = new DiskTO(iso, 3L, null, Volume.Type.ISO); profile.addDisk(disk); } - // FR283: pre-allocate an additional empty cdrom slot at diskSeq=4 (hdd). - // Hardcoded second slot for now; commit 2 makes this configurable. - TemplateObjectTO extraIso = new TemplateObjectTO(); - extraIso.setFormat(ImageFormat.ISO); - profile.addDisk(new DiskTO(extraIso, 4L, null, Volume.Type.ISO)); + // Empty ISO DiskTOs pre-allocate cdrom drives at boot so runtime attachIso can media-swap into them. + int cap = VmCdromMaxCount.value(); + for (int slot = 1; slot < cap; slot++) { + TemplateObjectTO extraIso = new TemplateObjectTO(); + extraIso.setFormat(ImageFormat.ISO); + profile.addDisk(new DiskTO(extraIso, 3L + slot, null, Volume.Type.ISO)); + } } private void prepareTemplateInOneStoragePool(final VMTemplateVO template, final StoragePoolVO pool) { @@ -2543,7 +2545,8 @@ public ConfigKey[] getConfigKeys() { return new ConfigKey[] {AllowPublicUserTemplates, TemplatePreloaderPoolSize, ValidateUrlIsResolvableBeforeRegisteringTemplate, - TemplateDeleteFromPrimaryStorage}; + TemplateDeleteFromPrimaryStorage, + VmCdromMaxCount}; } public List getTemplateAdapters() { From 0516f82b8bb2d8b47e255632ca4fd35b137a2606 Mon Sep 17 00:00:00 2001 From: Daman Arora Date: Mon, 4 May 2026 11:55:30 -0400 Subject: [PATCH 03/27] add vm_iso_map table + VO/DAO --- .../main/java/com/cloud/vm/VmIsoMapVO.java | 83 +++++++++++++++++++ .../java/com/cloud/vm/dao/VmIsoMapDao.java | 32 +++++++ .../com/cloud/vm/dao/VmIsoMapDaoImpl.java | 80 ++++++++++++++++++ .../META-INF/db/schema-42210to42300.sql | 14 ++++ 4 files changed, 209 insertions(+) create mode 100644 engine/schema/src/main/java/com/cloud/vm/VmIsoMapVO.java create mode 100644 engine/schema/src/main/java/com/cloud/vm/dao/VmIsoMapDao.java create mode 100644 engine/schema/src/main/java/com/cloud/vm/dao/VmIsoMapDaoImpl.java diff --git a/engine/schema/src/main/java/com/cloud/vm/VmIsoMapVO.java b/engine/schema/src/main/java/com/cloud/vm/VmIsoMapVO.java new file mode 100644 index 000000000000..f4a3f1168188 --- /dev/null +++ b/engine/schema/src/main/java/com/cloud/vm/VmIsoMapVO.java @@ -0,0 +1,83 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package com.cloud.vm; + +import java.util.Date; + +import javax.persistence.Column; +import javax.persistence.Entity; +import javax.persistence.GeneratedValue; +import javax.persistence.GenerationType; +import javax.persistence.Id; +import javax.persistence.Table; +import javax.persistence.Temporal; +import javax.persistence.TemporalType; + +import org.apache.cloudstack.api.InternalIdentity; + +@Entity +@Table(name = "vm_iso_map") +public class VmIsoMapVO implements InternalIdentity { + @Id + @GeneratedValue(strategy = GenerationType.IDENTITY) + @Column(name = "id") + private Long id; + + @Column(name = "vm_id") + private long vmId; + + @Column(name = "iso_id") + private long isoId; + + @Column(name = "device_seq") + private int deviceSeq; + + @Column(name = "created") + @Temporal(TemporalType.TIMESTAMP) + private Date created; + + public VmIsoMapVO() { + } + + public VmIsoMapVO(long vmId, long isoId, int deviceSeq) { + this.vmId = vmId; + this.isoId = isoId; + this.deviceSeq = deviceSeq; + this.created = new Date(); + } + + @Override + public long getId() { + return id; + } + + public long getVmId() { + return vmId; + } + + public long getIsoId() { + return isoId; + } + + public int getDeviceSeq() { + return deviceSeq; + } + + public Date getCreated() { + return created; + } +} diff --git a/engine/schema/src/main/java/com/cloud/vm/dao/VmIsoMapDao.java b/engine/schema/src/main/java/com/cloud/vm/dao/VmIsoMapDao.java new file mode 100644 index 000000000000..e088d4c604d9 --- /dev/null +++ b/engine/schema/src/main/java/com/cloud/vm/dao/VmIsoMapDao.java @@ -0,0 +1,32 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package com.cloud.vm.dao; + +import java.util.List; + +import com.cloud.utils.db.GenericDao; +import com.cloud.vm.VmIsoMapVO; + +public interface VmIsoMapDao extends GenericDao { + List listByVmId(long vmId); + + VmIsoMapVO findByVmIdDeviceSeq(long vmId, int deviceSeq); + + VmIsoMapVO findByVmIdIsoId(long vmId, long isoId); + + int removeByVmId(long vmId); +} diff --git a/engine/schema/src/main/java/com/cloud/vm/dao/VmIsoMapDaoImpl.java b/engine/schema/src/main/java/com/cloud/vm/dao/VmIsoMapDaoImpl.java new file mode 100644 index 000000000000..2d1f9b8c5c9b --- /dev/null +++ b/engine/schema/src/main/java/com/cloud/vm/dao/VmIsoMapDaoImpl.java @@ -0,0 +1,80 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package com.cloud.vm.dao; + +import java.util.List; + +import org.springframework.stereotype.Component; + +import com.cloud.utils.db.GenericDaoBase; +import com.cloud.utils.db.SearchBuilder; +import com.cloud.utils.db.SearchCriteria; +import com.cloud.vm.VmIsoMapVO; + +@Component +public class VmIsoMapDaoImpl extends GenericDaoBase implements VmIsoMapDao { + + private SearchBuilder ListByVmId; + private SearchBuilder ByVmIdDeviceSeq; + private SearchBuilder ByVmIdIsoId; + + protected VmIsoMapDaoImpl() { + ListByVmId = createSearchBuilder(); + ListByVmId.and("vmId", ListByVmId.entity().getVmId(), SearchCriteria.Op.EQ); + ListByVmId.done(); + + ByVmIdDeviceSeq = createSearchBuilder(); + ByVmIdDeviceSeq.and("vmId", ByVmIdDeviceSeq.entity().getVmId(), SearchCriteria.Op.EQ); + ByVmIdDeviceSeq.and("deviceSeq", ByVmIdDeviceSeq.entity().getDeviceSeq(), SearchCriteria.Op.EQ); + ByVmIdDeviceSeq.done(); + + ByVmIdIsoId = createSearchBuilder(); + ByVmIdIsoId.and("vmId", ByVmIdIsoId.entity().getVmId(), SearchCriteria.Op.EQ); + ByVmIdIsoId.and("isoId", ByVmIdIsoId.entity().getIsoId(), SearchCriteria.Op.EQ); + ByVmIdIsoId.done(); + } + + @Override + public List listByVmId(long vmId) { + SearchCriteria sc = ListByVmId.create(); + sc.setParameters("vmId", vmId); + return listBy(sc); + } + + @Override + public VmIsoMapVO findByVmIdDeviceSeq(long vmId, int deviceSeq) { + SearchCriteria sc = ByVmIdDeviceSeq.create(); + sc.setParameters("vmId", vmId); + sc.setParameters("deviceSeq", deviceSeq); + return findOneBy(sc); + } + + @Override + public VmIsoMapVO findByVmIdIsoId(long vmId, long isoId) { + SearchCriteria sc = ByVmIdIsoId.create(); + sc.setParameters("vmId", vmId); + sc.setParameters("isoId", isoId); + return findOneBy(sc); + } + + @Override + public int removeByVmId(long vmId) { + SearchCriteria sc = ListByVmId.create(); + sc.setParameters("vmId", vmId); + return remove(sc); + } +} diff --git a/engine/schema/src/main/resources/META-INF/db/schema-42210to42300.sql b/engine/schema/src/main/resources/META-INF/db/schema-42210to42300.sql index c0feb06e76ac..9cc23036a2de 100644 --- a/engine/schema/src/main/resources/META-INF/db/schema-42210to42300.sql +++ b/engine/schema/src/main/resources/META-INF/db/schema-42210to42300.sql @@ -127,3 +127,17 @@ CREATE TABLE IF NOT EXISTS `cloud_usage`.`quota_tariff_usage` ( PRIMARY KEY (`id`), CONSTRAINT `fk_quota_tariff_usage__tariff_id` FOREIGN KEY (`tariff_id`) REFERENCES `cloud_usage`.`quota_tariff` (`id`), CONSTRAINT `fk_quota_tariff_usage__quota_usage_id` FOREIGN KEY (`quota_usage_id`) REFERENCES `cloud_usage`.`quota_usage` (`id`)); + +--- Per-VM ISO attachments. user_vm.iso_id remains as the primary/bootable ISO pointer. +CREATE TABLE IF NOT EXISTS `cloud`.`vm_iso_map` ( + `id` bigint(20) unsigned NOT NULL AUTO_INCREMENT, + `vm_id` bigint(20) unsigned NOT NULL COMMENT 'foreign key to user_vm', + `iso_id` bigint(20) unsigned NOT NULL COMMENT 'foreign key to vm_template (ISOs are templates of format ISO)', + `device_seq` int(10) unsigned NOT NULL COMMENT 'cdrom slot index used to derive the libvirt device label (3=hdc, 4=hdd)', + `created` datetime NOT NULL, + PRIMARY KEY (`id`), + UNIQUE KEY `uc_vm_iso_map__vm_iso` (`vm_id`, `iso_id`), + UNIQUE KEY `uc_vm_iso_map__vm_seq` (`vm_id`, `device_seq`), + CONSTRAINT `fk_vm_iso_map__vm_id` FOREIGN KEY (`vm_id`) REFERENCES `cloud`.`user_vm` (`id`) ON DELETE CASCADE, + CONSTRAINT `fk_vm_iso_map__iso_id` FOREIGN KEY (`iso_id`) REFERENCES `cloud`.`vm_template` (`id`) +); From 945f2bd06afa3a104f8c7510fba2b9bbc1393139 Mon Sep 17 00:00:00 2001 From: Daman Arora Date: Mon, 4 May 2026 12:41:39 -0400 Subject: [PATCH 04/27] persist multi-ISO state via vm_iso_map --- ...spring-engine-schema-core-daos-context.xml | 1 + .../cloud/template/TemplateManagerImpl.java | 117 +++++++++++------- 2 files changed, 75 insertions(+), 43 deletions(-) diff --git a/engine/schema/src/main/resources/META-INF/cloudstack/core/spring-engine-schema-core-daos-context.xml b/engine/schema/src/main/resources/META-INF/cloudstack/core/spring-engine-schema-core-daos-context.xml index ad3722577c27..52a03fa92a5e 100644 --- a/engine/schema/src/main/resources/META-INF/cloudstack/core/spring-engine-schema-core-daos-context.xml +++ b/engine/schema/src/main/resources/META-INF/cloudstack/core/spring-engine-schema-core-daos-context.xml @@ -108,6 +108,7 @@ + diff --git a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java index 418c53f72f45..ea6382ff77d6 100755 --- a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java +++ b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java @@ -222,7 +222,9 @@ import com.cloud.vm.VirtualMachineProfile; import com.cloud.vm.VirtualMachineProfileImpl; import com.cloud.vm.VmDetailConstants; +import com.cloud.vm.VmIsoMapVO; import com.cloud.vm.dao.UserVmDao; +import com.cloud.vm.dao.VmIsoMapDao; import com.cloud.vm.dao.VMInstanceDao; import com.google.gson.Gson; import com.google.gson.GsonBuilder; @@ -256,6 +258,8 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager, @Inject private UserVmDao _userVmDao; @Inject + private VmIsoMapDao _vmIsoMapDao; + @Inject private VolumeDao _volumeDao; @Inject private SnapshotDao _snapshotDao; @@ -679,51 +683,52 @@ private String extract(Account caller, Long templateId, String url, Long zoneId, @Override public void prepareIsoForVmProfile(VirtualMachineProfile profile, DeployDestination dest) { UserVmVO vm = _userVmDao.findById(profile.getId()); - if (vm.getIsoId() != null) { - Map storageForDisks = dest.getStorageForDisks(); - Long poolId = null; - TemplateInfo template; - if (MapUtils.isNotEmpty(storageForDisks)) { - for (StoragePool storagePool : storageForDisks.values()) { - if (poolId != null && storagePool.getId() != poolId) { - throw new CloudRuntimeException("Cannot determine where to download ISO"); - } - poolId = storagePool.getId(); - } - } - template = prepareIso(vm.getIsoId(), vm.getDataCenterId(), dest.getHost().getId(), poolId); - - if (template == null){ - logger.error("Failed to prepare ISO on secondary or cache storage"); - throw new CloudRuntimeException("Failed to prepare ISO on secondary or cache storage"); - } - if (template.isBootable()) { - profile.setBootLoaderType(BootloaderType.CD); - } - GuestOSVO guestOS = _guestOSDao.findById(template.getGuestOSId()); - String displayName = null; - if (guestOS != null) { - displayName = guestOS.getDisplayName(); + Long poolId = null; + Map storageForDisks = dest.getStorageForDisks(); + if (MapUtils.isNotEmpty(storageForDisks)) { + for (StoragePool storagePool : storageForDisks.values()) { + if (poolId != null && storagePool.getId() != poolId) { + throw new CloudRuntimeException("Cannot determine where to download ISO"); + } + poolId = storagePool.getId(); } + } - TemplateObjectTO iso = (TemplateObjectTO)template.getTO(); - iso.setDirectDownload(template.isDirectDownload()); - iso.setGuestOsType(displayName); - DiskTO disk = new DiskTO(iso, 3L, null, Volume.Type.ISO); - profile.addDisk(disk); - } else { - TemplateObjectTO iso = new TemplateObjectTO(); - iso.setFormat(ImageFormat.ISO); - DiskTO disk = new DiskTO(iso, 3L, null, Volume.Type.ISO); - profile.addDisk(disk); + Map slotToIsoId = new HashMap<>(); + if (vm.getIsoId() != null) { + slotToIsoId.put(3, vm.getIsoId()); + } + for (VmIsoMapVO row : _vmIsoMapDao.listByVmId(vm.getId())) { + slotToIsoId.put(row.getDeviceSeq(), row.getIsoId()); } + // Empty ISO DiskTOs pre-allocate cdrom drives at boot so runtime attachIso can media-swap into them. int cap = VmCdromMaxCount.value(); - for (int slot = 1; slot < cap; slot++) { - TemplateObjectTO extraIso = new TemplateObjectTO(); - extraIso.setFormat(ImageFormat.ISO); - profile.addDisk(new DiskTO(extraIso, 3L + slot, null, Volume.Type.ISO)); + int neededForAttached = slotToIsoId.isEmpty() ? 0 : slotToIsoId.keySet().stream().max(Integer::compare).get() - 2; + int totalSlots = Math.max(cap, neededForAttached); + for (int slot = 0; slot < totalSlots; slot++) { + int diskSeq = 3 + slot; + Long isoId = slotToIsoId.get(diskSeq); + if (isoId != null) { + TemplateInfo template = prepareIso(isoId, vm.getDataCenterId(), dest.getHost().getId(), poolId); + if (template == null) { + logger.error("Failed to prepare ISO on secondary or cache storage"); + throw new CloudRuntimeException("Failed to prepare ISO on secondary or cache storage"); + } + if (diskSeq == 3 && template.isBootable()) { + profile.setBootLoaderType(BootloaderType.CD); + } + GuestOSVO guestOS = _guestOSDao.findById(template.getGuestOSId()); + TemplateObjectTO iso = (TemplateObjectTO) template.getTO(); + iso.setDirectDownload(template.isDirectDownload()); + iso.setGuestOsType(guestOS != null ? guestOS.getDisplayName() : null); + profile.addDisk(new DiskTO(iso, (long) diskSeq, null, Volume.Type.ISO)); + } else { + TemplateObjectTO empty = new TemplateObjectTO(); + empty.setFormat(ImageFormat.ISO); + profile.addDisk(new DiskTO(empty, (long) diskSeq, null, Volume.Type.ISO)); + } } } @@ -1415,16 +1420,42 @@ private boolean attachISOToVM(long vmId, long userId, long isoId, boolean attach boolean success = attachISOToVM(vmId, isoId, attach, forced, isVirtualRouter); if (success && attach && !isVirtualRouter) { - vm.setIsoId(iso.getId()); - _userVmDao.update(vmId, vm); + if (vm.getIsoId() == null) { + vm.setIsoId(iso.getId()); + _userVmDao.update(vmId, vm); + } else { + int nextDeviceSeq = nextFreeCdromDeviceSeq(vmId); + _vmIsoMapDao.persist(new VmIsoMapVO(vmId, iso.getId(), nextDeviceSeq)); + } } if (success && !attach && !isVirtualRouter) { - vm.setIsoId(null); - _userVmDao.update(vmId, vm); + List extras = _vmIsoMapDao.listByVmId(vmId); + if (!extras.isEmpty()) { + VmIsoMapVO last = extras.get(extras.size() - 1); + for (VmIsoMapVO entry : extras) { + if (entry.getDeviceSeq() > last.getDeviceSeq()) { + last = entry; + } + } + _vmIsoMapDao.remove(last.getId()); + } else { + vm.setIsoId(null); + _userVmDao.update(vmId, vm); + } } return success; } + private int nextFreeCdromDeviceSeq(long vmId) { + int max = 3; // iso_id occupies device_seq=3 + for (VmIsoMapVO row : _vmIsoMapDao.listByVmId(vmId)) { + if (row.getDeviceSeq() > max) { + max = row.getDeviceSeq(); + } + } + return max + 1; + } + @Override @ActionEvent(eventType = EventTypes.EVENT_TEMPLATE_DELETE, eventDescription = "Deleting Template", async = true) public boolean deleteTemplate(DeleteTemplateCmd cmd) { From 9570369c0f53a80a68ab5b7ca9f87b724a4a3028 Mon Sep 17 00:00:00 2001 From: Daman Arora Date: Mon, 4 May 2026 12:49:14 -0400 Subject: [PATCH 05/27] carry target cdrom slot through AttachCommand to KVM agent --- .../kvm/storage/KVMStorageProcessor.java | 13 +++--- .../cloud/template/TemplateManagerImpl.java | 43 ++++++++++--------- 2 files changed, 30 insertions(+), 26 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java index 6e03b84d20cf..5f2183c3a38b 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java @@ -1184,10 +1184,11 @@ private void deleteSnapshotOnPrimary(final CopyCommand cmd, final SnapshotObject } } - protected synchronized void attachOrDetachISO(final Connect conn, final String vmName, String isoPath, final boolean isAttach, Map params, DataStoreTO store) throws + protected synchronized void attachOrDetachISO(final Connect conn, final String vmName, String isoPath, final boolean isAttach, Map params, DataStoreTO store, Integer deviceSeq) throws LibvirtException, InternalErrorException { DiskDef iso = new DiskDef(); boolean isUefiEnabled = MapUtils.isNotEmpty(params) && params.containsKey("UEFI"); + Integer devId = (deviceSeq != null) ? deviceSeq : 3; if (isoPath != null && isAttach) { final int index = isoPath.lastIndexOf("/"); final String path = isoPath.substring(0, index); @@ -1203,9 +1204,9 @@ protected synchronized void attachOrDetachISO(final Connect conn, final String v final DiskDef.DiskType isoDiskType = LibvirtComputingResource.getDiskType(isoVol); isoPath = isoVol.getPath(); - iso.defISODisk(isoPath, isUefiEnabled, isoDiskType); + iso.defISODisk(isoPath, devId, isUefiEnabled, isoDiskType); } else { - iso.defISODisk(null, isUefiEnabled, DiskDef.DiskType.FILE); + iso.defISODisk(null, devId, isUefiEnabled, DiskDef.DiskType.FILE); } final List disks = resource.getDisks(conn, vmName); @@ -1225,11 +1226,12 @@ public Answer attachIso(final AttachCommand cmd) { final DiskTO disk = cmd.getDisk(); final TemplateObjectTO isoTO = (TemplateObjectTO)disk.getData(); final DataStoreTO store = isoTO.getDataStore(); + final Integer deviceSeq = (disk.getDiskSeq() != null) ? disk.getDiskSeq().intValue() : null; try { String dataStoreUrl = getDataStoreUrlFromStore(store); final Connect conn = LibvirtConnection.getConnectionByVmName(cmd.getVmName()); - attachOrDetachISO(conn, cmd.getVmName(), dataStoreUrl + File.separator + isoTO.getPath(), true, cmd.getControllerInfo(), store); + attachOrDetachISO(conn, cmd.getVmName(), dataStoreUrl + File.separator + isoTO.getPath(), true, cmd.getControllerInfo(), store, deviceSeq); } catch (final LibvirtException e) { return new Answer(cmd, false, e.toString()); } catch (final InternalErrorException e) { @@ -1246,11 +1248,12 @@ public Answer dettachIso(final DettachCommand cmd) { final DiskTO disk = cmd.getDisk(); final TemplateObjectTO isoTO = (TemplateObjectTO)disk.getData(); final DataStoreTO store = isoTO.getDataStore(); + final Integer deviceSeq = (disk.getDiskSeq() != null) ? disk.getDiskSeq().intValue() : null; try { String dataStoreUrl = getDataStoreUrlFromStore(store); final Connect conn = LibvirtConnection.getConnectionByVmName(cmd.getVmName()); - attachOrDetachISO(conn, cmd.getVmName(), dataStoreUrl + File.separator + isoTO.getPath(), false, cmd.getParams(), store); + attachOrDetachISO(conn, cmd.getVmName(), dataStoreUrl + File.separator + isoTO.getPath(), false, cmd.getParams(), store, deviceSeq); } catch (final LibvirtException e) { return new Answer(cmd, false, e.toString()); } catch (final InternalErrorException e) { diff --git a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java index ea6382ff77d6..60a62baea884 100755 --- a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java +++ b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java @@ -1372,7 +1372,7 @@ public TemplateInfo prepareIso(long isoId, long dcId, Long hostId, Long poolId) } } - private boolean attachISOToVM(long vmId, long isoId, boolean attach, boolean forced, boolean isVirtualRouter) { + private boolean attachISOToVM(long vmId, long isoId, int deviceSeq, boolean attach, boolean forced, boolean isVirtualRouter) { VirtualMachine vm = !isVirtualRouter ? _userVmDao.findById(vmId) : _vmInstanceDao.findById(vmId); if (vm == null || (isVirtualRouter && vm.getType() != VirtualMachine.Type.DomainRouter)) { @@ -1396,7 +1396,7 @@ private boolean attachISOToVM(long vmId, long isoId, boolean attach, boolean for } DataTO isoTO = tmplt.getTO(); - DiskTO disk = new DiskTO(isoTO, null, null, Volume.Type.ISO); + DiskTO disk = new DiskTO(isoTO, (long) deviceSeq, null, Volume.Type.ISO); HypervisorGuru hvGuru = _hvGuruMgr.getGuru(vm.getHypervisorType()); VirtualMachineProfile profile = new VirtualMachineProfileImpl(vm); @@ -1418,42 +1418,43 @@ private boolean attachISOToVM(long vmId, long userId, long isoId, boolean attach UserVmVO vm = _userVmDao.findById(vmId); VMTemplateVO iso = _tmpltDao.findById(isoId); - boolean success = attachISOToVM(vmId, isoId, attach, forced, isVirtualRouter); + VmIsoMapVO highestExtra = highestCdromMapEntry(vmId); + int targetSlot; + if (attach) { + targetSlot = vm.getIsoId() == null ? 3 : (highestExtra == null ? 4 : highestExtra.getDeviceSeq() + 1); + } else { + targetSlot = highestExtra != null ? highestExtra.getDeviceSeq() : 3; + } + + boolean success = attachISOToVM(vmId, isoId, targetSlot, attach, forced, isVirtualRouter); + if (success && attach && !isVirtualRouter) { - if (vm.getIsoId() == null) { + if (targetSlot == 3) { vm.setIsoId(iso.getId()); _userVmDao.update(vmId, vm); } else { - int nextDeviceSeq = nextFreeCdromDeviceSeq(vmId); - _vmIsoMapDao.persist(new VmIsoMapVO(vmId, iso.getId(), nextDeviceSeq)); + _vmIsoMapDao.persist(new VmIsoMapVO(vmId, iso.getId(), targetSlot)); } } if (success && !attach && !isVirtualRouter) { - List extras = _vmIsoMapDao.listByVmId(vmId); - if (!extras.isEmpty()) { - VmIsoMapVO last = extras.get(extras.size() - 1); - for (VmIsoMapVO entry : extras) { - if (entry.getDeviceSeq() > last.getDeviceSeq()) { - last = entry; - } - } - _vmIsoMapDao.remove(last.getId()); - } else { + if (targetSlot == 3) { vm.setIsoId(null); _userVmDao.update(vmId, vm); + } else if (highestExtra != null) { + _vmIsoMapDao.remove(highestExtra.getId()); } } return success; } - private int nextFreeCdromDeviceSeq(long vmId) { - int max = 3; // iso_id occupies device_seq=3 + private VmIsoMapVO highestCdromMapEntry(long vmId) { + VmIsoMapVO highest = null; for (VmIsoMapVO row : _vmIsoMapDao.listByVmId(vmId)) { - if (row.getDeviceSeq() > max) { - max = row.getDeviceSeq(); + if (highest == null || row.getDeviceSeq() > highest.getDeviceSeq()) { + highest = row; } } - return max + 1; + return highest; } @Override From 5fd487a7b643b29b3ded6b0f4897a3cf858ea5d7 Mon Sep 17 00:00:00 2001 From: Daman Arora Date: Mon, 4 May 2026 13:01:13 -0400 Subject: [PATCH 06/27] enforce per-VM cdrom cap, clamp to hypervisor max --- .../com/cloud/template/TemplateManagerImpl.java | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java index 60a62baea884..3f4435308a51 100755 --- a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java +++ b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java @@ -704,7 +704,7 @@ public void prepareIsoForVmProfile(VirtualMachineProfile profile, DeployDestinat } // Empty ISO DiskTOs pre-allocate cdrom drives at boot so runtime attachIso can media-swap into them. - int cap = VmCdromMaxCount.value(); + int cap = effectiveMaxCdroms(vm); int neededForAttached = slotToIsoId.isEmpty() ? 0 : slotToIsoId.keySet().stream().max(Integer::compare).get() - 2; int totalSlots = Math.max(cap, neededForAttached); for (int slot = 0; slot < totalSlots; slot++) { @@ -1333,6 +1333,14 @@ public boolean attachIso(long isoId, long vmId, Boolean... extraParams) { if (VMWARE_TOOLS_ISO.equals(iso.getUniqueName()) && vm.getHypervisorType() != Hypervisor.HypervisorType.VMware) { throw new InvalidParameterValueException("Cannot attach VMware tools drivers to incompatible hypervisor " + vm.getHypervisorType()); } + if (!isVirtualRouter) { + int effectiveMax = effectiveMaxCdroms(vm); + int attached = (((UserVm) vm).getIsoId() != null ? 1 : 0) + _vmIsoMapDao.listByVmId(vmId).size(); + if (attached >= effectiveMax) { + throw new InvalidParameterValueException(String.format( + "Instance has reached the maximum of %d attached CD-ROM(s); detach one before attaching another.", effectiveMax)); + } + } boolean result = attachISOToVM(vmId, userId, isoId, true, forced, isVirtualRouter); if (result) { return result; @@ -1457,6 +1465,13 @@ private VmIsoMapVO highestCdromMapEntry(long vmId) { return highest; } + private int effectiveMaxCdroms(VirtualMachine vm) { + int globalCap = VmCdromMaxCount.value(); + // i440fx/IDE: hda is root, our slot scheme puts cdroms at hdc/hdd → 2 max on KVM. + int hypervisorCap = (vm.getHypervisorType() == HypervisorType.KVM) ? 2 : 1; + return Math.min(globalCap, hypervisorCap); + } + @Override @ActionEvent(eventType = EventTypes.EVENT_TEMPLATE_DELETE, eventDescription = "Deleting Template", async = true) public boolean deleteTemplate(DeleteTemplateCmd cmd) { From 3a13ac75077aebf7a2fbfad0a331b5bbb9d876bb Mon Sep 17 00:00:00 2001 From: Daman Arora Date: Mon, 4 May 2026 13:09:59 -0400 Subject: [PATCH 07/27] make detachIso accepts an ISO id --- .../api/command/user/iso/DetachIsoCmd.java | 7 +++- .../cloud/template/TemplateManagerImpl.java | 36 ++++++++++++++++--- 2 files changed, 37 insertions(+), 6 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/iso/DetachIsoCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/iso/DetachIsoCmd.java index cf4aa41f795c..ef12a9e4b8ae 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/iso/DetachIsoCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/iso/DetachIsoCmd.java @@ -27,6 +27,7 @@ import org.apache.cloudstack.api.ServerApiException; import org.apache.cloudstack.api.command.user.UserCmd; import org.apache.cloudstack.api.command.user.vm.DeployVMCmd; +import org.apache.cloudstack.api.response.TemplateResponse; import org.apache.cloudstack.api.response.UserVmResponse; import com.cloud.event.EventTypes; @@ -51,6 +52,10 @@ public class DetachIsoCmd extends BaseAsyncCmd implements UserCmd { description = "If true, ejects the ISO before detaching on VMware. Default: false", since = "4.15.1") 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.") + protected Long id; + ///////////////////////////////////////////////////// /////////////////// Accessors /////////////////////// ///////////////////////////////////////////////////// @@ -104,7 +109,7 @@ public ApiCommandResourceType getApiResourceType() { @Override public void execute() { - boolean result = _templateService.detachIso(virtualMachineId, null, isForced()); + boolean result = _templateService.detachIso(virtualMachineId, id, isForced()); if (result) { UserVm userVm = _entityMgr.findById(UserVm.class, virtualMachineId); UserVmResponse response = _responseGenerator.createUserVmResponse(getResponseView(), "virtualmachine", userVm).get(0); diff --git a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java index 3f4435308a51..ff7dfcec47d6 100755 --- a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java +++ b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java @@ -1249,7 +1249,27 @@ public boolean detachIso(long vmId, Long isoParamId, Boolean... extraParams) { _accountMgr.checkAccess(caller, null, true, virtualMachine); - Long isoId = !isVirtualRouter ? ((UserVm) virtualMachine).getIsoId() : isoParamId; + Long isoId; + if (isVirtualRouter) { + isoId = isoParamId; + } else { + Long primaryIsoId = ((UserVm) virtualMachine).getIsoId(); + List extras = _vmIsoMapDao.listByVmId(vmId); + if (isoParamId != null) { + boolean attached = (primaryIsoId != null && primaryIsoId.equals(isoParamId)) + || extras.stream().anyMatch(r -> r.getIsoId() == isoParamId); + if (!attached) { + throw new InvalidParameterValueException("The specified ISO is not attached to this Instance."); + } + isoId = isoParamId; + } else if (primaryIsoId == null && extras.isEmpty()) { + throw new InvalidParameterValueException("The specified instance has no ISO attached to it."); + } else if (!extras.isEmpty()) { + throw new InvalidParameterValueException("Instance has more than one ISO attached; specify the 'id' parameter to choose which to detach."); + } else { + isoId = primaryIsoId; + } + } if (isoId == null) { throw new InvalidParameterValueException("The specified instance has no ISO attached to it."); } @@ -1426,12 +1446,18 @@ private boolean attachISOToVM(long vmId, long userId, long isoId, boolean attach UserVmVO vm = _userVmDao.findById(vmId); VMTemplateVO iso = _tmpltDao.findById(isoId); - VmIsoMapVO highestExtra = highestCdromMapEntry(vmId); int targetSlot; + VmIsoMapVO mapEntry = null; if (attach) { + VmIsoMapVO highestExtra = highestCdromMapEntry(vmId); targetSlot = vm.getIsoId() == null ? 3 : (highestExtra == null ? 4 : highestExtra.getDeviceSeq() + 1); } else { - targetSlot = highestExtra != null ? highestExtra.getDeviceSeq() : 3; + if (vm.getIsoId() != null && vm.getIsoId() == isoId) { + targetSlot = 3; + } else { + mapEntry = _vmIsoMapDao.findByVmIdIsoId(vmId, isoId); + targetSlot = mapEntry != null ? mapEntry.getDeviceSeq() : 3; + } } boolean success = attachISOToVM(vmId, isoId, targetSlot, attach, forced, isVirtualRouter); @@ -1448,8 +1474,8 @@ private boolean attachISOToVM(long vmId, long userId, long isoId, boolean attach if (targetSlot == 3) { vm.setIsoId(null); _userVmDao.update(vmId, vm); - } else if (highestExtra != null) { - _vmIsoMapDao.remove(highestExtra.getId()); + } else if (mapEntry != null) { + _vmIsoMapDao.remove(mapEntry.getId()); } } return success; From b05b9f9939dbd3292ba80ee14715df251dba52cd Mon Sep 17 00:00:00 2001 From: Daman Arora Date: Mon, 4 May 2026 13:15:40 -0400 Subject: [PATCH 08/27] expose attached ISOs as isos[] in listVirtualMachines response --- .../api/response/AttachedIsoResponse.java | 67 +++++++++++++++++++ .../api/response/UserVmResponse.java | 12 ++++ .../api/query/dao/UserVmJoinDaoImpl.java | 16 +++++ 3 files changed, 95 insertions(+) create mode 100644 api/src/main/java/org/apache/cloudstack/api/response/AttachedIsoResponse.java diff --git a/api/src/main/java/org/apache/cloudstack/api/response/AttachedIsoResponse.java b/api/src/main/java/org/apache/cloudstack/api/response/AttachedIsoResponse.java new file mode 100644 index 000000000000..39af8923d268 --- /dev/null +++ b/api/src/main/java/org/apache/cloudstack/api/response/AttachedIsoResponse.java @@ -0,0 +1,67 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package org.apache.cloudstack.api.response; + +import org.apache.cloudstack.api.BaseResponse; + +import com.cloud.serializer.Param; +import com.google.gson.annotations.SerializedName; + +public class AttachedIsoResponse extends BaseResponse { + + @SerializedName("id") + @Param(description = "The ID of the attached ISO") + private String id; + + @SerializedName("name") + @Param(description = "The name of the attached ISO") + private String name; + + @SerializedName("displaytext") + @Param(description = "The display text of the attached ISO") + private String displayText; + + @SerializedName("deviceseq") + @Param(description = "The cdrom slot that holds this ISO (3=hdc, 4=hdd, ...)") + private Integer deviceSeq; + + public AttachedIsoResponse() { + } + + public AttachedIsoResponse(String id, String name, String displayText, Integer deviceSeq) { + this.id = id; + this.name = name; + this.displayText = displayText; + this.deviceSeq = deviceSeq; + } + + public String getId() { + return id; + } + + public String getName() { + return name; + } + + public String getDisplayText() { + return displayText; + } + + public Integer getDeviceSeq() { + return deviceSeq; + } +} diff --git a/api/src/main/java/org/apache/cloudstack/api/response/UserVmResponse.java b/api/src/main/java/org/apache/cloudstack/api/response/UserVmResponse.java index a7f6dff96f88..98919758ebe9 100644 --- a/api/src/main/java/org/apache/cloudstack/api/response/UserVmResponse.java +++ b/api/src/main/java/org/apache/cloudstack/api/response/UserVmResponse.java @@ -166,6 +166,10 @@ public class UserVmResponse extends BaseResponseWithTagInformation implements Co @Param(description = "An alternate display text of the ISO attached to the Instance") private String isoDisplayText; + @SerializedName("isos") + @Param(description = "All ISOs attached to the Instance, keyed by cdrom slot. The first entry mirrors isoid/isoname for back-compat.", responseObject = AttachedIsoResponse.class, since = "4.23.0") + private List isos; + @SerializedName(ApiConstants.SERVICE_OFFERING_ID) @Param(description = "The ID of the service offering of the Instance") private String serviceOfferingId; @@ -871,6 +875,14 @@ public void setIsoId(String isoId) { this.isoId = isoId; } + public void setIsos(List isos) { + this.isos = isos; + } + + public List getIsos() { + return isos; + } + public void setIsoName(String isoName) { this.isoName = isoName; } diff --git a/server/src/main/java/com/cloud/api/query/dao/UserVmJoinDaoImpl.java b/server/src/main/java/com/cloud/api/query/dao/UserVmJoinDaoImpl.java index 72690091e40e..34cc68469f51 100644 --- a/server/src/main/java/com/cloud/api/query/dao/UserVmJoinDaoImpl.java +++ b/server/src/main/java/com/cloud/api/query/dao/UserVmJoinDaoImpl.java @@ -128,6 +128,8 @@ public class UserVmJoinDaoImpl extends GenericDaoBaseWithTagInformation VmDetailSearch; @@ -244,6 +246,20 @@ public UserVmResponse newUserVmResponse(ResponseView view, String objectName, Us userVmResponse.setIsoId(userVm.getIsoUuid()); userVmResponse.setIsoName(userVm.getIsoName()); userVmResponse.setIsoDisplayText(userVm.getIsoDisplayText()); + + java.util.List attachedIsos = new java.util.ArrayList<>(); + if (userVm.getIsoUuid() != null) { + attachedIsos.add(new org.apache.cloudstack.api.response.AttachedIsoResponse( + userVm.getIsoUuid(), userVm.getIsoName(), userVm.getIsoDisplayText(), 3)); + } + for (com.cloud.vm.VmIsoMapVO row : vmIsoMapDao.listByVmId(userVm.getId())) { + com.cloud.storage.VMTemplateVO tmpl = vmTemplateDao.findById(row.getIsoId()); + if (tmpl != null) { + attachedIsos.add(new org.apache.cloudstack.api.response.AttachedIsoResponse( + tmpl.getUuid(), tmpl.getName(), tmpl.getDisplayText(), row.getDeviceSeq())); + } + } + userVmResponse.setIsos(attachedIsos); } if (details.contains(VMDetails.all) || details.contains(VMDetails.servoff)) { userVmResponse.setServiceOfferingId(userVm.getServiceOfferingUuid()); From 6d2ae5a775f5d2c6d24cff3e9c672aea7a94311b Mon Sep 17 00:00:00 2001 From: Daman Arora Date: Mon, 4 May 2026 13:34:59 -0400 Subject: [PATCH 09/27] extract CDROM_PRIMARY_DEVICE_SEQ constant --- .../com/cloud/template/TemplateManager.java | 7 ++++++ .../kvm/storage/KVMStorageProcessor.java | 4 ++- .../api/query/dao/UserVmJoinDaoImpl.java | 3 ++- .../cloud/template/TemplateManagerImpl.java | 25 ++++++++++++------- 4 files changed, 28 insertions(+), 11 deletions(-) diff --git a/engine/components-api/src/main/java/com/cloud/template/TemplateManager.java b/engine/components-api/src/main/java/com/cloud/template/TemplateManager.java index 593fcca3e4ae..8aa140ae8b37 100644 --- a/engine/components-api/src/main/java/com/cloud/template/TemplateManager.java +++ b/engine/components-api/src/main/java/com/cloud/template/TemplateManager.java @@ -71,6 +71,13 @@ public interface TemplateManager { true, ConfigKey.Scope.Global); + /** + * Device sequence for the bootable / primary cdrom slot. user_vm.iso_id has always pointed at this + * slot; the KVM agent's getDevLabel() maps it to hdc on the IDE bus. Any additional cdrom slots + * (held in vm_iso_map) start at {@code CDROM_PRIMARY_DEVICE_SEQ + 1} (hdd, hde, ...). + */ + int CDROM_PRIMARY_DEVICE_SEQ = 3; + static final String VMWARE_TOOLS_ISO = "vmware-tools.iso"; static final String XS_TOOLS_ISO = "xs-tools.iso"; diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java index 5f2183c3a38b..2d45155b3233 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java @@ -21,6 +21,8 @@ import static com.cloud.utils.NumbersUtil.toHumanReadableSize; import static com.cloud.utils.storage.S3.S3Utils.putFile; +import com.cloud.template.TemplateManager; + import java.io.File; import java.io.FileNotFoundException; import java.io.FileOutputStream; @@ -1188,7 +1190,7 @@ protected synchronized void attachOrDetachISO(final Connect conn, final String v LibvirtException, InternalErrorException { DiskDef iso = new DiskDef(); boolean isUefiEnabled = MapUtils.isNotEmpty(params) && params.containsKey("UEFI"); - Integer devId = (deviceSeq != null) ? deviceSeq : 3; + Integer devId = (deviceSeq != null) ? deviceSeq : TemplateManager.CDROM_PRIMARY_DEVICE_SEQ; if (isoPath != null && isAttach) { final int index = isoPath.lastIndexOf("/"); final String path = isoPath.substring(0, index); diff --git a/server/src/main/java/com/cloud/api/query/dao/UserVmJoinDaoImpl.java b/server/src/main/java/com/cloud/api/query/dao/UserVmJoinDaoImpl.java index 34cc68469f51..93ac319ab29e 100644 --- a/server/src/main/java/com/cloud/api/query/dao/UserVmJoinDaoImpl.java +++ b/server/src/main/java/com/cloud/api/query/dao/UserVmJoinDaoImpl.java @@ -250,7 +250,8 @@ public UserVmResponse newUserVmResponse(ResponseView view, String objectName, Us java.util.List attachedIsos = new java.util.ArrayList<>(); if (userVm.getIsoUuid() != null) { attachedIsos.add(new org.apache.cloudstack.api.response.AttachedIsoResponse( - userVm.getIsoUuid(), userVm.getIsoName(), userVm.getIsoDisplayText(), 3)); + userVm.getIsoUuid(), userVm.getIsoName(), userVm.getIsoDisplayText(), + com.cloud.template.TemplateManager.CDROM_PRIMARY_DEVICE_SEQ)); } for (com.cloud.vm.VmIsoMapVO row : vmIsoMapDao.listByVmId(userVm.getId())) { com.cloud.storage.VMTemplateVO tmpl = vmTemplateDao.findById(row.getIsoId()); diff --git a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java index ff7dfcec47d6..e629325ef242 100755 --- a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java +++ b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java @@ -697,7 +697,7 @@ public void prepareIsoForVmProfile(VirtualMachineProfile profile, DeployDestinat Map slotToIsoId = new HashMap<>(); if (vm.getIsoId() != null) { - slotToIsoId.put(3, vm.getIsoId()); + slotToIsoId.put(CDROM_PRIMARY_DEVICE_SEQ, vm.getIsoId()); } for (VmIsoMapVO row : _vmIsoMapDao.listByVmId(vm.getId())) { slotToIsoId.put(row.getDeviceSeq(), row.getIsoId()); @@ -705,10 +705,11 @@ public void prepareIsoForVmProfile(VirtualMachineProfile profile, DeployDestinat // Empty ISO DiskTOs pre-allocate cdrom drives at boot so runtime attachIso can media-swap into them. int cap = effectiveMaxCdroms(vm); - int neededForAttached = slotToIsoId.isEmpty() ? 0 : slotToIsoId.keySet().stream().max(Integer::compare).get() - 2; + int neededForAttached = slotToIsoId.isEmpty() ? 0 + : slotToIsoId.keySet().stream().max(Integer::compare).get() - (CDROM_PRIMARY_DEVICE_SEQ - 1); int totalSlots = Math.max(cap, neededForAttached); for (int slot = 0; slot < totalSlots; slot++) { - int diskSeq = 3 + slot; + int diskSeq = CDROM_PRIMARY_DEVICE_SEQ + slot; Long isoId = slotToIsoId.get(diskSeq); if (isoId != null) { TemplateInfo template = prepareIso(isoId, vm.getDataCenterId(), dest.getHost().getId(), poolId); @@ -716,7 +717,7 @@ public void prepareIsoForVmProfile(VirtualMachineProfile profile, DeployDestinat logger.error("Failed to prepare ISO on secondary or cache storage"); throw new CloudRuntimeException("Failed to prepare ISO on secondary or cache storage"); } - if (diskSeq == 3 && template.isBootable()) { + if (diskSeq == CDROM_PRIMARY_DEVICE_SEQ && template.isBootable()) { profile.setBootLoaderType(BootloaderType.CD); } GuestOSVO guestOS = _guestOSDao.findById(template.getGuestOSId()); @@ -1450,20 +1451,26 @@ private boolean attachISOToVM(long vmId, long userId, long isoId, boolean attach VmIsoMapVO mapEntry = null; if (attach) { VmIsoMapVO highestExtra = highestCdromMapEntry(vmId); - targetSlot = vm.getIsoId() == null ? 3 : (highestExtra == null ? 4 : highestExtra.getDeviceSeq() + 1); + if (vm.getIsoId() == null) { + targetSlot = CDROM_PRIMARY_DEVICE_SEQ; + } else if (highestExtra == null) { + targetSlot = CDROM_PRIMARY_DEVICE_SEQ + 1; + } else { + targetSlot = highestExtra.getDeviceSeq() + 1; + } } else { if (vm.getIsoId() != null && vm.getIsoId() == isoId) { - targetSlot = 3; + targetSlot = CDROM_PRIMARY_DEVICE_SEQ; } else { mapEntry = _vmIsoMapDao.findByVmIdIsoId(vmId, isoId); - targetSlot = mapEntry != null ? mapEntry.getDeviceSeq() : 3; + targetSlot = mapEntry != null ? mapEntry.getDeviceSeq() : CDROM_PRIMARY_DEVICE_SEQ; } } boolean success = attachISOToVM(vmId, isoId, targetSlot, attach, forced, isVirtualRouter); if (success && attach && !isVirtualRouter) { - if (targetSlot == 3) { + if (targetSlot == CDROM_PRIMARY_DEVICE_SEQ) { vm.setIsoId(iso.getId()); _userVmDao.update(vmId, vm); } else { @@ -1471,7 +1478,7 @@ private boolean attachISOToVM(long vmId, long userId, long isoId, boolean attach } } if (success && !attach && !isVirtualRouter) { - if (targetSlot == 3) { + if (targetSlot == CDROM_PRIMARY_DEVICE_SEQ) { vm.setIsoId(null); _userVmDao.update(vmId, vm); } else if (mapEntry != null) { From a44b4fb7b1efb2ba81709f2afa559e7c1b95e77e Mon Sep 17 00:00:00 2001 From: Daman Arora Date: Mon, 4 May 2026 13:57:38 -0400 Subject: [PATCH 10/27] unit tests for cdrom slot allocation logic --- .../cloud/template/TemplateManagerImpl.java | 4 +- .../template/TemplateManagerImplTest.java | 59 +++++++++++++++++++ 2 files changed, 61 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java index e629325ef242..e1514a2acd2b 100755 --- a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java +++ b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java @@ -1443,7 +1443,7 @@ private boolean attachISOToVM(long vmId, long isoId, int deviceSeq, boolean atta return (a != null && a.getResult()); } - private boolean attachISOToVM(long vmId, long userId, long isoId, boolean attach, boolean forced, boolean isVirtualRouter) { + boolean attachISOToVM(long vmId, long userId, long isoId, boolean attach, boolean forced, boolean isVirtualRouter) { UserVmVO vm = _userVmDao.findById(vmId); VMTemplateVO iso = _tmpltDao.findById(isoId); @@ -1488,7 +1488,7 @@ private boolean attachISOToVM(long vmId, long userId, long isoId, boolean attach return success; } - private VmIsoMapVO highestCdromMapEntry(long vmId) { + VmIsoMapVO highestCdromMapEntry(long vmId) { VmIsoMapVO highest = null; for (VmIsoMapVO row : _vmIsoMapDao.listByVmId(vmId)) { if (highest == null || row.getDeviceSeq() > highest.getDeviceSeq()) { diff --git a/server/src/test/java/com/cloud/template/TemplateManagerImplTest.java b/server/src/test/java/com/cloud/template/TemplateManagerImplTest.java index 6288180a9f4e..1a27b9ddcd77 100755 --- a/server/src/test/java/com/cloud/template/TemplateManagerImplTest.java +++ b/server/src/test/java/com/cloud/template/TemplateManagerImplTest.java @@ -220,6 +220,12 @@ public class TemplateManagerImplTest extends TestCase { @Mock HeuristicRuleHelper heuristicRuleHelperMock; + @Mock + com.cloud.vm.dao.UserVmDao _userVmDao; + + @Mock + com.cloud.vm.dao.VmIsoMapDao _vmIsoMapDao; + public class CustomThreadPoolExecutor extends ThreadPoolExecutor { AtomicInteger ai = new AtomicInteger(0); public CustomThreadPoolExecutor(int corePoolSize, int maximumPoolSize, long keepAliveTime, TimeUnit unit, @@ -750,6 +756,59 @@ public void verifyHeuristicRulesForZoneTestTemplateNotISOFormatShouldCheckForTem Mockito.verify(heuristicRuleHelperMock, Mockito.times(1)).getImageStoreIfThereIsHeuristicRule(1L, HeuristicType.TEMPLATE, vmTemplateVOMock); } + @Test + public void highestCdromMapEntryReturnsNullWhenMapIsEmpty() { + Mockito.when(_vmIsoMapDao.listByVmId(1L)).thenReturn(new ArrayList<>()); + Assert.assertNull(templateManager.highestCdromMapEntry(1L)); + } + + @Test + public void highestCdromMapEntryReturnsEntryWithMaxDeviceSeq() { + com.cloud.vm.VmIsoMapVO low = new com.cloud.vm.VmIsoMapVO(1L, 100L, 4); + com.cloud.vm.VmIsoMapVO high = new com.cloud.vm.VmIsoMapVO(1L, 200L, 5); + Mockito.when(_vmIsoMapDao.listByVmId(1L)).thenReturn(java.util.Arrays.asList(low, high)); + com.cloud.vm.VmIsoMapVO result = templateManager.highestCdromMapEntry(1L); + Assert.assertNotNull(result); + Assert.assertEquals(5, result.getDeviceSeq()); + } + + @Test + public void attachISOToVMAttachWritesToIsoIdWhenPrimarySlotEmpty() { + com.cloud.vm.UserVmVO vm = Mockito.mock(com.cloud.vm.UserVmVO.class); + com.cloud.storage.VMTemplateVO iso = Mockito.mock(com.cloud.storage.VMTemplateVO.class); + Mockito.when(_userVmDao.findById(1L)).thenReturn(vm); + Mockito.when(vmTemplateDao.findById(42L)).thenReturn(iso); + Mockito.when(iso.getId()).thenReturn(42L); + Mockito.when(vm.getIsoId()).thenReturn(null); + Mockito.when(_vmIsoMapDao.listByVmId(1L)).thenReturn(new ArrayList<>()); + + boolean result = templateManager.attachISOToVM(1L, 1L, 42L, true, false, false); + + Assert.assertTrue(result); + Mockito.verify(vm).setIsoId(42L); + Mockito.verify(_userVmDao).update(eq(1L), eq(vm)); + Mockito.verify(_vmIsoMapDao, Mockito.never()).persist(any(com.cloud.vm.VmIsoMapVO.class)); + } + + @Test + public void attachISOToVMAttachWritesToVmIsoMapWhenPrimarySlotOccupied() { + com.cloud.vm.UserVmVO vm = Mockito.mock(com.cloud.vm.UserVmVO.class); + com.cloud.storage.VMTemplateVO iso = Mockito.mock(com.cloud.storage.VMTemplateVO.class); + Mockito.when(_userVmDao.findById(1L)).thenReturn(vm); + Mockito.when(vmTemplateDao.findById(42L)).thenReturn(iso); + Mockito.when(iso.getId()).thenReturn(42L); + Mockito.when(vm.getIsoId()).thenReturn(99L); + Mockito.when(_vmIsoMapDao.listByVmId(1L)).thenReturn(new ArrayList<>()); + + boolean result = templateManager.attachISOToVM(1L, 1L, 42L, true, false, false); + + Assert.assertTrue(result); + Mockito.verify(_vmIsoMapDao).persist(Mockito.argThat(row -> + row.getVmId() == 1L && row.getIsoId() == 42L + && row.getDeviceSeq() == TemplateManager.CDROM_PRIMARY_DEVICE_SEQ + 1)); + Mockito.verify(vm, Mockito.never()).setIsoId(anyLong()); + } + @Configuration @ComponentScan(basePackageClasses = {TemplateManagerImpl.class}, includeFilters = {@ComponentScan.Filter(value = TestConfiguration.Library.class, type = FilterType.CUSTOM)}, From a6afd26794ebc9afee39bba6c3c4f5f8d1aecbf2 Mon Sep 17 00:00:00 2001 From: Daman Arora Date: Mon, 4 May 2026 14:24:41 -0400 Subject: [PATCH 11/27] implement multi-ISO attachment and detachment for VMs with enhanced validation --- ui/src/config/section/compute.js | 29 +++-- ui/src/views/compute/AttachIso.vue | 108 +++++++++++------ ui/src/views/compute/DetachIso.vue | 188 +++++++++++++++++++++++++++++ 3 files changed, 278 insertions(+), 47 deletions(-) create mode 100644 ui/src/views/compute/DetachIso.vue diff --git a/ui/src/config/section/compute.js b/ui/src/config/section/compute.js index dd17116df02d..df2aa861af75 100644 --- a/ui/src/config/section/compute.js +++ b/ui/src/config/section/compute.js @@ -299,7 +299,15 @@ export default { docHelp: 'adminguide/templates.html#attaching-an-iso-to-a-vm', dataView: true, popup: true, - show: (record) => { return record.hypervisor !== 'External' && ['Running', 'Stopped'].includes(record.state) && !record.isoid && record.vmtype !== 'sharedfsvm' }, + show: (record) => { + if (record.hypervisor === 'External' || !['Running', 'Stopped'].includes(record.state) || record.vmtype === 'sharedfsvm') { + return false + } + + const attached = (record.isos && record.isos.length) ? record.isos.length : (record.isoid ? 1 : 0) + const cap = record.hypervisor === 'KVM' ? 2 : 1 + return attached < cap + }, disabled: (record) => { return record.hostcontrolstate === 'Offline' || record.hostcontrolstate === 'Maintenance' }, component: shallowRef(defineAsyncComponent(() => import('@/views/compute/AttachIso.vue'))) }, @@ -307,22 +315,17 @@ export default { api: 'detachIso', icon: 'link-outlined', label: 'label.action.detach.iso', - message: 'message.detach.iso.confirm', dataView: true, - args: (record, store) => { - var args = ['virtualmachineid'] - if (record && record.hypervisor && record.hypervisor === 'VMware') { - args.push('forced') + popup: true, + show: (record) => { + if (record.hypervisor === 'External' || !['Running', 'Stopped'].includes(record.state) || record.vmtype === 'sharedfsvm') { + return false } - return args + const attached = (record.isos && record.isos.length) ? record.isos.length : (record.isoid ? 1 : 0) + return attached > 0 }, - show: (record) => { return record.hypervisor !== 'External' && ['Running', 'Stopped'].includes(record.state) && 'isoid' in record && record.isoid && record.vmtype !== 'sharedfsvm' }, disabled: (record) => { return record.hostcontrolstate === 'Offline' || record.hostcontrolstate === 'Maintenance' }, - mapping: { - virtualmachineid: { - value: (record, params) => { return record.id } - } - } + component: shallowRef(defineAsyncComponent(() => import('@/views/compute/DetachIso.vue'))) }, { api: 'updateVMAffinityGroup', diff --git a/ui/src/views/compute/AttachIso.vue b/ui/src/views/compute/AttachIso.vue index 60694cb8f57b..936ea03c5957 100644 --- a/ui/src/views/compute/AttachIso.vue +++ b/ui/src/views/compute/AttachIso.vue @@ -23,17 +23,25 @@ :rules="rules" layout="vertical" @finish="handleSubmit"> - + - + {{ iso.displaytext || iso.name }} @@ -69,19 +77,46 @@ export default { data () { return { loading: false, - isos: [] + isos: [], + maxSelections: 1 } }, created () { + this.computeMaxSelections() this.initForm() this.fetchData() }, + watch: { + 'form.ids' (newVal) { + // Cap the multi-select at maxSelections — server-side cap enforcement still applies, but + // this prevents the user from picking more than the VM can hold and getting a partial-success + // response. + if (newVal && newVal.length > this.maxSelections) { + this.form.ids = newVal.slice(0, this.maxSelections) + this.$message.warning(this.$t('label.iso.name') + ': max ' + this.maxSelections) + } + } + }, methods: { + computeMaxSelections () { + // Mirrors the server-side effectiveMaxCdroms: KVM caps at 2 cdrom slots (IDE bus reality), + // other hypervisors at 1. Subtract whatever's already attached to know how many MORE + // can be added in this dialog. + const hypervisorCap = this.resource.hypervisor === 'KVM' ? 2 : 1 + const alreadyAttached = (this.resource.isos && this.resource.isos.length) || + (this.resource.isoid ? 1 : 0) + this.maxSelections = Math.max(1, hypervisorCap - alreadyAttached) + }, initForm () { this.formRef = ref() - this.form = reactive({}) + this.form = reactive({ ids: [] }) this.rules = reactive({ - id: [{ required: true, message: `${this.$t('label.required')}` }] + ids: [{ + required: true, + type: 'array', + min: 1, + message: `${this.$t('label.required')}` + }] }) }, fetchData () { @@ -93,9 +128,6 @@ export default { }) Promise.all(promises).then(() => { this.isos = _.uniqBy(this.isos, 'id') - if (this.isos.length > 0) { - this.form.id = this.isos[0].id - } }).catch((error) => { console.log(error) }).finally(() => { @@ -127,35 +159,43 @@ export default { if (this.loading) return this.formRef.value.validate().then(() => { const values = toRaw(this.form) - const params = { - id: values.id, - virtualmachineid: this.resource.id - } - - if (values.forced) { - params.forced = values.forced - } + const ids = values.ids || [] + if (ids.length === 0) return this.loading = true const title = this.$t('label.action.attach.iso') - postAPI('attachIso', params).then(json => { - const jobId = json.attachisoresponse.jobid - if (jobId) { - this.$pollJob({ - jobId, - title, - description: values.id, - successMessage: `${this.$t('label.action.attach.iso')} ${this.$t('label.success')}`, - loadingMessage: `${title} ${this.$t('label.in.progress')}`, - catchMessage: this.$t('error.fetching.async.job.result') - }) + // CloudStack's attachIso API is single-ISO. We fan out one call per selected ISO and + // surface the first error if any. The server-side cap already prevents over-attachment. + const sendOne = (isoId) => { + const params = { + id: isoId, + virtualmachineid: this.resource.id } - this.closeAction() - }).catch(error => { - this.$notifyError(error) - }).finally(() => { - this.loading = false - }) + if (values.forced) { + params.forced = values.forced + } + return new Promise((resolve, reject) => { + postAPI('attachIso', params).then(json => { + const jobId = json.attachisoresponse && json.attachisoresponse.jobid + if (jobId) { + this.$pollJob({ + jobId, + title, + description: isoId, + successMessage: `${this.$t('label.action.attach.iso')} ${this.$t('label.success')}`, + loadingMessage: `${title} ${this.$t('label.in.progress')}`, + catchMessage: this.$t('error.fetching.async.job.result') + }) + } + resolve() + }).catch(reject) + }) + } + + ids.reduce((p, id) => p.then(() => sendOne(id)), Promise.resolve()) + .then(() => { this.closeAction() }) + .catch(error => { this.$notifyError(error) }) + .finally(() => { this.loading = false }) }).catch(error => { this.formRef.value.scrollToField(error.errorFields[0].name) }) diff --git a/ui/src/views/compute/DetachIso.vue b/ui/src/views/compute/DetachIso.vue new file mode 100644 index 000000000000..d9287f4e9778 --- /dev/null +++ b/ui/src/views/compute/DetachIso.vue @@ -0,0 +1,188 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + + + From 71fe08298211d7153b0d9cf17fb58f629cdae46a Mon Sep 17 00:00:00 2001 From: Daman Arora Date: Mon, 4 May 2026 14:25:45 -0400 Subject: [PATCH 12/27] implement multi-ISO display in InstanceTab with computed property for attached ISOs --- ui/src/views/compute/InstanceTab.vue | 34 ++++++++++++++++++++++++---- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/ui/src/views/compute/InstanceTab.vue b/ui/src/views/compute/InstanceTab.vue index 9576e70c8d58..adfb194f69c4 100644 --- a/ui/src/views/compute/InstanceTab.vue +++ b/ui/src/views/compute/InstanceTab.vue @@ -28,10 +28,14 @@ - - - {{ vm.isoname }}
- {{ vm.isoid }} + +
+ + {{ iso.displaytext || iso.name }} + {{ slotLabel(iso.deviceseq) }} +
+ {{ iso.id }} +
0) { + return [...this.vm.isos].sort((a, b) => (a.deviceseq || 0) - (b.deviceseq || 0)) + } + if (this.vm.isoid) { + return [{ + id: this.vm.isoid, + name: this.vm.isoname, + displaytext: this.vm.isodisplaytext, + deviceseq: 3 + }] + } + return [] + } + }, methods: { + slotLabel (deviceseq) { + // 3 -> hdc, 4 -> hdd, ... matches LibvirtVMDef.getDevLabel for the IDE bus on KVM. + if (typeof deviceseq !== 'number') return '' + return 'hd' + String.fromCharCode('a'.charCodeAt(0) + deviceseq - 1) + }, setCurrentTab () { this.currentTab = this.$route.query.tab ? this.$route.query.tab : 'details' }, From 2ff3ce629bf919a3d3f5a0ec6de6c571576af929 Mon Sep 17 00:00:00 2001 From: Daman Arora Date: Mon, 4 May 2026 15:55:08 -0400 Subject: [PATCH 13/27] add warning alert for max CDROM selections and enhance global capacity fetching --- ui/src/views/compute/AttachIso.vue | 38 +++++++++++++++++++++++++----- 1 file changed, 32 insertions(+), 6 deletions(-) diff --git a/ui/src/views/compute/AttachIso.vue b/ui/src/views/compute/AttachIso.vue index 936ea03c5957..5a3c5a57e790 100644 --- a/ui/src/views/compute/AttachIso.vue +++ b/ui/src/views/compute/AttachIso.vue @@ -17,6 +17,12 @@