Skip to content

Commit ea6cbad

Browse files
authored
Multiple CD-ROM / ISO Support Per VM (#13101)
* pre-allocate a second empty cdrom slot at boot (hardcoded) * drive cdrom slot count via vm.cdrom.max.count ConfigKey * add vm_iso_map table + VO/DAO * persist multi-ISO state via vm_iso_map * carry target cdrom slot through AttachCommand to KVM agent * enforce per-VM cdrom cap, clamp to hypervisor max * make detachIso accepts an ISO id * expose attached ISOs as isos[] in listVirtualMachines response * extract CDROM_PRIMARY_DEVICE_SEQ constant * unit tests for cdrom slot allocation logic * implement multi-ISO attachment and detachment for VMs with enhanced validation * implement multi-ISO display in InstanceTab with computed property for attached ISOs * add warning alert for max CDROM selections and enhance global capacity fetching * enhance ISO attachment validation to handle multiple ISOs and prevent duplicates * refactor ISO attachment logic for detachment and validation * add unit tests for ISO detachment resolution and validation logic * add mock for VmIsoMapDao in UserVmJoinDaoImplTest and set lenient behavior for listByVmId * refactor ISO attachment logic and enhance UI for multi-CDROM management * refactor ISO attachment methods to use VM ID and improve parameter handling * remove unnecessary mock for VM ISO mapping in TemplateManagerImplTest * add 'since' attribute to ISO detach command parameter description * scope vm.cdrom.max.count to cluster * add support for configurable CD-ROM count per VM and improve handling in TemplateManager * add HostDetailsDao mock to UserVmJoinDaoImplTest * fix: handle null poolId when loading attached ISO slots in prepareIsoForVmProfile * implement listByIsoId method in VmIsoMapDao and update TemplateManagerImpl for ISO deletion checks * improve logging messages for ISO deletion checks * add unit tests for CD-ROM handling and enforce limits in TemplateManager * refactor: update configuration value handling and improve notification logic * refactor: rename CD-ROM references to ISO and update related logic * refactor: enhance effective CD-ROM max count logic to handle missing host IDs and improve cluster ID retrieval * refactor: enhance effective CD-ROM max count logic to handle misconfigurations during VM boot * refactor: enhance effective CD-ROM max count logic to retrieve host ID from candidates based on hypervisor type * refactor: enhance host ID retrieval logic for VMs based on hypervisor type * feat: add bootable ISO flag to AttachedIsoResponse and update UI to display it * refactor: simplify effectiveMaxCdroms method and improve logging for CD-ROM capacity * test: update AttachedIsoResponseTest to include bootable flag in constructor tests * feat: include bootable flag in AttachedIsoResponse for user VMs * feat: enhance CD-ROM management by defining empty slots for user VMs
1 parent 21e4475 commit ea6cbad

24 files changed

Lines changed: 1353 additions & 110 deletions

File tree

api/src/main/java/com/cloud/host/Host.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ public static String[] toStrings(Host.Type... types) {
6363
String HOST_OVFTOOL_VERSION = "host.ovftool.version";
6464
String HOST_VIRTV2V_VERSION = "host.virtv2v.version";
6565
String HOST_SSH_PORT = "host.ssh.port";
66+
String HOST_CDROM_MAX_COUNT = "host.cdrom.max.count";
6667
String GUEST_OS_CATEGORY_ID = "guest.os.category.id";
6768
String GUEST_OS_RULE = "guest.os.rule";
6869

api/src/main/java/org/apache/cloudstack/api/command/user/iso/DetachIsoCmd.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import org.apache.cloudstack.api.ServerApiException;
2828
import org.apache.cloudstack.api.command.user.UserCmd;
2929
import org.apache.cloudstack.api.command.user.vm.DeployVMCmd;
30+
import org.apache.cloudstack.api.response.TemplateResponse;
3031
import org.apache.cloudstack.api.response.UserVmResponse;
3132

3233
import com.cloud.event.EventTypes;
@@ -51,6 +52,10 @@ public class DetachIsoCmd extends BaseAsyncCmd implements UserCmd {
5152
description = "If true, ejects the ISO before detaching on VMware. Default: false", since = "4.15.1")
5253
protected Boolean forced;
5354

55+
@Parameter(name = ApiConstants.ID, type = CommandType.UUID, entityType = TemplateResponse.class,
56+
description = "The ID of the ISO to detach. Required when the Instance has more than one ISO attached.", since = "4.23.0")
57+
protected Long id;
58+
5459
/////////////////////////////////////////////////////
5560
/////////////////// Accessors ///////////////////////
5661
/////////////////////////////////////////////////////
@@ -104,7 +109,7 @@ public ApiCommandResourceType getApiResourceType() {
104109

105110
@Override
106111
public void execute() {
107-
boolean result = _templateService.detachIso(virtualMachineId, null, isForced());
112+
boolean result = _templateService.detachIso(virtualMachineId, id, isForced());
108113
if (result) {
109114
UserVm userVm = _entityMgr.findById(UserVm.class, virtualMachineId);
110115
UserVmResponse response = _responseGenerator.createUserVmResponse(getResponseView(), "virtualmachine", userVm).get(0);
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
package org.apache.cloudstack.api.response;
18+
19+
import org.apache.cloudstack.api.BaseResponse;
20+
21+
import com.cloud.serializer.Param;
22+
import com.google.gson.annotations.SerializedName;
23+
24+
public class AttachedIsoResponse extends BaseResponse {
25+
26+
@SerializedName("id")
27+
@Param(description = "The ID of the attached ISO")
28+
private String id;
29+
30+
@SerializedName("name")
31+
@Param(description = "The name of the attached ISO")
32+
private String name;
33+
34+
@SerializedName("displaytext")
35+
@Param(description = "The display text of the attached ISO")
36+
private String displayText;
37+
38+
@SerializedName("deviceseq")
39+
@Param(description = "The cdrom slot that holds this ISO (3=hdc, 4=hdd, ...)")
40+
private Integer deviceSeq;
41+
42+
@SerializedName("bootable")
43+
@Param(description = "Whether this is the bootable ISO for the VM")
44+
private Boolean bootable;
45+
46+
public AttachedIsoResponse() {
47+
}
48+
49+
public AttachedIsoResponse(String id, String name, String displayText, Integer deviceSeq, boolean bootable) {
50+
this.id = id;
51+
this.name = name;
52+
this.displayText = displayText;
53+
this.deviceSeq = deviceSeq;
54+
this.bootable = bootable;
55+
}
56+
57+
public String getId() {
58+
return id;
59+
}
60+
61+
public String getName() {
62+
return name;
63+
}
64+
65+
public String getDisplayText() {
66+
return displayText;
67+
}
68+
69+
public Integer getDeviceSeq() {
70+
return deviceSeq;
71+
}
72+
73+
public Boolean getBootable() {
74+
return bootable;
75+
}
76+
}

api/src/main/java/org/apache/cloudstack/api/response/UserVmResponse.java

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,14 @@ public class UserVmResponse extends BaseResponseWithTagInformation implements Co
166166
@Param(description = "An alternate display text of the ISO attached to the Instance")
167167
private String isoDisplayText;
168168

169+
@SerializedName("isos")
170+
@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")
171+
private List<AttachedIsoResponse> isos;
172+
173+
@SerializedName("isomaxcount")
174+
@Param(description = "Maximum number of ISOs that may be attached to this Instance, after applying the cluster-scoped vm.iso.max.count and the hypervisor's own cap.", since = "4.23.0")
175+
private Integer isoMaxCount;
176+
169177
@SerializedName(ApiConstants.SERVICE_OFFERING_ID)
170178
@Param(description = "The ID of the service offering of the Instance")
171179
private String serviceOfferingId;
@@ -871,6 +879,22 @@ public void setIsoId(String isoId) {
871879
this.isoId = isoId;
872880
}
873881

882+
public void setIsos(List<AttachedIsoResponse> isos) {
883+
this.isos = isos;
884+
}
885+
886+
public List<AttachedIsoResponse> getIsos() {
887+
return isos;
888+
}
889+
890+
public void setIsoMaxCount(Integer isoMaxCount) {
891+
this.isoMaxCount = isoMaxCount;
892+
}
893+
894+
public Integer getIsoMaxCount() {
895+
return isoMaxCount;
896+
}
897+
874898
public void setIsoName(String isoName) {
875899
this.isoName = isoName;
876900
}
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
package org.apache.cloudstack.api.response;
18+
19+
import org.junit.Assert;
20+
import org.junit.Test;
21+
import org.junit.runner.RunWith;
22+
import org.mockito.junit.MockitoJUnitRunner;
23+
24+
@RunWith(MockitoJUnitRunner.class)
25+
public final class AttachedIsoResponseTest {
26+
27+
@Test
28+
public void testFullConstructorPopulatesAllFields() {
29+
AttachedIsoResponse response = new AttachedIsoResponse("uuid-1", "alpine-iso", "Alpine boot", 3, true);
30+
Assert.assertEquals("uuid-1", response.getId());
31+
Assert.assertEquals("alpine-iso", response.getName());
32+
Assert.assertEquals("Alpine boot", response.getDisplayText());
33+
Assert.assertEquals(Integer.valueOf(3), response.getDeviceSeq());
34+
Assert.assertTrue(response.getBootable());
35+
}
36+
37+
@Test
38+
public void testNoArgConstructorLeavesFieldsNull() {
39+
AttachedIsoResponse response = new AttachedIsoResponse();
40+
Assert.assertNull(response.getId());
41+
Assert.assertNull(response.getName());
42+
Assert.assertNull(response.getDisplayText());
43+
Assert.assertNull(response.getDeviceSeq());
44+
Assert.assertNull(response.getBootable());
45+
}
46+
}

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,21 @@ public interface TemplateManager {
6464
true,
6565
ConfigKey.Scope.Global);
6666

67+
ConfigKey<Integer> VmIsoMaxCount = new ConfigKey<Integer>("Advanced",
68+
Integer.class,
69+
"vm.iso.max.count", "1",
70+
"Maximum number of ISOs that may be attached to a VM.",
71+
true,
72+
ConfigKey.Scope.Cluster);
73+
74+
// KVM/libvirt maps deviceSeq=3 to hdc (hda/hdb are taken by the root volume on i440fx/IDE).
75+
// user_vm.iso_id has always pointed at this slot; additional cdroms live in vm_iso_map.
76+
int CDROM_PRIMARY_DEVICE_SEQ = 3;
77+
78+
// Fallback per-VM cdrom cap when the placement host hasn't advertised host.cdrom.max.count
79+
// (older agent, never-deployed VM, etc.).
80+
int DEFAULT_CDROM_MAX_PER_VM = 1;
81+
6782
static final String VMWARE_TOOLS_ISO = "vmware-tools.iso";
6883
static final String XS_TOOLS_ISO = "xs-tools.iso";
6984

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
package com.cloud.vm;
18+
19+
import java.util.Date;
20+
21+
import javax.persistence.Column;
22+
import javax.persistence.Entity;
23+
import javax.persistence.GeneratedValue;
24+
import javax.persistence.GenerationType;
25+
import javax.persistence.Id;
26+
import javax.persistence.Table;
27+
import javax.persistence.Temporal;
28+
import javax.persistence.TemporalType;
29+
30+
import org.apache.cloudstack.api.InternalIdentity;
31+
32+
@Entity
33+
@Table(name = "vm_iso_map")
34+
public class VmIsoMapVO implements InternalIdentity {
35+
@Id
36+
@GeneratedValue(strategy = GenerationType.IDENTITY)
37+
@Column(name = "id")
38+
private Long id;
39+
40+
@Column(name = "vm_id")
41+
private long vmId;
42+
43+
@Column(name = "iso_id")
44+
private long isoId;
45+
46+
@Column(name = "device_seq")
47+
private int deviceSeq;
48+
49+
@Column(name = "created")
50+
@Temporal(TemporalType.TIMESTAMP)
51+
private Date created;
52+
53+
public VmIsoMapVO() {
54+
}
55+
56+
public VmIsoMapVO(long vmId, long isoId, int deviceSeq) {
57+
this.vmId = vmId;
58+
this.isoId = isoId;
59+
this.deviceSeq = deviceSeq;
60+
this.created = new Date();
61+
}
62+
63+
@Override
64+
public long getId() {
65+
return id;
66+
}
67+
68+
public long getVmId() {
69+
return vmId;
70+
}
71+
72+
public long getIsoId() {
73+
return isoId;
74+
}
75+
76+
public int getDeviceSeq() {
77+
return deviceSeq;
78+
}
79+
80+
public Date getCreated() {
81+
return created;
82+
}
83+
}
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
package com.cloud.vm.dao;
18+
19+
import java.util.List;
20+
21+
import com.cloud.utils.db.GenericDao;
22+
import com.cloud.vm.VmIsoMapVO;
23+
24+
public interface VmIsoMapDao extends GenericDao<VmIsoMapVO, Long> {
25+
List<VmIsoMapVO> listByVmId(long vmId);
26+
27+
List<VmIsoMapVO> listByIsoId(long isoId);
28+
29+
VmIsoMapVO findByVmIdDeviceSeq(long vmId, int deviceSeq);
30+
31+
VmIsoMapVO findByVmIdIsoId(long vmId, long isoId);
32+
33+
int removeByVmId(long vmId);
34+
}

0 commit comments

Comments
 (0)