Skip to content

Commit 48af308

Browse files
sudo87sudo87
committed
handle initial review comments
1 parent be93681 commit 48af308

File tree

3 files changed

+57
-47
lines changed

3 files changed

+57
-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: 54 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,62 @@ 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+
2023+
Boolean result = Transaction.execute(new TransactionCallback<Boolean>() {
2024+
@Override
2025+
public Boolean doInTransaction(TransactionStatus status) {
20402026

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);
2027+
logger.debug("Unmanaging VM {}", vm);
20482028

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

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

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

20702077
/**
@@ -4030,7 +4037,13 @@ private void checkAndSetEnterSetupMode(VirtualMachineTO vmTo, Map<VirtualMachine
40304037
vmTo.setEnterHardwareSetup(enterSetup == null ? false : enterSetup);
40314038
}
40324039

4033-
protected VirtualMachineTO prepareVmTO(Long vmId, Long hostId) {
4040+
/**
4041+
* This method helps constructing vmSpec for Unmanage operation for Stopped Instance
4042+
* @param vmId
4043+
* @param hostId
4044+
* @return VirtualMachineTO
4045+
*/
4046+
protected VirtualMachineTO prepVmSpecForUnmanageCmd(Long vmId, Long hostId) {
40344047
final VMInstanceVO vm = _vmDao.findById(vmId);
40354048
final Account owner = _entityMgr.findById(Account.class, vm.getAccountId());
40364049
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)