From 630484f2517ec2ff63c4c77722f34911e6c14f83 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Fri, 12 Aug 2022 18:24:44 +0530 Subject: [PATCH 01/10] [WIP] vmware: fix vm snapshot with datastore cluster, drs Signed-off-by: Abhishek Kumar --- .../vmsnapshot/DefaultVMSnapshotStrategy.java | 30 +++++-- .../vmware/manager/VmwareHostService.java | 3 + .../manager/VmwareStorageManagerImpl.java | 72 +++++++++------- .../vmware/resource/VmwareResource.java | 5 ++ ...VmwareSecondaryStorageResourceHandler.java | 15 ++-- .../resource/VmwareStorageProcessor.java | 86 ++++++++++--------- 6 files changed, 130 insertions(+), 81 deletions(-) diff --git a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java index 90f6ccd1f9ba..014ee84ec79f 100644 --- a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java +++ b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java @@ -25,14 +25,14 @@ 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.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.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 params) throws ConfigurationException { @@ -325,12 +329,24 @@ protected void finalizeRevert(VMSnapshotVO vmSnapshot, List volu private void updateVolumePath(List volumeTOs) { for (VolumeObjectTO volume : volumeTOs) { - if (volume.getPath() != null) { - VolumeVO volumeVO = volumeDao.findById(volume.getId()); + if (StringUtils.isAllEmpty(volume.getDataStoreUuid(), volume.getPath(), volume.getChainInfo())) { + continue; + } + 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()); - volumeVO.setVmSnapshotChainSize(volume.getSize()); - volumeDao.persist(volumeVO); } + if (StringUtils.isNotEmpty(volume.getChainInfo())) { + volumeVO.setChainInfo(volume.getChainInfo()); + } + volumeVO.setVmSnapshotChainSize(volume.getSize()); + volumeDao.persist(volumeVO); } } diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareHostService.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareHostService.java index d9ee3c5f5cad..4f680963035f 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareHostService.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareHostService.java @@ -20,6 +20,7 @@ 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(); } diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImpl.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImpl.java index 3ed9ffffb063..ca34462f31d6 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImpl.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImpl.java @@ -28,20 +28,9 @@ import java.util.Properties; import java.util.UUID; -import org.apache.log4j.Logger; - -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 +54,12 @@ 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; @@ -81,6 +72,7 @@ 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 +81,15 @@ import com.cloud.utils.script.Script; import com.cloud.vm.VirtualMachine; import com.cloud.vm.snapshot.VMSnapshot; +import com.vmware.vim25.DatastoreSummary; +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 +1135,18 @@ private long getVMSnapshotChainSize(VmwareContext context, VmwareHypervisorHost return size; } + 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 map = new HashMap<>(); + map.put(DiskTO.PROTOCOL_TYPE, "DatastoreCluster"); + disk.setDetails(map); + disk.setData(volumeTO); + storageProcessor.getSyncedVolume(virtualMachineMO, context, hypervisorHost, disk, volumeTO); + } + @Override public CreateVMSnapshotAnswer execute(VmwareHostService hostService, CreateVMSnapshotCommand cmd) { List volumeTOs = cmd.getVolumeTOs(); @@ -1180,10 +1193,9 @@ public CreateVMSnapshotAnswer execute(VmwareHostService hostService, CreateVMSna } else if (!vmMo.createSnapshot(vmSnapshotName, vmSnapshotDesc, snapshotMemory, quiescevm)) { return new CreateVMSnapshotAnswer(cmd, false, "Unable to create snapshot due to esxi internal failed"); } - Map mapNewDisk = getNewDiskMap(vmMo); - setVolumeToPathAndSize(volumeTOs, mapNewDisk, context, hyperHost, cmd.getVmName()); + setVolumeToPathAndSize(volumeTOs, vmMo, hostService, context, hyperHost); return new CreateVMSnapshotAnswer(cmd, cmd.getTarget(), volumeTOs); } @@ -1248,29 +1260,32 @@ private Map getNewDiskMap(VirtualMachineMO vmMo) throws Exceptio return mapNewDisk; } - private void setVolumeToPathAndSize(List volumeTOs, Map mapNewDisk, VmwareContext context, VmwareHypervisorHost hyperHost, String vmName) + private void setVolumeToPathAndSize(List volumeTOs, VirtualMachineMO vmMo, VmwareHostService hostService, VmwareContext context, VmwareHypervisorHost hyperHost) throws Exception { + String vmName = vmMo.getVmName(); for (VolumeObjectTO volumeTO : volumeTOs) { - String oldPath = volumeTO.getPath(); + syncVolume(hostService, vmMo, context, hyperHost, volumeTO); + String path = volumeTO.getPath(); final String baseName; // if this is managed storage - 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] + 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 = oldPath.substring(1, oldPath.length() - 1); + baseName = path.substring(1, path.length() - 1); } else { baseName = VmwareHelper.trimSnapshotDeltaPostfix(volumeTO.getPath()); } - 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, store); - long size = getVMSnapshotChainSize(context, hyperHost, baseName + "-*.vmdk", morDs, newPath, vmName); + ManagedObjectReference morDs = getDatastoreAsManagedObjectReference(baseName, hyperHost, volumeTO.getDataStoreUuid()); + DatastoreMO dsMo = new DatastoreMO(hyperHost.getContext(), morDs); + String type = dsMo.getDatastoreType(); + DatastoreSummary summary = dsMo.getDatastoreSummary(); + s_logger.info(String.format("-------------------------------%s", type)); + long size = getVMSnapshotChainSize(context, hyperHost, baseName + "-*.vmdk", morDs, path, vmName); if (volumeTO.getVolumeType() == Volume.Type.ROOT) { // add memory snapshot size @@ -1278,11 +1293,10 @@ private void setVolumeToPathAndSize(List volumeTOs, Map mapNewDisk = getNewDiskMap(vmMo); - setVolumeToPathAndSize(listVolumeTo, mapNewDisk, context, hyperHost, cmd.getVmName()); + setVolumeToPathAndSize(listVolumeTo, vmMo, hostService, context, hyperHost); return new DeleteVMSnapshotAnswer(cmd, listVolumeTo); } @@ -1405,7 +1419,7 @@ public RevertToVMSnapshotAnswer execute(VmwareHostService hostService, RevertToV if (result) { Map mapNewDisk = getNewDiskMap(vmMo); - setVolumeToPathAndSize(listVolumeTo, mapNewDisk, context, hyperHost, cmd.getVmName()); + setVolumeToPathAndSize(listVolumeTo, vmMo, hostService, context, hyperHost); if (!snapshotMemory) { vmState = VirtualMachine.PowerState.PowerOff; diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java index f224601dfc4e..a4b078f8e95d 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java @@ -7671,4 +7671,9 @@ private void logCommand(Command cmd) { 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; + } } diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareSecondaryStorageResourceHandler.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareSecondaryStorageResourceHandler.java index 44bcbf31e1a3..68947ef69042 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareSecondaryStorageResourceHandler.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareSecondaryStorageResourceHandler.java @@ -18,14 +18,10 @@ 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.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 String createLogMessageException(Throwable e, Command command) { return message; } + + @Override + public VmwareStorageProcessor getStorageProcessor() { + return null; + } } diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java index d2f7fc88a3b9..72f0e97991a3 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java @@ -3862,17 +3862,52 @@ public VirtualMachineMO cloneVMFromTemplate(VmwareHypervisorHost hyperHost, Stri } } + public Pair getSyncedVolume(VirtualMachineMO vmMo, VmwareContext context,VmwareHypervisorHost hyperHost, DiskTO disk, VolumeObjectTO volumeTO) throws Exception { + String volumePath = volumeTO.getPath(); + DataStoreTO primaryStore = volumeTO.getDataStore(); + boolean datastoreChangeObserved = false; + boolean volumePathChangeObserved = false; + String chainInfo = null; + String datastoreUUID = primaryStore.getUuid(); + if (disk.getDetails().get(DiskTO.PROTOCOL_TYPE) != null && disk.getDetails().get(DiskTO.PROTOCOL_TYPE).equalsIgnoreCase("DatastoreCluster")) { + 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; + volumeTO.setPath(volumePath); + chainInfo = _gson.toJson(matchingExistingDisk); + volumeTO.setChainInfo(chainInfo); + } + + 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; + volumeTO.setDataStoreUuid(datastoreUUID); + chainInfo = _gson.toJson(matchingExistingDisk); + volumeTO.setChainInfo(chainInfo); + } + } + } + } + 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 +3920,19 @@ public Answer syncVolumePath(SyncVolumePathCommand cmd) { throw new Exception(msg); } } - - String datastoreUUID = primaryStore.getUuid(); - if (disk.getDetails().get(DiskTO.PROTOCOL_TYPE) != null && disk.getDetails().get(DiskTO.PROTOCOL_TYPE).equalsIgnoreCase("DatastoreCluster")) { - 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); - } - } - } - } + Pair changes = getSyncedVolume(vmMo, context, hyperHost, disk, volumeTO); + boolean volumePathChangeObserved = changes.first(); + boolean datastoreChangeObserved = changes.second(); 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()) { - answer.setContextParam("chainInfo", chainInfo); + if ((volumePathChangeObserved || datastoreChangeObserved) && StringUtils.isNotEmpty(volumeTO.getChainInfo())) { + answer.setContextParam("chainInfo", volumeTO.getChainInfo()); } return answer; From 859b4f32f91d13abc12979805aa8f46e93cf725c Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Thu, 18 Aug 2022 14:35:26 +0530 Subject: [PATCH 02/10] fix Signed-off-by: Abhishek Kumar --- .../resource/VmwareStorageProcessor.java | 58 +++++++++---------- 1 file changed, 28 insertions(+), 30 deletions(-) diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java index 72f0e97991a3..eed980073f2c 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java @@ -3863,39 +3863,37 @@ public VirtualMachineMO cloneVMFromTemplate(VmwareHypervisorHost hyperHost, Stri } public Pair getSyncedVolume(VirtualMachineMO vmMo, VmwareContext context,VmwareHypervisorHost hyperHost, DiskTO disk, VolumeObjectTO volumeTO) throws Exception { - String volumePath = volumeTO.getPath(); DataStoreTO primaryStore = volumeTO.getDataStore(); boolean datastoreChangeObserved = false; boolean volumePathChangeObserved = false; - String chainInfo = null; - String datastoreUUID = primaryStore.getUuid(); - if (disk.getDetails().get(DiskTO.PROTOCOL_TYPE) != null && disk.getDetails().get(DiskTO.PROTOCOL_TYPE).equalsIgnoreCase("DatastoreCluster")) { - 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; - volumeTO.setPath(volumePath); - chainInfo = _gson.toJson(matchingExistingDisk); - volumeTO.setChainInfo(chainInfo); - } - - 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; - volumeTO.setDataStoreUuid(datastoreUUID); - chainInfo = _gson.toJson(matchingExistingDisk); - volumeTO.setChainInfo(chainInfo); - } + 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)); } } } From 795984be66ad75085e09edbf0fcb8e92040c8bdd Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Thu, 18 Aug 2022 14:35:41 +0530 Subject: [PATCH 03/10] refactor Signed-off-by: Abhishek Kumar --- .../resource/VmwareStorageProcessor.java | 38 ++++++------------- 1 file changed, 11 insertions(+), 27 deletions(-) diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java index eed980073f2c..16008908430d 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java @@ -2081,33 +2081,17 @@ private Answer attachVolume(Command cmd, DiskTO disk, boolean isAttach, boolean 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")) { - 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); - } - } - } + Pair changes = getSyncedVolume(vmMo, context, hyperHost, disk, volumeTO); + volumePathChangeObserved = changes.first(); + datastoreChangeObserved = changes.second(); + if (datastoreChangeObserved) { + datastoreUUID = volumeTO.getDataStoreUuid(); + } + if (volumePathChangeObserved) { + volumePath = volumeTO.getPath(); + } + if ((volumePathChangeObserved || datastoreChangeObserved) && StringUtils.isNotEmpty(volumeTO.getChainInfo())) { + chainInfo = volumeTO.getChainInfo(); } if (storagePort == DEFAULT_NFS_PORT) { morDs = HypervisorHostHelper.findDatastoreWithBackwardsCompatibility(hyperHost, isManaged ? VmwareResource.getDatastoreName(diskUuid) : datastoreUUID); From 62adc89842418d6a771dcf909fc4e1e3f048db6f Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Mon, 22 Aug 2022 14:35:07 +0530 Subject: [PATCH 04/10] sync only volumes from datastorecluster Signed-off-by: Abhishek Kumar --- .../manager/VmwareStorageManagerImpl.java | 47 +++++++++++-------- 1 file changed, 27 insertions(+), 20 deletions(-) diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImpl.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImpl.java index ca34462f31d6..abd646cdeae2 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImpl.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImpl.java @@ -28,6 +28,7 @@ import java.util.Properties; import java.util.UUID; +import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; import org.apache.cloudstack.storage.to.TemplateObjectTO; import org.apache.cloudstack.storage.to.VolumeObjectTO; import org.apache.log4j.Logger; @@ -69,6 +70,7 @@ 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; @@ -81,7 +83,6 @@ import com.cloud.utils.script.Script; import com.cloud.vm.VirtualMachine; import com.cloud.vm.snapshot.VMSnapshot; -import com.vmware.vim25.DatastoreSummary; import com.vmware.vim25.FileInfo; import com.vmware.vim25.FileQueryFlags; import com.vmware.vim25.HostDatastoreBrowserSearchResults; @@ -1135,13 +1136,21 @@ private long getVMSnapshotChainSize(VmwareContext context, VmwareHypervisorHost return size; } + private boolean isVolumeExistedOnDatastoreCluster(VolumeObjectTO volumeObjectTO) { + DataStoreTO dsTO = volumeObjectTO.getDataStore(); + if (!(dsTO instanceof PrimaryDataStoreTO)) { + return false; + } + return Storage.StoragePoolType.DatastoreCluster.equals(((PrimaryDataStoreTO)dsTO).getPoolType()); + } + 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 map = new HashMap<>(); - map.put(DiskTO.PROTOCOL_TYPE, "DatastoreCluster"); + map.put(DiskTO.PROTOCOL_TYPE, Storage.StoragePoolType.DatastoreCluster.toString()); disk.setDetails(map); disk.setData(volumeTO); storageProcessor.getSyncedVolume(virtualMachineMO, context, hypervisorHost, disk, volumeTO); @@ -1193,7 +1202,6 @@ public CreateVMSnapshotAnswer execute(VmwareHostService hostService, CreateVMSna } else if (!vmMo.createSnapshot(vmSnapshotName, vmSnapshotDesc, snapshotMemory, quiescevm)) { return new CreateVMSnapshotAnswer(cmd, false, "Unable to create snapshot due to esxi internal failed"); } - Map mapNewDisk = getNewDiskMap(vmMo); setVolumeToPathAndSize(volumeTOs, vmMo, hostService, context, hyperHost); @@ -1264,27 +1272,30 @@ private void setVolumeToPathAndSize(List volumeTOs, VirtualMachi throws Exception { String vmName = vmMo.getVmName(); for (VolumeObjectTO volumeTO : volumeTOs) { - syncVolume(hostService, vmMo, context, hyperHost, volumeTO); String path = volumeTO.getPath(); - final String baseName; - // 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 { + if (isVolumeExistedOnDatastoreCluster(volumeTO)) { + syncVolume(hostService, vmMo, context, hyperHost, volumeTO); + path = volumeTO.getPath(); baseName = VmwareHelper.trimSnapshotDeltaPostfix(volumeTO.getPath()); + } else { + Map 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); } // get volume's chain size for this VM snapshot; exclude current volume vdisk ManagedObjectReference morDs = getDatastoreAsManagedObjectReference(baseName, hyperHost, volumeTO.getDataStoreUuid()); - DatastoreMO dsMo = new DatastoreMO(hyperHost.getContext(), morDs); - String type = dsMo.getDatastoreType(); - DatastoreSummary summary = dsMo.getDatastoreSummary(); - s_logger.info(String.format("-------------------------------%s", type)); long size = getVMSnapshotChainSize(context, hyperHost, baseName + "-*.vmdk", morDs, path, vmName); if (volumeTO.getVolumeType() == Volume.Type.ROOT) { @@ -1349,8 +1360,6 @@ public DeleteVMSnapshotAnswer execute(VmwareHostService hostService, DeleteVMSna // after removed snapshot, the volumes' paths have been changed for the VM, needs to report new paths to manager - Map mapNewDisk = getNewDiskMap(vmMo); - setVolumeToPathAndSize(listVolumeTo, vmMo, hostService, context, hyperHost); return new DeleteVMSnapshotAnswer(cmd, listVolumeTo); @@ -1417,8 +1426,6 @@ public RevertToVMSnapshotAnswer execute(VmwareHostService hostService, RevertToV } if (result) { - Map mapNewDisk = getNewDiskMap(vmMo); - setVolumeToPathAndSize(listVolumeTo, vmMo, hostService, context, hyperHost); if (!snapshotMemory) { From 4e2f0b08cb7d5e64dc8f63ffdcd4a9bd7385f351 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Mon, 22 Aug 2022 17:14:40 +0530 Subject: [PATCH 05/10] fix test Signed-off-by: Abhishek Kumar --- .../storage/vmsnapshot/VMSnapshotStrategyTest.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/vmsnapshot/VMSnapshotStrategyTest.java b/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/vmsnapshot/VMSnapshotStrategyTest.java index 4420c19d6a09..5fd6dee90832 100644 --- a/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/vmsnapshot/VMSnapshotStrategyTest.java +++ b/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/vmsnapshot/VMSnapshotStrategyTest.java @@ -27,6 +27,7 @@ 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 DiskOfferingDao diskOfferingDao() { public HostDao hostDao() { return Mockito.mock(HostDao.class); } + + @Bean + public PrimaryDataStoreDao primaryDataStoreDao() { + return Mockito.mock(PrimaryDataStoreDao.class); + } } } From 72f42f9ad5749fdf75c88f691a72f0ee0cd89f43 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Tue, 23 Aug 2022 11:55:17 +0530 Subject: [PATCH 06/10] fix fo ds uuid npe Signed-off-by: Abhishek Kumar --- .../vmware/manager/VmwareStorageManagerImpl.java | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImpl.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImpl.java index abd646cdeae2..3b3ddb4efe2a 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImpl.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImpl.java @@ -1209,7 +1209,7 @@ public CreateVMSnapshotAnswer execute(VmwareHostService hostService, CreateVMSna } } 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) { @@ -1273,12 +1273,14 @@ private void setVolumeToPathAndSize(List volumeTOs, VirtualMachi String vmName = vmMo.getVmName(); for (VolumeObjectTO volumeTO : volumeTOs) { String path = volumeTO.getPath(); - final String baseName; + String baseName; + String datastoreUuid = volumeTO.getDataStore().getUuid(); if (isVolumeExistedOnDatastoreCluster(volumeTO)) { syncVolume(hostService, vmMo, context, hyperHost, volumeTO); path = volumeTO.getPath(); baseName = VmwareHelper.trimSnapshotDeltaPostfix(volumeTO.getPath()); + datastoreUuid = volumeTO.getDataStoreUuid(); } else { Map mapNewDisk = getNewDiskMap(vmMo); // if this is managed storage @@ -1295,7 +1297,7 @@ private void setVolumeToPathAndSize(List volumeTOs, VirtualMachi } // get volume's chain size for this VM snapshot; exclude current volume vdisk - ManagedObjectReference morDs = getDatastoreAsManagedObjectReference(baseName, hyperHost, volumeTO.getDataStoreUuid()); + ManagedObjectReference morDs = getDatastoreAsManagedObjectReference(baseName, hyperHost, datastoreUuid); long size = getVMSnapshotChainSize(context, hyperHost, baseName + "-*.vmdk", morDs, path, vmName); if (volumeTO.getVolumeType() == Volume.Type.ROOT) { @@ -1366,7 +1368,7 @@ public DeleteVMSnapshotAnswer execute(VmwareHostService hostService, DeleteVMSna } } 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); } @@ -1439,7 +1441,7 @@ public RevertToVMSnapshotAnswer execute(VmwareHostService hostService, RevertToV } } 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); } From b8154ca48516a2b35dbd22750e66883bf9a7257b Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Wed, 24 Aug 2022 16:47:20 +0530 Subject: [PATCH 07/10] add test Signed-off-by: Abhishek Kumar --- .../vmsnapshot/DefaultVMSnapshotStrategy.java | 2 +- .../DefaultVMSnapshotStrategyTest.java | 108 ++++++++++++++++++ 2 files changed, 109 insertions(+), 1 deletion(-) create mode 100644 engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategyTest.java diff --git a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java index 014ee84ec79f..e2815c005c44 100644 --- a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java +++ b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java @@ -327,7 +327,7 @@ protected void finalizeRevert(VMSnapshotVO vmSnapshot, List volu vmSnapshotDao.persist(vmSnapshot); } - private void updateVolumePath(List volumeTOs) { + protected void updateVolumePath(List volumeTOs) { for (VolumeObjectTO volume : volumeTOs) { if (StringUtils.isAllEmpty(volume.getDataStoreUuid(), volume.getPath(), volume.getChainInfo())) { continue; diff --git a/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategyTest.java b/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategyTest.java new file mode 100644 index 000000000000..7e7695d05727 --- /dev/null +++ b/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategyTest.java @@ -0,0 +1,108 @@ +/* + * 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 persistedVolumes = new ArrayList<>(); + + + private void setupVolumeDaoPersistMock() { + persistedVolumes.clear(); + Mockito.when(volumeDao.persist(Mockito.any())).thenAnswer((Answer) 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); + Mockito.when(vol2.getPoolId()).thenReturn(oldPoolId); + 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()); + } + +} \ No newline at end of file From 379a4afc8dd6bf283d52a076fc3e7d4a3416e1ad Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Wed, 24 Aug 2022 16:49:20 +0530 Subject: [PATCH 08/10] new line Signed-off-by: Abhishek Kumar --- .../storage/vmsnapshot/DefaultVMSnapshotStrategyTest.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategyTest.java b/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategyTest.java index 7e7695d05727..965b37a20b59 100644 --- a/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategyTest.java +++ b/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategyTest.java @@ -104,5 +104,4 @@ public void testUpdateVolumePath() { Assert.assertEquals(vmSnapshotChainSize, persistedVolume.getVmSnapshotChainSize()); Assert.assertEquals(newVolChain, persistedVolume.getChainInfo()); } - -} \ No newline at end of file +} From 17941978d34bf320632daf2354266e104bc0d5a4 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Wed, 24 Aug 2022 17:46:43 +0530 Subject: [PATCH 09/10] remove unnecessary mock Signed-off-by: Abhishek Kumar --- .../storage/vmsnapshot/DefaultVMSnapshotStrategyTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategyTest.java b/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategyTest.java index 965b37a20b59..da377f96ec32 100644 --- a/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategyTest.java +++ b/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategyTest.java @@ -85,7 +85,6 @@ public void testUpdateVolumePath() { Mockito.when(vol2.getChainInfo()).thenReturn(newVolChain); Mockito.when(vol2.getSize()).thenReturn(vmSnapshotChainSize); Mockito.when(vol2.getId()).thenReturn(volumeId); - Mockito.when(vol2.getPoolId()).thenReturn(oldPoolId); 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); From 39f3ecc75e54823adf6ec5ed7e31659660be67a3 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Thu, 25 Aug 2022 18:26:09 +0530 Subject: [PATCH 10/10] fix datastorecluster type check Signed-off-by: Abhishek Kumar --- .../cloudstack/storage/to/PrimaryDataStoreTO.java | 7 +++++++ .../subsystem/api/storage/PrimaryDataStore.java | 4 ++++ .../storage/datastore/PrimaryDataStoreImpl.java | 12 ++++++++++++ .../vmware/manager/VmwareStorageManagerImpl.java | 8 +++++--- 4 files changed, 28 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/org/apache/cloudstack/storage/to/PrimaryDataStoreTO.java b/core/src/main/java/org/apache/cloudstack/storage/to/PrimaryDataStoreTO.java index 9df2a6c955bc..a6a74176c136 100644 --- a/core/src/main/java/org/apache/cloudstack/storage/to/PrimaryDataStoreTO.java +++ b/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 PrimaryDataStoreTO(PrimaryDataStore dataStore) { this.url = dataStore.getUri(); this.details = dataStore.getDetails(); this.isManaged = dataStore.isManaged(); + this.parentPoolType = dataStore.getParentPoolType(); } public long getId() { @@ -172,4 +175,8 @@ public Boolean getDiskProvisioningStrictnessFlag() { public void setDiskProvisioningStrictnessFlag(Boolean diskProvisioningStrictnessFlag) { this.diskProvisioningStrictnessFlag = diskProvisioningStrictnessFlag; } + + public StoragePoolType getParentPoolType() { + return parentPoolType; + } } diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/PrimaryDataStore.java b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/PrimaryDataStore.java index 5546571ba6b4..718f63e77f50 100644 --- a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/PrimaryDataStore.java +++ b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/PrimaryDataStore.java @@ -22,6 +22,8 @@ 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(); } diff --git a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/PrimaryDataStoreImpl.java b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/PrimaryDataStoreImpl.java index f557ac35171e..eba597c3c555 100644 --- a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/PrimaryDataStoreImpl.java +++ b/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 void configure(StoragePoolVO pdsv, PrimaryDataStoreDriver driver, DataSto 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 DataStoreTO getTO() { } return to; } + + @Override + public StoragePoolType getParentPoolType() { + if (this.parentStoragePool != null) { + return this.parentStoragePool.getPoolType(); + } + return null; + } } diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImpl.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImpl.java index 3b3ddb4efe2a..1f57e00979da 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImpl.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImpl.java @@ -1136,12 +1136,14 @@ private long getVMSnapshotChainSize(VmwareContext context, VmwareHypervisorHost return size; } - private boolean isVolumeExistedOnDatastoreCluster(VolumeObjectTO volumeObjectTO) { + private boolean isVolumeOnDatastoreCluster(VolumeObjectTO volumeObjectTO) { DataStoreTO dsTO = volumeObjectTO.getDataStore(); if (!(dsTO instanceof PrimaryDataStoreTO)) { return false; } - return Storage.StoragePoolType.DatastoreCluster.equals(((PrimaryDataStoreTO)dsTO).getPoolType()); + 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, @@ -1276,7 +1278,7 @@ private void setVolumeToPathAndSize(List volumeTOs, VirtualMachi String baseName; String datastoreUuid = volumeTO.getDataStore().getUuid(); - if (isVolumeExistedOnDatastoreCluster(volumeTO)) { + if (isVolumeOnDatastoreCluster(volumeTO)) { syncVolume(hostService, vmMo, context, hyperHost, volumeTO); path = volumeTO.getPath(); baseName = VmwareHelper.trimSnapshotDeltaPostfix(volumeTO.getPath());