Repository: cloudstack Updated Branches: refs/heads/master 2a0236931 -> 5e90b75c9
CLOUDSTACK-6402: Fix StopCommand so that VMs are not removed accidentally as part of vmsync Added a new flag 'checkBeforeCleanup' to StopCommand based on which check is done to see if VM is running in HV host. If VM is running then in this case it is not stopped and the operation bails out. Also modified the MS code to call the StopCommand with appropriate value for the flag based on the context. Currently it is only set to 'true' when called from the new vmsync logic based on powerstate of VM. For rest it is set to 'false' meaning no change in behaviour. Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/5e90b75c Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/5e90b75c Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/5e90b75c Branch: refs/heads/master Commit: 5e90b75c984d9593a2d183d2baa4a1a20a039292 Parents: 2a02369 Author: Koushik Das <[email protected]> Authored: Mon Apr 14 16:12:35 2014 +0530 Committer: Koushik Das <[email protected]> Committed: Mon Apr 14 16:52:37 2014 +0530 ---------------------------------------------------------------------- core/src/com/cloud/agent/api/StopCommand.java | 14 +++++++++--- .../com/cloud/vm/VirtualMachineManagerImpl.java | 24 ++++++++++---------- .../cloud/vm/VirtualMachineManagerImplTest.java | 10 ++++---- .../HypervResource/HypervResourceController.cs | 11 +++++++++ .../HypervResourceController1Test.cs | 2 +- .../kvm/resource/LibvirtComputingResource.java | 12 ++++++++++ .../vmware/resource/VmwareResource.java | 12 ++++++++++ .../xen/resource/CitrixResourceBase.java | 6 +++++ 8 files changed, 70 insertions(+), 21 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cloudstack/blob/5e90b75c/core/src/com/cloud/agent/api/StopCommand.java ---------------------------------------------------------------------- diff --git a/core/src/com/cloud/agent/api/StopCommand.java b/core/src/com/cloud/agent/api/StopCommand.java index 00d7f5f..61d13e0 100755 --- a/core/src/com/cloud/agent/api/StopCommand.java +++ b/core/src/com/cloud/agent/api/StopCommand.java @@ -25,26 +25,30 @@ public class StopCommand extends RebootCommand { private String publicConsoleProxyIpAddress = null; boolean executeInSequence = false; private GPUDeviceTO gpuDevice; + boolean checkBeforeCleanup = false; protected StopCommand() { } - public StopCommand(VirtualMachine vm, boolean isProxy, String urlPort, String publicConsoleProxyIpAddress, boolean executeInSequence) { + public StopCommand(VirtualMachine vm, boolean isProxy, String urlPort, String publicConsoleProxyIpAddress, boolean executeInSequence, boolean checkBeforeCleanup) { super(vm); this.isProxy = isProxy; this.urlPort = urlPort; this.publicConsoleProxyIpAddress = publicConsoleProxyIpAddress; this.executeInSequence = executeInSequence; + this.checkBeforeCleanup = checkBeforeCleanup; } - public StopCommand(VirtualMachine vm, boolean executeInSequence) { + public StopCommand(VirtualMachine vm, boolean executeInSequence, boolean checkBeforeCleanup) { super(vm); this.executeInSequence = executeInSequence; + this.checkBeforeCleanup = checkBeforeCleanup; } - public StopCommand(String vmName, boolean executeInSequence) { + public StopCommand(String vmName, boolean executeInSequence, boolean checkBeforeCleanup) { super(vmName); this.executeInSequence = executeInSequence; + this.checkBeforeCleanup = checkBeforeCleanup; } @Override @@ -71,4 +75,8 @@ public class StopCommand extends RebootCommand { public void setGpuDevice(GPUDeviceTO gpuDevice) { this.gpuDevice = gpuDevice; } + + public boolean checkBeforeCleanup() { + return this.checkBeforeCleanup; + } } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/5e90b75c/engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java ---------------------------------------------------------------------- diff --git a/engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java b/engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java index c5a7efb..4376afb 100755 --- a/engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java @@ -1047,7 +1047,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac s_logger.info("The guru did not like the answers so stopping " + vm); } - StopCommand cmd = new StopCommand(vm, getExecuteInSequence(vm.getHypervisorType())); + StopCommand cmd = new StopCommand(vm, getExecuteInSequence(vm.getHypervisorType()), false); StopAnswer answer = (StopAnswer) _agentMgr.easySend(destHostId, cmd); if ( answer != null ) { @@ -1225,9 +1225,9 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac } } - protected boolean sendStop(VirtualMachineGuru guru, VirtualMachineProfile profile, boolean force) { + protected boolean sendStop(VirtualMachineGuru guru, VirtualMachineProfile profile, boolean force, boolean checkBeforeCleanup) { VirtualMachine vm = profile.getVirtualMachine(); - StopCommand stop = new StopCommand(vm, getExecuteInSequence(vm.getHypervisorType())); + StopCommand stop = new StopCommand(vm, getExecuteInSequence(vm.getHypervisorType()), checkBeforeCleanup); try { StopAnswer answer = (StopAnswer)_agentMgr.send(vm.getHostId(), stop); if (answer != null) { @@ -1282,7 +1282,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac if (step == Step.Started || step == Step.Starting || step == Step.Release) { if (vm.getHostId() != null) { - if (!sendStop(guru, profile, cleanUpEvenIfUnableToStop)) { + if (!sendStop(guru, profile, cleanUpEvenIfUnableToStop, false)) { s_logger.warn("Failed to stop vm " + vm + " in " + State.Starting + " state as a part of cleanup process"); return false; } @@ -1295,26 +1295,26 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac } } else if (state == State.Stopping) { if (vm.getHostId() != null) { - if (!sendStop(guru, profile, cleanUpEvenIfUnableToStop)) { + if (!sendStop(guru, profile, cleanUpEvenIfUnableToStop, false)) { s_logger.warn("Failed to stop vm " + vm + " in " + State.Stopping + " state as a part of cleanup process"); return false; } } } else if (state == State.Migrating) { if (vm.getHostId() != null) { - if (!sendStop(guru, profile, cleanUpEvenIfUnableToStop)) { + if (!sendStop(guru, profile, cleanUpEvenIfUnableToStop, false)) { s_logger.warn("Failed to stop vm " + vm + " in " + State.Migrating + " state as a part of cleanup process"); return false; } } if (vm.getLastHostId() != null) { - if (!sendStop(guru, profile, cleanUpEvenIfUnableToStop)) { + if (!sendStop(guru, profile, cleanUpEvenIfUnableToStop, false)) { s_logger.warn("Failed to stop vm " + vm + " in " + State.Migrating + " state as a part of cleanup process"); return false; } } } else if (state == State.Running) { - if (!sendStop(guru, profile, cleanUpEvenIfUnableToStop)) { + if (!sendStop(guru, profile, cleanUpEvenIfUnableToStop, false)) { s_logger.warn("Failed to stop vm " + vm + " in " + State.Running + " state as a part of cleanup process"); return false; } @@ -1488,7 +1488,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac vmGuru.prepareStop(profile); - StopCommand stop = new StopCommand(vm, getExecuteInSequence(vm.getHypervisorType())); + StopCommand stop = new StopCommand(vm, getExecuteInSequence(vm.getHypervisorType()), false); boolean stopped = false; StopAnswer answer = null; @@ -2464,11 +2464,11 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac } public Command cleanup(VirtualMachine vm) { - return new StopCommand(vm, getExecuteInSequence(vm.getHypervisorType())); + return new StopCommand(vm, getExecuteInSequence(vm.getHypervisorType()), false); } public Command cleanup(String vmName) { - return new StopCommand(vmName, getExecuteInSequence(null)); + return new StopCommand(vmName, getExecuteInSequence(null), false); } @@ -4257,7 +4257,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac VirtualMachineGuru vmGuru = getVmGuru(vm); VirtualMachineProfile profile = new VirtualMachineProfileImpl(vm); - sendStop(vmGuru, profile, true); + sendStop(vmGuru, profile, true, true); try { stateTransitTo(vm, VirtualMachine.Event.FollowAgentPowerOffReport, null); http://git-wip-us.apache.org/repos/asf/cloudstack/blob/5e90b75c/engine/orchestration/test/com/cloud/vm/VirtualMachineManagerImplTest.java ---------------------------------------------------------------------- diff --git a/engine/orchestration/test/com/cloud/vm/VirtualMachineManagerImplTest.java b/engine/orchestration/test/com/cloud/vm/VirtualMachineManagerImplTest.java index 06a2983..f6422f4 100644 --- a/engine/orchestration/test/com/cloud/vm/VirtualMachineManagerImplTest.java +++ b/engine/orchestration/test/com/cloud/vm/VirtualMachineManagerImplTest.java @@ -466,12 +466,12 @@ public class VirtualMachineManagerImplTest { VirtualMachineGuru guru = mock(VirtualMachineGuru.class); VirtualMachine vm = mock(VirtualMachine.class); VirtualMachineProfile profile = mock(VirtualMachineProfile.class); - StopAnswer answer = new StopAnswer(new StopCommand(vm, false), "ok", true); + StopAnswer answer = new StopAnswer(new StopCommand(vm, false, false), "ok", true); when(profile.getVirtualMachine()).thenReturn(vm); when(vm.getHostId()).thenReturn(1L); when(_agentMgr.send(anyLong(), (Command)any())).thenReturn(answer); - boolean actual = _vmMgr.sendStop(guru, profile, false); + boolean actual = _vmMgr.sendStop(guru, profile, false, false); Assert.assertTrue(actual); } @@ -481,12 +481,12 @@ public class VirtualMachineManagerImplTest { VirtualMachineGuru guru = mock(VirtualMachineGuru.class); VirtualMachine vm = mock(VirtualMachine.class); VirtualMachineProfile profile = mock(VirtualMachineProfile.class); - StopAnswer answer = new StopAnswer(new StopCommand(vm, false), "fail", false); + StopAnswer answer = new StopAnswer(new StopCommand(vm, false, false), "fail", false); when(profile.getVirtualMachine()).thenReturn(vm); when(vm.getHostId()).thenReturn(1L); when(_agentMgr.send(anyLong(), (Command)any())).thenReturn(answer); - boolean actual = _vmMgr.sendStop(guru, profile, false); + boolean actual = _vmMgr.sendStop(guru, profile, false, false); Assert.assertFalse(actual); } @@ -500,7 +500,7 @@ public class VirtualMachineManagerImplTest { when(vm.getHostId()).thenReturn(1L); when(_agentMgr.send(anyLong(), (Command)any())).thenReturn(null); - boolean actual = _vmMgr.sendStop(guru, profile, false); + boolean actual = _vmMgr.sendStop(guru, profile, false, false); Assert.assertFalse(actual); } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/5e90b75c/plugins/hypervisors/hyperv/DotNet/ServerResource/HypervResource/HypervResourceController.cs ---------------------------------------------------------------------- diff --git a/plugins/hypervisors/hyperv/DotNet/ServerResource/HypervResource/HypervResourceController.cs b/plugins/hypervisors/hyperv/DotNet/ServerResource/HypervResource/HypervResourceController.cs index 4f4c14e..ca3c206 100644 --- a/plugins/hypervisors/hyperv/DotNet/ServerResource/HypervResource/HypervResourceController.cs +++ b/plugins/hypervisors/hyperv/DotNet/ServerResource/HypervResource/HypervResourceController.cs @@ -1135,7 +1135,18 @@ namespace HypervResource logger.Info(CloudStackTypes.StopCommand + Utils.CleanString(cmd.ToString())); string details = null; bool result = false; + bool checkBeforeCleanup = cmd.checkBeforeCleanup; + String vmName = cmd.vmName; + if (checkBeforeCleanup == true) + { + ComputerSystem vm = wmiCallsV2.GetComputerSystem(vmName); + if (vm == null || vm.EnabledState == 2) + { + // VM is not available or vm in running state + return ReturnCloudStackTypedJArray(new { result = false, details = "VM is running on host, bailing out", vm = vmName, contextMap = contextMap }, CloudStackTypes.StopAnswer); + } + } try { wmiCallsV2.DestroyVm(cmd); http://git-wip-us.apache.org/repos/asf/cloudstack/blob/5e90b75c/plugins/hypervisors/hyperv/DotNet/ServerResource/ServerResource.Tests/HypervResourceController1Test.cs ---------------------------------------------------------------------- diff --git a/plugins/hypervisors/hyperv/DotNet/ServerResource/ServerResource.Tests/HypervResourceController1Test.cs b/plugins/hypervisors/hyperv/DotNet/ServerResource/ServerResource.Tests/HypervResourceController1Test.cs index fd0a4bf..87a12c9 100644 --- a/plugins/hypervisors/hyperv/DotNet/ServerResource/ServerResource.Tests/HypervResourceController1Test.cs +++ b/plugins/hypervisors/hyperv/DotNet/ServerResource/ServerResource.Tests/HypervResourceController1Test.cs @@ -281,7 +281,7 @@ namespace ServerResource.Tests HypervResourceController rsrcServer = new HypervResourceController(); HypervResourceController.wmiCallsV2 = wmiCallsV2; - String sampleStop = "{\"isProxy\":false,\"vmName\":\"i-2-17-VM\",\"contextMap\":{},\"wait\":0}"; + String sampleStop = "{\"isProxy\":false,\"vmName\":\"i-2-17-VM\",\"contextMap\":{},\"checkBeforeCleanup\":false,\"wait\":0}"; dynamic jsonStopCmd = JsonConvert.DeserializeObject(sampleStop); // Act http://git-wip-us.apache.org/repos/asf/cloudstack/blob/5e90b75c/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java ---------------------------------------------------------------------- diff --git a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java index e3a805d..2a3fcbe 100755 --- a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java +++ b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java @@ -3485,6 +3485,18 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv protected Answer execute(StopCommand cmd) { final String vmName = cmd.getVmName(); + if (cmd.checkBeforeCleanup()) { + try { + Connect conn = LibvirtConnection.getConnectionByVmName(vmName); + Domain vm = conn.domainLookupByName(cmd.getVmName()); + if (vm != null && vm.getInfo().state == DomainInfo.DomainState.VIR_DOMAIN_RUNNING) { + return new StopAnswer(cmd, "vm is still running on host", false); + } + } catch (Exception e) { + s_logger.debug("Failed to get vm status in case of checkboforecleanup is true", e); + } + } + State state = null; synchronized (_vms) { state = _vms.get(vmName); http://git-wip-us.apache.org/repos/asf/cloudstack/blob/5e90b75c/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java ---------------------------------------------------------------------- diff --git a/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java b/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java index 4f7d450..e89777d 100755 --- a/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java +++ b/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java @@ -2740,6 +2740,18 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa try { VirtualMachineMO vmMo = hyperHost.findVmOnHyperHost(cmd.getVmName()); if (vmMo != null) { + if (cmd.checkBeforeCleanup()) { + if (getVmPowerState(vmMo) != PowerState.PowerOff) { + String msg = "StopCommand is sent for cleanup and VM " + cmd.getVmName() + " is current running. ignore it."; + s_logger.warn(msg); + return new StopAnswer(cmd, msg, false); + } else { + String msg = "StopCommand is sent for cleanup and VM " + cmd.getVmName() + " is indeed stopped already."; + s_logger.info(msg); + return new StopAnswer(cmd, msg, true); + } + } + State state = null; synchronized (_vms) { state = _vms.get(cmd.getVmName()); http://git-wip-us.apache.org/repos/asf/cloudstack/blob/5e90b75c/plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java ---------------------------------------------------------------------- diff --git a/plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java b/plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java index ebfdab6..626a219 100644 --- a/plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java +++ b/plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java @@ -3628,6 +3628,12 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe return new StopAnswer(cmd, msg, platformstring, false); } + if (cmd.checkBeforeCleanup() && vmr.powerState == VmPowerState.RUNNING) { + String msg = "Vm " + vmName + " is running on host and checkBeforeCleanup flag is set, so bailing out"; + s_logger.debug(msg); + return new StopAnswer(cmd, msg, false); + } + State state = s_vms.getState(_cluster, vmName); synchronized (_cluster.intern()) {
