Skip to content

Commit 24c4280

Browse files
committed
handle initial review comments
1 parent be93681 commit 24c4280

File tree

3 files changed

+56
-47
lines changed

3 files changed

+56
-47
lines changed

core/src/main/java/com/cloud/agent/api/UnmanageInstanceCommand.java

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,6 @@ public class UnmanageInstanceCommand extends Command {
3030

3131
@Override
3232
public boolean executeInSequence() {
33-
//VR start doesn't go through queue
34-
if (instanceName != null && instanceName.startsWith("r-")) {
35-
return false;
36-
}
3733
return executeInSequence;
3834
}
3935

engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java

Lines changed: 53 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,6 @@
150150
import com.cloud.agent.api.StopCommand;
151151
import com.cloud.agent.api.UnPlugNicAnswer;
152152
import com.cloud.agent.api.UnPlugNicCommand;
153-
import com.cloud.agent.api.UnmanageInstanceAnswer;
154153
import com.cloud.agent.api.UnmanageInstanceCommand;
155154
import com.cloud.agent.api.UnregisterVMCommand;
156155
import com.cloud.agent.api.VmDiskStatsEntry;
@@ -1173,8 +1172,6 @@ protected void checkAndAttemptMigrateVmAcrossCluster(final VMInstanceVO vm, fina
11731172
markVolumesInPool(vm, answer);
11741173
}
11751174

1176-
1177-
11781175
protected void updateVmMetadataManufacturerAndProduct(VirtualMachineTO vmTO, VMInstanceVO vm) {
11791176
String metadataManufacturer = VmMetadataManufacturer.valueIn(vm.getDataCenterId());
11801177
if (StringUtils.isBlank(metadataManufacturer)) {
@@ -2019,52 +2016,61 @@ public boolean unmanage(String vmUuid) {
20192016
throw new ConcurrentOperationException(msg);
20202017
}
20212018

2022-
boolean isDomainXMLPreserved = true;
2023-
// persist domain for kvm host
20242019
if (HypervisorType.KVM.equals(vm.getHypervisorType())) {
2025-
long hostId = vm.getHostId();
2026-
UnmanageInstanceCommand unmanageInstanceCommand;
2027-
if (State.Stopped.equals(vm.getState())) {
2028-
hostId = vm.getLastHostId();
2029-
unmanageInstanceCommand = new UnmanageInstanceCommand(prepareVmTO(vm.getId(), hostId)); // reconstruct vmSpec
2030-
} else {
2031-
unmanageInstanceCommand = new UnmanageInstanceCommand(vm.getName());
2032-
}
2033-
try {
2034-
Answer answer = _agentMgr.send(hostId, unmanageInstanceCommand);
2035-
isDomainXMLPreserved = (answer instanceof UnmanageInstanceAnswer && answer.getResult());
2036-
} catch (Exception ex) {
2037-
isDomainXMLPreserved = false;
2038-
}
2020+
persistDomainForKVM(vm);
20392021
}
2022+
Boolean result = Transaction.execute(new TransactionCallback<Boolean>() {
2023+
@Override
2024+
public Boolean doInTransaction(TransactionStatus status) {
20402025

2041-
if (isDomainXMLPreserved) {
2042-
logger.debug("Unmanaging VM {}", vm);
2043-
Boolean result = Transaction.execute(new TransactionCallback<Boolean>() {
2044-
@Override
2045-
public Boolean doInTransaction(TransactionStatus status) {
2046-
final VirtualMachineProfile profile = new VirtualMachineProfileImpl(vm);
2047-
final VirtualMachineGuru guru = getVmGuru(vm);
2026+
logger.debug("Unmanaging VM {}", vm);
20482027

2049-
try {
2050-
unmanageVMSnapshots(vm);
2051-
unmanageVMNics(profile, vm);
2052-
unmanageVMVolumes(vm);
2028+
final VirtualMachineProfile profile = new VirtualMachineProfileImpl(vm);
2029+
final VirtualMachineGuru guru = getVmGuru(vm);
20532030

2054-
guru.finalizeUnmanage(vm);
2055-
} catch (Exception e) {
2056-
logger.error("Error while unmanaging VM {}", vm, e);
2057-
return false;
2058-
}
2031+
try {
2032+
unmanageVMSnapshots(vm);
2033+
unmanageVMNics(profile, vm);
2034+
unmanageVMVolumes(vm);
20592035

2060-
return true;
2036+
guru.finalizeUnmanage(vm);
2037+
} catch (Exception e) {
2038+
logger.error("Error while unmanaging VM {}", vm, e);
2039+
return false;
20612040
}
2062-
});
2063-
return BooleanUtils.isTrue(result);
2041+
2042+
return true;
2043+
}
2044+
});
2045+
2046+
return BooleanUtils.isTrue(result);
2047+
}
2048+
2049+
void persistDomainForKVM(VMInstanceVO vm) {
2050+
long hostId = vm.getHostId();
2051+
UnmanageInstanceCommand unmanageInstanceCommand;
2052+
if (State.Stopped.equals(vm.getState())) {
2053+
hostId = vm.getLastHostId();
2054+
unmanageInstanceCommand = new UnmanageInstanceCommand(prepVmSpecForUnmanageCmd(vm.getId(), hostId)); // reconstruct vmSpec for stopped instance
20642055
} else {
2065-
logger.error("Error encountered persisting domainXML for vm: {}", vm.getName());
2056+
unmanageInstanceCommand = new UnmanageInstanceCommand(vm.getName());
2057+
}
2058+
try {
2059+
Answer answer = _agentMgr.send(hostId, unmanageInstanceCommand);
2060+
if (!answer.getResult()) {
2061+
String errorMsg = "Failed to persist domainXML for instance: " + vm.getName();
2062+
logger.debug(errorMsg);
2063+
throw new CloudRuntimeException(errorMsg);
2064+
}
2065+
} catch (AgentUnavailableException e) {
2066+
String errorMsg = "Failed to send command, agent unavailable";
2067+
logger.error(errorMsg, e);
2068+
throw new CloudRuntimeException(errorMsg);
2069+
} catch (OperationTimedoutException e) {
2070+
String errorMsg = "Failed to send command, operation timed out";
2071+
logger.error(errorMsg, e);
2072+
throw new CloudRuntimeException(errorMsg);
20662073
}
2067-
return false;
20682074
}
20692075

20702076
/**
@@ -4030,7 +4036,13 @@ private void checkAndSetEnterSetupMode(VirtualMachineTO vmTo, Map<VirtualMachine
40304036
vmTo.setEnterHardwareSetup(enterSetup == null ? false : enterSetup);
40314037
}
40324038

4033-
protected VirtualMachineTO prepareVmTO(Long vmId, Long hostId) {
4039+
/**
4040+
* This method helps constructing vmSpec for Unmanage operation for Stopped Instance
4041+
* @param vmId
4042+
* @param hostId
4043+
* @return VirtualMachineTO
4044+
*/
4045+
protected VirtualMachineTO prepVmSpecForUnmanageCmd(Long vmId, Long hostId) {
40344046
final VMInstanceVO vm = _vmDao.findById(vmId);
40354047
final Account owner = _entityMgr.findById(Account.class, vm.getAccountId());
40364048
final ServiceOfferingVO offering = _offeringDao.findById(vm.getId(), vm.getServiceOfferingId());

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,8 +105,8 @@
105105
import org.apache.commons.lang3.builder.ReflectionToStringBuilder;
106106
import org.apache.commons.lang3.builder.ToStringStyle;
107107
import org.apache.logging.log4j.LogManager;
108-
import org.apache.logging.log4j.Logger;
109108
import org.apache.logging.log4j.ThreadContext;
109+
import org.apache.logging.log4j.Logger;
110110
import org.apache.xerces.impl.xpath.regex.Match;
111111
import org.joda.time.Duration;
112112
import org.libvirt.Connect;
@@ -170,6 +170,7 @@
170170
import com.cloud.host.Host.Type;
171171
import com.cloud.hypervisor.Hypervisor.HypervisorType;
172172
import com.cloud.hypervisor.kvm.dpdk.DpdkHelper;
173+
import com.cloud.hypervisor.kvm.resource.disconnecthook.DisconnectHook;
173174
import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.ChannelDef;
174175
import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.ClockDef;
175176
import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.ConsoleDef;
@@ -198,7 +199,6 @@
198199
import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.WatchDogDef;
199200
import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.WatchDogDef.WatchDogAction;
200201
import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.WatchDogDef.WatchDogModel;
201-
import com.cloud.hypervisor.kvm.resource.disconnecthook.DisconnectHook;
202202
import com.cloud.hypervisor.kvm.resource.rolling.maintenance.RollingMaintenanceAgentExecutor;
203203
import com.cloud.hypervisor.kvm.resource.rolling.maintenance.RollingMaintenanceExecutor;
204204
import com.cloud.hypervisor.kvm.resource.rolling.maintenance.RollingMaintenanceServiceExecutor;
@@ -240,6 +240,7 @@
240240
import com.cloud.vm.VirtualMachine;
241241
import com.cloud.vm.VirtualMachine.PowerState;
242242
import com.cloud.vm.VmDetailConstants;
243+
243244
import com.google.gson.Gson;
244245
import com.google.gson.JsonArray;
245246
import com.google.gson.JsonElement;

0 commit comments

Comments
 (0)