DaanHoogland commented on code in PR #8796:
URL: https://github.com/apache/cloudstack/pull/8796#discussion_r1537404842


##########
plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/driver/LinstorPrimaryDataStoreDriverImpl.java:
##########
@@ -1015,29 +1020,46 @@ protected Answer copySnapshot(DataObject srcData, 
DataObject destData) {
         options.put("fullSnapshot", fullSnapshot + "");
         options.put(SnapshotInfo.BackupSnapshotAfterTakingSnapshot.key(),
             
String.valueOf(SnapshotInfo.BackupSnapshotAfterTakingSnapshot.value()));
-        options.put("volumeSize", snapshotInfo.getBaseVolume().getSize() + "");
+        options.put("volumeSize", snapshotObject.getBaseVolume().getSize() + 
"");
 
         try {
+            final String rscName = LinstorUtil.RSC_PREFIX + 
snapshotObject.getBaseVolume().getUuid();
+            String snapshotName = LinstorUtil.RSC_PREFIX + 
snapshotObject.getUuid();
+
+            // vmsnapshots don't have our typical snapshot path set
+            // instead the path is the internal snapshot name e.g.: 
{vm}_VS_{datestr}
+            // we have to find out and modify the path here before
+            if (!(snapshotObject.getPath().startsWith("/dev/mapper/") ||

Review Comment:
   seems like this comment would serve as good javadoc for a new method..?



##########
plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/snapshot/LinstorVMSnapshotStrategy.java:
##########
@@ -0,0 +1,347 @@
+//
+//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.snapshot;
+
+import com.linbit.linstor.api.ApiException;
+import com.linbit.linstor.api.DevelopersApi;
+import com.linbit.linstor.api.model.ApiCallRcList;
+import com.linbit.linstor.api.model.CreateMultiSnapshotRequest;
+import com.linbit.linstor.api.model.Snapshot;
+
+import javax.inject.Inject;
+
+import java.util.Collections;
+import java.util.List;
+
+import com.cloud.agent.api.VMSnapshotTO;
+import com.cloud.event.EventTypes;
+import com.cloud.event.UsageEventUtils;
+import com.cloud.storage.DiskOfferingVO;
+import com.cloud.storage.VolumeVO;
+import com.cloud.storage.dao.DiskOfferingDao;
+import com.cloud.storage.dao.VolumeDao;
+import com.cloud.uservm.UserVm;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.utils.fsm.NoTransitionException;
+import com.cloud.vm.UserVmVO;
+import com.cloud.vm.VirtualMachine;
+import com.cloud.vm.dao.UserVmDao;
+import com.cloud.vm.snapshot.VMSnapshot;
+import com.cloud.vm.snapshot.VMSnapshotVO;
+import com.cloud.vm.snapshot.dao.VMSnapshotDao;
+import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority;
+import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
+import org.apache.cloudstack.storage.datastore.util.LinstorUtil;
+import org.apache.cloudstack.storage.to.VolumeObjectTO;
+import org.apache.cloudstack.storage.vmsnapshot.DefaultVMSnapshotStrategy;
+import org.apache.cloudstack.storage.vmsnapshot.VMSnapshotHelper;
+import org.apache.log4j.Logger;
+import org.springframework.stereotype.Component;
+
+@Component
+public class LinstorVMSnapshotStrategy extends DefaultVMSnapshotStrategy {
+    private static final Logger log = 
Logger.getLogger(LinstorVMSnapshotStrategy.class);
+
+    @Inject
+    private VMSnapshotHelper _vmSnapshotHelper;
+    @Inject
+    private UserVmDao _userVmDao;
+    @Inject
+    private VMSnapshotDao vmSnapshotDao;
+    @Inject
+    private VolumeDao volumeDao;
+    @Inject
+    private DiskOfferingDao diskOfferingDao;
+    @Inject
+    private PrimaryDataStoreDao _storagePoolDao;
+
+    private void linstorCreateMultiSnapshot(
+        DevelopersApi api, VMSnapshotVO vmSnapshotVO, List<VolumeObjectTO> 
volumeTOs)
+        throws ApiException {
+        CreateMultiSnapshotRequest cmsReq = new CreateMultiSnapshotRequest();
+        for (VolumeObjectTO vol : volumeTOs) {
+            Snapshot snap = new Snapshot();
+            snap.setName(vmSnapshotVO.getName());
+            snap.setResourceName(LinstorUtil.RSC_PREFIX + vol.getPath());
+            log.debug(String.format("Add volume %s;%s to snapshot", 
vol.getName(), snap.getResourceName()));
+            cmsReq.addSnapshotsItem(snap);
+        }
+        log.debug(String.format("Creating multi snapshot %s", 
vmSnapshotVO.getName()));
+        ApiCallRcList answers = api.createMultiSnapshot(cmsReq);
+        log.debug(String.format("Created multi snapshot %s", 
vmSnapshotVO.getName()));
+        if (answers.hasError()) {
+            throw new CloudRuntimeException(
+                "Error creating vm snapshots: " + 
LinstorUtil.getBestErrorMessage(answers));
+        }
+    }
+
+    @Override
+    public VMSnapshot takeVMSnapshot(VMSnapshot vmSnapshot) {
+        log.info("Take snapshot");
+        UserVm userVm = _userVmDao.findById(vmSnapshot.getVmId());
+        VMSnapshotVO vmSnapshotVO = (VMSnapshotVO) vmSnapshot;
+
+        try {
+            _vmSnapshotHelper.vmSnapshotStateTransitTo(vmSnapshotVO, 
VMSnapshot.Event.CreateRequested);
+        } catch (NoTransitionException e) {
+            throw new CloudRuntimeException("No transition: " + 
e.getMessage());
+        }
+
+        boolean result = false;
+        try {
+
+            List<VolumeObjectTO> volumeTOs = 
_vmSnapshotHelper.getVolumeTOList(userVm.getId());
+            final StoragePoolVO storagePool = 
_storagePoolDao.findById(volumeTOs.get(0).getPoolId());
+            final DevelopersApi api = 
LinstorUtil.getLinstorAPI(storagePool.getHostAddress());
+
+            long prev_chain_size = 0;
+            long virtual_size = 0;
+            for (VolumeObjectTO volume : volumeTOs) {
+                virtual_size += volume.getSize();
+                VolumeVO volumeVO = volumeDao.findById(volume.getId());
+                prev_chain_size += volumeVO.getVmSnapshotChainSize() == null ? 
0 : volumeVO.getVmSnapshotChainSize();
+            }
+
+            VMSnapshotTO current = null;
+            VMSnapshotVO currentSnapshot = 
vmSnapshotDao.findCurrentSnapshotByVmId(userVm.getId());
+            if (currentSnapshot != null) {
+                current = 
_vmSnapshotHelper.getSnapshotWithParents(currentSnapshot);
+            }
+
+            if (current == null) {
+                vmSnapshotVO.setParent(null);
+            } else {
+                vmSnapshotVO.setParent(current.getId());
+            }
+
+            linstorCreateMultiSnapshot(api, vmSnapshotVO, volumeTOs);
+
+            log.debug(String.format("finalize vm snapshot create for %s", 
vmSnapshotVO.getName()));
+            finalizeCreate(vmSnapshotVO, volumeTOs);
+
+            result = _vmSnapshotHelper.vmSnapshotStateTransitTo(vmSnapshot, 
VMSnapshot.Event.OperationSucceeded);
+            long new_chain_size = 0;
+            for (VolumeObjectTO volumeObjectTO : volumeTOs) {
+                publishUsageEvents(EventTypes.EVENT_VM_SNAPSHOT_CREATE, 
vmSnapshot, userVm, volumeObjectTO);
+                new_chain_size += volumeObjectTO.getSize();
+                log.info("EventTypes.EVENT_VM_SNAPSHOT_CREATE 
publishUsageEvent" + volumeObjectTO);
+            }
+            publishUsageEvents(
+                EventTypes.EVENT_VM_SNAPSHOT_ON_PRIMARY, vmSnapshot, userVm, 
new_chain_size - prev_chain_size, virtual_size);
+            return vmSnapshot;
+        } catch (Exception e) {
+            log.debug("Could not create VM snapshot:" + e.getMessage());
+            throw new CloudRuntimeException("Could not create VM snapshot:" + 
e.getMessage());
+        } finally {
+            if (!result) {
+                try {
+                    _vmSnapshotHelper.vmSnapshotStateTransitTo(vmSnapshot, 
VMSnapshot.Event.OperationFailed);
+                    log.info(String.format("VMSnapshot.Event.OperationFailed 
vmSnapshot=%s", vmSnapshot));
+                } catch (NoTransitionException nte) {
+                    log.error("Cannot set vm state:" + nte.getMessage());
+                }
+            }
+        }
+    }
+
+    @Override
+    public StrategyPriority canHandle(Long vmId, Long rootPoolId, boolean 
snapshotMemory) {
+        if (snapshotMemory) {
+            return StrategyPriority.CANT_HANDLE;
+        }
+        return allVolumesOnLinstor(vmId);
+    }
+
+    @Override
+    public StrategyPriority canHandle(VMSnapshot vmSnapshot) {
+        VMSnapshotVO vmSnapshotVO = (VMSnapshotVO) vmSnapshot;
+        if (vmSnapshotVO.getType() != VMSnapshot.Type.Disk) {
+            return StrategyPriority.CANT_HANDLE;
+        }
+        return allVolumesOnLinstor(vmSnapshot.getVmId());
+    }
+
+    private StrategyPriority allVolumesOnLinstor(Long vmId) {
+        List<VolumeObjectTO> volumeTOs = 
_vmSnapshotHelper.getVolumeTOList(vmId);
+        if (volumeTOs == null || volumeTOs.isEmpty()) {
+            return StrategyPriority.CANT_HANDLE;
+        }
+        for (VolumeObjectTO volumeTO : volumeTOs) {
+            Long poolId = volumeTO.getPoolId();
+            StoragePoolVO pool = _storagePoolDao.findById(poolId);
+            if 
(!pool.getStorageProviderName().equals(LinstorUtil.PROVIDER_NAME)) {
+                return StrategyPriority.CANT_HANDLE;
+            }
+        }
+        return StrategyPriority.HIGHEST;
+    }
+
+    private String linstorDeleteSnapshot(final DevelopersApi api, final String 
rscName, final String snapshotName) {
+        String resultMsg = null;
+        try {
+            ApiCallRcList answers = api.resourceSnapshotDelete(rscName, 
snapshotName, Collections.emptyList());
+            if (answers.hasError()) {
+                resultMsg = LinstorUtil.getBestErrorMessage(answers);
+            }
+        } catch (ApiException apiEx) {
+            log.error("Linstor: ApiEx - " + apiEx.getBestMessage());
+            resultMsg = apiEx.getBestMessage();
+        }
+
+        return resultMsg;
+    }
+
+    @Override
+    public boolean deleteVMSnapshot(VMSnapshot vmSnapshot) {
+        UserVmVO userVm = _userVmDao.findById(vmSnapshot.getVmId());
+        VMSnapshotVO vmSnapshotVO = (VMSnapshotVO) vmSnapshot;
+        try {
+            _vmSnapshotHelper.vmSnapshotStateTransitTo(vmSnapshot, 
VMSnapshot.Event.ExpungeRequested);
+        } catch (NoTransitionException e) {
+            log.debug("Failed to change vm snapshot state with event 
ExpungeRequested");
+            throw new CloudRuntimeException(
+                    "Failed to change vm snapshot state with event 
ExpungeRequested: " + e.getMessage());
+        }
+
+        List<VolumeObjectTO> volumeTOs = 
_vmSnapshotHelper.getVolumeTOList(vmSnapshot.getVmId());
+        final StoragePoolVO storagePool = 
_storagePoolDao.findById(volumeTOs.get(0).getPoolId());
+        final DevelopersApi api = 
LinstorUtil.getLinstorAPI(storagePool.getHostAddress());
+
+        final String snapshotName = vmSnapshotVO.getName();
+        for (VolumeObjectTO volumeObjectTO : volumeTOs) {
+            final String rscName = LinstorUtil.RSC_PREFIX + 
volumeObjectTO.getUuid();
+            String err = linstorDeleteSnapshot(api, rscName, snapshotName);
+
+            if (err != null)
+            {
+                throw new CloudRuntimeException(
+                    String.format("Unable to delete Linstor resource %s 
snapshot %s: %s", rscName, snapshotName, err));
+            }
+            log.info("Linstor: Deleted snapshot " + snapshotName + " for 
resource " + rscName);
+        }
+
+        finalizeDelete(vmSnapshotVO, volumeTOs);
+        vmSnapshotDao.remove(vmSnapshot.getId());
+
+        long full_chain_size = 0;
+        for (VolumeObjectTO volumeTo : volumeTOs) {
+            publishUsageEvents(EventTypes.EVENT_VM_SNAPSHOT_DELETE, 
vmSnapshot, userVm, volumeTo);
+            full_chain_size += volumeTo.getSize();
+        }
+        publishUsageEvents(EventTypes.EVENT_VM_SNAPSHOT_OFF_PRIMARY, 
vmSnapshot, userVm, full_chain_size, 0L);
+        return true;
+    }
+
+    private String linstorRevertSnapshot(final DevelopersApi api, final String 
rscName, final String snapshotName) {
+        String resultMsg = null;
+        try {
+            ApiCallRcList answers = api.resourceSnapshotRollback(rscName, 
snapshotName);
+            if (answers.hasError()) {
+                resultMsg = LinstorUtil.getBestErrorMessage(answers);
+            }
+        } catch (ApiException apiEx) {
+            log.error("Linstor: ApiEx - " + apiEx.getBestMessage());
+            resultMsg = apiEx.getBestMessage();
+        }
+
+        return resultMsg;
+    }
+
+    @Override
+    public boolean revertVMSnapshot(VMSnapshot vmSnapshot) {
+        log.debug("Revert vm snapshot");
+        VMSnapshotVO vmSnapshotVO = (VMSnapshotVO) vmSnapshot;
+        UserVmVO userVm = _userVmDao.findById(vmSnapshot.getVmId());
+
+        if (userVm.getState() == VirtualMachine.State.Running && 
vmSnapshotVO.getType() == VMSnapshot.Type.Disk) {
+            throw new CloudRuntimeException("Virtual machine should be in 
stopped state for revert operation");
+        }
+
+        try {
+            _vmSnapshotHelper.vmSnapshotStateTransitTo(vmSnapshotVO, 
VMSnapshot.Event.RevertRequested);
+        } catch (NoTransitionException e) {
+            throw new CloudRuntimeException(e.getMessage());
+        }
+
+        boolean result = false;
+        try {

Review Comment:
   several try's in one method. they are not nested but this last one is the 
complex on and could go in its own method..?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to