[CLOUDSTACK-9025][CLOUDSTACK-10128] solve problems of templates created based on snapshots (#2315)

The first PR(#1176) intended to solve #CLOUDSTACK-9025 was only tackling the problem for CloudStack deployments that use single hypervisor types (restricted to XenServer). Additionally, the lack of information regarding that solution (poor documentation, test cases and description in PRs and Jira ticket) led the code to be removed in #1124 after a long discussion and analysis in #1056. That piece of code seemed logicless (and it was!).  It would receive a hostId and then change that hostId for other hostId of the zone without doing any check; it was not even checking the hypervisor and storage in which the host was plugged into.

The problem reported in #CLOUDSTACK-9025 is caused by partial snapshots that are taken in XenServer. This means, we do not take a complete snapshot, but a partial one that contains only the modified data. This requires rebuilding the VHD hierarchy when creating a template out of the snapshot. The point is that the first hostId received is not a hostId, but a system VM ID(SSVM). That is why the code in #1176 fixed the problem for some deployment scenarios, but would cause problems for scenarios where we have multiple hypervisors in the same zone. We need to execute the creation of the VHD that represents the template in the hypervisor, so the VHD chain can be built using the parent links.

This commit changes the method com.cloud.hypervisor.XenServerGuru.getCommandHostDelegation(long, Command). From now on we replace the hostId that is intended to execute the “copy command” that will create the VHD of the template according to some conditions that were already in place. The idea is that starting with XenServer 6.2.0 hotFix ESP1004 we need to execute the command in the hypervisor host and not from the SSVM. Moreover, the method was improved making it readable and understandable; it was also created test cases assuring that from XenServer 6.2.0 hotFix ESP1004 and upward versions we change the hostId that will be used to execute the “copy command”.

Furthermore, we are not selecting a random host from a zone anymore. A new method was introduced in the HostDao called “findHostConnectedToSnapshotStoragePoolToExecuteCommand”, using this method we look for a host that is in the cluster that is using the storage pool where the volume from which the Snaphost is taken of. By doing this, we guarantee that the host that is connected to the primary storage where all of the snapshots parent VHDs are stored is used to create the template.

Consider using Disabled hosts when no Enabled hosts are found

This also closes #2317

api/src/com/cloud/hypervisor/HypervisorGuru.java

@@ -33,7 +33,7 @@ import com.cloud.vm.VirtualMachineProfile;
 
 public interface HypervisorGuru extends Adapter {
     static final ConfigKey<Boolean> VmwareFullClone = new ConfigKey<Boolean>("Advanced", Boolean.class, "vmware.create.full.clone", "true",
-                        "If set to true, creates guest VMs as full clones on ESX", false);
+            "If set to true, creates guest VMs as full clones on ESX", false);
     HypervisorType getHypervisorType();
 
     /**
@@ -45,7 +45,7 @@ public interface HypervisorGuru extends Adapter {
     VirtualMachineTO implement(VirtualMachineProfile vm);
 
     /**
-     * Give hypervisor guru opportunity to decide if certain command needs to be delegated to other host, mainly to secondary storage VM host
+     * Gives hypervisor guru opportunity to decide if certain commands need to be delegated to another host, for instance, we may have the opportunity to change from a system VM (is considered a host) to a real host to execute commands.
      *
      * @param hostId original hypervisor host
      * @param cmd command that is going to be sent, hypervisor guru usually needs to register various context objects into the command object

core/src/org/apache/cloudstack/storage/command/CopyCommand.java

@@ -24,7 +24,7 @@ import java.util.Map;
 
 import com.cloud.agent.api.to.DataTO;
 
-public final class CopyCommand extends StorageSubSystemCommand {
+public class CopyCommand extends StorageSubSystemCommand {
     private DataTO srcTO;
     private DataTO destTO;
     private DataTO cacheTO;

engine/schema/src/com/cloud/host/dao/HostDao.java

@@ -19,6 +19,8 @@ package com.cloud.host.dao;
 import java.util.Date;
 import java.util.List;
 
+import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
+
 import com.cloud.host.Host;
 import com.cloud.host.Host.Type;
 import com.cloud.host.HostVO;
@@ -72,14 +74,6 @@ public interface HostDao extends GenericDao<HostVO, Long>, StateDao<Status, Stat
 
     List<HostVO> findHypervisorHostInCluster(long clusterId);
 
-    /**
-     * @param type
-     * @param clusterId
-     * @param podId
-     * @param dcId
-     * @param haTag TODO
-     * @return
-     */
     List<HostVO> listAllUpAndEnabledNonHAHosts(Type type, Long clusterId, Long podId, long dcId, String haTag);
 
     List<HostVO> findByDataCenterId(Long zoneId);
@@ -103,4 +97,16 @@ public interface HostDao extends GenericDao<HostVO, Long>, StateDao<Status, Stat
     List<HostVO> listByType(Type type);
 
     HostVO findByIp(String ip);
+
+    /**
+     * This method will look for a host that is of the same hypervisor and same zone as the storage pool where the volume of the Snapshot is stored.
+     * <ul>
+     * <li>(this is applicable only for XenServer) If the storage pool is managed, then we will look for a host that has the property 'supportsResign' in cloud.cluster_details
+     * <li>We give priority to 'Enabled' hosts, but if no 'Enabled' hosts are found, we use 'Disabled' hosts
+     * <li>If no host is found, we throw a runtime exception
+     * </ul>
+     *
+     * Side note: this method is currently only used in XenServerGuru; therefore, it was designed to meet XenServer deployment scenarios requirements.
+     */
+    HostVO findHostToOperateOnSnapshotBasedOnStoragePool(StoragePoolVO storagePoolVO);
 }

engine/schema/src/com/cloud/host/dao/HostDaoImpl.java

@@ -30,13 +30,15 @@ import javax.annotation.PostConstruct;
 import javax.inject.Inject;
 import javax.persistence.TableGenerator;
 
-import com.cloud.configuration.ManagementServiceConfiguration;
+import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
+import org.apache.commons.lang3.StringUtils;
 import org.apache.log4j.Logger;
 import org.springframework.stereotype.Component;
 
 import com.cloud.agent.api.VgpuTypesInfo;
 import com.cloud.cluster.agentlb.HostTransferMapVO;
 import com.cloud.cluster.agentlb.dao.HostTransferMapDao;
+import com.cloud.configuration.ManagementServiceConfiguration;
 import com.cloud.dc.ClusterVO;
 import com.cloud.dc.dao.ClusterDao;
 import com.cloud.gpu.dao.HostGpuGroupsDao;
@@ -305,11 +307,11 @@ public class HostDaoImpl extends GenericDaoBase<HostVO, Long> implements HostDao
         }
         HostTransferSearch.and("id", HostTransferSearch.entity().getId(), SearchCriteria.Op.NULL);
         UnmanagedDirectConnectSearch.join("hostTransferSearch", HostTransferSearch, HostTransferSearch.entity().getId(), UnmanagedDirectConnectSearch.entity().getId(),
-            JoinType.LEFTOUTER);
+                JoinType.LEFTOUTER);
         ClusterManagedSearch = _clusterDao.createSearchBuilder();
         ClusterManagedSearch.and("managed", ClusterManagedSearch.entity().getManagedState(), SearchCriteria.Op.EQ);
-        UnmanagedDirectConnectSearch.join("ClusterManagedSearch", ClusterManagedSearch, ClusterManagedSearch.entity().getId(), UnmanagedDirectConnectSearch.entity()
-            .getClusterId(), JoinType.INNER);
+        UnmanagedDirectConnectSearch.join("ClusterManagedSearch", ClusterManagedSearch, ClusterManagedSearch.entity().getId(), UnmanagedDirectConnectSearch.entity().getClusterId(),
+                JoinType.INNER);
         UnmanagedDirectConnectSearch.done();
 
         DirectConnectSearch = createSearchBuilder();
