From 5d5ac17c685787556bb2015ca3c0dd4f2c9ace11 Mon Sep 17 00:00:00 2001
From: Abhishek Kumar <abhishek.mrt22@gmail.com>
Date: Wed, 9 Apr 2025 13:03:01 +0530
Subject: [PATCH] xenserver: do not destroy halted hypervisor vm (#9175)

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
---
 .../resource/CitrixResourceBase.java          | 38 ++++++++++++++-----
 .../xen56/XenServer56FenceCommandWrapper.java |  2 +-
 .../XenServer56FP1FenceCommandWrapper.java    |  2 +-
 .../CitrixCreateVMSnapshotCommandWrapper.java |  9 ++---
 .../CitrixDeleteVMSnapshotCommandWrapper.java |  2 +-
 ...itrixRevertToVMSnapshotCommandWrapper.java |  3 +-
 .../xenbase/CitrixStartCommandWrapper.java    |  2 +-
 .../xenbase/CitrixStopCommandWrapper.java     |  2 +-
 8 files changed, 40 insertions(+), 20 deletions(-)

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
index f53a70de722..322f69c9531 100644
--- 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 java.util.concurrent.TimeoutException;
 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.io.FileUtils;
 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;
 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 abstract class CitrixResourceBase extends ServerResourceBase implements S
 
                 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 abstract class CitrixResourceBase extends ServerResourceBase implements S
                 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 @@ public abstract class CitrixResourceBase extends ServerResourceBase implements S
             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 abstract class CitrixResourceBase extends ServerResourceBase implements S
             }
             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 @@ public abstract class CitrixResourceBase extends ServerResourceBase implements S
                 }
                 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 @@ public abstract class CitrixResourceBase extends ServerResourceBase implements S
             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
index 3cebbd9a342..713807ffaee 100644
--- 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 final class XenServer56FenceCommandWrapper extends CommandWrapper<FenceCo
             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
index bc7a4434bd0..dda87101de7 100644
--- 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 final class XenServer56FP1FenceCommandWrapper extends CommandWrapper<Fenc
                 }
                 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
index 68c295717c3..47c08b0cebe 100644
--- 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 final class CitrixCreateVMSnapshotCommandWrapper extends CommandWrapper<C
                     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 final class CitrixCreateVMSnapshotCommandWrapper extends CommandWrapper<C
                                 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
index b74111e8441..0bcbe13c558 100644
--- 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 final class CitrixDeleteVMSnapshotCommandWrapper extends CommandWrapper<D
             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
index f8bb1b89242..445e764a38c 100644
--- 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 final class CitrixRevertToVMSnapshotCommandWrapper extends CommandWrapper
             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 final class CitrixRevertToVMSnapshotCommandWrapper extends CommandWrapper
             }
 
             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
index ad76b7f4541..5867a151c85 100644
--- 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 final class CitrixStartCommandWrapper extends CommandWrapper<StartCommand
                 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
index 8e7eb4caec2..87c2cb76822 100644
--- 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 final class CitrixStopCommandWrapper extends CommandWrapper<StopCommand,
                             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);