xenserver: do not destroy halted hypervisor vm (apache#9175)

shwstppr · web-flow · commit 5d5ac17c6857 · 2025-04-09T09:33:01.000+02:00
Signed-off-by: Abhishek Kumar &lt;abhishek.mrt22@gmail.com&gt;
diff --git a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java
@@ -51,10 +51,6 @@
 import javax.naming.ConfigurationException;
 import javax.xml.parsers.ParserConfigurationException;
 
-import com.trilead.ssh2.SFTPException;
-import com.trilead.ssh2.SFTPv3Client;
-import com.trilead.ssh2.SFTPv3DirectoryEntry;
-import com.trilead.ssh2.SFTPv3FileAttributes;
 import org.apache.cloudstack.api.ApiConstants;
 import org.apache.cloudstack.diagnostics.CopyToSecondaryStorageAnswer;
 import org.apache.cloudstack.diagnostics.CopyToSecondaryStorageCommand;
@@ -72,6 +68,7 @@
 import org.apache.commons.lang3.BooleanUtils;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.log4j.Logger;
+import org.apache.maven.artifact.versioning.ComparableVersion;
 import org.apache.xmlrpc.XmlRpcException;
 import org.joda.time.Duration;
 import org.w3c.dom.Document;
@@ -153,6 +150,10 @@
 import com.cloud.vm.VirtualMachine.PowerState;
 import com.cloud.vm.VmDetailConstants;
 import com.trilead.ssh2.SCPClient;
+import com.trilead.ssh2.SFTPException;
+import com.trilead.ssh2.SFTPv3Client;
+import com.trilead.ssh2.SFTPv3DirectoryEntry;
+import com.trilead.ssh2.SFTPv3FileAttributes;
 import com.xensource.xenapi.Bond;
 import com.xensource.xenapi.Connection;
 import com.xensource.xenapi.Console;
@@ -627,7 +628,7 @@ public boolean cleanupHaltedVms(final Connection conn) throws XenAPIException, X
 
                 if (VmPowerState.HALTED.equals(vmRec.powerState) && vmRec.affinity.equals(host) && !isAlienVm(vm, conn)) {
                     try {
-                        vm.destroy(conn);
+                        destroyVm(vm, conn);
                     } catch (final Exception e) {
                         s_logger.warn("Catch Exception " + e.getClass().getName() + ": unable to destroy VM " + vmRec.nameLabel + " due to ", e);
                         success = false;
@@ -1452,7 +1453,7 @@ public VM createVmFromTemplate(final Connection conn, final VirtualMachineTO vmS
                 vm.setPVBootloader(conn, "pygrub");
                 vm.setPVBootloaderArgs(conn, CitrixHelper.getPVbootloaderArgs(guestOsTypeName));
             } else {
-                vm.destroy(conn);
+                destroyVm(vm, conn, true);
                 throw new CloudRuntimeException("Unable to handle boot loader type: " + vmSpec.getBootloader());
             }
         }
@@ -2038,7 +2039,7 @@ void forceShutdownVM(final Connection conn, final VM vm) {
             final Long domId = vm.getDomid(conn);
             callHostPlugin(conn, "vmopspremium", "forceShutdownVM", "domId", domId.toString());
             vm.powerStateReset(conn);
-            vm.destroy(conn);
+            destroyVm(vm, conn);
         } catch (final Exception e) {
             final String msg = "forceShutdown failed due to " + e.toString();
             s_logger.warn(msg, e);
@@ -3690,7 +3691,7 @@ public String handleVmStartFailure(final Connection conn, final String vmName, f
             }
             if (vm.getPowerState(conn) == VmPowerState.HALTED) {
                 try {
-                    vm.destroy(conn);
+                    destroyVm(vm, conn, true);
                 } catch (final Exception e) {
                     s_logger.warn("VM destroy failed due to ", e);
                 }
@@ -5208,7 +5209,7 @@ protected void startvmfailhandle(final Connection conn, final VM vm, final List<
                 }
                 if (vm.getPowerState(conn) == VmPowerState.HALTED) {
                     try {
-                        vm.destroy(conn);
+                        destroyVm(vm, conn, true);
                     } catch (final Exception e) {
                         final String msg = "VM destroy failed due to " + e.toString();
                         s_logger.warn(msg, e);
@@ -5875,4 +5876,23 @@ private void umountNfs(Connection conn, String remoteDir, String localDir) {
             s_logger.warn(errMsg);
         }
     }
+
+    public boolean isDestroyHaltedVms() {
+        ComparableVersion version = new ComparableVersion(getHost().getProductVersion());
+        if (version.compareTo(new ComparableVersion("8.0")) >= 0) {
+            return false;
+        }
+        return true;
+    }
+
+    public void destroyVm(VM vm, Connection connection, boolean forced) throws XenAPIException, XmlRpcException {
+        if (!isDestroyHaltedVms() && !forced) {
+            return;
+        }
+        vm.destroy(connection);
+    }
+
+    public void destroyVm(VM vm, Connection connection) throws XenAPIException, XmlRpcException {
+        destroyVm(vm, connection, false);
+    }
 }
diff --git a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xen56/XenServer56FenceCommandWrapper.java b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xen56/XenServer56FenceCommandWrapper.java
@@ -56,7 +56,7 @@ public Answer execute(final FenceCommand command, final XenServer56Resource xenS
             for (final VM vm : vms) {
                 s_logger.info("Fence command for VM " + command.getVmName());
                 vm.powerStateReset(conn);
-                vm.destroy(conn);
+                xenServer56.destroyVm(vm, conn);
             }
             return new FenceAnswer(command);
         } catch (final XmlRpcException e) {
diff --git a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xen56p1/XenServer56FP1FenceCommandWrapper.java b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xen56p1/XenServer56FP1FenceCommandWrapper.java
@@ -68,7 +68,7 @@ public Answer execute(final FenceCommand command, final XenServer56Resource xenS
                 }
                 s_logger.info("Fence command for VM " + command.getVmName());
                 vm.powerStateReset(conn);
-                vm.destroy(conn);
+                xenServer56.destroyVm(vm, conn);
                 for (final VDI vdi : vdis) {
                     final Map<String, String> smConfig = vdi.getSmConfig(conn);
                     for (final String key : smConfig.keySet()) {
diff --git a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixCreateVMSnapshotCommandWrapper.java b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixCreateVMSnapshotCommandWrapper.java
@@ -100,6 +100,7 @@ public Answer execute(final CreateVMSnapshotCommand command, final CitrixResourc
                     vm = citrixResourceBase.getVM(conn, vmName);
                     vmState = vm.getPowerState(conn);
                 } catch (final Exception e) {
+                    s_logger.debug(String.format("Failed to find VM with name: %s due to:", vmName), e);
                     if (!snapshotMemory) {
                         vm = citrixResourceBase.createWorkingVM(conn, vmName, guestOSType, platformEmulator, listVolumeTo);
                     }
@@ -178,13 +179,11 @@ public Answer execute(final CreateVMSnapshotCommand command, final CitrixResourc
                                 vdi.destroy(conn);
                             }
                         }
-                        vmSnapshot.destroy(conn);
+                        citrixResourceBase.destroyVm(vmSnapshot, conn, true);
                     }
                 }
-                if (vmState == VmPowerState.HALTED) {
-                    if (vm != null) {
-                        vm.destroy(conn);
-                    }
+                if (vmState == VmPowerState.HALTED && vm != null) {
+                    citrixResourceBase.destroyVm(vm, conn);
                 }
             } catch (final Exception e2) {
                 s_logger.error("delete snapshot error due to " + e2.getMessage());
diff --git a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixDeleteVMSnapshotCommandWrapper.java b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixDeleteVMSnapshotCommandWrapper.java
@@ -68,7 +68,7 @@ public Answer execute(final DeleteVMSnapshotCommand command, final CitrixResourc
             if (command.getTarget().getType() == VMSnapshot.Type.DiskAndMemory) {
                 vdiList.add(snapshot.getSuspendVDI(conn));
             }
-            snapshot.destroy(conn);
+            citrixResourceBase.destroyVm(snapshot, conn, true);
             for (final VDI vdi : vdiList) {
                 vdi.destroy(conn);
             }
diff --git a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixRevertToVMSnapshotCommandWrapper.java b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixRevertToVMSnapshotCommandWrapper.java
@@ -68,6 +68,7 @@ public Answer execute(final RevertToVMSnapshotCommand command, final CitrixResou
             try {
                 vm = citrixResourceBase.getVM(conn, vmName);
             } catch (final Exception e) {
+                s_logger.debug(String.format("Failed to find VM with name: %s due to:", vmName), e);
                 vm = citrixResourceBase.createWorkingVM(conn, vmName, command.getGuestOSType(), command.getPlatformEmulator(), listVolumeTo);
             }
 
@@ -90,7 +91,7 @@ public Answer execute(final RevertToVMSnapshotCommand command, final CitrixResou
             }
 
             if (!snapshotMemory) {
-                vm.destroy(conn);
+                citrixResourceBase.destroyVm(vm, conn);
                 vmState = PowerState.PowerOff;
             } else {
                 vmState = PowerState.PowerOn;
diff --git a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixStartCommandWrapper.java b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixStartCommandWrapper.java
@@ -75,7 +75,7 @@ public Answer execute(final StartCommand command, final CitrixResourceBase citri
                 for (final VM v : vms) {
                     final VM.Record vRec = v.getRecord(conn);
                     if (vRec.powerState == VmPowerState.HALTED) {
-                        v.destroy(conn);
+                        citrixResourceBase.destroyVm(v, conn, true);
                     } else if (vRec.powerState == VmPowerState.RUNNING) {
                         final String host = vRec.residentOn.getUuid(conn);
                         final String msg = "VM " + vmName + " is runing on host " + host;
diff --git a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixStopCommandWrapper.java b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixStopCommandWrapper.java
@@ -143,7 +143,7 @@ public Answer execute(final StopCommand command, final CitrixResourceBase citrix
                             for (final VIF vif : vifs) {
                                 networks.add(vif.getNetwork(conn));
                             }
-                            vm.destroy(conn);
+                            citrixResourceBase.destroyVm(vm, conn);
                             final SR sr = citrixResourceBase.getISOSRbyVmName(conn, command.getVmName(), false);
                             citrixResourceBase.removeSR(conn, sr);
                             final SR configDriveSR = citrixResourceBase.getISOSRbyVmName(conn, command.getVmName(), true);

Original file line number	Diff line number	Diff line change
`@@ -56,7 +56,7 @@ public Answer execute(final FenceCommand command, final XenServer56Resource xenS`
`56`	`56`	`for (final VM vm : vms) {`
`57`	`57`	`s_logger.info("Fence command for VM " + command.getVmName());`
`58`	`58`	`vm.powerStateReset(conn);`
`59`		`- vm.destroy(conn);`
	`59`	`+ xenServer56.destroyVm(vm, conn);`
`60`	`60`	`}`
`61`	`61`	`return new FenceAnswer(command);`
`62`	`62`	`} catch (final XmlRpcException e) {`
Original file line number	Diff line number	Diff line change
`@@ -68,7 +68,7 @@ public Answer execute(final FenceCommand command, final XenServer56Resource xenS`
`68`	`68`	`}`
`69`	`69`	`s_logger.info("Fence command for VM " + command.getVmName());`
`70`	`70`	`vm.powerStateReset(conn);`
`71`		`- vm.destroy(conn);`
	`71`	`+ xenServer56.destroyVm(vm, conn);`
`72`	`72`	`for (final VDI vdi : vdis) {`
`73`	`73`	`final Map<String, String> smConfig = vdi.getSmConfig(conn);`
`74`	`74`	`for (final String key : smConfig.keySet()) {`
Original file line number	Diff line number	Diff line change
`@@ -100,6 +100,7 @@ public Answer execute(final CreateVMSnapshotCommand command, final CitrixResourc`
`100`	`100`	`vm = citrixResourceBase.getVM(conn, vmName);`
`101`	`101`	`vmState = vm.getPowerState(conn);`
`102`	`102`	`} catch (final Exception e) {`
	`103`	`+ s_logger.debug(String.format("Failed to find VM with name: %s due to:", vmName), e);`
`103`	`104`	`if (!snapshotMemory) {`
`104`	`105`	`vm = citrixResourceBase.createWorkingVM(conn, vmName, guestOSType, platformEmulator, listVolumeTo);`
`105`	`106`	`}`
`@@ -178,13 +179,11 @@ public Answer execute(final CreateVMSnapshotCommand command, final CitrixResourc`
`178`	`179`	`vdi.destroy(conn);`
`179`	`180`	`}`
`180`	`181`	`}`
`181`		`- vmSnapshot.destroy(conn);`
	`182`	`+ citrixResourceBase.destroyVm(vmSnapshot, conn, true);`
`182`	`183`	`}`
`183`	`184`	`}`
`184`		`- if (vmState == VmPowerState.HALTED) {`
`185`		`- if (vm != null) {`
`186`		`- vm.destroy(conn);`
`187`		`- }`
	`185`	`+ if (vmState == VmPowerState.HALTED && vm != null) {`
	`186`	`+ citrixResourceBase.destroyVm(vm, conn);`
`188`	`187`	`}`
`189`	`188`	`} catch (final Exception e2) {`
`190`	`189`	`s_logger.error("delete snapshot error due to " + e2.getMessage());`
Original file line number	Diff line number	Diff line change
`@@ -68,6 +68,7 @@ public Answer execute(final RevertToVMSnapshotCommand command, final CitrixResou`
`68`	`68`	`try {`
`69`	`69`	`vm = citrixResourceBase.getVM(conn, vmName);`
`70`	`70`	`} catch (final Exception e) {`
	`71`	`+ s_logger.debug(String.format("Failed to find VM with name: %s due to:", vmName), e);`
`71`	`72`	`vm = citrixResourceBase.createWorkingVM(conn, vmName, command.getGuestOSType(), command.getPlatformEmulator(), listVolumeTo);`
`72`	`73`	`}`
`73`	`74`
`@@ -90,7 +91,7 @@ public Answer execute(final RevertToVMSnapshotCommand command, final CitrixResou`
`90`	`91`	`}`
`91`	`92`
`92`	`93`	`if (!snapshotMemory) {`
`93`		`- vm.destroy(conn);`
	`94`	`+ citrixResourceBase.destroyVm(vm, conn);`
`94`	`95`	`vmState = PowerState.PowerOff;`
`95`	`96`	`} else {`
`96`	`97`	`vmState = PowerState.PowerOn;`
Original file line number	Diff line number	Diff line change
`@@ -143,7 +143,7 @@ public Answer execute(final StopCommand command, final CitrixResourceBase citrix`
`143`	`143`	`for (final VIF vif : vifs) {`
`144`	`144`	`networks.add(vif.getNetwork(conn));`
`145`	`145`	`}`
`146`		`- vm.destroy(conn);`
	`146`	`+ citrixResourceBase.destroyVm(vm, conn);`
`147`	`147`	`final SR sr = citrixResourceBase.getISOSRbyVmName(conn, command.getVmName(), false);`
`148`	`148`	`citrixResourceBase.removeSR(conn, sr);`
`149`	`149`	`final SR configDriveSR = citrixResourceBase.getISOSRbyVmName(conn, command.getVmName(), true);`