@@ -388,7 +390,8 @@ public class HostDaoImpl extends GenericDaoBase<HostVO, Long> implements HostDao
 
         ClusterManagedSearch = _clusterDao.createSearchBuilder();
         ClusterManagedSearch.and("managed", ClusterManagedSearch.entity().getManagedState(), SearchCriteria.Op.EQ);
-        ClustersForHostsNotOwnedByAnyMSSearch.join("ClusterManagedSearch", ClusterManagedSearch, ClusterManagedSearch.entity().getId(), ClustersForHostsNotOwnedByAnyMSSearch.entity().getClusterId(), JoinType.INNER);
+        ClustersForHostsNotOwnedByAnyMSSearch.join("ClusterManagedSearch", ClusterManagedSearch, ClusterManagedSearch.entity().getId(),
+                ClustersForHostsNotOwnedByAnyMSSearch.entity().getClusterId(), JoinType.INNER);
 
         ClustersForHostsNotOwnedByAnyMSSearch.done();
 
@@ -520,18 +523,7 @@ public class HostDaoImpl extends GenericDaoBase<HostVO, Long> implements HostDao
         return clusters;
     }
 
-    /*
-     * Returns a list of all cluster Ids
-     */
-    private List<Long> listAllClusters() {
-        SearchCriteria<Long> sc = AllClustersSearch.create();
-        sc.setParameters("managed", Managed.ManagedState.Managed);
-
-        List<Long> clusters = _clusterDao.customSearch(sc, null);
-        return clusters;
-    }
-
-    /*
+    /**
      * This determines if hosts belonging to cluster(@clusterId) are up for grabs
      *
      * This is used for handling following cases:
@@ -656,7 +648,7 @@ public class HostDaoImpl extends GenericDaoBase<HostVO, Long> implements HostDao
         SearchCriteria<HostVO> sc = UnmanagedApplianceSearch.create();
         sc.setParameters("lastPinged", lastPingSecondsAfter);
         sc.setParameters("types", Type.ExternalDhcp, Type.ExternalFirewall, Type.ExternalLoadBalancer, Type.BaremetalDhcp, Type.BaremetalPxe, Type.TrafficMonitor,
-            Type.L2Networking, Type.NetScalerControlCenter);
+                Type.L2Networking, Type.NetScalerControlCenter);
         List<HostVO> hosts = lockRows(sc, null, true);
 
         for (HostVO host : hosts) {
@@ -790,10 +782,8 @@ public class HostDaoImpl extends GenericDaoBase<HostVO, Long> implements HostDao
     @Override
     public List<HostVO> findLostHosts(long timeout) {
         List<HostVO> result = new ArrayList<HostVO>();
-        String sql =
-                "select h.id from host h left join  cluster c on h.cluster_id=c.id where h.mgmt_server_id is not null and h.last_ping < ? and h.status in ('Up', 'Updating', 'Disconnected', 'Connecting') and h.type not in ('ExternalFirewall', 'ExternalLoadBalancer', 'TrafficMonitor', 'SecondaryStorage', 'LocalSecondaryStorage', 'L2Networking') and (h.cluster_id is null or c.managed_state = 'Managed') ;";
-        try (
-                TransactionLegacy txn = TransactionLegacy.currentTxn();
+        String sql = "select h.id from host h left join  cluster c on h.cluster_id=c.id where h.mgmt_server_id is not null and h.last_ping < ? and h.status in ('Up', 'Updating', 'Disconnected', 'Connecting') and h.type not in ('ExternalFirewall', 'ExternalLoadBalancer', 'TrafficMonitor', 'SecondaryStorage', 'LocalSecondaryStorage', 'L2Networking') and (h.cluster_id is null or c.managed_state = 'Managed') ;";
+        try (TransactionLegacy txn = TransactionLegacy.currentTxn();
                 PreparedStatement pstmt = txn.prepareStatement(sql);) {
             pstmt.setLong(1, timeout);
             try (ResultSet rs = pstmt.executeQuery();) {
@@ -887,8 +877,7 @@ public class HostDaoImpl extends GenericDaoBase<HostVO, Long> implements HostDao
     @Override
     @DB
     public List<RunningHostCountInfo> getRunningHostCounts(Date cutTime) {
-        String sql =
-            "select * from (" + "select h.data_center_id, h.type, count(*) as count from host as h INNER JOIN mshost as m ON h.mgmt_server_id=m.msid "
+        String sql = "select * from (" + "select h.data_center_id, h.type, count(*) as count from host as h INNER JOIN mshost as m ON h.mgmt_server_id=m.msid "
                 + "where h.status='Up' and h.type='SecondaryStorage' and m.last_update > ? " + "group by h.data_center_id, h.type " + "UNION ALL "
                 + "select h.data_center_id, h.type, count(*) as count from host as h INNER JOIN mshost as m ON h.mgmt_server_id=m.msid "
                 + "where h.status='Up' and h.type='Routing' and m.last_update > ? " + "group by h.data_center_id, h.type) as t " + "ORDER by t.data_center_id, t.type";
@@ -1006,24 +995,12 @@ public class HostDaoImpl extends GenericDaoBase<HostVO, Long> implements HostDao
 
                 StringBuilder str = new StringBuilder("Unable to update host for event:").append(event.toString());
                 str.append(". Name=").append(host.getName());
-                str.append("; New=[status=")
-                        .append(newStatus.toString())
-                        .append(":msid=")
-                        .append(newStatus.lostConnection() ? "null" : host.getManagementServerId())
-                        .append(":lastpinged=")
-                        .append(host.getLastPinged())
-                        .append("]");
+                str.append("; New=[status=").append(newStatus.toString()).append(":msid=").append(newStatus.lostConnection() ? "null" : host.getManagementServerId())
+                .append(":lastpinged=").append(host.getLastPinged()).append("]");
                 str.append("; Old=[status=").append(oldStatus.toString()).append(":msid=").append(host.getManagementServerId()).append(":lastpinged=").append(oldPingTime)
-                        .append("]");
-                str.append("; DB=[status=")
-                        .append(vo.getStatus().toString())
-                        .append(":msid=")
-                        .append(vo.getManagementServerId())
-                        .append(":lastpinged=")
-                        .append(vo.getLastPinged())
-                        .append(":old update count=")
-                        .append(oldUpdateCount)
-                        .append("]");
+                .append("]");
+                str.append("; DB=[status=").append(vo.getStatus().toString()).append(":msid=").append(vo.getManagementServerId()).append(":lastpinged=").append(vo.getLastPinged())
+                .append(":old update count=").append(oldUpdateCount).append("]");
                 status_logger.debug(str.toString());
             } else {
                 StringBuilder msg = new StringBuilder("Agent status update: [");
@@ -1190,4 +1167,48 @@ public class HostDaoImpl extends GenericDaoBase<HostVO, Long> implements HostDao
         sc.setParameters("type", type);
         return listBy(sc);
     }
+
+    String sqlFindHostConnectedToStoragePoolToExecuteCommand = "select h.id from storage_pool pool "
+            + " join cluster c on pool.cluster_id = c.id "
+            + " %s "
+            + " join host h on h.data_center_id = c.data_center_id and h.hypervisor_type = c.hypervisor_type"
+            + " where pool.id = ? and h.status = 'Up' and h.type = 'Routing' and resource_state = '%s' "
+            + " ORDER by rand() limit 1 ";
+
+    @Override
+    public HostVO findHostToOperateOnSnapshotBasedOnStoragePool(StoragePoolVO storagePoolVO) {
+        try (TransactionLegacy tx = TransactionLegacy.currentTxn()) {
+            String sql = createSqlFindHostConnectedToStoragePoolToExecuteCommand(storagePoolVO, false);
+            ResultSet rs = executeSqlGetResultSetForMethodFindHostToOperateBasedOnStoragePool(storagePoolVO, tx, sql);
+            if (rs.next()) {
+                return findById(rs.getLong("id"));
+            }
+            sql = createSqlFindHostConnectedToStoragePoolToExecuteCommand(storagePoolVO, true);
+            rs = executeSqlGetResultSetForMethodFindHostToOperateBasedOnStoragePool(storagePoolVO, tx, sql);
+            if (!rs.next()) {
+                throw new CloudRuntimeException(String.format("Could not find a host connected to the storage pool [storagepool=%d]. ", storagePoolVO.getId()));
+            }
+            return findById(rs.getLong("id"));
+        } catch (SQLException e) {
+            throw new CloudRuntimeException(e);
+        }
+    }
+
+    private ResultSet executeSqlGetResultSetForMethodFindHostToOperateBasedOnStoragePool(StoragePoolVO storagePoolVO, TransactionLegacy tx, String sql) throws SQLException {
+        PreparedStatement pstmt = tx.prepareAutoCloseStatement(sql);
+        pstmt.setLong(1, storagePoolVO.getId());
+        return pstmt.executeQuery();
+    }
+
+    private String createSqlFindHostConnectedToStoragePoolToExecuteCommand(StoragePoolVO storagePoolVO, boolean useDisabledHosts) {
+        String hostResourceStatus = "Enabled";
+        if (useDisabledHosts) {
+            hostResourceStatus = "Disabled";
+        }
+        String joinForManagedStorage = StringUtils.EMPTY;
+        if (storagePoolVO.isManaged()) {
+            joinForManagedStorage = " join cluster_details cd on cd.cluster_id = c.id and cd.name = 'supportsResign' and cd.value = 'true' ";
+        }
+        return String.format(sqlFindHostConnectedToStoragePoolToExecuteCommand, joinForManagedStorage, hostResourceStatus);
+    }
 }

engine/schema/src/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDao.java

@@ -123,4 +123,11 @@ public interface PrimaryDataStoreDao extends GenericDao<StoragePoolVO, Long> {
     List<StoragePoolVO> listLocalStoragePoolByPath(long datacenterId, String path);
 
     void deletePoolTags(long poolId);
+
+    /**
+     *  Looks for a storage pool where the original volume of the snapshot was taken.
+     *  Even if the volume has already been deleted, we will return the last storage pool where it was stored.
+     */
+    StoragePoolVO findStoragePoolForSnapshot(long snapshotId);
+
 }

