vmware: fix vm snapshot with datastore cluster, drs (#6643)

Fixes #6595
Sync volume datastore, path and chaininfo info while calculating snapshot chain size after snapshot operation is complete from vCenter.

core/src/main/java/org/apache/cloudstack/storage/to/PrimaryDataStoreTO.java

@@ -55,6 +55,8 @@ public class PrimaryDataStoreTO implements DataStoreTO {
     private Boolean diskProvisioningStrictnessFlag;
     private final boolean isManaged;
 
+    private final StoragePoolType parentPoolType;
+
     public PrimaryDataStoreTO(PrimaryDataStore dataStore) {
         this.uuid = dataStore.getUuid();
         this.name = dataStore.getName();
@@ -66,6 +68,7 @@ public class PrimaryDataStoreTO implements DataStoreTO {
         this.url = dataStore.getUri();
         this.details = dataStore.getDetails();
         this.isManaged = dataStore.isManaged();
+        this.parentPoolType = dataStore.getParentPoolType();
     }
 
     public long getId() {
@@ -172,4 +175,8 @@ public class PrimaryDataStoreTO implements DataStoreTO {
     public void setDiskProvisioningStrictnessFlag(Boolean diskProvisioningStrictnessFlag) {
         this.diskProvisioningStrictnessFlag = diskProvisioningStrictnessFlag;
     }
+
+    public StoragePoolType getParentPoolType() {
+        return parentPoolType;
+    }
 }

engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/PrimaryDataStore.java

@@ -22,6 +22,8 @@ import java.util.List;
 
 import org.apache.cloudstack.engine.subsystem.api.storage.disktype.DiskFormat;
 
+import com.cloud.storage.Storage;
+
 public interface PrimaryDataStore extends DataStore, PrimaryDataStoreInfo {
     DataObject create(DataObject dataObject, String configuration);
 
@@ -38,4 +40,6 @@ public interface PrimaryDataStore extends DataStore, PrimaryDataStoreInfo {
     SnapshotInfo getSnapshot(long snapshotId);
 
     DiskFormat getDefaultDiskType();
+
+    Storage.StoragePoolType getParentPoolType();
 }

engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java

@@ -25,14 +25,14 @@ import java.util.Map;
 import javax.inject.Inject;
 import javax.naming.ConfigurationException;
 
-import com.cloud.event.UsageEventVO;
-import org.apache.log4j.Logger;
-
 import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority;
 import org.apache.cloudstack.engine.subsystem.api.storage.VMSnapshotOptions;
 import org.apache.cloudstack.engine.subsystem.api.storage.VMSnapshotStrategy;
 import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
 import org.apache.cloudstack.storage.to.VolumeObjectTO;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.log4j.Logger;
 
 import com.cloud.agent.AgentManager;
 import com.cloud.agent.api.Answer;
@@ -45,6 +45,7 @@ import com.cloud.agent.api.RevertToVMSnapshotCommand;
 import com.cloud.agent.api.VMSnapshotTO;
 import com.cloud.event.EventTypes;
 import com.cloud.event.UsageEventUtils;
+import com.cloud.event.UsageEventVO;
 import com.cloud.exception.AgentUnavailableException;
 import com.cloud.exception.OperationTimedoutException;
 import com.cloud.host.HostVO;
@@ -53,6 +54,7 @@ import com.cloud.storage.DiskOfferingVO;
 import com.cloud.storage.GuestOSHypervisorVO;
 import com.cloud.storage.GuestOSVO;
 import com.cloud.storage.Storage.ImageFormat;
+import com.cloud.storage.StoragePool;
 import com.cloud.storage.VolumeVO;
 import com.cloud.storage.dao.DiskOfferingDao;
 import com.cloud.storage.dao.GuestOSDao;
@@ -97,6 +99,8 @@ public class DefaultVMSnapshotStrategy extends ManagerBase implements VMSnapshot
     DiskOfferingDao diskOfferingDao;
     @Inject
     HostDao hostDao;
+    @Inject
+    PrimaryDataStoreDao primaryDataStoreDao;
 
     @Override
     public boolean configure(String name, Map<String, Object> params) throws ConfigurationException {
@@ -323,14 +327,26 @@ public class DefaultVMSnapshotStrategy extends ManagerBase implements VMSnapshot
         vmSnapshotDao.persist(vmSnapshot);
     }
 
-    private void updateVolumePath(List<VolumeObjectTO> volumeTOs) {
+    protected void updateVolumePath(List<VolumeObjectTO> volumeTOs) {
         for (VolumeObjectTO volume : volumeTOs) {
-            if (volume.getPath() != null) {
+            if (StringUtils.isAllEmpty(volume.getDataStoreUuid(), volume.getPath(), volume.getChainInfo())) {
-                VolumeVO volumeVO = volumeDao.findById(volume.getId());
+                continue;
-                volumeVO.setPath(volume.getPath());
-                volumeVO.setVmSnapshotChainSize(volume.getSize());
-                volumeDao.persist(volumeVO);
             }
+            VolumeVO volumeVO = volumeDao.findById(volume.getId());
+            if (StringUtils.isNotEmpty(volume.getDataStoreUuid())) {
+                StoragePool pool = primaryDataStoreDao.findPoolByUUID(volume.getDataStoreUuid());
+                if (pool != null && pool.getId() != volumeVO.getPoolId()) {
+                    volumeVO.setPoolId(pool.getId());
+                }
+            }
+            if (StringUtils.isNotEmpty(volume.getPath())) {
+                volumeVO.setPath(volume.getPath());
+            }
+            if (StringUtils.isNotEmpty(volume.getChainInfo())) {
+                volumeVO.setChainInfo(volume.getChainInfo());
+            }
+            volumeVO.setVmSnapshotChainSize(volume.getSize());
+            volumeDao.persist(volumeVO);
         }
     }
 

engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategyTest.java

@@ -0,0 +1,106 @@
+/*
+ * 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 org.apache.cloudstack.storage.vmsnapshot;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.UUID;
+
+import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
+import org.apache.cloudstack.storage.to.VolumeObjectTO;
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.Spy;
+import org.mockito.junit.MockitoJUnitRunner;
+import org.mockito.stubbing.Answer;
+
+import com.cloud.storage.Storage;
+import com.cloud.storage.Volume;
+import com.cloud.storage.VolumeVO;
+import com.cloud.storage.dao.VolumeDao;
+
+@RunWith(MockitoJUnitRunner.class)
+public class DefaultVMSnapshotStrategyTest {
+    @Mock
+    VolumeDao volumeDao;
+    @Mock
+    PrimaryDataStoreDao primaryDataStoreDao;
+
+    @Spy
+    @InjectMocks
+    private final DefaultVMSnapshotStrategy defaultVMSnapshotStrategy = new DefaultVMSnapshotStrategy();
+
+    protected List<VolumeVO> persistedVolumes = new ArrayList<>();
+
+
+    private void setupVolumeDaoPersistMock() {
+        persistedVolumes.clear();
+        Mockito.when(volumeDao.persist(Mockito.any())).thenAnswer((Answer<VolumeVO>) invocation -> {
+            VolumeVO volume = (VolumeVO)invocation.getArguments()[0];
+            persistedVolumes.add(volume);
+            return volume;
+        });
+    }
+
+    @Test
+    public void testUpdateVolumePath() {
+        setupVolumeDaoPersistMock();
+        VolumeObjectTO vol1 = Mockito.mock(VolumeObjectTO.class);
+        Mockito.when(vol1.getDataStoreUuid()).thenReturn(null);
+        Mockito.when(vol1.getPath()).thenReturn(null);
+        Mockito.when(vol1.getChainInfo()).thenReturn(null);
+        VolumeObjectTO vol2 = Mockito.mock(VolumeObjectTO.class);
+        Long volumeId = 1L;
+        String newDSUuid = UUID.randomUUID().toString();
+        String oldVolPath = "old";
+        String newVolPath = "new";
+        String oldVolChain = "old-chain";
+        String newVolChain = "new-chain";
+        Long vmSnapshotChainSize = 1000L;
+        Long oldPoolId = 1L;
+        Long newPoolId = 2L;
+        Mockito.when(vol2.getDataStoreUuid()).thenReturn(newDSUuid);
+        Mockito.when(vol2.getPath()).thenReturn(newVolPath);
+        Mockito.when(vol2.getChainInfo()).thenReturn(newVolChain);
+        Mockito.when(vol2.getSize()).thenReturn(vmSnapshotChainSize);
+        Mockito.when(vol2.getId()).thenReturn(volumeId);
+        VolumeVO volumeVO = new VolumeVO("name", 0l, 0l, 0l, 0l, 0l, "folder", "path", Storage.ProvisioningType.THIN, 0l, Volume.Type.ROOT);
+        volumeVO.setPoolId(oldPoolId);
+        volumeVO.setChainInfo(oldVolChain);
+        volumeVO.setPath(oldVolPath);
+        Mockito.when(volumeDao.findById(volumeId)).thenReturn(volumeVO);
+        StoragePoolVO storagePoolVO = Mockito.mock(StoragePoolVO.class);
+        Mockito.when(storagePoolVO.getId()).thenReturn(newPoolId);
+        Mockito.when(primaryDataStoreDao.findPoolByUUID(newDSUuid)).thenReturn(storagePoolVO);
+        Mockito.when(volumeDao.findById(volumeId)).thenReturn(volumeVO);
+        defaultVMSnapshotStrategy.updateVolumePath(List.of(vol1, vol2));
+        Assert.assertEquals(1, persistedVolumes.size());
+        VolumeVO persistedVolume = persistedVolumes.get(0);
+        Assert.assertNotNull(persistedVolume);
+        Assert.assertEquals(newPoolId, persistedVolume.getPoolId());
+        Assert.assertEquals(newVolPath, persistedVolume.getPath());
+        Assert.assertEquals(vmSnapshotChainSize, persistedVolume.getVmSnapshotChainSize());
+        Assert.assertEquals(newVolChain, persistedVolume.getChainInfo());
+    }
+}

engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/vmsnapshot/VMSnapshotStrategyTest.java

@@ -27,6 +27,7 @@ import javax.inject.Inject;
 
 import org.apache.cloudstack.engine.subsystem.api.storage.VMSnapshotStrategy;
 import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
 import org.apache.cloudstack.storage.to.VolumeObjectTO;
 import org.apache.cloudstack.test.utils.SpringUtils;
 import org.junit.Before;
@@ -92,6 +93,8 @@ public class VMSnapshotStrategyTest extends TestCase {
     VMSnapshotDao vmSnapshotDao;
     @Inject
     HostDao hostDao;
+    @Inject
+    PrimaryDataStoreDao primaryDataStoreDao;
 
     @Override
     @Before
@@ -304,5 +307,10 @@ public class VMSnapshotStrategyTest extends TestCase {
         public HostDao hostDao() {
             return Mockito.mock(HostDao.class);
         }
+
+        @Bean
+        public PrimaryDataStoreDao primaryDataStoreDao() {
+            return Mockito.mock(PrimaryDataStoreDao.class);
+        }
     }
 }

engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/PrimaryDataStoreImpl.java

@@ -72,6 +72,7 @@ public class PrimaryDataStoreImpl implements PrimaryDataStore {
 
     protected PrimaryDataStoreDriver driver;
     protected StoragePoolVO pdsv;
+    protected StoragePoolVO parentStoragePool;
     @Inject
     protected PrimaryDataStoreDao dataStoreDao;
     protected PrimaryDataStoreLifeCycle lifeCycle;
@@ -99,6 +100,9 @@ public class PrimaryDataStoreImpl implements PrimaryDataStore {
         this.pdsv = pdsv;
         this.driver = driver;
         this.provider = provider;
+        if (pdsv.getParent() != null && pdsv.getParent() > 0L) {
+            this.parentStoragePool = dataStoreDao.findById(pdsv.getParent());
+        }
     }
 
     public static PrimaryDataStoreImpl createDataStore(StoragePoolVO pdsv, PrimaryDataStoreDriver driver, DataStoreProvider provider) {
@@ -447,4 +451,12 @@ public class PrimaryDataStoreImpl implements PrimaryDataStore {
         }
         return to;
     }
+
+    @Override
+    public StoragePoolType getParentPoolType() {
+        if (this.parentStoragePool != null) {
+            return this.parentStoragePool.getPoolType();
+        }
+        return null;
+    }
 }

plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareHostService.java

@@ -20,6 +20,7 @@ import com.cloud.agent.api.Command;
 import com.cloud.hypervisor.vmware.mo.DatastoreMO;
 import com.cloud.hypervisor.vmware.mo.VmwareHypervisorHost;
 import com.cloud.hypervisor.vmware.util.VmwareContext;
+import com.cloud.storage.resource.VmwareStorageProcessor;
 
 public interface VmwareHostService {
     VmwareContext getServiceContext(Command cmd);
@@ -31,4 +32,6 @@ public interface VmwareHostService {
     String getWorkerName(VmwareContext context, Command cmd, int workerSequence, DatastoreMO dsMo) throws Exception;
 
     String createLogMessageException(Throwable e, Command command);
+
+    VmwareStorageProcessor getStorageProcessor();
 }

plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImpl.java

@@ -28,20 +28,10 @@ import java.util.Map;
 import java.util.Properties;
 import java.util.UUID;
 
-import org.apache.log4j.Logger;
+import org.apache.cloudstack.storage.to.PrimaryDataStoreTO;
-
-import com.cloud.hypervisor.vmware.mo.DatastoreFile;
-import com.vmware.vim25.FileInfo;
-import com.vmware.vim25.FileQueryFlags;
-import com.vmware.vim25.HostDatastoreBrowserSearchResults;
-import com.vmware.vim25.HostDatastoreBrowserSearchSpec;
-import com.vmware.vim25.ManagedObjectReference;
-import com.vmware.vim25.TaskInfo;
-import com.vmware.vim25.TaskInfoState;
-import com.vmware.vim25.VirtualDisk;
-
 import org.apache.cloudstack.storage.to.TemplateObjectTO;
 import org.apache.cloudstack.storage.to.VolumeObjectTO;
+import org.apache.log4j.Logger;
 
 import com.cloud.agent.api.Answer;
 import com.cloud.agent.api.BackupSnapshotAnswer;
@@ -65,10 +55,12 @@ import com.cloud.agent.api.storage.PrimaryStorageDownloadCommand;
 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.DiskTO;
 import com.cloud.agent.api.to.NfsTO;
 import com.cloud.agent.api.to.StorageFilerTO;
 import com.cloud.hypervisor.vmware.mo.CustomFieldConstants;
 import com.cloud.hypervisor.vmware.mo.DatacenterMO;
+import com.cloud.hypervisor.vmware.mo.DatastoreFile;
 import com.cloud.hypervisor.vmware.mo.DatastoreMO;
 import com.cloud.hypervisor.vmware.mo.HostDatastoreBrowserMO;
 import com.cloud.hypervisor.vmware.mo.HostMO;
@@ -78,9 +70,11 @@ import com.cloud.hypervisor.vmware.mo.VmwareHypervisorHost;
 import com.cloud.hypervisor.vmware.util.VmwareContext;
 import com.cloud.hypervisor.vmware.util.VmwareHelper;
 import com.cloud.storage.JavaStorageLayer;
+import com.cloud.storage.Storage;
 import com.cloud.storage.Storage.ImageFormat;
 import com.cloud.storage.StorageLayer;
 import com.cloud.storage.Volume;
+import com.cloud.storage.resource.VmwareStorageProcessor;
 import com.cloud.storage.template.OVAProcessor;
 import com.cloud.utils.NumbersUtil;
 import com.cloud.utils.Pair;
@@ -89,6 +83,14 @@ import com.cloud.utils.exception.CloudRuntimeException;
 import com.cloud.utils.script.Script;
 import com.cloud.vm.VirtualMachine;
 import com.cloud.vm.snapshot.VMSnapshot;
+import com.vmware.vim25.FileInfo;
+import com.vmware.vim25.FileQueryFlags;
+import com.vmware.vim25.HostDatastoreBrowserSearchResults;
+import com.vmware.vim25.HostDatastoreBrowserSearchSpec;
+import com.vmware.vim25.ManagedObjectReference;
+import com.vmware.vim25.TaskInfo;
+import com.vmware.vim25.TaskInfoState;
+import com.vmware.vim25.VirtualDisk;
 
 public class VmwareStorageManagerImpl implements VmwareStorageManager {
 
@@ -1134,6 +1136,28 @@ public class VmwareStorageManagerImpl implements VmwareStorageManager {
         return size;
     }
 
+    private boolean isVolumeOnDatastoreCluster(VolumeObjectTO volumeObjectTO) {
+        DataStoreTO dsTO = volumeObjectTO.getDataStore();
+        if (!(dsTO instanceof PrimaryDataStoreTO)) {
+            return false;
+        }
+        PrimaryDataStoreTO primaryDataStoreTO = (PrimaryDataStoreTO)dsTO;
+        return Storage.StoragePoolType.DatastoreCluster.equals(primaryDataStoreTO.getPoolType()) ||
+                Storage.StoragePoolType.DatastoreCluster.equals(primaryDataStoreTO.getParentPoolType());
+    }
+
+    private void syncVolume(VmwareHostService hostService, VirtualMachineMO virtualMachineMO, VmwareContext context,
+                             VmwareHypervisorHost hypervisorHost, VolumeObjectTO volumeTO) throws Exception {
+        if (hostService.getStorageProcessor() == null) return;
+        VmwareStorageProcessor storageProcessor = hostService.getStorageProcessor();
+        DiskTO disk = new DiskTO();
+        Map<String, String> map = new HashMap<>();
+        map.put(DiskTO.PROTOCOL_TYPE, Storage.StoragePoolType.DatastoreCluster.toString());
+        disk.setDetails(map);
+        disk.setData(volumeTO);
+        storageProcessor.getSyncedVolume(virtualMachineMO, context, hypervisorHost, disk, volumeTO);
+    }
+
     @Override
     public CreateVMSnapshotAnswer execute(VmwareHostService hostService, CreateVMSnapshotCommand cmd) {
         List<VolumeObjectTO> volumeTOs = cmd.getVolumeTOs();
@@ -1181,15 +1205,13 @@ public class VmwareStorageManagerImpl implements VmwareStorageManager {
                     return new CreateVMSnapshotAnswer(cmd, false, "Unable to create snapshot due to esxi internal failed");
                 }
 
-                Map<String, String> mapNewDisk = getNewDiskMap(vmMo);
+                setVolumeToPathAndSize(volumeTOs, vmMo, hostService, context, hyperHost);
-
-                setVolumeToPathAndSize(volumeTOs, mapNewDisk, context, hyperHost, cmd.getVmName());
 
                 return new CreateVMSnapshotAnswer(cmd, cmd.getTarget(), volumeTOs);
             }
         } catch (Exception e) {
             String msg = e.getMessage();
-            s_logger.error("failed to create snapshot for vm:" + vmName + " due to " + msg);
+            s_logger.error("failed to create snapshot for vm:" + vmName + " due to " + msg, e);
 
             try {
                 if (vmMo.getSnapshotMor(vmSnapshotName) != null) {
@@ -1248,29 +1270,37 @@ public class VmwareStorageManagerImpl implements VmwareStorageManager {
         return mapNewDisk;
     }
 
-    private void setVolumeToPathAndSize(List<VolumeObjectTO> volumeTOs, Map<String, String> mapNewDisk, VmwareContext context, VmwareHypervisorHost hyperHost, String vmName)
+    private void setVolumeToPathAndSize(List<VolumeObjectTO> volumeTOs, VirtualMachineMO vmMo, VmwareHostService hostService, VmwareContext context, VmwareHypervisorHost hyperHost)
             throws Exception {
+        String vmName = vmMo.getVmName();
         for (VolumeObjectTO volumeTO : volumeTOs) {
-            String oldPath = volumeTO.getPath();
+            String path = volumeTO.getPath();
+            String baseName;
+            String datastoreUuid = volumeTO.getDataStore().getUuid();
 
-            final String baseName;
+            if (isVolumeOnDatastoreCluster(volumeTO)) {
-
+                syncVolume(hostService, vmMo, context, hyperHost, volumeTO);
-            // if this is managed storage
+                path = volumeTO.getPath();
-            if (oldPath.startsWith("[-iqn.")) { // ex. [-iqn.2010-01.com.company:3y8w.vol-10.64-0] -iqn.2010-01.com.company:3y8w.vol-10.64-0-000001.vmdk
-                oldPath = oldPath.split(" ")[0]; // ex. [-iqn.2010-01.com.company:3y8w.vol-10.64-0]
-
-                // remove '[' and ']'
-                baseName = oldPath.substring(1, oldPath.length() - 1);
-            } else {
                 baseName = VmwareHelper.trimSnapshotDeltaPostfix(volumeTO.getPath());
+                datastoreUuid = volumeTO.getDataStoreUuid();
+            } else {
+                Map<String, String> mapNewDisk = getNewDiskMap(vmMo);
+                // if this is managed storage
+                if (path.startsWith("[-iqn.")) { // ex. [-iqn.2010-01.com.company:3y8w.vol-10.64-0] -iqn.2010-01.com.company:3y8w.vol-10.64-0-000001.vmdk
+                    path = path.split(" ")[0]; // ex. [-iqn.2010-01.com.company:3y8w.vol-10.64-0]
+
+                    // remove '[' and ']'
+                    baseName = path.substring(1, path.length() - 1);
+                } else {
+                    baseName = VmwareHelper.trimSnapshotDeltaPostfix(path);
+                }
+                path = mapNewDisk.get(baseName);
+                volumeTO.setPath(path);
             }
 
-            String newPath = mapNewDisk.get(baseName);
-
             // get volume's chain size for this VM snapshot; exclude current volume vdisk
-            DataStoreTO store = volumeTO.getDataStore();
+            ManagedObjectReference morDs = getDatastoreAsManagedObjectReference(baseName, hyperHost, datastoreUuid);
-            ManagedObjectReference morDs = getDatastoreAsManagedObjectReference(baseName, hyperHost, store);
+            long size = getVMSnapshotChainSize(context, hyperHost, baseName + "-*.vmdk", morDs, path, vmName);
-            long size = getVMSnapshotChainSize(context, hyperHost, baseName + "-*.vmdk", morDs, newPath, vmName);
 
             if (volumeTO.getVolumeType() == Volume.Type.ROOT) {
                 // add memory snapshot size
@@ -1278,11 +1308,10 @@ public class VmwareStorageManagerImpl implements VmwareStorageManager {
             }
 
             volumeTO.setSize(size);
-            volumeTO.setPath(newPath);
         }
     }
 
-    private ManagedObjectReference getDatastoreAsManagedObjectReference(String baseName, VmwareHypervisorHost hyperHost, DataStoreTO store) throws Exception {
+    private ManagedObjectReference getDatastoreAsManagedObjectReference(String baseName, VmwareHypervisorHost hyperHost, String storeUuid) throws Exception {
         try {
             // if baseName equates to a datastore name, this should be managed storage
             ManagedObjectReference morDs = hyperHost.findDatastoreByName(baseName);
@@ -1295,7 +1324,7 @@ public class VmwareStorageManagerImpl implements VmwareStorageManager {
         }
 
         // not managed storage, so use the standard way of getting a ManagedObjectReference for a datastore
-        return HypervisorHostHelper.findDatastoreWithBackwardsCompatibility(hyperHost, store.getUuid());
+        return HypervisorHostHelper.findDatastoreWithBackwardsCompatibility(hyperHost, storeUuid);
     }
 
     @Override
@@ -1335,15 +1364,13 @@ public class VmwareStorageManagerImpl implements VmwareStorageManager {
 
                 // after removed snapshot, the volumes' paths have been changed for the VM, needs to report new paths to manager
 
-                Map<String, String> mapNewDisk = getNewDiskMap(vmMo);
+                setVolumeToPathAndSize(listVolumeTo, vmMo, hostService, context, hyperHost);
-
-                setVolumeToPathAndSize(listVolumeTo, mapNewDisk, context, hyperHost, cmd.getVmName());
 
                 return new DeleteVMSnapshotAnswer(cmd, listVolumeTo);
             }
         } catch (Exception e) {
             String msg = e.getMessage();
-            s_logger.error("failed to delete vm snapshot " + vmSnapshotName + " of vm " + vmName + " due to " + msg);
+            s_logger.error("failed to delete vm snapshot " + vmSnapshotName + " of vm " + vmName + " due to " + msg, e);
 
             return new DeleteVMSnapshotAnswer(cmd, false, msg);
         }
@@ -1403,9 +1430,7 @@ public class VmwareStorageManagerImpl implements VmwareStorageManager {
                 }
 
                 if (result) {
-                    Map<String, String> mapNewDisk = getNewDiskMap(vmMo);
+                    setVolumeToPathAndSize(listVolumeTo, vmMo, hostService, context, hyperHost);
-
-                    setVolumeToPathAndSize(listVolumeTo, mapNewDisk, context, hyperHost, cmd.getVmName());
 
                     if (!snapshotMemory) {
                         vmState = VirtualMachine.PowerState.PowerOff;
@@ -1418,7 +1443,7 @@ public class VmwareStorageManagerImpl implements VmwareStorageManager {
             }
         } catch (Exception e) {
             String msg = "revert vm " + vmName + " to snapshot " + snapshotName + " failed due to " + e.getMessage();
-            s_logger.error(msg);
+            s_logger.error(msg, e);
 
             return new RevertToVMSnapshotAnswer(cmd, false, msg);
         }

plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java

@@ -7671,4 +7671,9 @@ public class VmwareResource extends ServerResourceBase implements StoragePoolRes
             s_logger.error(String.format("Failed to log command %s due to: [%s].", cmd.getClass().getSimpleName(), e.getMessage()), e);
         }
     }
+
+    @Override
+    public VmwareStorageProcessor getStorageProcessor() {
+        return _storageProcessor;
+    }
 }

plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareSecondaryStorageResourceHandler.java

@@ -18,14 +18,10 @@ package com.cloud.storage.resource;
 
 import java.util.List;
 
-import org.apache.log4j.Logger;
-import org.apache.log4j.NDC;
-
-import com.google.gson.Gson;
-import com.vmware.vim25.ManagedObjectReference;
-
 import org.apache.cloudstack.storage.command.StorageSubSystemCommand;
 import org.apache.cloudstack.storage.resource.SecondaryStorageResourceHandler;
+import org.apache.log4j.Logger;
+import org.apache.log4j.NDC;
 
 import com.cloud.agent.api.Answer;
 import com.cloud.agent.api.BackupSnapshotCommand;
@@ -52,6 +48,8 @@ import com.cloud.serializer.GsonHelper;
 import com.cloud.utils.NumbersUtil;
 import com.cloud.utils.Pair;
 import com.cloud.utils.StringUtils;
+import com.google.gson.Gson;
+import com.vmware.vim25.ManagedObjectReference;
 
 public class VmwareSecondaryStorageResourceHandler implements SecondaryStorageResourceHandler, VmwareHostService, VmwareStorageMount {
     private static final Logger s_logger = Logger.getLogger(VmwareSecondaryStorageResourceHandler.class);
@@ -326,4 +324,9 @@ public class VmwareSecondaryStorageResourceHandler implements SecondaryStorageRe
 
         return message;
     }
+
+    @Override
+    public VmwareStorageProcessor getStorageProcessor() {
+        return null;
+    }
 }

plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java

@@ -2081,33 +2081,17 @@ public class VmwareStorageProcessor implements StorageProcessor {
                     datastoreVolumePath = dsMo.getDatastorePath((vmdkPath != null ? vmdkPath : dsMo.getName()) + ".vmdk");
                 } else {
                     String datastoreUUID = primaryStore.getUuid();
-                    if (disk.getDetails().get(DiskTO.PROTOCOL_TYPE) != null && disk.getDetails().get(DiskTO.PROTOCOL_TYPE).equalsIgnoreCase("DatastoreCluster")) {
+                    Pair<Boolean, Boolean> changes = getSyncedVolume(vmMo, context, hyperHost, disk, volumeTO);
-                        VirtualMachineDiskInfo matchingExistingDisk = getMatchingExistingDisk(hyperHost, context, vmMo, disk);
+                    volumePathChangeObserved = changes.first();
-                        VirtualMachineDiskInfoBuilder diskInfoBuilder = vmMo.getDiskInfoBuilder();
+                    datastoreChangeObserved = changes.second();
-                        if (diskInfoBuilder != null && matchingExistingDisk != null) {
+                    if (datastoreChangeObserved) {
-                            String[] diskChain = matchingExistingDisk.getDiskChain();
+                        datastoreUUID = volumeTO.getDataStoreUuid();
-                            assert (diskChain.length > 0);
+                    }
-                            DatastoreFile file = new DatastoreFile(diskChain[0]);
+                    if (volumePathChangeObserved) {
-                            if (!file.getFileBaseName().equalsIgnoreCase(volumePath)) {
+                        volumePath = volumeTO.getPath();
-                                if (s_logger.isInfoEnabled())
+                    }
-                                    s_logger.info("Detected disk-chain top file change on volume: " + volumeTO.getId() + " " + volumePath + " -> " + file.getFileBaseName());
+                    if ((volumePathChangeObserved || datastoreChangeObserved) && StringUtils.isNotEmpty(volumeTO.getChainInfo())) {
-                                volumePathChangeObserved = true;
+                        chainInfo = volumeTO.getChainInfo();
-                                volumePath = file.getFileBaseName();
-                                volumeTO.setPath(volumePath);
-                                chainInfo = _gson.toJson(matchingExistingDisk);
-                            }
-
-                            DatastoreMO diskDatastoreMofromVM = getDiskDatastoreMofromVM(hyperHost, context, vmMo, disk, diskInfoBuilder);
-                            if (diskDatastoreMofromVM != null) {
-                                String actualPoolUuid = diskDatastoreMofromVM.getCustomFieldValue(CustomFieldConstants.CLOUD_UUID);
-                                if (!actualPoolUuid.equalsIgnoreCase(primaryStore.getUuid())) {
-                                    s_logger.warn(String.format("Volume %s found to be in a different storage pool %s", volumePath, actualPoolUuid));
-                                    datastoreChangeObserved = true;
-                                    datastoreUUID = actualPoolUuid;
-                                    chainInfo = _gson.toJson(matchingExistingDisk);
-                                }
-                            }
-                        }
                     }
                     if (storagePort == DEFAULT_NFS_PORT) {
                         morDs = HypervisorHostHelper.findDatastoreWithBackwardsCompatibility(hyperHost, isManaged ? VmwareResource.getDatastoreName(diskUuid) : datastoreUUID);
@@ -3862,17 +3846,50 @@ public class VmwareStorageProcessor implements StorageProcessor {
         }
     }
 
+    public Pair<Boolean, Boolean> getSyncedVolume(VirtualMachineMO vmMo, VmwareContext context,VmwareHypervisorHost hyperHost, DiskTO disk, VolumeObjectTO volumeTO) throws Exception {
+        DataStoreTO primaryStore = volumeTO.getDataStore();
+        boolean datastoreChangeObserved = false;
+        boolean volumePathChangeObserved = false;
+        if (!"DatastoreCluster".equalsIgnoreCase(disk.getDetails().get(DiskTO.PROTOCOL_TYPE))) {
+            return new Pair<>(volumePathChangeObserved, datastoreChangeObserved);
+        }
+        VirtualMachineDiskInfo matchingExistingDisk = getMatchingExistingDisk(hyperHost, context, vmMo, disk);
+        VirtualMachineDiskInfoBuilder diskInfoBuilder = vmMo.getDiskInfoBuilder();
+        if (diskInfoBuilder != null && matchingExistingDisk != null) {
+            String[] diskChain = matchingExistingDisk.getDiskChain();
+            assert (diskChain.length > 0);
+            DatastoreFile file = new DatastoreFile(diskChain[0]);
+            String volumePath = volumeTO.getPath();
+            if (!file.getFileBaseName().equalsIgnoreCase(volumePath)) {
+                if (s_logger.isInfoEnabled()) {
+                    s_logger.info("Detected disk-chain top file change on volume: " + volumeTO.getId() + " " + volumePath + " -> " + file.getFileBaseName());
+                }
+                volumePathChangeObserved = true;
+                volumePath = file.getFileBaseName();
+                volumeTO.setPath(volumePath);
+                volumeTO.setChainInfo(_gson.toJson(matchingExistingDisk));
+            }
+
+            DatastoreMO diskDatastoreMoFromVM = getDiskDatastoreMofromVM(hyperHost, context, vmMo, disk, diskInfoBuilder);
+            if (diskDatastoreMoFromVM != null) {
+                String actualPoolUuid = diskDatastoreMoFromVM.getCustomFieldValue(CustomFieldConstants.CLOUD_UUID);
+                if (!actualPoolUuid.equalsIgnoreCase(primaryStore.getUuid())) {
+                    s_logger.warn(String.format("Volume %s found to be in a different storage pool %s", volumePath, actualPoolUuid));
+                    datastoreChangeObserved = true;
+                    volumeTO.setDataStoreUuid(actualPoolUuid);
+                    volumeTO.setChainInfo(_gson.toJson(matchingExistingDisk));
+                }
+            }
+        }
+        return new Pair<>(volumePathChangeObserved, datastoreChangeObserved);
+    }
+
     @Override
     public Answer syncVolumePath(SyncVolumePathCommand cmd) {
         DiskTO disk = cmd.getDisk();
         VolumeObjectTO volumeTO = (VolumeObjectTO)disk.getData();
-        DataStoreTO primaryStore = volumeTO.getDataStore();
-        String volumePath = volumeTO.getPath();
         String vmName = volumeTO.getVmName();
 
-        boolean datastoreChangeObserved = false;
-        boolean volumePathChangeObserved = false;
-        String chainInfo = null;
         try {
             VmwareContext context = hostService.getServiceContext(null);
             VmwareHypervisorHost hyperHost = hostService.getHyperHost(context, null);
@@ -3885,46 +3902,19 @@ public class VmwareStorageProcessor implements StorageProcessor {
                     throw new Exception(msg);
                 }
             }
-
+            Pair<Boolean, Boolean> changes = getSyncedVolume(vmMo, context, hyperHost, disk, volumeTO);
-            String datastoreUUID = primaryStore.getUuid();
+            boolean volumePathChangeObserved = changes.first();
-            if (disk.getDetails().get(DiskTO.PROTOCOL_TYPE) != null && disk.getDetails().get(DiskTO.PROTOCOL_TYPE).equalsIgnoreCase("DatastoreCluster")) {
+            boolean datastoreChangeObserved = changes.second();
-                VirtualMachineDiskInfo matchingExistingDisk = getMatchingExistingDisk(hyperHost, context, vmMo, disk);
-                VirtualMachineDiskInfoBuilder diskInfoBuilder = vmMo.getDiskInfoBuilder();
-                if (diskInfoBuilder != null && matchingExistingDisk != null) {
-                    String[] diskChain = matchingExistingDisk.getDiskChain();
-                    assert (diskChain.length > 0);
-                    DatastoreFile file = new DatastoreFile(diskChain[0]);
-                    if (!file.getFileBaseName().equalsIgnoreCase(volumePath)) {
-                        if (s_logger.isInfoEnabled())
-                            s_logger.info("Detected disk-chain top file change on volume: " + volumeTO.getId() + " " + volumePath + " -> " + file.getFileBaseName());
-                        volumePathChangeObserved = true;
-                        volumePath = file.getFileBaseName();
-                        volumeTO.setPath(volumePath);
-                        chainInfo = _gson.toJson(matchingExistingDisk);
-                    }
-
-                    DatastoreMO diskDatastoreMofromVM = getDiskDatastoreMofromVM(hyperHost, context, vmMo, disk, diskInfoBuilder);
-                    if (diskDatastoreMofromVM != null) {
-                        String actualPoolUuid = diskDatastoreMofromVM.getCustomFieldValue(CustomFieldConstants.CLOUD_UUID);
-                        if (!actualPoolUuid.equalsIgnoreCase(primaryStore.getUuid())) {
-                            s_logger.warn(String.format("Volume %s found to be in a different storage pool %s", volumePath, actualPoolUuid));
-                            datastoreChangeObserved = true;
-                            datastoreUUID = actualPoolUuid;
-                            chainInfo = _gson.toJson(matchingExistingDisk);
-                        }
-                    }
-                }
-            }
 
             SyncVolumePathAnswer answer = new SyncVolumePathAnswer(disk);
             if (datastoreChangeObserved) {
-                answer.setContextParam("datastoreName", datastoreUUID);
+                answer.setContextParam("datastoreName", volumeTO.getDataStoreUuid());
             }
             if (volumePathChangeObserved) {
-                answer.setContextParam("volumePath", volumePath);
+                answer.setContextParam("volumePath", volumeTO.getPath());
             }
-            if (chainInfo != null && !chainInfo.isEmpty()) {
+            if ((volumePathChangeObserved || datastoreChangeObserved) && StringUtils.isNotEmpty(volumeTO.getChainInfo())) {
-                answer.setContextParam("chainInfo", chainInfo);
+                answer.setContextParam("chainInfo", volumeTO.getChainInfo());
             }
 
             return answer;