Skip to content

Commit 98e2ed3

Browse files
Pearl1594Pearl Dsilva
Pearl1594
and
Pearl Dsilva
authored
vmware: Add force parameter to iso attach/detach operations (#4907)
Fixes: #4808, #4941 This PR adds a force flag to the attachIso / detachIso commands, especially for VMware where it is noticed that when trying to either detach an iso or attach an iso when there already exists another present it fails to do the necessary operation as from ACS end we either answer the question returned by Esxi for CDRom disconnect operation as No (for detach operation) or do not answer the question at all (for Attach operation). Co-authored-by: Pearl Dsilva <pearl.dsilva@shapeblue.com>
1 parent 73f82ae commit 98e2ed3

13 files changed

Lines changed: 215 additions & 138 deletions

File tree

‎api/src/main/java/com/cloud/template/TemplateApiService.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,9 @@ public interface TemplateApiService {
5656

5757
VirtualMachineTemplate prepareTemplate(long templateId, long zoneId, Long storageId);
5858

59-
boolean detachIso(long vmId);
59+
boolean detachIso(long vmId, boolean forced);
6060

61-
boolean attachIso(long isoId, long vmId);
61+
boolean attachIso(long isoId, long vmId, boolean forced);
6262

6363
/**
6464
* Deletes a template

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,10 @@ public class AttachIsoCmd extends BaseAsyncCmd implements UserCmd {
5454
required = true, description = "the ID of the virtual machine")
5555
protected Long virtualMachineId;
5656

57+
@Parameter(name = ApiConstants.FORCED, type = CommandType.BOOLEAN,
58+
description = "If true, ejects existing ISO before attaching on VMware. Default: false", since = "4.15.1")
59+
protected Boolean forced;
60+
5761
/////////////////////////////////////////////////////
5862
/////////////////// Accessors ///////////////////////
5963
/////////////////////////////////////////////////////
@@ -66,6 +70,10 @@ public Long getVirtualMachineId() {
6670
return virtualMachineId;
6771
}
6872

73+
public Boolean isForced() {
74+
return forced != null;
75+
}
76+
6977
/////////////////////////////////////////////////////
7078
/////////////// API Implementation///////////////////
7179
/////////////////////////////////////////////////////
@@ -98,7 +106,7 @@ public String getEventDescription() {
98106
@Override
99107
public void execute() {
100108
CallContext.current().setEventDetails("Vm Id: " + getVirtualMachineId() + " ISO ID: " + getId());
101-
boolean result = _templateService.attachIso(id, virtualMachineId);
109+
boolean result = _templateService.attachIso(id, virtualMachineId, isForced());
102110
if (result) {
103111
UserVm userVm = _responseGenerator.findUserVmById(virtualMachineId);
104112
if (userVm != null) {

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

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,14 @@ public class DetachIsoCmd extends BaseAsyncCmd implements UserCmd {
4444
//////////////// API parameters /////////////////////
4545
/////////////////////////////////////////////////////
4646

47-
@Parameter(name=ApiConstants.VIRTUAL_MACHINE_ID, type=CommandType.UUID, entityType = UserVmResponse.class,
48-
required=true, description="The ID of the virtual machine")
47+
@Parameter(name = ApiConstants.VIRTUAL_MACHINE_ID, type = CommandType.UUID, entityType = UserVmResponse.class,
48+
required = true, description = "The ID of the virtual machine")
4949
protected Long virtualMachineId;
5050

51+
@Parameter(name = ApiConstants.FORCED, type = CommandType.BOOLEAN,
52+
description = "If true, ejects the ISO before detaching on VMware. Default: false", since = "4.15.1")
53+
protected Boolean forced;
54+
5155
/////////////////////////////////////////////////////
5256
/////////////////// Accessors ///////////////////////
5357
/////////////////////////////////////////////////////
@@ -56,6 +60,10 @@ public Long getVirtualMachineId() {
5660
return virtualMachineId;
5761
}
5862

63+
public Boolean isForced() {
64+
return forced != null;
65+
}
66+
5967
/////////////////////////////////////////////////////
6068
/////////////// API Implementation///////////////////
6169
/////////////////////////////////////////////////////
@@ -87,7 +95,7 @@ public String getEventDescription() {
8795

8896
@Override
8997
public void execute() {
90-
boolean result = _templateService.detachIso(virtualMachineId);
98+
boolean result = _templateService.detachIso(virtualMachineId, isForced());
9199
if (result) {
92100
UserVm userVm = _entityMgr.findById(UserVm.class, virtualMachineId);
93101
UserVmResponse response = _responseGenerator.createUserVmResponse(getResponseView(), "virtualmachine", userVm).get(0);

‎core/src/main/java/org/apache/cloudstack/storage/command/AttachCommand.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ public final class AttachCommand extends StorageSubSystemCommand {
2727
private DiskTO disk;
2828
private String vmName;
2929
private boolean inSeq = false;
30+
private boolean forced = false;
3031
private Map<String, String> controllerInfo;
3132

3233
public AttachCommand(final DiskTO disk, final String vmName) {
@@ -69,6 +70,14 @@ public void setVmName(final String vmName) {
6970
this.vmName = vmName;
7071
}
7172

73+
public boolean isForced() {
74+
return forced;
75+
}
76+
77+
public void setForced(boolean forced) {
78+
this.forced = forced;
79+
}
80+
7281
@Override
7382
public void setExecuteInSequence(final boolean inSeq) {
7483
this.inSeq = inSeq;

‎core/src/main/java/org/apache/cloudstack/storage/command/DettachCommand.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ public class DettachCommand extends StorageSubSystemCommand {
3131
private String _storageHost;
3232
private int _storagePort;
3333
private Map<String, String> params;
34+
private boolean forced;
3435

3536
public DettachCommand(final DiskTO disk, final String vmName) {
3637
super();
@@ -106,6 +107,14 @@ public void setParams(Map<String, String> params) {
106107
this.params = params;
107108
}
108109

110+
public boolean isForced() {
111+
return forced;
112+
}
113+
114+
public void setForced(boolean forced) {
115+
this.forced = forced;
116+
}
117+
109118
@Override
110119
public void setExecuteInSequence(final boolean inSeq) {
111120

‎plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java‎

Lines changed: 51 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -2019,35 +2019,7 @@ protected StartAnswer execute(StartCommand cmd) {
20192019
if (volIso != null) {
20202020
for (DiskTO vol : disks) {
20212021
if (vol.getType() == Volume.Type.ISO) {
2022-
2023-
TemplateObjectTO iso = (TemplateObjectTO) vol.getData();
2024-
2025-
if (iso.getPath() != null && !iso.getPath().isEmpty()) {
2026-
DataStoreTO imageStore = iso.getDataStore();
2027-
if (!(imageStore instanceof NfsTO)) {
2028-
s_logger.debug("unsupported protocol");
2029-
throw new Exception("unsupported protocol");
2030-
}
2031-
NfsTO nfsImageStore = (NfsTO) imageStore;
2032-
String isoPath = nfsImageStore.getUrl() + File.separator + iso.getPath();
2033-
Pair<String, ManagedObjectReference> isoDatastoreInfo = getIsoDatastoreInfo(hyperHost, isoPath);
2034-
assert (isoDatastoreInfo != null);
2035-
assert (isoDatastoreInfo.second() != null);
2036-
2037-
deviceConfigSpecArray[i] = new VirtualDeviceConfigSpec();
2038-
Pair<VirtualDevice, Boolean> isoInfo =
2039-
VmwareHelper.prepareIsoDevice(vmMo, isoDatastoreInfo.first(), isoDatastoreInfo.second(), true, true, ideUnitNumber++, i + 1);
2040-
deviceConfigSpecArray[i].setDevice(isoInfo.first());
2041-
if (isoInfo.second()) {
2042-
if (s_logger.isDebugEnabled())
2043-
s_logger.debug("Prepare ISO volume at new device " + _gson.toJson(isoInfo.first()));
2044-
deviceConfigSpecArray[i].setOperation(VirtualDeviceConfigSpecOperation.ADD);
2045-
} else {
2046-
if (s_logger.isDebugEnabled())
2047-
s_logger.debug("Prepare ISO volume at existing device " + _gson.toJson(isoInfo.first()));
2048-
deviceConfigSpecArray[i].setOperation(VirtualDeviceConfigSpecOperation.EDIT);
2049-
}
2050-
}
2022+
configureIso(hyperHost, vmMo, vol, deviceConfigSpecArray, ideUnitNumber++, i);
20512023
i++;
20522024
}
20532025
}
@@ -2075,8 +2047,16 @@ protected StartAnswer execute(StartCommand cmd) {
20752047
//
20762048
// Setup ROOT/DATA disk devices
20772049
//
2050+
if (multipleIsosAtached(sortedDisks) && deployAsIs) {
2051+
sortedDisks = getDisks(sortedDisks);
2052+
}
2053+
20782054
for (DiskTO vol : sortedDisks) {
20792055
if (vol.getType() == Volume.Type.ISO) {
2056+
if (deployAsIs) {
2057+
configureIso(hyperHost, vmMo, vol, deviceConfigSpecArray, ideUnitNumber++, i);
2058+
i++;
2059+
}
20802060
continue;
20812061
}
20822062

@@ -2457,6 +2437,46 @@ protected StartAnswer execute(StartCommand cmd) {
24572437
}
24582438
}
24592439

2440+
private boolean multipleIsosAtached(DiskTO[] sortedDisks) {
2441+
return Arrays.stream(sortedDisks).filter(disk -> disk.getType() == Volume.Type.ISO).count() > 1;
2442+
}
2443+
2444+
private DiskTO[] getDisks(DiskTO[] sortedDisks) {
2445+
return Arrays.stream(sortedDisks).filter(vol -> ((vol.getPath() != null &&
2446+
vol.getPath().contains("configdrive"))) || (vol.getType() != Volume.Type.ISO)).toArray(DiskTO[]::new);
2447+
}
2448+
private void configureIso(VmwareHypervisorHost hyperHost, VirtualMachineMO vmMo, DiskTO vol,
2449+
VirtualDeviceConfigSpec[] deviceConfigSpecArray, int ideUnitNumber, int i) throws Exception {
2450+
TemplateObjectTO iso = (TemplateObjectTO) vol.getData();
2451+
2452+
if (iso.getPath() != null && !iso.getPath().isEmpty()) {
2453+
DataStoreTO imageStore = iso.getDataStore();
2454+
if (!(imageStore instanceof NfsTO)) {
2455+
s_logger.debug("unsupported protocol");
2456+
throw new Exception("unsupported protocol");
2457+
}
2458+
NfsTO nfsImageStore = (NfsTO) imageStore;
2459+
String isoPath = nfsImageStore.getUrl() + File.separator + iso.getPath();
2460+
Pair<String, ManagedObjectReference> isoDatastoreInfo = getIsoDatastoreInfo(hyperHost, isoPath);
2461+
assert (isoDatastoreInfo != null);
2462+
assert (isoDatastoreInfo.second() != null);
2463+
2464+
deviceConfigSpecArray[i] = new VirtualDeviceConfigSpec();
2465+
Pair<VirtualDevice, Boolean> isoInfo =
2466+
VmwareHelper.prepareIsoDevice(vmMo, isoDatastoreInfo.first(), isoDatastoreInfo.second(), true, true, ideUnitNumber, i + 1);
2467+
deviceConfigSpecArray[i].setDevice(isoInfo.first());
2468+
if (isoInfo.second()) {
2469+
if (s_logger.isDebugEnabled())
2470+
s_logger.debug("Prepare ISO volume at new device " + _gson.toJson(isoInfo.first()));
2471+
deviceConfigSpecArray[i].setOperation(VirtualDeviceConfigSpecOperation.ADD);
2472+
} else {
2473+
if (s_logger.isDebugEnabled())
2474+
s_logger.debug("Prepare ISO volume at existing device " + _gson.toJson(isoInfo.first()));
2475+
deviceConfigSpecArray[i].setOperation(VirtualDeviceConfigSpecOperation.EDIT);
2476+
}
2477+
}
2478+
}
2479+
24602480
private String mapAdapterType(String adapterStringFromOVF) {
24612481
if (StringUtils.isBlank(adapterStringFromOVF) || adapterStringFromOVF.equalsIgnoreCase(VirtualEthernetCardType.E1000.toString())) {
24622482
return VirtualEthernetCardType.E1000.toString();
@@ -5382,7 +5402,7 @@ protected AttachIsoAnswer execute(AttachIsoCommand cmd) {
53825402
"Failed to unmount vmware-tools installer ISO as the corresponding CDROM device is locked by VM. Please unmount the CDROM device inside the VM and ret-try.");
53835403
}
53845404
} catch (Throwable e) {
5385-
vmMo.detachIso(null);
5405+
vmMo.detachIso(null, cmd.isForce());
53865406
}
53875407
}
53885408

@@ -5411,7 +5431,7 @@ protected AttachIsoAnswer execute(AttachIsoCommand cmd) {
54115431
String isoDatastorePath = String.format("[%s] %s%s", storeName, isoStorePathFromRoot, isoFileName);
54125432

54135433
if (cmd.isAttach()) {
5414-
vmMo.attachIso(isoDatastorePath, morSecondaryDs, true, false, cmd.getDeviceKey());
5434+
vmMo.attachIso(isoDatastorePath, morSecondaryDs, true, false, cmd.getDeviceKey(), cmd.isForce());
54155435
return new AttachIsoAnswer(cmd);
54165436
} else {
54175437
int key = vmMo.detachIso(isoDatastorePath, cmd.isForce());

‎plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2045,7 +2045,7 @@ public Answer backupSnapshot(CopyCommand cmd) {
20452045

20462046
@Override
20472047
public Answer attachIso(AttachCommand cmd) {
2048-
return this.attachIso(cmd.getDisk(), true, cmd.getVmName());
2048+
return this.attachIso(cmd.getDisk(), true, cmd.getVmName(), cmd.isForced());
20492049
}
20502050

20512051
@Override
@@ -2411,7 +2411,7 @@ private synchronized ManagedObjectReference prepareSecondaryDatastoreOnHost(Stri
24112411
return morDatastore;
24122412
}
24132413

2414-
private Answer attachIso(DiskTO disk, boolean isAttach, String vmName) {
2414+
private Answer attachIso(DiskTO disk, boolean isAttach, String vmName, boolean force) {
24152415
try {
24162416
VmwareContext context = hostService.getServiceContext(null);
24172417
VmwareHypervisorHost hyperHost = hostService.getHyperHost(context, null);
@@ -2441,7 +2441,7 @@ private Answer attachIso(DiskTO disk, boolean isAttach, String vmName) {
24412441
return new AttachAnswer("Failed to unmount vmware-tools installer ISO as the corresponding CDROM device is locked by VM. Please unmount the CDROM device inside the VM and ret-try.");
24422442
}
24432443
} catch(Throwable e){
2444-
vmMo.detachIso(null);
2444+
vmMo.detachIso(null, force);
24452445
}
24462446
}
24472447

@@ -2469,9 +2469,9 @@ private Answer attachIso(DiskTO disk, boolean isAttach, String vmName) {
24692469
String isoDatastorePath = String.format("[%s] %s/%s", storeName, isoStorePathFromRoot, isoFileName);
24702470

24712471
if (isAttach) {
2472-
vmMo.attachIso(isoDatastorePath, morSecondaryDs, true, false);
2472+
vmMo.attachIso(isoDatastorePath, morSecondaryDs, true, false, force);
24732473
} else {
2474-
vmMo.detachIso(isoDatastorePath);
2474+
vmMo.detachIso(isoDatastorePath, force);
24752475
}
24762476

24772477
return new AttachAnswer(disk);
@@ -2497,7 +2497,7 @@ private Answer attachIso(DiskTO disk, boolean isAttach, String vmName) {
24972497

24982498
@Override
24992499
public Answer dettachIso(DettachCommand cmd) {
2500-
return this.attachIso(cmd.getDisk(), false, cmd.getVmName());
2500+
return this.attachIso(cmd.getDisk(), false, cmd.getVmName(), cmd.isForced());
25012501
}
25022502

25032503
@Override

‎plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterActionWorker.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -319,7 +319,7 @@ protected void attachIsoKubernetesVMs(List<UserVm> clusterVMs, final KubernetesS
319319
}
320320
for (UserVm vm : clusterVMs) {
321321
try {
322-
templateService.attachIso(iso.getId(), vm.getId());
322+
templateService.attachIso(iso.getId(), vm.getId(), true);
323323
if (LOGGER.isInfoEnabled()) {
324324
LOGGER.info(String.format("Attached binaries ISO for VM : %s in cluster: %s", vm.getDisplayName(), kubernetesCluster.getName()));
325325
}
@@ -337,7 +337,7 @@ protected void detachIsoKubernetesVMs(List<UserVm> clusterVMs) {
337337
for (UserVm vm : clusterVMs) {
338338
boolean result = false;
339339
try {
340-
result = templateService.detachIso(vm.getId());
340+
result = templateService.detachIso(vm.getId(), true);
341341
} catch (CloudRuntimeException ex) {
342342
LOGGER.warn(String.format("Failed to detach binaries ISO from VM : %s in the Kubernetes cluster : %s ", vm.getDisplayName(), kubernetesCluster.getName()), ex);
343343
}

‎server/src/main/java/com/cloud/template/TemplateManagerImpl.java‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1150,7 +1150,7 @@ public boolean templateIsDeleteable(long templateId) {
11501150

11511151
@Override
11521152
@ActionEvent(eventType = EventTypes.EVENT_ISO_DETACH, eventDescription = "detaching ISO", async = true)
1153-
public boolean detachIso(long vmId) {
1153+
public boolean detachIso(long vmId, boolean forced) {
11541154
Account caller = CallContext.current().getCallingAccount();
11551155
Long userId = CallContext.current().getCallingUserId();
11561156

@@ -1178,7 +1178,7 @@ public boolean detachIso(long vmId) {
11781178
throw new InvalidParameterValueException("Please specify a VM that is either Stopped or Running.");
11791179
}
11801180

1181-
boolean result = attachISOToVM(vmId, userId, isoId, false); // attach=false
1181+
boolean result = attachISOToVM(vmId, userId, isoId, false, forced); // attach=false
11821182
// => detach
11831183
if (result) {
11841184
return result;
@@ -1189,7 +1189,7 @@ public boolean detachIso(long vmId) {
11891189

11901190
@Override
11911191
@ActionEvent(eventType = EventTypes.EVENT_ISO_ATTACH, eventDescription = "attaching ISO", async = true)
1192-
public boolean attachIso(long isoId, long vmId) {
1192+
public boolean attachIso(long isoId, long vmId, boolean forced) {
11931193
Account caller = CallContext.current().getCallingAccount();
11941194
Long userId = CallContext.current().getCallingUserId();
11951195

@@ -1231,7 +1231,7 @@ public boolean attachIso(long isoId, long vmId) {
12311231
if ("vmware-tools.iso".equals(iso.getName()) && vm.getHypervisorType() != Hypervisor.HypervisorType.VMware) {
12321232
throw new InvalidParameterValueException("Cannot attach VMware tools drivers to incompatible hypervisor " + vm.getHypervisorType());
12331233
}
1234-
boolean result = attachISOToVM(vmId, userId, isoId, true);
1234+
boolean result = attachISOToVM(vmId, userId, isoId, true, forced);
12351235
if (result) {
12361236
return result;
12371237
} else {
@@ -1270,7 +1270,7 @@ public TemplateInfo prepareIso(long isoId, long dcId, Long hostId, Long poolId)
12701270
}
12711271
}
12721272

1273-
private boolean attachISOToVM(long vmId, long isoId, boolean attach) {
1273+
private boolean attachISOToVM(long vmId, long isoId, boolean attach, boolean forced) {
12741274
UserVmVO vm = _userVmDao.findById(vmId);
12751275

12761276
if (vm == null) {
@@ -1302,19 +1302,21 @@ private boolean attachISOToVM(long vmId, long isoId, boolean attach) {
13021302

13031303
Command cmd = null;
13041304
if (attach) {
1305-
cmd = new AttachCommand(disk, vmName, vmTO.getDetails());
1305+
cmd = new AttachCommand(disk, vmName);
1306+
((AttachCommand)cmd).setForced(forced);
13061307
} else {
1307-
cmd = new DettachCommand(disk, vmName, vmTO.getDetails());
1308+
cmd = new DettachCommand(disk, vmName);
1309+
((DettachCommand)cmd).setForced(forced);
13081310
}
13091311
Answer a = _agentMgr.easySend(vm.getHostId(), cmd);
13101312
return (a != null && a.getResult());
13111313
}
13121314

1313-
private boolean attachISOToVM(long vmId, long userId, long isoId, boolean attach) {
1315+
private boolean attachISOToVM(long vmId, long userId, long isoId, boolean attach, boolean forced) {
13141316
UserVmVO vm = _userVmDao.findById(vmId);
13151317
VMTemplateVO iso = _tmpltDao.findById(isoId);
13161318

1317-
boolean success = attachISOToVM(vmId, isoId, attach);
1319+
boolean success = attachISOToVM(vmId, isoId, attach, forced);
13181320
if (success && attach) {
13191321
vm.setIsoId(iso.getId());
13201322
_userVmDao.update(vmId, vm);

‎server/src/main/java/com/cloud/vm/UserVmManagerImpl.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5350,7 +5350,7 @@ public UserVm createVirtualMachine(DeployVMCmd cmd) throws InsufficientCapacityE
53505350
for (VMTemplateVO tmpl: child_templates){
53515351
if (tmpl.getFormat() == Storage.ImageFormat.ISO){
53525352
s_logger.info("MDOV trying to attach disk to the VM " + tmpl.getId() + " vmid=" + vm.getId());
5353-
_tmplService.attachIso(tmpl.getId(), vm.getId());
5353+
_tmplService.attachIso(tmpl.getId(), vm.getId(), true);
53545354
}
53555355
}
53565356

0 commit comments

Comments
 (0)