engine/schema/src/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java

@@ -50,33 +50,29 @@ import com.cloud.utils.exception.CloudRuntimeException;
 
 @DB()
 public class PrimaryDataStoreDaoImpl extends GenericDaoBase<StoragePoolVO, Long> implements PrimaryDataStoreDao {
-    protected final SearchBuilder<StoragePoolVO> AllFieldSearch;
-    protected final SearchBuilder<StoragePoolVO> DcPodSearch;
-    protected final SearchBuilder<StoragePoolVO> DcPodAnyClusterSearch;
-    protected final SearchBuilder<StoragePoolVO> DeleteLvmSearch;
-    protected final SearchBuilder<StoragePoolVO> DcLocalStorageSearch;
-    protected final GenericSearchBuilder<StoragePoolVO, Long> StatusCountSearch;
+    private final SearchBuilder<StoragePoolVO> AllFieldSearch;
+    private final SearchBuilder<StoragePoolVO> DcPodSearch;
+    private final SearchBuilder<StoragePoolVO> DcPodAnyClusterSearch;
+    private final SearchBuilder<StoragePoolVO> DeleteLvmSearch;
+    private final SearchBuilder<StoragePoolVO> DcLocalStorageSearch;
+    private final GenericSearchBuilder<StoragePoolVO, Long> StatusCountSearch;
 
     @Inject
-    protected StoragePoolDetailsDao _detailsDao;
+    private StoragePoolDetailsDao _detailsDao;
     @Inject
-    protected StoragePoolHostDao _hostDao;
+    private StoragePoolHostDao _hostDao;
     @Inject
-    protected StoragePoolTagsDao _tagsDao;
+    private StoragePoolTagsDao _tagsDao;
 
-    protected final String DetailsSqlPrefix =
-        "SELECT storage_pool.* from storage_pool LEFT JOIN storage_pool_details ON storage_pool.id = storage_pool_details.pool_id WHERE storage_pool.removed is null and storage_pool.status = 'Up' and storage_pool.data_center_id = ? and (storage_pool.pod_id = ? or storage_pool.pod_id is null) and storage_pool.scope = ? and (";
+    protected final String DetailsSqlPrefix = "SELECT storage_pool.* from storage_pool LEFT JOIN storage_pool_details ON storage_pool.id = storage_pool_details.pool_id WHERE storage_pool.removed is null and storage_pool.status = 'Up' and storage_pool.data_center_id = ? and (storage_pool.pod_id = ? or storage_pool.pod_id is null) and storage_pool.scope = ? and (";
     protected final String DetailsSqlSuffix = ") GROUP BY storage_pool_details.pool_id HAVING COUNT(storage_pool_details.name) >= ?";
-    protected final String ZoneWideTagsSqlPrefix =
-        "SELECT storage_pool.* from storage_pool LEFT JOIN storage_pool_tags ON storage_pool.id = storage_pool_tags.pool_id WHERE storage_pool.removed is null and storage_pool.status = 'Up' and storage_pool.data_center_id = ? and storage_pool.scope = ? and (";
-    protected final String ZoneWideTagsSqlSuffix = ") GROUP BY storage_pool_tags.pool_id HAVING COUNT(storage_pool_tags.tag) >= ?";
+    private final String ZoneWideTagsSqlPrefix = "SELECT storage_pool.* from storage_pool LEFT JOIN storage_pool_tags ON storage_pool.id = storage_pool_tags.pool_id WHERE storage_pool.removed is null and storage_pool.status = 'Up' and storage_pool.data_center_id = ? and storage_pool.scope = ? and (";
+    private final String ZoneWideTagsSqlSuffix = ") GROUP BY storage_pool_tags.pool_id HAVING COUNT(storage_pool_tags.tag) >= ?";
 
     // Storage tags are now separate from storage_pool_details, leaving only details on that table
     protected final String TagsSqlPrefix = "SELECT storage_pool.* from storage_pool LEFT JOIN storage_pool_tags ON storage_pool.id = storage_pool_tags.pool_id WHERE storage_pool.removed is null and storage_pool.status = 'Up' and storage_pool.data_center_id = ? and (storage_pool.pod_id = ? or storage_pool.pod_id is null) and storage_pool.scope = ? and (";
     protected final String TagsSqlSuffix = ") GROUP BY storage_pool_tags.pool_id HAVING COUNT(storage_pool_tags.tag) >= ?";
 
-    protected final String FindPoolTags = "SELECT storage_pool_tags.tag FROM storage_pool_tags WHERE pool_id = ?";
-
     /**
      * Used in method findPoolsByDetailsOrTagsInternal
      */
@@ -300,7 +296,8 @@ public class PrimaryDataStoreDaoImpl extends GenericDaoBase<StoragePoolVO, Long>
      * @param valuesLength values length
      * @return list of storage pools matching conditions
      */
-    protected List<StoragePoolVO> findPoolsByDetailsOrTagsInternal(long dcId, long podId, Long clusterId, ScopeType scope, String sqlValues, ValueType valuesType, int valuesLength) {
+    protected List<StoragePoolVO> findPoolsByDetailsOrTagsInternal(long dcId, long podId, Long clusterId, ScopeType scope, String sqlValues, ValueType valuesType,
+            int valuesLength) {
         String sqlPrefix = valuesType.equals(ValueType.DETAILS) ? DetailsSqlPrefix : TagsSqlPrefix;
         String sqlSuffix = valuesType.equals(ValueType.DETAILS) ? DetailsSqlSuffix : TagsSqlSuffix;
         String sql = getSqlPreparedStatement(sqlPrefix, sqlSuffix, sqlValues, clusterId);
@@ -321,7 +318,7 @@ public class PrimaryDataStoreDaoImpl extends GenericDaoBase<StoragePoolVO, Long>
     protected List<StoragePoolVO> searchStoragePoolsPreparedStatement(String sql, long dcId, Long podId, Long clusterId, ScopeType scope, int valuesLength) {
         TransactionLegacy txn = TransactionLegacy.currentTxn();
         List<StoragePoolVO> pools = new ArrayList<StoragePoolVO>();
-        try (PreparedStatement pstmt = txn.prepareStatement(sql);){
+        try (PreparedStatement pstmt = txn.prepareStatement(sql);) {
             if (pstmt != null) {
                 int i = 1;
                 pstmt.setLong(i++, dcId);
@@ -333,11 +330,11 @@ public class PrimaryDataStoreDaoImpl extends GenericDaoBase<StoragePoolVO, Long>
                     pstmt.setLong(i++, clusterId);
                 }
                 pstmt.setInt(i++, valuesLength);
-                try(ResultSet rs = pstmt.executeQuery();) {
+                try (ResultSet rs = pstmt.executeQuery();) {
                     while (rs.next()) {
                         pools.add(toEntityBean(rs, false));
                     }
-                }catch (SQLException e) {
+                } catch (SQLException e) {
                     throw new CloudRuntimeException("Unable to execute :" + e.getMessage(), e);
                 }
             }
@@ -347,14 +344,6 @@ public class PrimaryDataStoreDaoImpl extends GenericDaoBase<StoragePoolVO, Long>
         return pools;
     }
 
-    /**
-     * Return SQL prepared statement given prefix, values and suffix
-     * @param sqlPrefix prefix
-     * @param sqlSuffix suffix
-     * @param sqlValues tags or details values
-     * @param clusterId cluster id
-     * @return sql prepared statement
-     */
     protected String getSqlPreparedStatement(String sqlPrefix, String sqlSuffix, String sqlValues, Long clusterId) {
         StringBuilder sql = new StringBuilder(sqlPrefix);
         if (clusterId != null) {
@@ -375,11 +364,7 @@ public class PrimaryDataStoreDaoImpl extends GenericDaoBase<StoragePoolVO, Long>
     protected String getSqlValuesFromDetails(Map<String, String> details) {
         StringBuilder sqlValues = new StringBuilder();
         for (Map.Entry<String, String> detail : details.entrySet()) {
-            sqlValues.append("((storage_pool_details.name='")
-                .append(detail.getKey())
-                .append("') AND (storage_pool_details.value='")
-                .append(detail.getValue())
-                .append("')) OR ");
+            sqlValues.append("((storage_pool_details.name='").append(detail.getKey()).append("') AND (storage_pool_details.value='").append(detail.getValue()).append("')) OR ");
         }
         sqlValues.delete(sqlValues.length() - 4, sqlValues.length());
         return sqlValues.toString();
@@ -395,9 +380,7 @@ public class PrimaryDataStoreDaoImpl extends GenericDaoBase<StoragePoolVO, Long>
     protected String getSqlValuesFromStorageTags(String[] tags) throws NullPointerException, IndexOutOfBoundsException {
         StringBuilder sqlValues = new StringBuilder();
         for (String tag : tags) {
-            sqlValues.append("(storage_pool_tags.tag='")
-                .append(tag)
-                .append("') OR ");
+            sqlValues.append("(storage_pool_tags.tag='").append(tag).append("') OR ");
         }
         sqlValues.delete(sqlValues.length() - 4, sqlValues.length());
         return sqlValues.toString();
@@ -460,7 +443,8 @@ public class PrimaryDataStoreDaoImpl extends GenericDaoBase<StoragePoolVO, Long>
     public List<StoragePoolVO> findLocalStoragePoolsByHostAndTags(long hostId, String[] tags) {
         SearchBuilder<StoragePoolVO> hostSearch = createSearchBuilder();
         SearchBuilder<StoragePoolHostVO> hostPoolSearch = _hostDao.createSearchBuilder();
-        SearchBuilder<StoragePoolTagVO> tagPoolSearch = _tagsDao.createSearchBuilder();;
+        SearchBuilder<StoragePoolTagVO> tagPoolSearch = _tagsDao.createSearchBuilder();
+        ;
 
         // Search for pools on the host
         hostPoolSearch.and("hostId", hostPoolSearch.entity().getHostId(), Op.EQ);
@@ -470,17 +454,17 @@ public class PrimaryDataStoreDaoImpl extends GenericDaoBase<StoragePoolVO, Long>
         hostSearch.and("status", hostSearch.entity().getStatus(), Op.EQ);
         hostSearch.join("hostJoin", hostPoolSearch, hostSearch.entity().getId(), hostPoolSearch.entity().getPoolId(), JoinBuilder.JoinType.INNER);
 
-        if (!(tags == null || tags.length == 0 )) {
+        if (!(tags == null || tags.length == 0)) {
             tagPoolSearch.and("tag", tagPoolSearch.entity().getTag(), Op.EQ);
             hostSearch.join("tagJoin", tagPoolSearch, hostSearch.entity().getId(), tagPoolSearch.entity().getPoolId(), JoinBuilder.JoinType.INNER);
         }
 
         SearchCriteria<StoragePoolVO> sc = hostSearch.create();
-        sc.setJoinParameters("hostJoin", "hostId", hostId );
+        sc.setJoinParameters("hostJoin", "hostId", hostId);
         sc.setParameters("scope", ScopeType.HOST.toString());
         sc.setParameters("status", Status.Up.toString());
 
-        if (!(tags == null || tags.length == 0 )) {
+        if (!(tags == null || tags.length == 0)) {
             for (String tag : tags) {
                 sc.setJoinParameters("tagJoin", "tag", tag);
             }
@@ -516,7 +500,7 @@ public class PrimaryDataStoreDaoImpl extends GenericDaoBase<StoragePoolVO, Long>
                 detailsVO.add(new StoragePoolDetailVO(poolId, key, details.get(key), true));
             }
             _detailsDao.saveDetails(detailsVO);
-            if(details.size() == 0) {
+            if (details.size() == 0) {
                 _detailsDao.removeDetails(poolId);
             }
         }
@@ -571,4 +555,24 @@ public class PrimaryDataStoreDaoImpl extends GenericDaoBase<StoragePoolVO, Long>
         _tagsDao.deleteTags(poolId);
     }
 
+    private String sqlIsSnapshotStoragePoolManaged = "select pool.id from snapshots s "
+            + " join volumes v on v.id = s.volume_id "
+            + " join storage_pool pool on pool.id = v.pool_id  "
+            + " where s.id = ?";
+
+    @Override
+    public StoragePoolVO findStoragePoolForSnapshot(long snapshotId) {
+        try (TransactionLegacy tx = TransactionLegacy.currentTxn();
+                PreparedStatement pstmt = tx.prepareAutoCloseStatement(sqlIsSnapshotStoragePoolManaged);) {
+            pstmt.setLong(1, snapshotId);
+            ResultSet rs = pstmt.executeQuery();
+            if (!rs.next()) {
+                throw new CloudRuntimeException(String.format("Could not find a storage pool for snapshot [snapshotId=%d]. ", snapshotId));
+            }
+            long storagePoolId = rs.getLong("id");
+            return findById(storagePoolId);
+        } catch (SQLException e) {
+            throw new CloudRuntimeException(e);
+        }
+    }
 }

engine/schema/test/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImplTest.java

@@ -63,7 +63,7 @@ public class PrimaryDataStoreDaoImplTest extends TestCase {
 
     private static final String DETAIL_KEY = "storage.overprovisioning.factor";
     private static final String DETAIL_VALUE = "2.0";
-    private static final Map<String, String> STORAGE_POOL_DETAILS = new HashMap<String, String>(){{ put(DETAIL_KEY, DETAIL_VALUE); }};
+    private static final Map<String, String> STORAGE_POOL_DETAILS = new HashMap<>();
 
     private static final String EXPECTED_RESULT_SQL_STORAGE_TAGS = "(storage_pool_tags.tag='" + STORAGE_TAG_1 + "') OR (storage_pool_tags.tag='" + STORAGE_TAG_2 + "')";
     private static final String EXPECTED_RESULT_SQL_DETAILS = "((storage_pool_details.name='" + DETAIL_KEY + "') AND (storage_pool_details.value='" + DETAIL_VALUE +"'))";
@@ -79,9 +79,10 @@ public class PrimaryDataStoreDaoImplTest extends TestCase {
 
     @Before
     public void setup() {
+        STORAGE_POOL_DETAILS.put(DETAIL_KEY, DETAIL_VALUE);
         doReturn(Arrays.asList(storagePoolVO)).when(primaryDataStoreDao).
-            searchStoragePoolsPreparedStatement(Matchers.anyString(), Matchers.anyLong(), Matchers.anyLong(), Matchers.anyLong(),
-                    Matchers.any(ScopeType.class), Matchers.anyInt());
+        searchStoragePoolsPreparedStatement(Matchers.anyString(), Matchers.anyLong(), Matchers.anyLong(), Matchers.anyLong(),
+                Matchers.any(ScopeType.class), Matchers.anyInt());
     }
 
     @Test

plugins/hypervisors/xenserver/src/com/cloud/hypervisor/XenServerGuru.java

@@ -32,6 +32,7 @@ import org.apache.cloudstack.storage.command.DettachCommand;
 import org.apache.cloudstack.storage.command.StorageSubSystemCommand;
 import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
 import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
+import org.apache.commons.lang.StringUtils;
 import org.apache.log4j.Logger;
 
 import com.cloud.agent.api.Command;
@@ -58,23 +59,25 @@ import com.cloud.vm.VirtualMachineProfile;
 import com.cloud.vm.dao.UserVmDao;
 
 public class XenServerGuru extends HypervisorGuruBase implements HypervisorGuru, Configurable {
-    private final Logger LOGGER = Logger.getLogger(XenServerGuru.class);
+
+    private Logger logger = Logger.getLogger(getClass());
+
     @Inject
-    private GuestOSDao _guestOsDao;
+    private GuestOSDao guestOsDao;
     @Inject
-    private GuestOSHypervisorDao _guestOsHypervisorDao;
+    private GuestOSHypervisorDao guestOsHypervisorDao;
     @Inject
     private HostDao hostDao;
     @Inject
-    private VolumeDao _volumeDao;
+    private VolumeDao volumeDao;
     @Inject
-    private PrimaryDataStoreDao _storagePoolDao;
+    private PrimaryDataStoreDao storagePoolDao;
     @Inject
-    private VolumeDataFactory _volFactory;
+    private VolumeDataFactory volFactory;
     @Inject
-    private UserVmDao _userVmDao;
+    private UserVmDao userVmDao;
     @Inject
-    GuestOsDetailsDao _guestOsDetailsDao;
+    private GuestOsDetailsDao guestOsDetailsDao;
 
     private static final ConfigKey<Integer> MaxNumberOfVCPUSPerVM = new ConfigKey<Integer>("Advanced", Integer.class, "xen.vm.vcpu.max", "16",
             "Maximum number of VCPUs that VM can get in XenServer.", true, ConfigKey.Scope.Cluster);
@@ -91,7 +94,7 @@ public class XenServerGuru extends HypervisorGuruBase implements HypervisorGuru,
             bt = vm.getBootLoaderType();
         }
         VirtualMachineTO to = toVirtualMachineTO(vm);
-        UserVmVO userVmVO = _userVmDao.findById(vm.getId());
+        UserVmVO userVmVO = userVmDao.findById(vm.getId());
         if (userVmVO != null) {
             HostVO host = hostDao.findById(userVmVO.getHostId());
             if (host != null) {
@@ -102,9 +105,9 @@ public class XenServerGuru extends HypervisorGuruBase implements HypervisorGuru,
         to.setBootloader(bt);
 
         // Determine the VM's OS description
-        GuestOSVO guestOS = _guestOsDao.findByIdIncludingRemoved(vm.getVirtualMachine().getGuestOSId());
+        GuestOSVO guestOS = guestOsDao.findByIdIncludingRemoved(vm.getVirtualMachine().getGuestOSId());
 
-        Map<String, String> guestOsDetails = _guestOsDetailsDao.listDetailsKeyPairs(vm.getVirtualMachine().getGuestOSId());
+        Map<String, String> guestOsDetails = guestOsDetailsDao.listDetailsKeyPairs(vm.getVirtualMachine().getGuestOSId());
 
         to.setGuestOsDetails(guestOsDetails);
 
@@ -112,7 +115,7 @@ public class XenServerGuru extends HypervisorGuruBase implements HypervisorGuru,
         HostVO host = hostDao.findById(vm.getVirtualMachine().getHostId());
         GuestOSHypervisorVO guestOsMapping = null;
         if (host != null) {
-            guestOsMapping = _guestOsHypervisorDao.findByOsIdAndHypervisor(guestOS.getId(), getHypervisorType().toString(), host.getHypervisorVersion());
+            guestOsMapping = guestOsHypervisorDao.findByOsIdAndHypervisor(guestOS.getId(), getHypervisorType().toString(), host.getHypervisorVersion());
         }
         if (guestOsMapping == null || host == null) {
             to.setPlatformEmulator(null);
@@ -132,20 +135,20 @@ public class XenServerGuru extends HypervisorGuruBase implements HypervisorGuru,
     public List<Command> finalizeExpungeVolumes(VirtualMachine vm) {
         List<Command> commands = new ArrayList<Command>();
 
-        List<VolumeVO> volumes = _volumeDao.findByInstance(vm.getId());
+        List<VolumeVO> volumes = volumeDao.findByInstance(vm.getId());
 
         // it's OK in this case to send a detach command to the host for a root volume as this
         // will simply lead to the SR that supports the root volume being removed
         if (volumes != null) {
             for (VolumeVO volume : volumes) {
-                StoragePoolVO storagePool = _storagePoolDao.findById(volume.getPoolId());
+                StoragePoolVO storagePool = storagePoolDao.findById(volume.getPoolId());
 
                 // storagePool should be null if we are expunging a volume that was never
                 // attached to a VM that was started (the "trick" for storagePool to be null
                 // is that none of the VMs this volume may have been attached to were ever started,
                 // so the volume was never assigned to a storage pool)
                 if (storagePool != null && storagePool.isManaged()) {
-                    DataTO volTO = _volFactory.getVolume(volume.getId()).getTO();
+                    DataTO volTO = volFactory.getVolume(volume.getId()).getTO();
                     DiskTO disk = new DiskTO(volTO, volume.getDeviceId(), volume.getPath(), volume.getVolumeType());
 
                     DettachCommand cmd = new DettachCommand(disk, vm.getInstanceName());
@@ -167,35 +170,62 @@ public class XenServerGuru extends HypervisorGuruBase implements HypervisorGuru,
 
     @Override
     public Pair<Boolean, Long> getCommandHostDelegation(long hostId, Command cmd) {
-        LOGGER.debug("getCommandHostDelegation: " + cmd.getClass());
         if (cmd instanceof StorageSubSystemCommand) {
             StorageSubSystemCommand c = (StorageSubSystemCommand)cmd;
             c.setExecuteInSequence(true);
         }
-        if (cmd instanceof CopyCommand) {
-            CopyCommand cpyCommand = (CopyCommand)cmd;
-            DataTO srcData = cpyCommand.getSrcTO();
-            DataTO destData = cpyCommand.getDestTO();
-
-            if (srcData.getHypervisorType() == HypervisorType.XenServer && srcData.getObjectType() == DataObjectType.SNAPSHOT &&
-                    destData.getObjectType() == DataObjectType.TEMPLATE) {
-                DataStoreTO srcStore = srcData.getDataStore();
-                DataStoreTO destStore = destData.getDataStore();
-                if (srcStore instanceof NfsTO && destStore instanceof NfsTO) {
-                    HostVO host = hostDao.findById(hostId);
-                    hostDao.loadDetails(host);
-                    String hypervisorVersion = host.getHypervisorVersion();
-                    String snapshotHotFixVersion = host.getDetail(XenserverConfigs.XS620HotFix);
-                    if (hypervisorVersion != null && !hypervisorVersion.equalsIgnoreCase("6.1.0")) {
-                        if (!(hypervisorVersion.equalsIgnoreCase("6.2.0") &&
-                                !(snapshotHotFixVersion != null && snapshotHotFixVersion.equalsIgnoreCase(XenserverConfigs.XSHotFix62ESP1004)))) {
-                            return new Pair<Boolean, Long>(Boolean.TRUE, new Long(host.getId()));
-                        }
-                    }
-                }
-            }
+        boolean isCopyCommand = cmd instanceof CopyCommand;
+        Pair<Boolean, Long> defaultHostToExecuteCommands = super.getCommandHostDelegation(hostId, cmd);
+        if (!isCopyCommand) {
+            logger.debug("We are returning the default host to execute commands because the command is not of Copy type.");
+            return defaultHostToExecuteCommands;
         }
-        return new Pair<Boolean, Long>(Boolean.FALSE, new Long(hostId));
+        CopyCommand copyCommand = (CopyCommand)cmd;
+        DataTO srcData = copyCommand.getSrcTO();
+        DataTO destData = copyCommand.getDestTO();
+
+        boolean isSourceDataHypervisorXenServer = srcData.getHypervisorType() == HypervisorType.XenServer;
+        if (!isSourceDataHypervisorXenServer) {
+            logger.debug("We are returning the default host to execute commands because the target hypervisor of the source data is not XenServer.");
+            return defaultHostToExecuteCommands;
+        }
+        DataStoreTO srcStore = srcData.getDataStore();
+        DataStoreTO destStore = destData.getDataStore();
+        boolean isSourceAndDestinationNfsObjects = srcStore instanceof NfsTO && destStore instanceof NfsTO;
+        if (!isSourceAndDestinationNfsObjects) {
+            logger.debug("We are returning the default host to execute commands because the source and destination objects are not NFS type.");
+            return defaultHostToExecuteCommands;
+        }
+        boolean isSourceObjectSnapshotTypeAndDestinationObjectTemplateType = srcData.getObjectType() == DataObjectType.SNAPSHOT
+                && destData.getObjectType() == DataObjectType.TEMPLATE;
+        if (!isSourceObjectSnapshotTypeAndDestinationObjectTemplateType) {
+            logger.debug("We are returning the default host to execute commands because the source and destination objects are not snapshot and template respectively.");
+            return defaultHostToExecuteCommands;
+        }
+        long snapshotId = srcData.getId();
+        StoragePoolVO storagePoolVO = storagePoolDao.findStoragePoolForSnapshot(snapshotId);
+        HostVO hostCandidateToExecutedCommand = hostDao.findHostToOperateOnSnapshotBasedOnStoragePool(storagePoolVO);
+        hostDao.loadDetails(hostCandidateToExecutedCommand);
+        String hypervisorVersion = hostCandidateToExecutedCommand.getHypervisorVersion();
+        if (StringUtils.isBlank(hypervisorVersion)) {
+            logger.debug("We are returning the default host to execute commands because the hypervisor version is blank.");
+            return defaultHostToExecuteCommands;
+        }
+        boolean isXenServer610 = StringUtils.equals(hypervisorVersion, "6.1.0");
+        if (isXenServer610) {
+            logger.debug("We are returning the default host to execute commands because the hypervisor version is 6.1.0.");
+            return defaultHostToExecuteCommands;
+        }
+        String snapshotHotFixVersion = hostCandidateToExecutedCommand.getDetail(XenserverConfigs.XS620HotFix);
+        boolean isXenServer620 = StringUtils.equals(hypervisorVersion, "6.2.0");
+        if (isXenServer620 && !StringUtils.equalsIgnoreCase(XenserverConfigs.XSHotFix62ESP1004, snapshotHotFixVersion)) {
+            logger.debug(String.format(
+                    "We are returning the default host to execute commands because the hypervisor version is not 6.2.0 with hotfix ESP1004 [hypervisorVersion=%s, hotfixVersion=%s]",
+                    hypervisorVersion, snapshotHotFixVersion));
+            return defaultHostToExecuteCommands;
+        }
+        logger.debug(String.format("We are changing the hostId to executed command from %d to %d.", hostId, hostCandidateToExecutedCommand.getId()));
+        return new Pair<Boolean, Long>(Boolean.TRUE, new Long(hostCandidateToExecutedCommand.getId()));
     }
 
     @Override

plugins/hypervisors/xenserver/test/com/cloud/hypervisor/XenServerGuruTest.java

@@ -0,0 +1,235 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package com.cloud.hypervisor;
+
+import org.apache.cloudstack.hypervisor.xenserver.XenserverConfigs;
+import org.apache.cloudstack.storage.command.CopyCommand;
+import org.apache.cloudstack.storage.command.StorageSubSystemCommand;
+import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
+import org.apache.commons.lang.StringUtils;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.runners.MockitoJUnitRunner;
+
+import com.cloud.agent.api.Command;
+import com.cloud.agent.api.to.DataObjectType;
+import com.cloud.agent.api.to.DataStoreTO;
+import com.cloud.agent.api.to.DataTO;
+import com.cloud.agent.api.to.NfsTO;
+import com.cloud.host.HostVO;
+import com.cloud.host.dao.HostDao;
+import com.cloud.hypervisor.Hypervisor.HypervisorType;
+import com.cloud.utils.Pair;
+
+@RunWith(MockitoJUnitRunner.class)
+public class XenServerGuruTest {
+
+    @InjectMocks
+    private XenServerGuru xenServerGuru = new XenServerGuru();
+
+    @Mock
+    private HostDao hostDaoMock;
+
+    @Mock
+    private PrimaryDataStoreDao storagePoolDao;
+
+    private Long defaultHostId = 1l;
+
+    @Mock
+    private CopyCommand copyCommandMock;
+
+    @Mock
+    private DataTO sourceDataMock;
+
+    @Mock
+    private DataTO destinationDataMock;
+
+    @Mock
+    private HostVO hostMock;
+
+    @Mock
+    private StoragePoolVO storagePoolMock;
+
+    private Long changedHostId = 12l;
+
+    private long snapshotId = 5l;
+
+    @Before
+    public void beforeTest() {
+        Mockito.when(copyCommandMock.getSrcTO()).thenReturn(sourceDataMock);
+        Mockito.when(copyCommandMock.getDestTO()).thenReturn(destinationDataMock);
+        Mockito.when(hostMock.getId()).thenReturn(changedHostId);
+        Mockito.when(sourceDataMock.getId()).thenReturn(snapshotId);
+        Mockito.when(storagePoolDao.findStoragePoolForSnapshot(snapshotId)).thenReturn(storagePoolMock);
+    }
+
+    @Test
+    public void getCommandHostDelegationTestCommandNotCopyCommand() {
+        Pair<Boolean, Long> pairHostToExecuteCommand = xenServerGuru.getCommandHostDelegation(defaultHostId, Mockito.mock(Command.class));
+
+        assertPairOfHostToExecuteCommandIsTheDefaultHostId(pairHostToExecuteCommand);
+    }
+
+    private void assertPairOfHostToExecuteCommandIsTheDefaultHostId(Pair<Boolean, Long> pairHostToExecuteCommand) {
+        Assert.assertFalse(pairHostToExecuteCommand.first());
+        Assert.assertEquals(defaultHostId, pairHostToExecuteCommand.second());
+    }
+
+    @Test
+    public void getCommandHostDelegationTestCommanIsStorageSubSystemCommand() {
+        StorageSubSystemCommand storageSubSystemCommandMock = Mockito.mock(StorageSubSystemCommand.class);
+        Pair<Boolean, Long> pairHostToExecuteCommand = xenServerGuru.getCommandHostDelegation(defaultHostId, storageSubSystemCommandMock);
+
+        assertPairOfHostToExecuteCommandIsTheDefaultHostId(pairHostToExecuteCommand);
+
+        Mockito.verify(storageSubSystemCommandMock).setExecuteInSequence(true);
+    }
+
+    @Test
+    public void getCommandHostDelegationTestCommandIsCopyCommandButSourceDataHypervisorIsNotXenServer() {
+        Mockito.when(sourceDataMock.getHypervisorType()).thenReturn(HypervisorType.Any);
+
+        Pair<Boolean, Long> pairHostToExecuteCommand = xenServerGuru.getCommandHostDelegation(defaultHostId, copyCommandMock);
+
+        assertPairOfHostToExecuteCommandIsTheDefaultHostId(pairHostToExecuteCommand);
+    }
+
+    @Test
+    public void getCommandHostDelegationTestCommandIsCopyCommandAndSourceDataHypervisorIsXenServerButSourceAndDestinationAreNotNfsObjects() {
+        Mockito.when(sourceDataMock.getDataStore()).thenReturn(Mockito.mock(DataStoreTO.class));
+        Mockito.when(destinationDataMock.getDataStore()).thenReturn(Mockito.mock(DataStoreTO.class));
+
+        Mockito.when(sourceDataMock.getHypervisorType()).thenReturn(HypervisorType.XenServer);
+
+        Pair<Boolean, Long> pairHostToExecuteCommand = xenServerGuru.getCommandHostDelegation(defaultHostId, copyCommandMock);
+
+        assertPairOfHostToExecuteCommandIsTheDefaultHostId(pairHostToExecuteCommand);
+    }
+
+    @Test
+    public void getCommandHostDelegationTestCommandIsCopyCommandAndSourceDataHypervisorIsXenServerAndSourceAndDestinationAreNfsObjectsButSourceIsNotSnapshotType() {
+        configureSourceAndDestinationDataMockDataStoreAsNfsToType();
+
+        Mockito.when(sourceDataMock.getHypervisorType()).thenReturn(HypervisorType.XenServer);
+        Mockito.when(sourceDataMock.getObjectType()).thenReturn(DataObjectType.VOLUME);
+
+        Pair<Boolean, Long> pairHostToExecuteCommand = xenServerGuru.getCommandHostDelegation(defaultHostId, copyCommandMock);
+
+        assertPairOfHostToExecuteCommandIsTheDefaultHostId(pairHostToExecuteCommand);
+    }
+
+    private void configureSourceAndDestinationDataMockDataStoreAsNfsToType() {
+        Mockito.when(sourceDataMock.getDataStore()).thenReturn(Mockito.mock(NfsTO.class));
+        Mockito.when(destinationDataMock.getDataStore()).thenReturn(Mockito.mock(NfsTO.class));
+    }
+
+    @Test
+    public void getCommandHostDelegationTestCommandIsCopyCommandAndSourceDataHypervisorIsXenServerAndSourceAndDestinationAreNfsObjectsAndSourceIsSnapshotTypeButDestinationIsNotTemplateType() {
+        configureSourceAndDestinationDataMockDataStoreAsNfsToType();
+
+        Mockito.when(sourceDataMock.getHypervisorType()).thenReturn(HypervisorType.XenServer);
+        Mockito.when(sourceDataMock.getObjectType()).thenReturn(DataObjectType.SNAPSHOT);
+        Mockito.when(destinationDataMock.getObjectType()).thenReturn(DataObjectType.VOLUME);
+
+        Pair<Boolean, Long> pairHostToExecuteCommand = xenServerGuru.getCommandHostDelegation(defaultHostId, copyCommandMock);
+
+        assertPairOfHostToExecuteCommandIsTheDefaultHostId(pairHostToExecuteCommand);
+    }
+
+    @Test
+    public void getCommandHostDelegationTestCommandIsCopyCommandAndSourceDataHypervisorIsXenServerAndSourceAndDestinationAreNfsObjectsAndSourceIsSnapshotAndDestinationIsTemplateButHypervisorVersionIsBlank() {
+        configureSourceAndDestinationDataMockDataStoreAsNfsToType();
+        configureSourceHypervisorAsXenServerAndSourceTypeAsSnapshotAndDestinationTypeAsTemplate();
+
+        Mockito.when(hostMock.getHypervisorVersion()).thenReturn(StringUtils.EMPTY);
+        Mockito.when(hostDaoMock.findHostToOperateOnSnapshotBasedOnStoragePool(storagePoolMock)).thenReturn(hostMock);
+
+        Pair<Boolean, Long> pairHostToExecuteCommand = xenServerGuru.getCommandHostDelegation(defaultHostId, copyCommandMock);
+
+        assertPairOfHostToExecuteCommandIsTheDefaultHostId(pairHostToExecuteCommand);
+    }
+
+    private void configureSourceHypervisorAsXenServerAndSourceTypeAsSnapshotAndDestinationTypeAsTemplate() {
+        Mockito.when(sourceDataMock.getHypervisorType()).thenReturn(HypervisorType.XenServer);
+        Mockito.when(sourceDataMock.getObjectType()).thenReturn(DataObjectType.SNAPSHOT);
+        Mockito.when(destinationDataMock.getObjectType()).thenReturn(DataObjectType.TEMPLATE);
+    }
+
+    @Test
+    public void getCommandHostDelegationTestCommandIsCopyCommandAndSourceDataHypervisorIsXenServerAndSourceAndDestinationAreNfsObjectsAndSourceIsSnapshotAndDestinationIsTemplateButHypervisorVersionIsXenServer610() {
+        configureSourceAndDestinationDataMockDataStoreAsNfsToType();
+        configureSourceHypervisorAsXenServerAndSourceTypeAsSnapshotAndDestinationTypeAsTemplate();
+
+        Mockito.when(hostMock.getHypervisorVersion()).thenReturn("6.1.0");
+        Mockito.when(hostDaoMock.findHostToOperateOnSnapshotBasedOnStoragePool(storagePoolMock)).thenReturn(hostMock);
+
+        Pair<Boolean, Long> pairHostToExecuteCommand = xenServerGuru.getCommandHostDelegation(defaultHostId, copyCommandMock);
+
+        assertPairOfHostToExecuteCommandIsTheDefaultHostId(pairHostToExecuteCommand);
+    }
+
+    @Test
+    public void getCommandHostDelegationTestCommandIsCopyCommandAndSourceDataHypervisorIsXenServerAndSourceAndDestinationAreNfsObjectsAndSourceIsSnapshotAndDestinationIsTemplateAndHypervisorVersionIsXenServer620WithoutHotfixOfSnapshots() {
+        configureSourceAndDestinationDataMockDataStoreAsNfsToType();
+        configureSourceHypervisorAsXenServerAndSourceTypeAsSnapshotAndDestinationTypeAsTemplate();
+
+        Mockito.when(hostMock.getHypervisorVersion()).thenReturn("6.2.0");
+        Mockito.when(hostDaoMock.findHostToOperateOnSnapshotBasedOnStoragePool(storagePoolMock)).thenReturn(hostMock);
+
+        Pair<Boolean, Long> pairHostToExecuteCommand = xenServerGuru.getCommandHostDelegation(defaultHostId, copyCommandMock);
+
+        assertPairOfHostToExecuteCommandIsTheDefaultHostId(pairHostToExecuteCommand);
+    }
+
+    @Test
+    public void getCommandHostDelegationTestCommandIsCopyCommandAndSourceDataHypervisorIsXenServerAndSourceAndDestinationAreNfsObjectsAndSourceIsSnapshotAndDestinationIsTemplateAndHypervisorVersionIsXenServer620WithHotfixOfSnapshots() {
+        configureSourceAndDestinationDataMockDataStoreAsNfsToType();
+        configureSourceHypervisorAsXenServerAndSourceTypeAsSnapshotAndDestinationTypeAsTemplate();
+
+        Mockito.when(hostMock.getHypervisorVersion()).thenReturn("6.2.0");
+        Mockito.when(hostMock.getDetail(XenserverConfigs.XS620HotFix)).thenReturn(XenserverConfigs.XSHotFix62ESP1004);
+
+        Mockito.when(hostDaoMock.findHostToOperateOnSnapshotBasedOnStoragePool(storagePoolMock)).thenReturn(hostMock);
+
+        Pair<Boolean, Long> pairHostToExecuteCommand = xenServerGuru.getCommandHostDelegation(defaultHostId, copyCommandMock);
+
+        Assert.assertTrue(pairHostToExecuteCommand.first());
+        Assert.assertEquals(changedHostId, pairHostToExecuteCommand.second());
+    }
+
+    @Test
+    public void getCommandHostDelegationTestCommandIsCopyCommandAndSourceDataHypervisorIsXenServerAndSourceAndDestinationAreNfsObjectsAndSourceIsSnapshotAndDestinationIsTemplateAndHypervisorVersionIsXenServer650() {
+        configureSourceAndDestinationDataMockDataStoreAsNfsToType();
+        configureSourceHypervisorAsXenServerAndSourceTypeAsSnapshotAndDestinationTypeAsTemplate();
+
+        Mockito.when(hostMock.getHypervisorVersion()).thenReturn("6.5.0");
+
+        Mockito.when(hostDaoMock.findHostToOperateOnSnapshotBasedOnStoragePool(storagePoolMock)).thenReturn(hostMock);
+
+        Pair<Boolean, Long> pairHostToExecuteCommand = xenServerGuru.getCommandHostDelegation(defaultHostId, copyCommandMock);
+
+        Assert.assertTrue(pairHostToExecuteCommand.first());
+        Assert.assertEquals(changedHostId, pairHostToExecuteCommand.second());
+    }
+}

pom.xml

@@ -19,7 +19,7 @@
 <project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
   xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
   <modelVersion>4.0.0</modelVersion>
-
+ 
   <parent>
     <groupId>org.apache</groupId>
     <artifactId>apache</artifactId>

server/src/com/cloud/hypervisor/HypervisorGuruBase.java

@@ -131,16 +131,15 @@ public abstract class HypervisorGuruBase extends AdapterBase implements Hypervis
         Long minMemory = (long)(offering.getRamSize() / vmProfile.getMemoryOvercommitRatio());
         int minspeed = (int)(offering.getSpeed() / vmProfile.getCpuOvercommitRatio());
         int maxspeed = (offering.getSpeed());
-        VirtualMachineTO to =
-                new VirtualMachineTO(vm.getId(), vm.getInstanceName(), vm.getType(), offering.getCpu(), minspeed, maxspeed, minMemory * 1024l * 1024l,
-                        offering.getRamSize() * 1024l * 1024l, null, null, vm.isHaEnabled(), vm.limitCpuUse(), vm.getVncPassword());
+        VirtualMachineTO to = new VirtualMachineTO(vm.getId(), vm.getInstanceName(), vm.getType(), offering.getCpu(), minspeed, maxspeed, minMemory * 1024l * 1024l,
+                offering.getRamSize() * 1024l * 1024l, null, null, vm.isHaEnabled(), vm.limitCpuUse(), vm.getVncPassword());
         to.setBootArgs(vmProfile.getBootArgs());
 
         List<NicProfile> nicProfiles = vmProfile.getNics();
         NicTO[] nics = new NicTO[nicProfiles.size()];
         int i = 0;
         for (NicProfile nicProfile : nicProfiles) {
-            if(vm.getType() == VirtualMachine.Type.NetScalerVm) {
+            if (vm.getType() == VirtualMachine.Type.NetScalerVm) {
                 nicProfile.setBroadcastType(BroadcastDomainType.Native);
             }
             NicTO nicTo = toNicTO(nicProfile);
@@ -173,7 +172,7 @@ public abstract class HypervisorGuruBase extends AdapterBase implements Hypervis
 
         // Set GPU details
         ServiceOfferingDetailsVO offeringDetail = null;
-        if ((offeringDetail  = _serviceOfferingDetailsDao.findDetail(offering.getId(), GPU.Keys.vgpuType.toString())) != null) {
+        if ((offeringDetail = _serviceOfferingDetailsDao.findDetail(offering.getId(), GPU.Keys.vgpuType.toString())) != null) {
             ServiceOfferingDetailsVO groupName = _serviceOfferingDetailsDao.findDetail(offering.getId(), GPU.Keys.pciDevice.toString());
             to.setGpuDevice(_resourceMgr.getGPUDevice(vm.getHostId(), groupName.getValue(), offeringDetail.getValue()));
         }
@@ -194,6 +193,13 @@ public abstract class HypervisorGuruBase extends AdapterBase implements Hypervis
     }
 
     @Override
+    /**
+     * The basic implementation assumes that the initial "host" defined to execute the command is the host that is in fact going to execute it.
+     * However, subclasses can extend this behavior, changing the host that is going to execute the command in runtime.
+     * The first element of the 'Pair' indicates if the hostId has been changed; this means, if you change the hostId, but you do not inform this action in the return 'Pair' object, we will use the original "hostId".
+     *
+     * Side note: it seems that the 'hostId' received here is normally the ID of the SSVM that has an entry at the host table. Therefore, this methods gives the opportunity to change from the SSVM to a real host to execute a command.
+     */
     public Pair<Boolean, Long> getCommandHostDelegation(long hostId, Command cmd) {
         return new Pair<Boolean, Long>(Boolean.FALSE, new Long(hostId));
     }