[PR] [WIP] backup: Simple NAS backup plugin for KVM (experimental) [cloudstack]

["rohityadavcloud (via GitHub)" <gi...@apache.org>]

rohityadavcloud opened a new pull request, #9451:
URL: https://github.com/apache/cloudstack/pull/9451

   This is an experimental simple NAS backup plugin for KVM which may be later expanded for other hypervisors. This backup plugin aims to use shared NAS storage on KVM hosts such as NFS or CephFS, which is used to backup fully cloned VMs for backup & restore operations. This may not be as efficient and performant as some of the other B&R providers, but maybe useful for some KVM environments.
   
   Design & Implementation follows the `networker` B&R plugin, which is simply:
   
   - Implement B&R plugin interfaces
   - Use cmd-answer pattern to execute backup and restore operations on KVM host when VM is running (or needs to be restored) - instead of a B&R API client, relies on answers from KVM agent which executes the operations
   - Backups are full VM domain snapshots, copied to a VM-specific folders on a NAS target (NFS or CephFS WIP) along with a domain XML
   
   ### Types of changes
   
   - [ ] Breaking change (fix or feature that would cause existing functionality to change)
   - [x] New feature (non-breaking change which adds functionality)
   - [ ] Bug fix (non-breaking change which fixes an issue)
   - [ ] Enhancement (improves an existing feature and functionality)
   - [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
   - [ ] build/CI
   - [ ] test (unit or integration test code)
   
   ### Feature/Enhancement Scale or Bug Severity
   
   #### Feature/Enhancement Scale
   
   - [x] Major
   - [ ] Minor


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["Pearl1594 (via GitHub)" <gi...@apache.org>]

Pearl1594 commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2330417956

   @blueorangutan package


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM (experimental) [cloudstack]

["blueorangutan (via GitHub)" <gi...@apache.org>]

blueorangutan commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2326678039

   <b>[SF] Trillian test result (tid-11304)</b>
   Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
   Total time taken: 49496 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9451-t11304-kvm-ol8.zip
   Smoke tests completed. 139 look OK, 0 have errors, 0 did not run
   Only failed and skipped tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM (experimental) [cloudstack]

["shwstppr (via GitHub)" <gi...@apache.org>]

shwstppr commented on code in PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#discussion_r1735973652


##########
plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java:
##########
@@ -0,0 +1,434 @@
+// 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.backup;
+
+import com.cloud.agent.AgentManager;
+import com.cloud.dc.dao.ClusterDao;
+import com.cloud.exception.AgentUnavailableException;
+import com.cloud.exception.OperationTimedoutException;
+import com.cloud.host.Host;
+import com.cloud.host.HostVO;
+import com.cloud.host.Status;
+import com.cloud.host.dao.HostDao;
+import com.cloud.hypervisor.Hypervisor;
+import com.cloud.storage.ScopeType;
+import com.cloud.storage.StoragePoolHostVO;
+import com.cloud.storage.Volume;
+import com.cloud.storage.VolumeVO;
+import com.cloud.storage.dao.StoragePoolHostDao;
+import com.cloud.storage.dao.VolumeDao;
+import com.cloud.utils.Pair;
+import com.cloud.utils.component.AdapterBase;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.vm.VirtualMachine;
+import com.cloud.vm.dao.VMInstanceDao;
+import org.apache.cloudstack.backup.dao.BackupDao;
+import org.apache.cloudstack.backup.dao.BackupOfferingDao;
+import org.apache.cloudstack.backup.dao.BackupRepositoryDao;
+import org.apache.cloudstack.framework.config.ConfigKey;
+import org.apache.cloudstack.framework.config.Configurable;
+import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
+import org.apache.commons.collections.CollectionUtils;
+import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.LogManager;
+import javax.inject.Inject;
+import java.text.SimpleDateFormat;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.Date;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import java.util.HashMap;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.UUID;
+
+public class NASBackupProvider extends AdapterBase implements BackupProvider, Configurable {
+    private static final Logger LOG = LogManager.getLogger(NASBackupProvider.class);
+
+    @Inject
+    private BackupDao backupDao;
+
+    @Inject
+    private BackupRepositoryDao backupRepositoryDao;
+
+    @Inject
+    private BackupOfferingDao backupOfferingDao;
+
+    @Inject
+    private HostDao hostDao;
+
+    @Inject
+    private ClusterDao clusterDao;
+
+    @Inject
+    private VolumeDao volumeDao;
+
+    @Inject
+    private StoragePoolHostDao storagePoolHostDao;
+
+    @Inject
+    private VMInstanceDao vmInstanceDao;
+
+    @Inject
+    private PrimaryDataStoreDao primaryDataStoreDao;
+
+    @Inject
+    private AgentManager agentManager;
+
+    protected Host getLastVMHypervisorHost(VirtualMachine vm) {
+        Long hostId = vm.getLastHostId();
+        if (hostId == null) {
+            LOG.debug("Cannot find last host for vm. This should never happen, please check your database.");

Review Comment:
   Not sure if relevant but I think it can happen in case of cross-cluster migrations



##########
plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java:
##########
@@ -0,0 +1,434 @@
+// 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.backup;
+
+import com.cloud.agent.AgentManager;
+import com.cloud.dc.dao.ClusterDao;
+import com.cloud.exception.AgentUnavailableException;
+import com.cloud.exception.OperationTimedoutException;
+import com.cloud.host.Host;
+import com.cloud.host.HostVO;
+import com.cloud.host.Status;
+import com.cloud.host.dao.HostDao;
+import com.cloud.hypervisor.Hypervisor;
+import com.cloud.storage.ScopeType;
+import com.cloud.storage.StoragePoolHostVO;
+import com.cloud.storage.Volume;
+import com.cloud.storage.VolumeVO;
+import com.cloud.storage.dao.StoragePoolHostDao;
+import com.cloud.storage.dao.VolumeDao;
+import com.cloud.utils.Pair;
+import com.cloud.utils.component.AdapterBase;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.vm.VirtualMachine;
+import com.cloud.vm.dao.VMInstanceDao;
+import org.apache.cloudstack.backup.dao.BackupDao;
+import org.apache.cloudstack.backup.dao.BackupOfferingDao;
+import org.apache.cloudstack.backup.dao.BackupRepositoryDao;
+import org.apache.cloudstack.framework.config.ConfigKey;
+import org.apache.cloudstack.framework.config.Configurable;
+import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
+import org.apache.commons.collections.CollectionUtils;
+import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.LogManager;
+import javax.inject.Inject;
+import java.text.SimpleDateFormat;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.Date;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import java.util.HashMap;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.UUID;
+
+public class NASBackupProvider extends AdapterBase implements BackupProvider, Configurable {
+    private static final Logger LOG = LogManager.getLogger(NASBackupProvider.class);
+
+    @Inject
+    private BackupDao backupDao;
+
+    @Inject
+    private BackupRepositoryDao backupRepositoryDao;
+
+    @Inject
+    private BackupOfferingDao backupOfferingDao;
+
+    @Inject
+    private HostDao hostDao;
+
+    @Inject
+    private ClusterDao clusterDao;
+
+    @Inject
+    private VolumeDao volumeDao;
+
+    @Inject
+    private StoragePoolHostDao storagePoolHostDao;
+
+    @Inject
+    private VMInstanceDao vmInstanceDao;
+
+    @Inject
+    private PrimaryDataStoreDao primaryDataStoreDao;
+
+    @Inject
+    private AgentManager agentManager;
+
+    protected Host getLastVMHypervisorHost(VirtualMachine vm) {
+        Long hostId = vm.getLastHostId();
+        if (hostId == null) {
+            LOG.debug("Cannot find last host for vm. This should never happen, please check your database.");
+            return null;
+        }
+        Host host = hostDao.findById(hostId);
+
+        if (host.getStatus() == Status.Up) {
+            return host;
+        } else {
+            // Try to find any Up host in the same cluster
+            for (final Host hostInCluster : hostDao.findHypervisorHostInCluster(host.getClusterId())) {
+                if (hostInCluster.getStatus() == Status.Up) {
+                    LOG.debug("Found Host " + hostInCluster.getName());
+                    return hostInCluster;
+                }
+            }
+        }
+        // Try to find any Host in the zone
+        for (final HostVO hostInZone : hostDao.listByDataCenterIdAndHypervisorType(host.getDataCenterId(), Hypervisor.HypervisorType.KVM)) {
+            if (hostInZone.getStatus() == Status.Up) {
+                LOG.debug("Found Host " + hostInZone.getName());
+                return hostInZone;
+            }
+        }
+        return null;
+    }
+
+    protected Host getVMHypervisorHost(VirtualMachine vm) {
+        Long hostId = vm.getHostId();
+        if (hostId == null && VirtualMachine.State.Running.equals(vm.getState())) {
+            throw new CloudRuntimeException(String.format("Unable to find the hypervisor host for %s. Make sure the virtual machine is running", vm.getName()));
+        }
+        if (VirtualMachine.State.Stopped.equals(vm.getState())) {
+            hostId = vm.getLastHostId();
+        }
+        if (hostId == null) {
+            throw new CloudRuntimeException(String.format("Unable to find the hypervisor host for stopped VM: %s", vm));
+        }
+        final Host host = hostDao.findById(hostId);
+        if (host == null || !Status.Up.equals(host.getStatus()) || !Hypervisor.HypervisorType.KVM.equals(host.getHypervisorType())) {
+            throw new CloudRuntimeException("Unable to contact backend control plane to initiate backup");
+        }
+        return host;
+    }
+
+    @Override
+    public boolean takeBackup(final VirtualMachine vm) {
+        final Host host = getVMHypervisorHost(vm);
+
+        final BackupRepository backupRepository = backupRepositoryDao.findByBackupOfferingId(vm.getBackupOfferingId());
+        if (backupRepository == null) {
+            throw new CloudRuntimeException("No valid backup repository found for the VM, please check the attached backup offering");
+        }
+
+        final Date creationDate = new Date();
+        final String backupPath = String.format("%s/%s", vm.getInstanceName(),
+                new SimpleDateFormat("yyyy.MM.dd.HH.mm.ss").format(creationDate));
+
+        TakeBackupCommand command = new TakeBackupCommand(vm.getInstanceName(), backupPath);
+        command.setBackupRepoType(backupRepository.getType());
+        command.setBackupRepoAddress(backupRepository.getAddress());
+        command.setMountOptions(backupRepository.getMountOptions());
+
+        if (VirtualMachine.State.Stopped.equals(vm.getState())) {
+            List<VolumeVO> vmVolumes = volumeDao.findByInstance(vm.getId());
+            List<String> volumePaths = getVolumePaths(vmVolumes);
+            command.setVolumePaths(volumePaths);
+        }
+
+        BackupAnswer answer = null;
+        try {
+            answer = (BackupAnswer) agentManager.send(host.getId(), command);
+        } catch (AgentUnavailableException e) {
+            throw new CloudRuntimeException("Unable to contact backend control plane to initiate backup");
+        } catch (OperationTimedoutException e) {
+            throw new CloudRuntimeException("Operation to initiate backup timed out, please try again");
+        }
+
+        if (answer != null && answer.getResult()) {
+            BackupVO backup = new BackupVO();
+            backup.setVmId(vm.getId());
+            backup.setExternalId(backupPath);
+            backup.setType("FULL");
+            backup.setDate(creationDate);
+            backup.setSize(answer.getSize());
+            long virtualSize = 0L;
+            for (final Volume volume: volumeDao.findByInstance(vm.getId())) {
+                if (Volume.State.Ready.equals(volume.getState())) {
+                    virtualSize += volume.getSize();
+                }
+            }
+            backup.setProtectedSize(Long.valueOf(virtualSize));
+            backup.setStatus(Backup.Status.BackedUp);
+            backup.setBackupOfferingId(vm.getBackupOfferingId());
+            backup.setAccountId(vm.getAccountId());
+            backup.setDomainId(vm.getDomainId());
+            backup.setZoneId(vm.getDataCenterId());
+            return backupDao.persist(backup) != null;
+        }
+
+        return answer.getResult();
+    }
+
+    @Override
+    public boolean restoreVMFromBackup(VirtualMachine vm, Backup backup) {
+        List<VolumeVO> volumes = volumeDao.findByInstance(vm.getId());
+        volumes.sort(Comparator.comparing(VolumeVO::getDeviceId));
+
+        LOG.debug(String.format("Restoring vm %s from backup %s on the NAS Backup Provider", vm.getUuid(), backup.getUuid()));
+        BackupRepository backupRepository = getBackupRepository(vm, backup);
+
+        final Host host = getLastVMHypervisorHost(vm);
+        RestoreBackupCommand restoreCommand = new RestoreBackupCommand();
+        restoreCommand.setBackupPath(backup.getExternalId());
+        restoreCommand.setBackupRepoType(backupRepository.getType());
+        restoreCommand.setBackupRepoAddress(backupRepository.getAddress());
+        restoreCommand.setVmName(vm.getName());
+        restoreCommand.setVolumePaths(getVolumePaths(volumes));
+        restoreCommand.setVmExists(vm.getRemoved() == null);
+
+        BackupAnswer answer = null;
+        try {
+            answer = (BackupAnswer) agentManager.send(host.getId(), restoreCommand);
+        } catch (AgentUnavailableException e) {
+            throw new CloudRuntimeException("Unable to contact backend control plane to initiate backup");
+        } catch (OperationTimedoutException e) {
+            throw new CloudRuntimeException("Operation to initiate backup timed out, please try again");
+        }
+        return answer.getResult();
+    }
+
+    private List<String> getVolumePaths(List<VolumeVO> volumes) {
+        List<String> volumePaths = new ArrayList<>();
+        for (VolumeVO volume : volumes) {
+            StoragePoolVO storagePool = primaryDataStoreDao.findById(volume.getPoolId());
+            if (Objects.isNull(storagePool)) {
+                throw new CloudRuntimeException("Unable to find storage pool associated to the volume");
+            }
+            String volumePathPrefix = String.format("/mnt/%s", storagePool.getUuid());
+            if (ScopeType.HOST.equals(storagePool.getScope())) {
+                volumePathPrefix = storagePool.getPath();
+            }
+            volumePaths.add(String.format("%s/%s", volumePathPrefix, volume.getPath()));
+        }
+        return volumePaths;
+    }
+
+    @Override
+    public Pair<Boolean, String> restoreBackedUpVolume(Backup backup, String volumeUuid, String hostIp, String dataStoreUuid) {
+        final VolumeVO volume = volumeDao.findByUuid(volumeUuid);
+        final VirtualMachine backupSourceVm = vmInstanceDao.findById(backup.getVmId());
+        final StoragePoolHostVO dataStore = storagePoolHostDao.findByUuid(dataStoreUuid);
+        final HostVO hostVO = hostDao.findByIp(hostIp);
+        final Long zoneId = backup.getZoneId();
+
+        // TODO: Find volume from backup volumes
+        Optional<Backup.VolumeInfo> matchingVolume = getBackedUpVolumeInfo(backupSourceVm.getBackupVolumeList(), volumeUuid);
+        Long backedUpVolumeSize = matchingVolume.isPresent() ? matchingVolume.get().getSize() : 0L;
+
+        LOG.debug("Restoring vm volume" + volumeUuid + "from backup " + backup.getUuid() + " on the NAS Backup Provider");
+        BackupRepository backupRepository = getBackupRepository(backupSourceVm, backup);
+
+        VolumeVO restoredVolume = new VolumeVO(Volume.Type.DATADISK, null, backup.getZoneId(),
+                backup.getDomainId(), backup.getAccountId(), 0, null,
+                backup.getSize(), null, null, null);
+        String volumeUUID = UUID.randomUUID().toString();
+        restoredVolume.setName("RestoredVol-"+volume.getName());
+        restoredVolume.setProvisioningType(volume.getProvisioningType());
+        restoredVolume.setUpdated(new Date());
+        restoredVolume.setUuid(volumeUUID);
+        restoredVolume.setRemoved(null);
+        restoredVolume.setDisplayVolume(true);
+        restoredVolume.setPoolId(dataStore.getPoolId());
+        restoredVolume.setPath(restoredVolume.getUuid());
+        restoredVolume.setState(Volume.State.Copying);
+        restoredVolume.setSize(backedUpVolumeSize);
+        restoredVolume.setDiskOfferingId(volume.getDiskOfferingId());
+
+        RestoreBackupCommand restoreCommand = new RestoreBackupCommand();
+        restoreCommand.setBackupPath(backup.getExternalId());
+        restoreCommand.setBackupRepoType(backupRepository.getType());
+        restoreCommand.setBackupRepoAddress(backupRepository.getAddress());
+        restoreCommand.setVmName(backupSourceVm.getName());
+        restoreCommand.setVolumePaths(Collections.singletonList(String.format("%s/%s", dataStore.getLocalPath(), volumeUUID)));
+        restoreCommand.setDiskType(volume.getVolumeType().name().toLowerCase(Locale.ROOT));
+        restoreCommand.setDeviceId(volume.getDeviceId());
+        restoreCommand.setVmExists(null);
+        restoreCommand.setRestoreVolumeUUID(volumeUuid);
+
+        BackupAnswer answer = null;
+        try {
+            answer = (BackupAnswer) agentManager.send(hostVO.getId(), restoreCommand);
+        } catch (AgentUnavailableException e) {
+            throw new CloudRuntimeException("Unable to contact backend control plane to initiate backup");
+        } catch (OperationTimedoutException e) {
+            throw new CloudRuntimeException("Operation to initiate backup timed out, please try again");
+        }
+
+        if (answer.getResult()) {
+            try {
+                volumeDao.persist(restoredVolume);
+            } catch (Exception e) {
+                throw new CloudRuntimeException("Unable to create restored volume due to: " + e);
+            }
+        }
+
+        return new Pair<>(answer.getResult(), answer.getDetails());
+    }
+
+    private BackupRepository getBackupRepository(VirtualMachine vm, Backup backup) {
+        BackupRepository backupRepository = backupRepositoryDao.findByBackupOfferingId(vm.getBackupOfferingId());
+        final String errorMessage = "No valid backup repository found for the VM, please check the attached backup offering";
+        if (backupRepository == null) {
+            logger.warn(errorMessage + "Re-attempting with the backup offering associated with the backup");
+        }
+        backupRepository = backupRepositoryDao.findByBackupOfferingId(backup.getBackupOfferingId());
+        if (backupRepository == null) {
+            throw new CloudRuntimeException(errorMessage);
+        }
+        return backupRepository;
+    }
+
+    private Optional<Backup.VolumeInfo> getBackedUpVolumeInfo(List<Backup.VolumeInfo> backedUpVolumes, String volumeUuid) {
+        return backedUpVolumes.stream()
+                .filter(v -> v.getUuid().equals(volumeUuid))
+                .findFirst();
+    }
+
+    @Override
+    public boolean deleteBackup(Backup backup, boolean forced) {
+        final BackupRepository backupRepository = backupRepositoryDao.findByBackupOfferingId(backup.getBackupOfferingId());
+        if (backupRepository == null) {
+            throw new CloudRuntimeException("No valid backup repository found for the VM, please check the attached backup offering");
+        }
+
+        // TODO: this can be any host in the cluster or last host

Review Comment:
   This need to be addressed?



##########
ui/src/components/view/ListView.vue:
##########
@@ -174,7 +174,7 @@
       <template v-if="record.clustertype === 'ExternalManaged' && $route.path.split('/')[1] === 'kubernetes' && ['kubernetesversionname', 'cpunumber', 'memory', 'size'].includes(column.key)">
         <span>{{ text <= 0 || !text ? 'N/A' : text }}</span>
       </template>
-      <template v-else-if="column.key === 'size'">
+      <template v-else-if="column.key === 'size' || column.key === 'virtualsize'">

Review Comment:
   minor (presentational)
   ```suggestion
         <template v-else-if="['size', 'virtualsize'].includes(column.key)">
   ```



##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java:
##########
@@ -0,0 +1,181 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package com.cloud.hypervisor.kvm.resource.wrapper;
+
+import com.cloud.agent.api.Answer;
+import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
+import com.cloud.resource.CommandWrapper;
+import com.cloud.resource.ResourceWrapper;
+import com.cloud.utils.Pair;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.utils.script.Script;
+import org.apache.cloudstack.backup.BackupAnswer;
+import org.apache.cloudstack.backup.RestoreBackupCommand;
+import org.apache.commons.lang3.RandomStringUtils;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.nio.file.StandardCopyOption;
+import java.util.List;
+import java.util.Locale;
+import java.util.Objects;
+
+@ResourceWrapper(handles = RestoreBackupCommand.class)
+public class LibvirtRestoreBackupCommandWrapper extends CommandWrapper<RestoreBackupCommand, Answer, LibvirtComputingResource> {
+    private static final String BACKUP_TEMP_FILE_PREFIx = "csbackup";
+    private static final String MOUNT_COMMAND = "sudo mount -t %s %s %s";
+    private static final String UMOUNT_COMMAND = "sudo umount %s";
+    private static final String FILE_PATH_PLACEHOLDER = "%s/%s";
+
+    @Override
+    public Answer execute(RestoreBackupCommand command, LibvirtComputingResource serverResource) {
+        String vmName = command.getVmName();
+        String backupPath = command.getBackupPath();
+        String backupRepoAddress = command.getBackupRepoAddress();
+        String backupRepoType = command.getBackupRepoType();
+        String mountOptions = command.getMountOptions();
+        Boolean vmExists = command.isVmExists();
+        String diskType = command.getDiskType();
+        Long deviceId = command.getDeviceId();
+        List<String> volumePaths = command.getVolumePaths();
+        String restoreVolumeUuid = command.getRestoreVolumeUUID();
+
+        String newVolumeId = null;
+        if (Objects.isNull(vmExists)) {
+            String volumePath = volumePaths.get(0);
+            int lastIndex = volumePath.lastIndexOf("/");
+            newVolumeId = volumePath.substring(lastIndex + 1);
+            restoreVolume(backupPath, backupRepoType, backupRepoAddress, volumePath, diskType, deviceId, restoreVolumeUuid);
+        } else if (vmExists) {
+            restoreVolumesOfExistingVM(volumePaths, backupPath, backupRepoType, backupRepoAddress, mountOptions);
+        } else {
+            restoreVolumesOfDestroyedVMs(volumePaths, vmName, backupPath, backupRepoType, backupRepoAddress, mountOptions);
+        }
+
+        return new BackupAnswer(command, true, newVolumeId);
+    }
+
+    private void restoreVolumesOfExistingVM(List<String> volumePaths, String backupPath,
+                                             String backupRepoType, String backupRepoAddress, String mountOptions) {
+        String diskType = "root";
+        String mountDirectory = mountBackupDirectory(backupRepoAddress, backupRepoType);
+        try {
+            for (int idx = 0; idx < volumePaths.size(); idx++) {
+                String volumePath = volumePaths.get(idx);
+                Pair<String, String> bkpPathAndVolUuid = getBackupPath(mountDirectory, volumePath, backupPath, diskType, idx, null);
+                diskType = "datadisk";
+                try {
+                    replaceVolumeWithBackup(volumePath, bkpPathAndVolUuid.first());
+                } catch (IOException e) {
+                    throw new CloudRuntimeException(String.format("Unable to revert backup for volume [%s] due to [%s].", bkpPathAndVolUuid.second(), e.getMessage()), e);

Review Comment:
   Any handling needed if there are multiple volumes and the replace operations fails after some



##########
server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java:
##########
@@ -749,20 +759,26 @@ public boolean restoreBackupVolumeAndAttachToVM(final String backedUpVolumeUuid,
             throw new CloudRuntimeException("VM reference for the provided VM backup not found");
         }
         accountManager.checkAccess(CallContext.current().getCallingAccount(), null, true, vmFromBackup);
+        final BackupOffering offering = backupOfferingDao.findByIdIncludingRemoved(backup.getBackupOfferingId());
+        if (offering == null) {
+            throw new CloudRuntimeException("Failed to find VM backup offering");
+        }
+
+        BackupProvider backupProvider = getBackupProvider(offering.getProvider());
+        VolumeVO backedUpVolume = volumeDao.findByUuid(backedUpVolumeUuid);
+        Pair<HostVO, StoragePoolVO> restoreInfo;
+        if (!Objects.equals(offering.getProvider(), "nas")) {

Review Comment:
   ```suggestion
           if (!"nas".equals(offering.getProvider())) {
   ```



##########
plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java:
##########
@@ -0,0 +1,434 @@
+// 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.backup;
+
+import com.cloud.agent.AgentManager;
+import com.cloud.dc.dao.ClusterDao;
+import com.cloud.exception.AgentUnavailableException;
+import com.cloud.exception.OperationTimedoutException;
+import com.cloud.host.Host;
+import com.cloud.host.HostVO;
+import com.cloud.host.Status;
+import com.cloud.host.dao.HostDao;
+import com.cloud.hypervisor.Hypervisor;
+import com.cloud.storage.ScopeType;
+import com.cloud.storage.StoragePoolHostVO;
+import com.cloud.storage.Volume;
+import com.cloud.storage.VolumeVO;
+import com.cloud.storage.dao.StoragePoolHostDao;
+import com.cloud.storage.dao.VolumeDao;
+import com.cloud.utils.Pair;
+import com.cloud.utils.component.AdapterBase;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.vm.VirtualMachine;
+import com.cloud.vm.dao.VMInstanceDao;
+import org.apache.cloudstack.backup.dao.BackupDao;
+import org.apache.cloudstack.backup.dao.BackupOfferingDao;
+import org.apache.cloudstack.backup.dao.BackupRepositoryDao;
+import org.apache.cloudstack.framework.config.ConfigKey;
+import org.apache.cloudstack.framework.config.Configurable;
+import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
+import org.apache.commons.collections.CollectionUtils;
+import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.LogManager;
+import javax.inject.Inject;
+import java.text.SimpleDateFormat;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.Date;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import java.util.HashMap;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.UUID;
+
+public class NASBackupProvider extends AdapterBase implements BackupProvider, Configurable {
+    private static final Logger LOG = LogManager.getLogger(NASBackupProvider.class);
+
+    @Inject
+    private BackupDao backupDao;
+
+    @Inject
+    private BackupRepositoryDao backupRepositoryDao;
+
+    @Inject
+    private BackupOfferingDao backupOfferingDao;
+
+    @Inject
+    private HostDao hostDao;
+
+    @Inject
+    private ClusterDao clusterDao;
+
+    @Inject
+    private VolumeDao volumeDao;
+
+    @Inject
+    private StoragePoolHostDao storagePoolHostDao;
+
+    @Inject
+    private VMInstanceDao vmInstanceDao;
+
+    @Inject
+    private PrimaryDataStoreDao primaryDataStoreDao;
+
+    @Inject
+    private AgentManager agentManager;
+
+    protected Host getLastVMHypervisorHost(VirtualMachine vm) {
+        Long hostId = vm.getLastHostId();
+        if (hostId == null) {
+            LOG.debug("Cannot find last host for vm. This should never happen, please check your database.");
+            return null;
+        }
+        Host host = hostDao.findById(hostId);
+
+        if (host.getStatus() == Status.Up) {
+            return host;
+        } else {
+            // Try to find any Up host in the same cluster
+            for (final Host hostInCluster : hostDao.findHypervisorHostInCluster(host.getClusterId())) {
+                if (hostInCluster.getStatus() == Status.Up) {
+                    LOG.debug("Found Host " + hostInCluster.getName());
+                    return hostInCluster;
+                }
+            }
+        }
+        // Try to find any Host in the zone
+        for (final HostVO hostInZone : hostDao.listByDataCenterIdAndHypervisorType(host.getDataCenterId(), Hypervisor.HypervisorType.KVM)) {
+            if (hostInZone.getStatus() == Status.Up) {
+                LOG.debug("Found Host " + hostInZone.getName());
+                return hostInZone;
+            }
+        }
+        return null;
+    }
+
+    protected Host getVMHypervisorHost(VirtualMachine vm) {
+        Long hostId = vm.getHostId();
+        if (hostId == null && VirtualMachine.State.Running.equals(vm.getState())) {
+            throw new CloudRuntimeException(String.format("Unable to find the hypervisor host for %s. Make sure the virtual machine is running", vm.getName()));
+        }
+        if (VirtualMachine.State.Stopped.equals(vm.getState())) {
+            hostId = vm.getLastHostId();
+        }
+        if (hostId == null) {
+            throw new CloudRuntimeException(String.format("Unable to find the hypervisor host for stopped VM: %s", vm));
+        }
+        final Host host = hostDao.findById(hostId);
+        if (host == null || !Status.Up.equals(host.getStatus()) || !Hypervisor.HypervisorType.KVM.equals(host.getHypervisorType())) {
+            throw new CloudRuntimeException("Unable to contact backend control plane to initiate backup");
+        }
+        return host;
+    }
+
+    @Override
+    public boolean takeBackup(final VirtualMachine vm) {
+        final Host host = getVMHypervisorHost(vm);
+
+        final BackupRepository backupRepository = backupRepositoryDao.findByBackupOfferingId(vm.getBackupOfferingId());
+        if (backupRepository == null) {
+            throw new CloudRuntimeException("No valid backup repository found for the VM, please check the attached backup offering");
+        }
+
+        final Date creationDate = new Date();
+        final String backupPath = String.format("%s/%s", vm.getInstanceName(),
+                new SimpleDateFormat("yyyy.MM.dd.HH.mm.ss").format(creationDate));
+
+        TakeBackupCommand command = new TakeBackupCommand(vm.getInstanceName(), backupPath);
+        command.setBackupRepoType(backupRepository.getType());
+        command.setBackupRepoAddress(backupRepository.getAddress());
+        command.setMountOptions(backupRepository.getMountOptions());
+
+        if (VirtualMachine.State.Stopped.equals(vm.getState())) {
+            List<VolumeVO> vmVolumes = volumeDao.findByInstance(vm.getId());
+            List<String> volumePaths = getVolumePaths(vmVolumes);
+            command.setVolumePaths(volumePaths);
+        }

Review Comment:
   ISO attached to the VM needs to be considered?



##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java:
##########
@@ -0,0 +1,181 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package com.cloud.hypervisor.kvm.resource.wrapper;
+
+import com.cloud.agent.api.Answer;
+import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
+import com.cloud.resource.CommandWrapper;
+import com.cloud.resource.ResourceWrapper;
+import com.cloud.utils.Pair;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.utils.script.Script;
+import org.apache.cloudstack.backup.BackupAnswer;
+import org.apache.cloudstack.backup.RestoreBackupCommand;
+import org.apache.commons.lang3.RandomStringUtils;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.nio.file.StandardCopyOption;
+import java.util.List;
+import java.util.Locale;
+import java.util.Objects;
+
+@ResourceWrapper(handles = RestoreBackupCommand.class)
+public class LibvirtRestoreBackupCommandWrapper extends CommandWrapper<RestoreBackupCommand, Answer, LibvirtComputingResource> {
+    private static final String BACKUP_TEMP_FILE_PREFIx = "csbackup";

Review Comment:
   ```suggestion
       private static final String BACKUP_TEMP_FILE_PREFIX = "csbackup";
   ```



##########
ui/src/components/view/ListResourceTable.vue:
##########
@@ -50,6 +50,10 @@
             {{ $toLocaleDate(text) }}
           </template>
 
+          <template v-else-if="column.key === 'size' || column.key === 'virtualsize'">
+            {{ parseFloat(text / (1024.0 * 1024.0 * 1024.0)).toFixed(2) }} GB
+          </template>
+

Review Comment:
   would it be better to use - `bytesToHumanReadableSize` method?



##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java:
##########
@@ -0,0 +1,181 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package com.cloud.hypervisor.kvm.resource.wrapper;
+
+import com.cloud.agent.api.Answer;
+import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
+import com.cloud.resource.CommandWrapper;
+import com.cloud.resource.ResourceWrapper;
+import com.cloud.utils.Pair;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.utils.script.Script;
+import org.apache.cloudstack.backup.BackupAnswer;
+import org.apache.cloudstack.backup.RestoreBackupCommand;
+import org.apache.commons.lang3.RandomStringUtils;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.nio.file.StandardCopyOption;
+import java.util.List;
+import java.util.Locale;
+import java.util.Objects;
+
+@ResourceWrapper(handles = RestoreBackupCommand.class)
+public class LibvirtRestoreBackupCommandWrapper extends CommandWrapper<RestoreBackupCommand, Answer, LibvirtComputingResource> {
+    private static final String BACKUP_TEMP_FILE_PREFIx = "csbackup";
+    private static final String MOUNT_COMMAND = "sudo mount -t %s %s %s";
+    private static final String UMOUNT_COMMAND = "sudo umount %s";
+    private static final String FILE_PATH_PLACEHOLDER = "%s/%s";
+
+    @Override
+    public Answer execute(RestoreBackupCommand command, LibvirtComputingResource serverResource) {
+        String vmName = command.getVmName();
+        String backupPath = command.getBackupPath();
+        String backupRepoAddress = command.getBackupRepoAddress();
+        String backupRepoType = command.getBackupRepoType();
+        String mountOptions = command.getMountOptions();
+        Boolean vmExists = command.isVmExists();
+        String diskType = command.getDiskType();
+        Long deviceId = command.getDeviceId();
+        List<String> volumePaths = command.getVolumePaths();
+        String restoreVolumeUuid = command.getRestoreVolumeUUID();
+
+        String newVolumeId = null;
+        if (Objects.isNull(vmExists)) {
+            String volumePath = volumePaths.get(0);
+            int lastIndex = volumePath.lastIndexOf("/");
+            newVolumeId = volumePath.substring(lastIndex + 1);
+            restoreVolume(backupPath, backupRepoType, backupRepoAddress, volumePath, diskType, deviceId, restoreVolumeUuid);
+        } else if (vmExists) {
+            restoreVolumesOfExistingVM(volumePaths, backupPath, backupRepoType, backupRepoAddress, mountOptions);
+        } else {
+            restoreVolumesOfDestroyedVMs(volumePaths, vmName, backupPath, backupRepoType, backupRepoAddress, mountOptions);
+        }
+
+        return new BackupAnswer(command, true, newVolumeId);
+    }
+
+    private void restoreVolumesOfExistingVM(List<String> volumePaths, String backupPath,
+                                             String backupRepoType, String backupRepoAddress, String mountOptions) {
+        String diskType = "root";
+        String mountDirectory = mountBackupDirectory(backupRepoAddress, backupRepoType);
+        try {
+            for (int idx = 0; idx < volumePaths.size(); idx++) {
+                String volumePath = volumePaths.get(idx);
+                Pair<String, String> bkpPathAndVolUuid = getBackupPath(mountDirectory, volumePath, backupPath, diskType, idx, null);
+                diskType = "datadisk";
+                try {
+                    replaceVolumeWithBackup(volumePath, bkpPathAndVolUuid.first());
+                } catch (IOException e) {
+                    throw new CloudRuntimeException(String.format("Unable to revert backup for volume [%s] due to [%s].", bkpPathAndVolUuid.second(), e.getMessage()), e);
+                }
+            }
+        } finally {
+            unmountBackupDirectory(mountDirectory);
+            deleteTemporaryDirectory(mountDirectory);
+        }
+
+    }
+
+    private void restoreVolumesOfDestroyedVMs(List<String> volumePaths, String vmName, String backupPath,
+                                              String backupRepoType, String backupRepoAddress, String mountOptions) {
+        String mountDirectory = mountBackupDirectory(backupRepoAddress, backupRepoType);
+        String diskType = "root";
+        try {
+            for (int i = 0; i < volumePaths.size(); i++) {
+                String volumePath = volumePaths.get(i);
+                Pair<String, String> bkpPathAndVolUuid = getBackupPath(mountDirectory, volumePath, backupPath, diskType, i, null);
+                diskType = "datadisk";
+                try {
+                    replaceVolumeWithBackup(volumePath, bkpPathAndVolUuid.first());
+                } catch (IOException e) {
+                    throw new CloudRuntimeException(String.format("Unable to revert backup for volume [%s] due to [%s].", bkpPathAndVolUuid.second(), e.getMessage()), e);
+                }
+            }
+        } finally {
+            unmountBackupDirectory(mountDirectory);
+            deleteTemporaryDirectory(mountDirectory);
+        }
+    }
+
+    private void restoreVolume(String backupPath, String backupRepoType, String backupRepoAddress, String volumePath,
+                               String diskType, Long deviceId, String volumeUUID) {
+        String mountDirectory = mountBackupDirectory(backupRepoAddress, backupRepoType);
+        Pair<String, String> bkpPathAndVolUuid;
+        try {
+            bkpPathAndVolUuid = getBackupPath(mountDirectory, volumePath, backupPath, diskType, deviceId.intValue(), volumeUUID);
+            try {
+                replaceVolumeWithBackup(volumePath, bkpPathAndVolUuid.first());
+            } catch (IOException e) {
+                throw new CloudRuntimeException(String.format("Unable to revert backup for volume [%s] due to [%s].", bkpPathAndVolUuid.second(), e.getMessage()), e);
+            }
+        } catch (Exception e) {
+            throw new CloudRuntimeException("Failed to restore volume", e);
+        } finally {
+            unmountBackupDirectory(mountDirectory);
+            deleteTemporaryDirectory(mountDirectory);
+        }
+    }
+
+
+    private String mountBackupDirectory(String backupRepoAddress, String backupRepoType) {
+        String randomChars = RandomStringUtils.random(5, true, false);
+        String mountDirectory = String.format("%s.%s",BACKUP_TEMP_FILE_PREFIx , randomChars);
+        try {
+            mountDirectory = Files.createTempDirectory(mountDirectory).toString();
+            String mount = String.format(MOUNT_COMMAND, backupRepoType, backupRepoAddress, mountDirectory);
+            Script.runSimpleBashScript(mount);

Review Comment:
   It may be better to use `Script.executeCommand*`



##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java:
##########
@@ -0,0 +1,181 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package com.cloud.hypervisor.kvm.resource.wrapper;
+
+import com.cloud.agent.api.Answer;
+import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
+import com.cloud.resource.CommandWrapper;
+import com.cloud.resource.ResourceWrapper;
+import com.cloud.utils.Pair;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.utils.script.Script;
+import org.apache.cloudstack.backup.BackupAnswer;
+import org.apache.cloudstack.backup.RestoreBackupCommand;
+import org.apache.commons.lang3.RandomStringUtils;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.nio.file.StandardCopyOption;
+import java.util.List;
+import java.util.Locale;
+import java.util.Objects;
+
+@ResourceWrapper(handles = RestoreBackupCommand.class)
+public class LibvirtRestoreBackupCommandWrapper extends CommandWrapper<RestoreBackupCommand, Answer, LibvirtComputingResource> {
+    private static final String BACKUP_TEMP_FILE_PREFIx = "csbackup";
+    private static final String MOUNT_COMMAND = "sudo mount -t %s %s %s";
+    private static final String UMOUNT_COMMAND = "sudo umount %s";
+    private static final String FILE_PATH_PLACEHOLDER = "%s/%s";
+
+    @Override
+    public Answer execute(RestoreBackupCommand command, LibvirtComputingResource serverResource) {
+        String vmName = command.getVmName();
+        String backupPath = command.getBackupPath();
+        String backupRepoAddress = command.getBackupRepoAddress();
+        String backupRepoType = command.getBackupRepoType();
+        String mountOptions = command.getMountOptions();
+        Boolean vmExists = command.isVmExists();
+        String diskType = command.getDiskType();
+        Long deviceId = command.getDeviceId();
+        List<String> volumePaths = command.getVolumePaths();
+        String restoreVolumeUuid = command.getRestoreVolumeUUID();
+
+        String newVolumeId = null;
+        if (Objects.isNull(vmExists)) {
+            String volumePath = volumePaths.get(0);
+            int lastIndex = volumePath.lastIndexOf("/");
+            newVolumeId = volumePath.substring(lastIndex + 1);
+            restoreVolume(backupPath, backupRepoType, backupRepoAddress, volumePath, diskType, deviceId, restoreVolumeUuid);
+        } else if (vmExists) {
+            restoreVolumesOfExistingVM(volumePaths, backupPath, backupRepoType, backupRepoAddress, mountOptions);
+        } else {
+            restoreVolumesOfDestroyedVMs(volumePaths, vmName, backupPath, backupRepoType, backupRepoAddress, mountOptions);
+        }
+
+        return new BackupAnswer(command, true, newVolumeId);
+    }
+
+    private void restoreVolumesOfExistingVM(List<String> volumePaths, String backupPath,
+                                             String backupRepoType, String backupRepoAddress, String mountOptions) {
+        String diskType = "root";
+        String mountDirectory = mountBackupDirectory(backupRepoAddress, backupRepoType);
+        try {
+            for (int idx = 0; idx < volumePaths.size(); idx++) {
+                String volumePath = volumePaths.get(idx);
+                Pair<String, String> bkpPathAndVolUuid = getBackupPath(mountDirectory, volumePath, backupPath, diskType, idx, null);
+                diskType = "datadisk";
+                try {
+                    replaceVolumeWithBackup(volumePath, bkpPathAndVolUuid.first());
+                } catch (IOException e) {
+                    throw new CloudRuntimeException(String.format("Unable to revert backup for volume [%s] due to [%s].", bkpPathAndVolUuid.second(), e.getMessage()), e);
+                }
+            }
+        } finally {
+            unmountBackupDirectory(mountDirectory);
+            deleteTemporaryDirectory(mountDirectory);
+        }
+
+    }
+
+    private void restoreVolumesOfDestroyedVMs(List<String> volumePaths, String vmName, String backupPath,
+                                              String backupRepoType, String backupRepoAddress, String mountOptions) {
+        String mountDirectory = mountBackupDirectory(backupRepoAddress, backupRepoType);
+        String diskType = "root";
+        try {
+            for (int i = 0; i < volumePaths.size(); i++) {
+                String volumePath = volumePaths.get(i);
+                Pair<String, String> bkpPathAndVolUuid = getBackupPath(mountDirectory, volumePath, backupPath, diskType, i, null);
+                diskType = "datadisk";
+                try {
+                    replaceVolumeWithBackup(volumePath, bkpPathAndVolUuid.first());
+                } catch (IOException e) {
+                    throw new CloudRuntimeException(String.format("Unable to revert backup for volume [%s] due to [%s].", bkpPathAndVolUuid.second(), e.getMessage()), e);
+                }
+            }
+        } finally {
+            unmountBackupDirectory(mountDirectory);
+            deleteTemporaryDirectory(mountDirectory);
+        }
+    }
+
+    private void restoreVolume(String backupPath, String backupRepoType, String backupRepoAddress, String volumePath,
+                               String diskType, Long deviceId, String volumeUUID) {
+        String mountDirectory = mountBackupDirectory(backupRepoAddress, backupRepoType);
+        Pair<String, String> bkpPathAndVolUuid;
+        try {
+            bkpPathAndVolUuid = getBackupPath(mountDirectory, volumePath, backupPath, diskType, deviceId.intValue(), volumeUUID);
+            try {
+                replaceVolumeWithBackup(volumePath, bkpPathAndVolUuid.first());
+            } catch (IOException e) {
+                throw new CloudRuntimeException(String.format("Unable to revert backup for volume [%s] due to [%s].", bkpPathAndVolUuid.second(), e.getMessage()), e);
+            }
+        } catch (Exception e) {
+            throw new CloudRuntimeException("Failed to restore volume", e);
+        } finally {
+            unmountBackupDirectory(mountDirectory);
+            deleteTemporaryDirectory(mountDirectory);
+        }
+    }
+
+
+    private String mountBackupDirectory(String backupRepoAddress, String backupRepoType) {
+        String randomChars = RandomStringUtils.random(5, true, false);
+        String mountDirectory = String.format("%s.%s",BACKUP_TEMP_FILE_PREFIx , randomChars);
+        try {
+            mountDirectory = Files.createTempDirectory(mountDirectory).toString();
+            String mount = String.format(MOUNT_COMMAND, backupRepoType, backupRepoAddress, mountDirectory);
+            Script.runSimpleBashScript(mount);
+        } catch (Exception e) {
+            throw new CloudRuntimeException(String.format("Failed to mount %s to %s", backupRepoType, backupRepoAddress), e);
+        }
+        return mountDirectory;
+    }
+
+    private void unmountBackupDirectory(String backupDirectory) {
+        try {
+            String umountCmd = String.format(UMOUNT_COMMAND, backupDirectory);
+            Script.runSimpleBashScript(umountCmd);

Review Comment:
   same as above



##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtTakeBackupCommandWrapper.java:
##########
@@ -0,0 +1,83 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package com.cloud.hypervisor.kvm.resource.wrapper;
+
+import com.cloud.agent.api.Answer;
+import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
+import com.cloud.resource.CommandWrapper;
+import com.cloud.resource.ResourceWrapper;
+import com.cloud.utils.Pair;
+import com.cloud.utils.script.Script;
+import org.apache.cloudstack.backup.BackupAnswer;
+import org.apache.cloudstack.backup.TakeBackupCommand;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+
+@ResourceWrapper(handles = TakeBackupCommand.class)
+public class LibvirtTakeBackupCommandWrapper extends CommandWrapper<TakeBackupCommand, Answer, LibvirtComputingResource> {
+    @Override
+    public Answer execute(TakeBackupCommand command, LibvirtComputingResource libvirtComputingResource) {
+        final String vmName = command.getVmName();
+        final String backupPath = command.getBackupPath();
+        final String backupRepoType = command.getBackupRepoType();
+        final String backupRepoAddress = command.getBackupRepoAddress();
+        final String mountOptions = command.getMountOptions();
+        final List<String> diskPaths = command.getVolumePaths();
+
+        List<String[]> commands = new ArrayList<>();
+        commands.add(new String[]{
+                libvirtComputingResource.getNasBackupPath(),
+                "-o", "backup",
+                "-v", vmName,
+                "-t", backupRepoType,
+                "-s", backupRepoAddress,
+                "-m", Objects.nonNull(mountOptions) ? mountOptions : "",
+                "-p", backupPath,
+                "-d", (Objects.nonNull(diskPaths) && !diskPaths.isEmpty()) ? String.join(",", diskPaths) : ""
+        });
+
+        Pair<Integer, String> result = Script.executePipedCommands(commands, libvirtComputingResource.getCmdsTimeout());
+
+        if (result.first() != 0) {
+            logger.debug("Failed to take VM backup: " + result.second());
+            return new BackupAnswer(command, false, result.second().trim());
+        }
+
+        long backupSize = 0L;
+        if (Objects.isNull(diskPaths) || diskPaths.isEmpty()) {

Review Comment:
   use CollectionUtils?



##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java:
##########
@@ -0,0 +1,181 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package com.cloud.hypervisor.kvm.resource.wrapper;
+
+import com.cloud.agent.api.Answer;
+import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
+import com.cloud.resource.CommandWrapper;
+import com.cloud.resource.ResourceWrapper;
+import com.cloud.utils.Pair;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.utils.script.Script;
+import org.apache.cloudstack.backup.BackupAnswer;
+import org.apache.cloudstack.backup.RestoreBackupCommand;
+import org.apache.commons.lang3.RandomStringUtils;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.nio.file.StandardCopyOption;
+import java.util.List;
+import java.util.Locale;
+import java.util.Objects;
+
+@ResourceWrapper(handles = RestoreBackupCommand.class)
+public class LibvirtRestoreBackupCommandWrapper extends CommandWrapper<RestoreBackupCommand, Answer, LibvirtComputingResource> {
+    private static final String BACKUP_TEMP_FILE_PREFIx = "csbackup";
+    private static final String MOUNT_COMMAND = "sudo mount -t %s %s %s";
+    private static final String UMOUNT_COMMAND = "sudo umount %s";
+    private static final String FILE_PATH_PLACEHOLDER = "%s/%s";
+
+    @Override
+    public Answer execute(RestoreBackupCommand command, LibvirtComputingResource serverResource) {
+        String vmName = command.getVmName();
+        String backupPath = command.getBackupPath();
+        String backupRepoAddress = command.getBackupRepoAddress();
+        String backupRepoType = command.getBackupRepoType();
+        String mountOptions = command.getMountOptions();
+        Boolean vmExists = command.isVmExists();
+        String diskType = command.getDiskType();
+        Long deviceId = command.getDeviceId();
+        List<String> volumePaths = command.getVolumePaths();
+        String restoreVolumeUuid = command.getRestoreVolumeUUID();
+
+        String newVolumeId = null;
+        if (Objects.isNull(vmExists)) {
+            String volumePath = volumePaths.get(0);
+            int lastIndex = volumePath.lastIndexOf("/");
+            newVolumeId = volumePath.substring(lastIndex + 1);
+            restoreVolume(backupPath, backupRepoType, backupRepoAddress, volumePath, diskType, deviceId, restoreVolumeUuid);
+        } else if (vmExists) {
+            restoreVolumesOfExistingVM(volumePaths, backupPath, backupRepoType, backupRepoAddress, mountOptions);
+        } else {
+            restoreVolumesOfDestroyedVMs(volumePaths, vmName, backupPath, backupRepoType, backupRepoAddress, mountOptions);
+        }
+
+        return new BackupAnswer(command, true, newVolumeId);
+    }
+
+    private void restoreVolumesOfExistingVM(List<String> volumePaths, String backupPath,
+                                             String backupRepoType, String backupRepoAddress, String mountOptions) {
+        String diskType = "root";
+        String mountDirectory = mountBackupDirectory(backupRepoAddress, backupRepoType);
+        try {
+            for (int idx = 0; idx < volumePaths.size(); idx++) {
+                String volumePath = volumePaths.get(idx);
+                Pair<String, String> bkpPathAndVolUuid = getBackupPath(mountDirectory, volumePath, backupPath, diskType, idx, null);
+                diskType = "datadisk";
+                try {
+                    replaceVolumeWithBackup(volumePath, bkpPathAndVolUuid.first());
+                } catch (IOException e) {
+                    throw new CloudRuntimeException(String.format("Unable to revert backup for volume [%s] due to [%s].", bkpPathAndVolUuid.second(), e.getMessage()), e);
+                }
+            }
+        } finally {
+            unmountBackupDirectory(mountDirectory);
+            deleteTemporaryDirectory(mountDirectory);
+        }
+
+    }
+
+    private void restoreVolumesOfDestroyedVMs(List<String> volumePaths, String vmName, String backupPath,
+                                              String backupRepoType, String backupRepoAddress, String mountOptions) {
+        String mountDirectory = mountBackupDirectory(backupRepoAddress, backupRepoType);
+        String diskType = "root";
+        try {
+            for (int i = 0; i < volumePaths.size(); i++) {
+                String volumePath = volumePaths.get(i);
+                Pair<String, String> bkpPathAndVolUuid = getBackupPath(mountDirectory, volumePath, backupPath, diskType, i, null);
+                diskType = "datadisk";
+                try {
+                    replaceVolumeWithBackup(volumePath, bkpPathAndVolUuid.first());
+                } catch (IOException e) {
+                    throw new CloudRuntimeException(String.format("Unable to revert backup for volume [%s] due to [%s].", bkpPathAndVolUuid.second(), e.getMessage()), e);
+                }
+            }
+        } finally {
+            unmountBackupDirectory(mountDirectory);
+            deleteTemporaryDirectory(mountDirectory);
+        }
+    }
+
+    private void restoreVolume(String backupPath, String backupRepoType, String backupRepoAddress, String volumePath,
+                               String diskType, Long deviceId, String volumeUUID) {
+        String mountDirectory = mountBackupDirectory(backupRepoAddress, backupRepoType);
+        Pair<String, String> bkpPathAndVolUuid;
+        try {
+            bkpPathAndVolUuid = getBackupPath(mountDirectory, volumePath, backupPath, diskType, deviceId.intValue(), volumeUUID);
+            try {
+                replaceVolumeWithBackup(volumePath, bkpPathAndVolUuid.first());
+            } catch (IOException e) {
+                throw new CloudRuntimeException(String.format("Unable to revert backup for volume [%s] due to [%s].", bkpPathAndVolUuid.second(), e.getMessage()), e);
+            }
+        } catch (Exception e) {
+            throw new CloudRuntimeException("Failed to restore volume", e);
+        } finally {
+            unmountBackupDirectory(mountDirectory);
+            deleteTemporaryDirectory(mountDirectory);
+        }
+    }
+
+
+    private String mountBackupDirectory(String backupRepoAddress, String backupRepoType) {
+        String randomChars = RandomStringUtils.random(5, true, false);
+        String mountDirectory = String.format("%s.%s",BACKUP_TEMP_FILE_PREFIx , randomChars);
+        try {
+            mountDirectory = Files.createTempDirectory(mountDirectory).toString();
+            String mount = String.format(MOUNT_COMMAND, backupRepoType, backupRepoAddress, mountDirectory);
+            Script.runSimpleBashScript(mount);
+        } catch (Exception e) {
+            throw new CloudRuntimeException(String.format("Failed to mount %s to %s", backupRepoType, backupRepoAddress), e);
+        }
+        return mountDirectory;
+    }
+
+    private void unmountBackupDirectory(String backupDirectory) {
+        try {
+            String umountCmd = String.format(UMOUNT_COMMAND, backupDirectory);
+            Script.runSimpleBashScript(umountCmd);
+        } catch (Exception e) {
+            throw new CloudRuntimeException(String.format("Failed to unmount backup directory: %s", backupDirectory), e);
+        }
+    }
+
+    private void deleteTemporaryDirectory(String backupDirectory) {
+        try {
+            Files.deleteIfExists(Paths.get(backupDirectory));
+        } catch (IOException e) {
+            throw new CloudRuntimeException(String.format("Failed to delete backup directory: %s", backupDirectory), e);
+        }
+    }
+
+    private Pair<String, String> getBackupPath(String mountDirectory, String volumePath, String backupPath, String diskType, int deviceId, String volumeUuid) {
+        String bkpPath = String.format(FILE_PATH_PLACEHOLDER, mountDirectory, backupPath);
+        int lastIndex = volumePath.lastIndexOf("/");

Review Comment:
   maybe?
   ```suggestion
           int lastIndex = volumePath.lastIndexOf(File.separator);
   ```



##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java:
##########
@@ -0,0 +1,181 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package com.cloud.hypervisor.kvm.resource.wrapper;
+
+import com.cloud.agent.api.Answer;
+import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
+import com.cloud.resource.CommandWrapper;
+import com.cloud.resource.ResourceWrapper;
+import com.cloud.utils.Pair;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.utils.script.Script;
+import org.apache.cloudstack.backup.BackupAnswer;
+import org.apache.cloudstack.backup.RestoreBackupCommand;
+import org.apache.commons.lang3.RandomStringUtils;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.nio.file.StandardCopyOption;
+import java.util.List;
+import java.util.Locale;
+import java.util.Objects;
+
+@ResourceWrapper(handles = RestoreBackupCommand.class)
+public class LibvirtRestoreBackupCommandWrapper extends CommandWrapper<RestoreBackupCommand, Answer, LibvirtComputingResource> {
+    private static final String BACKUP_TEMP_FILE_PREFIx = "csbackup";
+    private static final String MOUNT_COMMAND = "sudo mount -t %s %s %s";
+    private static final String UMOUNT_COMMAND = "sudo umount %s";
+    private static final String FILE_PATH_PLACEHOLDER = "%s/%s";
+
+    @Override
+    public Answer execute(RestoreBackupCommand command, LibvirtComputingResource serverResource) {
+        String vmName = command.getVmName();
+        String backupPath = command.getBackupPath();
+        String backupRepoAddress = command.getBackupRepoAddress();
+        String backupRepoType = command.getBackupRepoType();
+        String mountOptions = command.getMountOptions();
+        Boolean vmExists = command.isVmExists();
+        String diskType = command.getDiskType();
+        Long deviceId = command.getDeviceId();
+        List<String> volumePaths = command.getVolumePaths();
+        String restoreVolumeUuid = command.getRestoreVolumeUUID();
+
+        String newVolumeId = null;
+        if (Objects.isNull(vmExists)) {
+            String volumePath = volumePaths.get(0);
+            int lastIndex = volumePath.lastIndexOf("/");
+            newVolumeId = volumePath.substring(lastIndex + 1);
+            restoreVolume(backupPath, backupRepoType, backupRepoAddress, volumePath, diskType, deviceId, restoreVolumeUuid);
+        } else if (vmExists) {
+            restoreVolumesOfExistingVM(volumePaths, backupPath, backupRepoType, backupRepoAddress, mountOptions);
+        } else {
+            restoreVolumesOfDestroyedVMs(volumePaths, vmName, backupPath, backupRepoType, backupRepoAddress, mountOptions);
+        }
+
+        return new BackupAnswer(command, true, newVolumeId);
+    }
+
+    private void restoreVolumesOfExistingVM(List<String> volumePaths, String backupPath,
+                                             String backupRepoType, String backupRepoAddress, String mountOptions) {
+        String diskType = "root";
+        String mountDirectory = mountBackupDirectory(backupRepoAddress, backupRepoType);
+        try {
+            for (int idx = 0; idx < volumePaths.size(); idx++) {
+                String volumePath = volumePaths.get(idx);
+                Pair<String, String> bkpPathAndVolUuid = getBackupPath(mountDirectory, volumePath, backupPath, diskType, idx, null);
+                diskType = "datadisk";
+                try {
+                    replaceVolumeWithBackup(volumePath, bkpPathAndVolUuid.first());
+                } catch (IOException e) {
+                    throw new CloudRuntimeException(String.format("Unable to revert backup for volume [%s] due to [%s].", bkpPathAndVolUuid.second(), e.getMessage()), e);
+                }
+            }
+        } finally {
+            unmountBackupDirectory(mountDirectory);
+            deleteTemporaryDirectory(mountDirectory);
+        }
+
+    }
+
+    private void restoreVolumesOfDestroyedVMs(List<String> volumePaths, String vmName, String backupPath,
+                                              String backupRepoType, String backupRepoAddress, String mountOptions) {
+        String mountDirectory = mountBackupDirectory(backupRepoAddress, backupRepoType);
+        String diskType = "root";
+        try {
+            for (int i = 0; i < volumePaths.size(); i++) {
+                String volumePath = volumePaths.get(i);
+                Pair<String, String> bkpPathAndVolUuid = getBackupPath(mountDirectory, volumePath, backupPath, diskType, i, null);
+                diskType = "datadisk";

Review Comment:
   Can these be taken from some constants?



##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java:
##########
@@ -0,0 +1,181 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package com.cloud.hypervisor.kvm.resource.wrapper;
+
+import com.cloud.agent.api.Answer;
+import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
+import com.cloud.resource.CommandWrapper;
+import com.cloud.resource.ResourceWrapper;
+import com.cloud.utils.Pair;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.utils.script.Script;
+import org.apache.cloudstack.backup.BackupAnswer;
+import org.apache.cloudstack.backup.RestoreBackupCommand;
+import org.apache.commons.lang3.RandomStringUtils;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.nio.file.StandardCopyOption;
+import java.util.List;
+import java.util.Locale;
+import java.util.Objects;
+
+@ResourceWrapper(handles = RestoreBackupCommand.class)
+public class LibvirtRestoreBackupCommandWrapper extends CommandWrapper<RestoreBackupCommand, Answer, LibvirtComputingResource> {
+    private static final String BACKUP_TEMP_FILE_PREFIx = "csbackup";
+    private static final String MOUNT_COMMAND = "sudo mount -t %s %s %s";
+    private static final String UMOUNT_COMMAND = "sudo umount %s";
+    private static final String FILE_PATH_PLACEHOLDER = "%s/%s";
+
+    @Override
+    public Answer execute(RestoreBackupCommand command, LibvirtComputingResource serverResource) {
+        String vmName = command.getVmName();
+        String backupPath = command.getBackupPath();
+        String backupRepoAddress = command.getBackupRepoAddress();
+        String backupRepoType = command.getBackupRepoType();
+        String mountOptions = command.getMountOptions();
+        Boolean vmExists = command.isVmExists();
+        String diskType = command.getDiskType();
+        Long deviceId = command.getDeviceId();
+        List<String> volumePaths = command.getVolumePaths();
+        String restoreVolumeUuid = command.getRestoreVolumeUUID();
+
+        String newVolumeId = null;
+        if (Objects.isNull(vmExists)) {
+            String volumePath = volumePaths.get(0);
+            int lastIndex = volumePath.lastIndexOf("/");
+            newVolumeId = volumePath.substring(lastIndex + 1);
+            restoreVolume(backupPath, backupRepoType, backupRepoAddress, volumePath, diskType, deviceId, restoreVolumeUuid);
+        } else if (vmExists) {
+            restoreVolumesOfExistingVM(volumePaths, backupPath, backupRepoType, backupRepoAddress, mountOptions);
+        } else {
+            restoreVolumesOfDestroyedVMs(volumePaths, vmName, backupPath, backupRepoType, backupRepoAddress, mountOptions);
+        }
+
+        return new BackupAnswer(command, true, newVolumeId);
+    }
+
+    private void restoreVolumesOfExistingVM(List<String> volumePaths, String backupPath,
+                                             String backupRepoType, String backupRepoAddress, String mountOptions) {
+        String diskType = "root";
+        String mountDirectory = mountBackupDirectory(backupRepoAddress, backupRepoType);
+        try {
+            for (int idx = 0; idx < volumePaths.size(); idx++) {
+                String volumePath = volumePaths.get(idx);
+                Pair<String, String> bkpPathAndVolUuid = getBackupPath(mountDirectory, volumePath, backupPath, diskType, idx, null);
+                diskType = "datadisk";
+                try {
+                    replaceVolumeWithBackup(volumePath, bkpPathAndVolUuid.first());
+                } catch (IOException e) {
+                    throw new CloudRuntimeException(String.format("Unable to revert backup for volume [%s] due to [%s].", bkpPathAndVolUuid.second(), e.getMessage()), e);
+                }
+            }
+        } finally {
+            unmountBackupDirectory(mountDirectory);
+            deleteTemporaryDirectory(mountDirectory);
+        }
+
+    }
+
+    private void restoreVolumesOfDestroyedVMs(List<String> volumePaths, String vmName, String backupPath,
+                                              String backupRepoType, String backupRepoAddress, String mountOptions) {
+        String mountDirectory = mountBackupDirectory(backupRepoAddress, backupRepoType);
+        String diskType = "root";
+        try {
+            for (int i = 0; i < volumePaths.size(); i++) {
+                String volumePath = volumePaths.get(i);
+                Pair<String, String> bkpPathAndVolUuid = getBackupPath(mountDirectory, volumePath, backupPath, diskType, i, null);
+                diskType = "datadisk";
+                try {
+                    replaceVolumeWithBackup(volumePath, bkpPathAndVolUuid.first());
+                } catch (IOException e) {
+                    throw new CloudRuntimeException(String.format("Unable to revert backup for volume [%s] due to [%s].", bkpPathAndVolUuid.second(), e.getMessage()), e);
+                }
+            }
+        } finally {
+            unmountBackupDirectory(mountDirectory);
+            deleteTemporaryDirectory(mountDirectory);
+        }
+    }
+
+    private void restoreVolume(String backupPath, String backupRepoType, String backupRepoAddress, String volumePath,
+                               String diskType, Long deviceId, String volumeUUID) {
+        String mountDirectory = mountBackupDirectory(backupRepoAddress, backupRepoType);
+        Pair<String, String> bkpPathAndVolUuid;
+        try {
+            bkpPathAndVolUuid = getBackupPath(mountDirectory, volumePath, backupPath, diskType, deviceId.intValue(), volumeUUID);
+            try {
+                replaceVolumeWithBackup(volumePath, bkpPathAndVolUuid.first());
+            } catch (IOException e) {
+                throw new CloudRuntimeException(String.format("Unable to revert backup for volume [%s] due to [%s].", bkpPathAndVolUuid.second(), e.getMessage()), e);
+            }
+        } catch (Exception e) {
+            throw new CloudRuntimeException("Failed to restore volume", e);
+        } finally {
+            unmountBackupDirectory(mountDirectory);
+            deleteTemporaryDirectory(mountDirectory);
+        }
+    }
+
+
+    private String mountBackupDirectory(String backupRepoAddress, String backupRepoType) {
+        String randomChars = RandomStringUtils.random(5, true, false);
+        String mountDirectory = String.format("%s.%s",BACKUP_TEMP_FILE_PREFIx , randomChars);
+        try {
+            mountDirectory = Files.createTempDirectory(mountDirectory).toString();
+            String mount = String.format(MOUNT_COMMAND, backupRepoType, backupRepoAddress, mountDirectory);
+            Script.runSimpleBashScript(mount);
+        } catch (Exception e) {
+            throw new CloudRuntimeException(String.format("Failed to mount %s to %s", backupRepoType, backupRepoAddress), e);
+        }
+        return mountDirectory;
+    }
+
+    private void unmountBackupDirectory(String backupDirectory) {
+        try {
+            String umountCmd = String.format(UMOUNT_COMMAND, backupDirectory);
+            Script.runSimpleBashScript(umountCmd);
+        } catch (Exception e) {
+            throw new CloudRuntimeException(String.format("Failed to unmount backup directory: %s", backupDirectory), e);
+        }
+    }
+
+    private void deleteTemporaryDirectory(String backupDirectory) {
+        try {
+            Files.deleteIfExists(Paths.get(backupDirectory));
+        } catch (IOException e) {
+            throw new CloudRuntimeException(String.format("Failed to delete backup directory: %s", backupDirectory), e);
+        }
+    }
+
+    private Pair<String, String> getBackupPath(String mountDirectory, String volumePath, String backupPath, String diskType, int deviceId, String volumeUuid) {
+        String bkpPath = String.format(FILE_PATH_PLACEHOLDER, mountDirectory, backupPath);
+        int lastIndex = volumePath.lastIndexOf("/");
+        String volUuid = Objects.isNull(volumeUuid) ? volumePath.substring(lastIndex + 1) : volumeUuid;
+        String backupFileName = String.format("%s.%s.%s.qcow2", deviceId, diskType.toLowerCase(Locale.ROOT), volUuid);

Review Comment:
   Will it always be QCOW2 even if the volume format is RAW?



-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM (experimental) [cloudstack]

["rohityadavcloud (via GitHub)" <gi...@apache.org>]

rohityadavcloud commented on code in PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#discussion_r1738378610


##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java:
##########
@@ -0,0 +1,181 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package com.cloud.hypervisor.kvm.resource.wrapper;
+
+import com.cloud.agent.api.Answer;
+import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
+import com.cloud.resource.CommandWrapper;
+import com.cloud.resource.ResourceWrapper;
+import com.cloud.utils.Pair;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.utils.script.Script;
+import org.apache.cloudstack.backup.BackupAnswer;
+import org.apache.cloudstack.backup.RestoreBackupCommand;
+import org.apache.commons.lang3.RandomStringUtils;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.nio.file.StandardCopyOption;
+import java.util.List;
+import java.util.Locale;
+import java.util.Objects;
+
+@ResourceWrapper(handles = RestoreBackupCommand.class)
+public class LibvirtRestoreBackupCommandWrapper extends CommandWrapper<RestoreBackupCommand, Answer, LibvirtComputingResource> {
+    private static final String BACKUP_TEMP_FILE_PREFIx = "csbackup";
+    private static final String MOUNT_COMMAND = "sudo mount -t %s %s %s";
+    private static final String UMOUNT_COMMAND = "sudo umount %s";
+    private static final String FILE_PATH_PLACEHOLDER = "%s/%s";
+
+    @Override
+    public Answer execute(RestoreBackupCommand command, LibvirtComputingResource serverResource) {
+        String vmName = command.getVmName();
+        String backupPath = command.getBackupPath();
+        String backupRepoAddress = command.getBackupRepoAddress();
+        String backupRepoType = command.getBackupRepoType();
+        String mountOptions = command.getMountOptions();
+        Boolean vmExists = command.isVmExists();
+        String diskType = command.getDiskType();
+        Long deviceId = command.getDeviceId();
+        List<String> volumePaths = command.getVolumePaths();
+        String restoreVolumeUuid = command.getRestoreVolumeUUID();
+
+        String newVolumeId = null;
+        if (Objects.isNull(vmExists)) {
+            String volumePath = volumePaths.get(0);
+            int lastIndex = volumePath.lastIndexOf("/");
+            newVolumeId = volumePath.substring(lastIndex + 1);
+            restoreVolume(backupPath, backupRepoType, backupRepoAddress, volumePath, diskType, deviceId, restoreVolumeUuid);
+        } else if (vmExists) {
+            restoreVolumesOfExistingVM(volumePaths, backupPath, backupRepoType, backupRepoAddress, mountOptions);
+        } else {
+            restoreVolumesOfDestroyedVMs(volumePaths, vmName, backupPath, backupRepoType, backupRepoAddress, mountOptions);
+        }
+
+        return new BackupAnswer(command, true, newVolumeId);
+    }
+
+    private void restoreVolumesOfExistingVM(List<String> volumePaths, String backupPath,
+                                             String backupRepoType, String backupRepoAddress, String mountOptions) {
+        String diskType = "root";
+        String mountDirectory = mountBackupDirectory(backupRepoAddress, backupRepoType);
+        try {
+            for (int idx = 0; idx < volumePaths.size(); idx++) {
+                String volumePath = volumePaths.get(idx);
+                Pair<String, String> bkpPathAndVolUuid = getBackupPath(mountDirectory, volumePath, backupPath, diskType, idx, null);
+                diskType = "datadisk";
+                try {
+                    replaceVolumeWithBackup(volumePath, bkpPathAndVolUuid.first());
+                } catch (IOException e) {
+                    throw new CloudRuntimeException(String.format("Unable to revert backup for volume [%s] due to [%s].", bkpPathAndVolUuid.second(), e.getMessage()), e);
+                }
+            }
+        } finally {
+            unmountBackupDirectory(mountDirectory);
+            deleteTemporaryDirectory(mountDirectory);
+        }
+
+    }
+
+    private void restoreVolumesOfDestroyedVMs(List<String> volumePaths, String vmName, String backupPath,
+                                              String backupRepoType, String backupRepoAddress, String mountOptions) {
+        String mountDirectory = mountBackupDirectory(backupRepoAddress, backupRepoType);
+        String diskType = "root";
+        try {
+            for (int i = 0; i < volumePaths.size(); i++) {
+                String volumePath = volumePaths.get(i);
+                Pair<String, String> bkpPathAndVolUuid = getBackupPath(mountDirectory, volumePath, backupPath, diskType, i, null);
+                diskType = "datadisk";
+                try {
+                    replaceVolumeWithBackup(volumePath, bkpPathAndVolUuid.first());
+                } catch (IOException e) {
+                    throw new CloudRuntimeException(String.format("Unable to revert backup for volume [%s] due to [%s].", bkpPathAndVolUuid.second(), e.getMessage()), e);
+                }
+            }
+        } finally {
+            unmountBackupDirectory(mountDirectory);
+            deleteTemporaryDirectory(mountDirectory);
+        }
+    }
+
+    private void restoreVolume(String backupPath, String backupRepoType, String backupRepoAddress, String volumePath,
+                               String diskType, Long deviceId, String volumeUUID) {
+        String mountDirectory = mountBackupDirectory(backupRepoAddress, backupRepoType);
+        Pair<String, String> bkpPathAndVolUuid;
+        try {
+            bkpPathAndVolUuid = getBackupPath(mountDirectory, volumePath, backupPath, diskType, deviceId.intValue(), volumeUUID);
+            try {
+                replaceVolumeWithBackup(volumePath, bkpPathAndVolUuid.first());
+            } catch (IOException e) {
+                throw new CloudRuntimeException(String.format("Unable to revert backup for volume [%s] due to [%s].", bkpPathAndVolUuid.second(), e.getMessage()), e);
+            }
+        } catch (Exception e) {
+            throw new CloudRuntimeException("Failed to restore volume", e);
+        } finally {
+            unmountBackupDirectory(mountDirectory);
+            deleteTemporaryDirectory(mountDirectory);
+        }
+    }
+
+
+    private String mountBackupDirectory(String backupRepoAddress, String backupRepoType) {
+        String randomChars = RandomStringUtils.random(5, true, false);
+        String mountDirectory = String.format("%s.%s",BACKUP_TEMP_FILE_PREFIx , randomChars);
+        try {
+            mountDirectory = Files.createTempDirectory(mountDirectory).toString();
+            String mount = String.format(MOUNT_COMMAND, backupRepoType, backupRepoAddress, mountDirectory);
+            Script.runSimpleBashScript(mount);
+        } catch (Exception e) {
+            throw new CloudRuntimeException(String.format("Failed to mount %s to %s", backupRepoType, backupRepoAddress), e);
+        }
+        return mountDirectory;
+    }
+
+    private void unmountBackupDirectory(String backupDirectory) {
+        try {
+            String umountCmd = String.format(UMOUNT_COMMAND, backupDirectory);
+            Script.runSimpleBashScript(umountCmd);
+        } catch (Exception e) {
+            throw new CloudRuntimeException(String.format("Failed to unmount backup directory: %s", backupDirectory), e);
+        }
+    }
+
+    private void deleteTemporaryDirectory(String backupDirectory) {
+        try {
+            Files.deleteIfExists(Paths.get(backupDirectory));
+        } catch (IOException e) {
+            throw new CloudRuntimeException(String.format("Failed to delete backup directory: %s", backupDirectory), e);
+        }
+    }
+
+    private Pair<String, String> getBackupPath(String mountDirectory, String volumePath, String backupPath, String diskType, int deviceId, String volumeUuid) {
+        String bkpPath = String.format(FILE_PATH_PLACEHOLDER, mountDirectory, backupPath);
+        int lastIndex = volumePath.lastIndexOf("/");
+        String volUuid = Objects.isNull(volumeUuid) ? volumePath.substring(lastIndex + 1) : volumeUuid;
+        String backupFileName = String.format("%s.%s.%s.qcow2", deviceId, diskType.toLowerCase(Locale.ROOT), volUuid);

Review Comment:
   That's a limitation of the B&R plugin, it will be always qcow2 - see docs at https://github.com/apache/cloudstack-documentation/pull/429 - this can change in future.



-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM (experimental) [cloudstack]

["shwstppr (via GitHub)" <gi...@apache.org>]

shwstppr commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2320065668

   @blueorangutan package


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] [WIP] backup: Simple NAS backup plugin for KVM (experimental) [cloudstack]

["blueorangutan (via GitHub)" <gi...@apache.org>]

blueorangutan commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2272307064

   Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 10583


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] [WIP] backup: Simple NAS backup plugin for KVM (experimental) [cloudstack]

["rohityadavcloud (via GitHub)" <gi...@apache.org>]

rohityadavcloud commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2262953524

   Update: take backup & delete backup implemented for NFS; TBD/TODO more operations.


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM (experimental) [cloudstack]

["DaanHoogland (via GitHub)" <gi...@apache.org>]

DaanHoogland commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2320968034

   > Thanks @shwstppr @DaanHoogland for the review, let me try & address as much as I could.
   > 
   > @DaanHoogland to answer your comments;
   > 
   >     1. yes, this is mostly for KVM; however, we think this may be in future expanded to other hypervisors; as most hypervisors support NFS (at least) among the supported NAS types; the method to take/restore backup could use full VM snapshots of non-KVM hypervisors.
   
   ok, great.
   
   >     2. yes, backup repository concept for now is specific to only this plugin - however, if the scope expands to a general purpose B&R plugin, then this becomes non-specific to the plugin. Also I think the networkers & veeam plugins have some concept of backup repository or backup storage which can reuse this concept. That said, I'm happy to move the dao/vo logic to the plugin if you really insist (and refactor in future).
   
   If it is that's fine. It was just not that obvious to me.


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM (experimental) [cloudstack]

["andrijapanicsb (via GitHub)" <gi...@apache.org>]

andrijapanicsb commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2315278613

   @Pearl1594 do we have more (internal?) details on the design of this one? Looks really interesting.


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] [WIP] backup: Simple NAS backup plugin for KVM (experimental) [cloudstack]

["blueorangutan (via GitHub)" <gi...@apache.org>]

blueorangutan commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2271595088

   @Pearl1594 a [SL] Jenkins job has been kicked to build packages. It will be bundled with  KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] [WIP] backup: Simple NAS backup plugin for KVM (experimental) [cloudstack]

["blueorangutan (via GitHub)" <gi...@apache.org>]

blueorangutan commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2284733578

   @DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with  KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["blueorangutan (via GitHub)" <gi...@apache.org>]

blueorangutan commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2330313945

   Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 10971


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["sureshanaparti (via GitHub)" <gi...@apache.org>]

sureshanaparti commented on code in PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#discussion_r1745223231


##########
engine/schema/src/main/resources/META-INF/db/schema-41910to42000.sql:
##########
@@ -234,6 +254,15 @@ CALL `cloud`.`IDEMPOTENT_MODIFY_COLUMN_CHAR_SET`('vpc_offerings', 'display_text'
 
 CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.roles','state', 'varchar(10) NOT NULL default "enabled" COMMENT "role state"');
 
+-- Drop foreign key on backup_schedule, drop unique key on vm_id and re-add foreign key to allow multiple backup schedules to be created
+ALTER TABLE `cloud`.`backup_schedule` DROP FOREIGN KEY fk_backup_schedule__vm_id;
+ALTER TABLE `cloud`.`backup_schedule` DROP INDEX vm_id;
+ALTER TABLE `cloud`.`backup_schedule` ADD CONSTRAINT fk_backup_schedule__vm_id
+    FOREIGN KEY (vm_id) REFERENCES vm_instance(id) ON DELETE CASCADE;
+
+-- Add volume details to the backups table to keep track of the volumes being backed up
+CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.backups', 'backed_volumes', 'text DEFAULT NULL COMMENT "details of backed-up volumes" ');
+

Review Comment:
   ```suggestion
   CALL `cloud`.`IDEMPOTENT_MODIFY_COLUMN_CHAR_SET`('backups', 'backed_volumes', 'TEXT', 'DEFAULT NULL COMMENT \'details of backed-up volumes\'')
   ```
   
   add this, to support unicode chars for the backed up volume details



-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["Pearl1594 (via GitHub)" <gi...@apache.org>]

Pearl1594 commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2330217137

   @blueorangutan package


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["blueorangutan (via GitHub)" <gi...@apache.org>]

blueorangutan commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2328577656

   Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 10951


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["rohityadavcloud (via GitHub)" <gi...@apache.org>]

rohityadavcloud commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2331243952

   @blueorangutan package


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["DaanHoogland (via GitHub)" <gi...@apache.org>]

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


##########
tools/marvin/setup.py:
##########
@@ -27,7 +27,7 @@
         raise RuntimeError("python setuptools is required to build Marvin")
 
 
-VERSION = "4.20.0.0-SNAPSHOT"
+VERSION = "4.20.0.0"

Review Comment:
   I think this change should not be here, but also this is easy to miss. should it be removed?



-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] [WIP] backup: Simple NAS backup plugin for KVM (experimental) [cloudstack]

["rohityadavcloud (via GitHub)" <gi...@apache.org>]

rohityadavcloud commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2265218556

   Tested NFS, CephFS & samba, works when configuration is correctly configured; for example;
   
   ```
   mysql> select * from backup_repository\G;
   *************************** 1. row ***************************
               id: 1
             uuid: d3b911d4-50a6-11ef-bc8b-525400f93984
             name: NFS-backup
          zone_id: 1
         provider: nas
             type: nfs
          address: 172.20.0.1:/export/testing/qa1-cs420-kvm-1cdf7971-kvm/backup
       mount_opts:
       used_bytes: NULL
   capacity_bytes: NULL
          created: 2024-08-02 08:11:31
          removed: NULL
   *************************** 2. row ***************************
               id: 2
             uuid: 09c2efd1-50c4-11ef-bc8b-525400f93984
             name: Ceph-backup
          zone_id: 1
         provider: nas
             type: ceph
          address: 10.10.1.10,10.10.1.11,10.10.1.12:/backup
       mount_opts: name=rohit,secret=XXXX==
       used_bytes: NULL
   capacity_bytes: NULL
          created: 2024-08-02 11:40:37
          removed: NULL
   *************************** 3. row ***************************
               id: 3
             uuid: 876b1da4-50c6-11ef-bc8b-525400f93984
             name: Samba-backup
          zone_id: 1
         provider: nas
             type: cifs
          address: //192.168.1.10/backup
       mount_opts: vers=3.0,username=rohit,password=XXXX
       used_bytes: NULL
   capacity_bytes: NULL
          created: 2024-08-02 11:58:27
          removed: NULL
   3 rows in set (0.00 sec)
   ```


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] [WIP] backup: Simple NAS backup plugin for KVM (experimental) [cloudstack]

["DaanHoogland (via GitHub)" <gi...@apache.org>]

DaanHoogland commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2283886921

   nice feature idea @rohityadavcloud :+1: 


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] [WIP] backup: Simple NAS backup plugin for KVM (experimental) [cloudstack]

["blueorangutan (via GitHub)" <gi...@apache.org>]

blueorangutan commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2272216350

   @Pearl1594 a [SL] Jenkins job has been kicked to build packages. It will be bundled with  KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] [WIP] backup: Simple NAS backup plugin for KVM (experimental) [cloudstack]

["blueorangutan (via GitHub)" <gi...@apache.org>]

blueorangutan commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2284679260

   Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 10621


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM (experimental) [cloudstack]

["Pearl1594 (via GitHub)" <gi...@apache.org>]

Pearl1594 commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2327225953

   @blueorangutan package


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["Pearl1594 (via GitHub)" <gi...@apache.org>]

Pearl1594 commented on code in PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#discussion_r1743745411


##########
ui/src/views/compute/backup/BackupSchedule.vue:
##########
@@ -147,18 +147,15 @@ export default {
   mounted () {
     this.dataSchedules = []
     if (this.dataSource && Object.keys(this.dataSource).length > 0) {
-      this.dataSchedules.push(this.dataSource)
+      this.dataSchedules = this.dataSource

Review Comment:
   Yes, that's right. Earlier, i.e before this PR, backup schedules was always 1 per instance due to a limitation (https://github.com/apache/cloudstack/issues/9344) . But now that it is a list, this would cause issues. 



-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["blueorangutan (via GitHub)" <gi...@apache.org>]

blueorangutan commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2330419607

   @Pearl1594 a [SL] Jenkins job has been kicked to build packages. It will be bundled with  KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["github-actions[bot] (via GitHub)" <gi...@apache.org>]

github-actions[bot] commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2330553279

   This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["blueorangutan (via GitHub)" <gi...@apache.org>]

blueorangutan commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2331322753

   @rohityadavcloud a [SL] Jenkins job has been kicked to build packages. It will be bundled with  KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM (experimental) [cloudstack]

["blueorangutan (via GitHub)" <gi...@apache.org>]

blueorangutan commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2315360122

   Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 10847


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] [WIP] backup: Simple NAS backup plugin for KVM (experimental) [cloudstack]

["Pearl1594 (via GitHub)" <gi...@apache.org>]

Pearl1594 commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2271836087

   @blueorangutan package


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM (experimental) [cloudstack]

["blueorangutan (via GitHub)" <gi...@apache.org>]

blueorangutan commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2327392502

   Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 10935


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM (experimental) [cloudstack]

["blueorangutan (via GitHub)" <gi...@apache.org>]

blueorangutan commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2328060630

   <b>[SF] Trillian Build Failed (tid-11323)<b/>


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM (experimental) [cloudstack]

["blueorangutan (via GitHub)" <gi...@apache.org>]

blueorangutan commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2327273021

   Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 10934


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["Pearl1594 (via GitHub)" <gi...@apache.org>]

Pearl1594 commented on code in PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#discussion_r1745689233


##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java:
##########
@@ -0,0 +1,181 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package com.cloud.hypervisor.kvm.resource.wrapper;
+
+import com.cloud.agent.api.Answer;
+import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
+import com.cloud.resource.CommandWrapper;
+import com.cloud.resource.ResourceWrapper;
+import com.cloud.utils.Pair;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.utils.script.Script;
+import org.apache.cloudstack.backup.BackupAnswer;
+import org.apache.cloudstack.backup.RestoreBackupCommand;
+import org.apache.commons.lang3.RandomStringUtils;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.nio.file.StandardCopyOption;
+import java.util.List;
+import java.util.Locale;
+import java.util.Objects;
+
+@ResourceWrapper(handles = RestoreBackupCommand.class)
+public class LibvirtRestoreBackupCommandWrapper extends CommandWrapper<RestoreBackupCommand, Answer, LibvirtComputingResource> {
+    private static final String BACKUP_TEMP_FILE_PREFIx = "csbackup";
+    private static final String MOUNT_COMMAND = "sudo mount -t %s %s %s";
+    private static final String UMOUNT_COMMAND = "sudo umount %s";
+    private static final String FILE_PATH_PLACEHOLDER = "%s/%s";
+
+    @Override
+    public Answer execute(RestoreBackupCommand command, LibvirtComputingResource serverResource) {
+        String vmName = command.getVmName();
+        String backupPath = command.getBackupPath();
+        String backupRepoAddress = command.getBackupRepoAddress();
+        String backupRepoType = command.getBackupRepoType();
+        String mountOptions = command.getMountOptions();
+        Boolean vmExists = command.isVmExists();
+        String diskType = command.getDiskType();
+        Long deviceId = command.getDeviceId();
+        List<String> volumePaths = command.getVolumePaths();
+        String restoreVolumeUuid = command.getRestoreVolumeUUID();
+
+        String newVolumeId = null;
+        if (Objects.isNull(vmExists)) {
+            String volumePath = volumePaths.get(0);
+            int lastIndex = volumePath.lastIndexOf("/");
+            newVolumeId = volumePath.substring(lastIndex + 1);
+            restoreVolume(backupPath, backupRepoType, backupRepoAddress, volumePath, diskType, deviceId, restoreVolumeUuid);
+        } else if (vmExists) {
+            restoreVolumesOfExistingVM(volumePaths, backupPath, backupRepoType, backupRepoAddress, mountOptions);
+        } else {
+            restoreVolumesOfDestroyedVMs(volumePaths, vmName, backupPath, backupRepoType, backupRepoAddress, mountOptions);
+        }
+
+        return new BackupAnswer(command, true, newVolumeId);
+    }
+
+    private void restoreVolumesOfExistingVM(List<String> volumePaths, String backupPath,
+                                             String backupRepoType, String backupRepoAddress, String mountOptions) {
+        String diskType = "root";
+        String mountDirectory = mountBackupDirectory(backupRepoAddress, backupRepoType);
+        try {
+            for (int idx = 0; idx < volumePaths.size(); idx++) {
+                String volumePath = volumePaths.get(idx);
+                Pair<String, String> bkpPathAndVolUuid = getBackupPath(mountDirectory, volumePath, backupPath, diskType, idx, null);
+                diskType = "datadisk";
+                try {
+                    replaceVolumeWithBackup(volumePath, bkpPathAndVolUuid.first());
+                } catch (IOException e) {
+                    throw new CloudRuntimeException(String.format("Unable to revert backup for volume [%s] due to [%s].", bkpPathAndVolUuid.second(), e.getMessage()), e);
+                }
+            }
+        } finally {
+            unmountBackupDirectory(mountDirectory);
+            deleteTemporaryDirectory(mountDirectory);
+        }
+
+    }
+
+    private void restoreVolumesOfDestroyedVMs(List<String> volumePaths, String vmName, String backupPath,
+                                              String backupRepoType, String backupRepoAddress, String mountOptions) {
+        String mountDirectory = mountBackupDirectory(backupRepoAddress, backupRepoType);
+        String diskType = "root";
+        try {
+            for (int i = 0; i < volumePaths.size(); i++) {
+                String volumePath = volumePaths.get(i);
+                Pair<String, String> bkpPathAndVolUuid = getBackupPath(mountDirectory, volumePath, backupPath, diskType, i, null);
+                diskType = "datadisk";
+                try {
+                    replaceVolumeWithBackup(volumePath, bkpPathAndVolUuid.first());
+                } catch (IOException e) {
+                    throw new CloudRuntimeException(String.format("Unable to revert backup for volume [%s] due to [%s].", bkpPathAndVolUuid.second(), e.getMessage()), e);
+                }
+            }
+        } finally {
+            unmountBackupDirectory(mountDirectory);
+            deleteTemporaryDirectory(mountDirectory);
+        }
+    }
+
+    private void restoreVolume(String backupPath, String backupRepoType, String backupRepoAddress, String volumePath,
+                               String diskType, Long deviceId, String volumeUUID) {
+        String mountDirectory = mountBackupDirectory(backupRepoAddress, backupRepoType);
+        Pair<String, String> bkpPathAndVolUuid;
+        try {
+            bkpPathAndVolUuid = getBackupPath(mountDirectory, volumePath, backupPath, diskType, deviceId.intValue(), volumeUUID);
+            try {
+                replaceVolumeWithBackup(volumePath, bkpPathAndVolUuid.first());
+            } catch (IOException e) {
+                throw new CloudRuntimeException(String.format("Unable to revert backup for volume [%s] due to [%s].", bkpPathAndVolUuid.second(), e.getMessage()), e);
+            }
+        } catch (Exception e) {
+            throw new CloudRuntimeException("Failed to restore volume", e);
+        } finally {
+            unmountBackupDirectory(mountDirectory);
+            deleteTemporaryDirectory(mountDirectory);
+        }
+    }
+
+
+    private String mountBackupDirectory(String backupRepoAddress, String backupRepoType) {
+        String randomChars = RandomStringUtils.random(5, true, false);
+        String mountDirectory = String.format("%s.%s",BACKUP_TEMP_FILE_PREFIx , randomChars);
+        try {
+            mountDirectory = Files.createTempDirectory(mountDirectory).toString();
+            String mount = String.format(MOUNT_COMMAND, backupRepoType, backupRepoAddress, mountDirectory);
+            Script.runSimpleBashScript(mount);

Review Comment:
   Will bear in mind this improvement as part of an improvement PR that will be raised.



-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["rohityadavcloud (via GitHub)" <gi...@apache.org>]

rohityadavcloud commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2331460802

   Rekicked pkg job manually in the backend jenkins.


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["rohityadavcloud (via GitHub)" <gi...@apache.org>]

rohityadavcloud commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2331617343

   @blueorangutan package


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] [WIP] backup: Simple NAS backup plugin for KVM (experimental) [cloudstack]

["blueorangutan (via GitHub)" <gi...@apache.org>]

blueorangutan commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2265033088

   @rohityadavcloud a [SL] Jenkins job has been kicked to build packages. It will be bundled with  KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] [WIP] backup: Simple NAS backup plugin for KVM (experimental) [cloudstack]

["codecov[bot] (via GitHub)" <gi...@apache.org>]

codecov[bot] commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2252093628

   ## [Codecov](https://app.codecov.io/gh/apache/cloudstack/pull/9451?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   All modified and coverable lines are covered by tests :white_check_mark:
   > Project coverage is 4.18%. Comparing base [(`817251f`)](https://app.codecov.io/gh/apache/cloudstack/commit/817251f1f8955a15232b6ee8032534ae64ff9921?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`f2baa68`)](https://app.codecov.io/gh/apache/cloudstack/commit/f2baa688025f4b2e966be8ea0bd19f5def5faba2?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   > Report is 1 commits behind head on main.
   
   > :exclamation:  There is a different number of reports uploaded between BASE (817251f) and HEAD (f2baa68). Click for more details.
   > 
   > <details><summary>HEAD has 1 upload less than BASE</summary>
   >
   >| Flag | BASE (817251f) | HEAD (f2baa68) |
   >|------|------|------|
   >|unittests|1|0|
   ></details>
   
   <details><summary>Additional details and impacted files</summary>
   
   
   ```diff
   @@             Coverage Diff              @@
   ##               main   #9451       +/-   ##
   ============================================
   - Coverage     15.54%   4.18%   -11.36%     
   ============================================
     Files          5494     372     -5122     
     Lines        481056   30448   -450608     
     Branches      59747    5398    -54349     
   ============================================
   - Hits          74770    1274    -73496     
   + Misses       398024   29030   -368994     
   + Partials       8262     144     -8118     
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/cloudstack/pull/9451/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [uitests](https://app.codecov.io/gh/apache/cloudstack/pull/9451/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `4.18% <ø> (ø)` | |
   | [unittests](https://app.codecov.io/gh/apache/cloudstack/pull/9451/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   </details>
   
   [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/cloudstack/pull/9451?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] [WIP] backup: Simple NAS backup plugin for KVM (experimental) [cloudstack]

["blueorangutan (via GitHub)" <gi...@apache.org>]

blueorangutan commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2284851877

   Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 10624


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] [WIP] backup: Simple NAS backup plugin for KVM (experimental) [cloudstack]

["DaanHoogland (via GitHub)" <gi...@apache.org>]

DaanHoogland commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2284730457

   @blueorangutan package


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM (experimental) [cloudstack]

["rajujith (via GitHub)" <gi...@apache.org>]

rajujith commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2320292455

   @blueorangutan test


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM (experimental) [cloudstack]

["blueorangutan (via GitHub)" <gi...@apache.org>]

blueorangutan commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2316928279

   @rohityadavcloud a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] [WIP] backup: Simple NAS backup plugin for KVM (experimental) [cloudstack]

["Pearl1594 (via GitHub)" <gi...@apache.org>]

Pearl1594 commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2272214946

   @blueorangutan package


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM (experimental) [cloudstack]

["rohityadavcloud (via GitHub)" <gi...@apache.org>]

rohityadavcloud commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2315219411

   Update cc @DaanHoogland @sureshanaparti @shwstppr @rajujith @NuxRo the experimental plugin is ready for review & testing, pending a bit of cleanup, maybe tests & doc work. Tested following cases:
   
   - Backup of VM with root disk & VM with root & data disks
   - Restore of backup of VM with root disk & VM with root & data disks
   - Deleting backups of VM with root disk & VM with root & data disks
   - Restore a volume of a backup of VM
   
   Case WIP: restoring VM using backup for a VM which is expunged
   
   @blueorangutan package


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM (experimental) [cloudstack]

["rohityadavcloud (via GitHub)" <gi...@apache.org>]

rohityadavcloud commented on code in PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#discussion_r1734599823


##########
tools/marvin/setup.py:
##########
@@ -27,7 +27,7 @@
         raise RuntimeError("python setuptools is required to build Marvin")
 
 
-VERSION = "4.20.0.0-SNAPSHOT"
+VERSION = "4.20.0.0"

Review Comment:
   ```suggestion
   VERSION = "4.20.0.0-SNAPSHOT"
   ```



-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["Pearl1594 (via GitHub)" <gi...@apache.org>]

Pearl1594 commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2331907787

   Few comments will be addressed as improvements in a subsequent PR. 


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["DaanHoogland (via GitHub)" <gi...@apache.org>]

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


##########
ui/src/views/compute/backup/BackupSchedule.vue:
##########
@@ -147,18 +147,15 @@ export default {
   mounted () {
     this.dataSchedules = []
     if (this.dataSource && Object.keys(this.dataSource).length > 0) {
-      this.dataSchedules.push(this.dataSource)
+      this.dataSchedules = this.dataSource

Review Comment:
   so never add, always replace. is this right?



-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["blueorangutan (via GitHub)" <gi...@apache.org>]

blueorangutan commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2330945563

   Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 10986


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM (experimental) [cloudstack]

["blueorangutan (via GitHub)" <gi...@apache.org>]

blueorangutan commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2319128581

   <b>[SF] Trillian test result (tid-11228)</b>
   Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
   Total time taken: 49421 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9451-t11228-kvm-ol8.zip
   Smoke tests completed. 139 look OK, 0 have errors, 0 did not run
   Only failed and skipped tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM (experimental) [cloudstack]

["DaanHoogland (via GitHub)" <gi...@apache.org>]

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


##########
core/src/main/java/org/apache/cloudstack/backup/TakeBackupCommand.java:
##########
@@ -0,0 +1,94 @@
+//
+// 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.backup;
+
+import com.cloud.agent.api.Command;
+import com.cloud.agent.api.LogLevel;
+
+import java.util.List;
+
+public class TakeBackupCommand extends Command {

Review Comment:
   nice but strange name. would this better be `MakeBackupCommand` or `CreateBackupCommand`? (not a biggy by any means)



-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM (experimental) [cloudstack]

["rohityadavcloud (via GitHub)" <gi...@apache.org>]

rohityadavcloud commented on code in PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#discussion_r1738373828


##########
core/src/main/java/org/apache/cloudstack/backup/TakeBackupCommand.java:
##########
@@ -0,0 +1,94 @@
+//
+// 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.backup;
+
+import com.cloud.agent.api.Command;
+import com.cloud.agent.api.LogLevel;
+
+import java.util.List;
+
+public class TakeBackupCommand extends Command {

Review Comment:
   There's are take snapshot related method/classes, so we followed that naming convention. CreateBackup name also makes sense.



-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM (experimental) [cloudstack]

["blueorangutan (via GitHub)" <gi...@apache.org>]

blueorangutan commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2321871750

   @Pearl1594 a [SL] Jenkins job has been kicked to build packages. It will be bundled with  KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM (experimental) [cloudstack]

["blueorangutan (via GitHub)" <gi...@apache.org>]

blueorangutan commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2327472573

   @Pearl1594 a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] [WIP] backup: Simple NAS backup plugin for KVM (experimental) [cloudstack]

["blueorangutan (via GitHub)" <gi...@apache.org>]

blueorangutan commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2271697363

   Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 10579


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["rohityadavcloud (via GitHub)" <gi...@apache.org>]

rohityadavcloud commented on code in PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#discussion_r1743421838


##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java:
##########
@@ -0,0 +1,181 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package com.cloud.hypervisor.kvm.resource.wrapper;
+
+import com.cloud.agent.api.Answer;
+import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
+import com.cloud.resource.CommandWrapper;
+import com.cloud.resource.ResourceWrapper;
+import com.cloud.utils.Pair;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.utils.script.Script;
+import org.apache.cloudstack.backup.BackupAnswer;
+import org.apache.cloudstack.backup.RestoreBackupCommand;
+import org.apache.commons.lang3.RandomStringUtils;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.nio.file.StandardCopyOption;
+import java.util.List;
+import java.util.Locale;
+import java.util.Objects;
+
+@ResourceWrapper(handles = RestoreBackupCommand.class)
+public class LibvirtRestoreBackupCommandWrapper extends CommandWrapper<RestoreBackupCommand, Answer, LibvirtComputingResource> {
+    private static final String BACKUP_TEMP_FILE_PREFIx = "csbackup";

Review Comment:
   @Pearl1594 to check & advise.



-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["rohityadavcloud (via GitHub)" <gi...@apache.org>]

rohityadavcloud merged PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["rajujith (via GitHub)" <gi...@apache.org>]

rajujith commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2330783709

   @blueorangutan package


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["blueorangutan (via GitHub)" <gi...@apache.org>]

blueorangutan commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2330573810

   @rohityadavcloud a [SL] Jenkins job has been kicked to build packages. It will be bundled with  KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["sureshanaparti (via GitHub)" <gi...@apache.org>]

sureshanaparti commented on code in PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#discussion_r1745231807


##########
plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java:
##########
@@ -0,0 +1,442 @@
+// 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.backup;
+
+import com.cloud.agent.AgentManager;
+import com.cloud.dc.dao.ClusterDao;
+import com.cloud.exception.AgentUnavailableException;
+import com.cloud.exception.OperationTimedoutException;
+import com.cloud.host.Host;
+import com.cloud.host.HostVO;
+import com.cloud.host.Status;
+import com.cloud.host.dao.HostDao;
+import com.cloud.hypervisor.Hypervisor;
+import com.cloud.storage.ScopeType;
+import com.cloud.storage.StoragePoolHostVO;
+import com.cloud.storage.Volume;
+import com.cloud.storage.VolumeVO;
+import com.cloud.storage.dao.StoragePoolHostDao;
+import com.cloud.storage.dao.VolumeDao;
+import com.cloud.utils.Pair;
+import com.cloud.utils.component.AdapterBase;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.vm.VirtualMachine;
+import com.cloud.vm.dao.VMInstanceDao;
+import org.apache.cloudstack.backup.dao.BackupDao;
+import org.apache.cloudstack.backup.dao.BackupOfferingDao;
+import org.apache.cloudstack.backup.dao.BackupRepositoryDao;
+import org.apache.cloudstack.framework.config.ConfigKey;
+import org.apache.cloudstack.framework.config.Configurable;
+import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
+import org.apache.commons.collections.CollectionUtils;
+import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.LogManager;
+import javax.inject.Inject;
+import java.text.SimpleDateFormat;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Date;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import java.util.HashMap;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.UUID;
+import java.util.stream.Collectors;
+
+public class NASBackupProvider extends AdapterBase implements BackupProvider, Configurable {
+    private static final Logger LOG = LogManager.getLogger(NASBackupProvider.class);
+
+    @Inject
+    private BackupDao backupDao;
+
+    @Inject
+    private BackupRepositoryDao backupRepositoryDao;
+
+    @Inject
+    private BackupOfferingDao backupOfferingDao;
+
+    @Inject
+    private HostDao hostDao;
+
+    @Inject
+    private ClusterDao clusterDao;
+
+    @Inject
+    private VolumeDao volumeDao;
+
+    @Inject
+    private StoragePoolHostDao storagePoolHostDao;
+
+    @Inject
+    private VMInstanceDao vmInstanceDao;
+
+    @Inject
+    private PrimaryDataStoreDao primaryDataStoreDao;
+
+    @Inject
+    private AgentManager agentManager;
+
+    protected Host getLastVMHypervisorHost(VirtualMachine vm) {
+        Long hostId = vm.getLastHostId();
+        if (hostId == null) {
+            LOG.debug("Cannot find last host for vm. This should never happen, please check your database.");
+            return null;
+        }
+        Host host = hostDao.findById(hostId);
+
+        if (host.getStatus() == Status.Up) {
+            return host;
+        } else {
+            // Try to find any Up host in the same cluster
+            for (final Host hostInCluster : hostDao.findHypervisorHostInCluster(host.getClusterId())) {
+                if (hostInCluster.getStatus() == Status.Up) {
+                    LOG.debug("Found Host " + hostInCluster.getName());
+                    return hostInCluster;
+                }
+            }
+        }
+        // Try to find any Host in the zone
+        for (final HostVO hostInZone : hostDao.listByDataCenterIdAndHypervisorType(host.getDataCenterId(), Hypervisor.HypervisorType.KVM)) {
+            if (hostInZone.getStatus() == Status.Up) {
+                LOG.debug("Found Host " + hostInZone.getName());
+                return hostInZone;
+            }
+        }
+        return null;
+    }
+
+    protected Host getVMHypervisorHost(VirtualMachine vm) {
+        Long hostId = vm.getHostId();
+        if (hostId == null && VirtualMachine.State.Running.equals(vm.getState())) {
+            throw new CloudRuntimeException(String.format("Unable to find the hypervisor host for %s. Make sure the virtual machine is running", vm.getName()));
+        }
+        if (VirtualMachine.State.Stopped.equals(vm.getState())) {
+            hostId = vm.getLastHostId();
+        }
+        if (hostId == null) {
+            throw new CloudRuntimeException(String.format("Unable to find the hypervisor host for stopped VM: %s", vm));
+        }
+        final Host host = hostDao.findById(hostId);
+        if (host == null || !Status.Up.equals(host.getStatus()) || !Hypervisor.HypervisorType.KVM.equals(host.getHypervisorType())) {
+            throw new CloudRuntimeException("Unable to contact backend control plane to initiate backup");
+        }
+        return host;
+    }
+
+    @Override
+    public boolean takeBackup(final VirtualMachine vm) {
+        final Host host = getVMHypervisorHost(vm);
+
+        final BackupRepository backupRepository = backupRepositoryDao.findByBackupOfferingId(vm.getBackupOfferingId());
+        if (backupRepository == null) {
+            throw new CloudRuntimeException("No valid backup repository found for the VM, please check the attached backup offering");
+        }
+
+        final Date creationDate = new Date();
+        final String backupPath = String.format("%s/%s", vm.getInstanceName(),
+                new SimpleDateFormat("yyyy.MM.dd.HH.mm.ss").format(creationDate));
+
+        BackupVO backupVO = createBackupObject(vm, backupPath);
+        TakeBackupCommand command = new TakeBackupCommand(vm.getInstanceName(), backupPath);
+        command.setBackupRepoType(backupRepository.getType());
+        command.setBackupRepoAddress(backupRepository.getAddress());
+        command.setMountOptions(backupRepository.getMountOptions());
+
+        if (VirtualMachine.State.Stopped.equals(vm.getState())) {
+            List<VolumeVO> vmVolumes = volumeDao.findByInstance(vm.getId());
+            List<String> volumePaths = getVolumePaths(vmVolumes);
+            command.setVolumePaths(volumePaths);
+        }
+
+        BackupAnswer answer = null;
+        try {
+            answer = (BackupAnswer) agentManager.send(host.getId(), command);
+        } catch (AgentUnavailableException e) {
+            throw new CloudRuntimeException("Unable to contact backend control plane to initiate backup");
+        } catch (OperationTimedoutException e) {
+            throw new CloudRuntimeException("Operation to initiate backup timed out, please try again");
+        }
+
+        if (answer != null && answer.getResult()) {
+            backupVO.setDate(new Date());
+            backupVO.setSize(answer.getSize());
+            backupVO.setStatus(Backup.Status.BackedUp);
+            backupVO.setBackedUpVolumes(BackupManagerImpl.createVolumeInfoFromVolumes(volumeDao.findByInstance(vm.getId())));
+            return backupDao.update(backupVO.getId(), backupVO);
+        } else {
+            backupVO.setStatus(Backup.Status.Failed);
+            backupDao.remove(backupVO.getId());
+        }
+        return Objects.nonNull(answer) && answer.getResult();
+    }
+
+    private BackupVO createBackupObject(VirtualMachine vm, String backupPath) {
+        BackupVO backup = new BackupVO();
+        backup.setVmId(vm.getId());
+        backup.setExternalId(backupPath);
+        backup.setType("FULL");
+        backup.setDate(new Date());
+        long virtualSize = 0L;
+        for (final Volume volume: volumeDao.findByInstance(vm.getId())) {
+            if (Volume.State.Ready.equals(volume.getState())) {
+                virtualSize += volume.getSize();
+            }
+        }
+        backup.setProtectedSize(Long.valueOf(virtualSize));
+        backup.setStatus(Backup.Status.BackingUp);
+        backup.setBackupOfferingId(vm.getBackupOfferingId());
+        backup.setAccountId(vm.getAccountId());
+        backup.setDomainId(vm.getDomainId());
+        backup.setZoneId(vm.getDataCenterId());
+        return backupDao.persist(backup);
+    }
+
+    @Override
+    public boolean restoreVMFromBackup(VirtualMachine vm, Backup backup) {
+        List<Backup.VolumeInfo> backedVolumes = backup.getBackedUpVolumes();
+        List<VolumeVO> volumes = backedVolumes.stream().map(volume -> volumeDao.findByUuid(volume.getUuid())).collect(Collectors.toList());
+
+        LOG.debug("Restoring vm {} from backup {} on the NAS Backup Provider", vm.getUuid(), backup.getUuid());
+        BackupRepository backupRepository = getBackupRepository(vm, backup);
+
+        final Host host = getLastVMHypervisorHost(vm);
+        RestoreBackupCommand restoreCommand = new RestoreBackupCommand();
+        restoreCommand.setBackupPath(backup.getExternalId());
+        restoreCommand.setBackupRepoType(backupRepository.getType());
+        restoreCommand.setBackupRepoAddress(backupRepository.getAddress());
+        restoreCommand.setVmName(vm.getName());
+        restoreCommand.setVolumePaths(getVolumePaths(volumes));
+        restoreCommand.setVmExists(vm.getRemoved() == null);
+        restoreCommand.setVmState(vm.getState());
+
+        BackupAnswer answer = null;
+        try {
+            answer = (BackupAnswer) agentManager.send(host.getId(), restoreCommand);
+        } catch (AgentUnavailableException e) {
+            throw new CloudRuntimeException("Unable to contact backend control plane to initiate backup");
+        } catch (OperationTimedoutException e) {
+            throw new CloudRuntimeException("Operation to initiate backup timed out, please try again");
+        }
+        return answer.getResult();
+    }
+
+    private List<String> getVolumePaths(List<VolumeVO> volumes) {
+        List<String> volumePaths = new ArrayList<>();
+        for (VolumeVO volume : volumes) {
+            StoragePoolVO storagePool = primaryDataStoreDao.findById(volume.getPoolId());
+            if (Objects.isNull(storagePool)) {
+                throw new CloudRuntimeException("Unable to find storage pool associated to the volume");
+            }
+            String volumePathPrefix = String.format("/mnt/%s", storagePool.getUuid());
+            if (ScopeType.HOST.equals(storagePool.getScope())) {
+                volumePathPrefix = storagePool.getPath();
+            }
+            volumePaths.add(String.format("%s/%s", volumePathPrefix, volume.getPath()));
+        }
+        return volumePaths;
+    }
+
+    @Override
+    public Pair<Boolean, String> restoreBackedUpVolume(Backup backup, String volumeUuid, String hostIp, String dataStoreUuid, Pair<String, VirtualMachine.State> vmNameAndState) {
+        final VolumeVO volume = volumeDao.findByUuid(volumeUuid);
+        final VirtualMachine backupSourceVm = vmInstanceDao.findById(backup.getVmId());
+        final StoragePoolHostVO dataStore = storagePoolHostDao.findByUuid(dataStoreUuid);
+        final HostVO hostVO = hostDao.findByIp(hostIp);
+
+        Optional<Backup.VolumeInfo> matchingVolume = getBackedUpVolumeInfo(backupSourceVm.getBackupVolumeList(), volumeUuid);
+        Long backedUpVolumeSize = matchingVolume.isPresent() ? matchingVolume.get().getSize() : 0L;
+
+        LOG.debug("Restoring vm volume" + volumeUuid + "from backup " + backup.getUuid() + " on the NAS Backup Provider");
+        BackupRepository backupRepository = getBackupRepository(backupSourceVm, backup);
+
+        VolumeVO restoredVolume = new VolumeVO(Volume.Type.DATADISK, null, backup.getZoneId(),
+                backup.getDomainId(), backup.getAccountId(), 0, null,
+                backup.getSize(), null, null, null);
+        String volumeUUID = UUID.randomUUID().toString();
+        restoredVolume.setName("RestoredVol-"+volume.getName());
+        restoredVolume.setProvisioningType(volume.getProvisioningType());
+        restoredVolume.setUpdated(new Date());
+        restoredVolume.setUuid(volumeUUID);
+        restoredVolume.setRemoved(null);
+        restoredVolume.setDisplayVolume(true);
+        restoredVolume.setPoolId(dataStore.getPoolId());
+        restoredVolume.setPath(restoredVolume.getUuid());
+        restoredVolume.setState(Volume.State.Copying);
+        restoredVolume.setSize(backedUpVolumeSize);
+        restoredVolume.setDiskOfferingId(volume.getDiskOfferingId());
+
+        RestoreBackupCommand restoreCommand = new RestoreBackupCommand();
+        restoreCommand.setBackupPath(backup.getExternalId());
+        restoreCommand.setBackupRepoType(backupRepository.getType());
+        restoreCommand.setBackupRepoAddress(backupRepository.getAddress());
+        restoreCommand.setVmName(vmNameAndState.first());
+        restoreCommand.setVolumePaths(Collections.singletonList(String.format("%s/%s", dataStore.getLocalPath(), volumeUUID)));
+        restoreCommand.setDiskType(volume.getVolumeType().name().toLowerCase(Locale.ROOT));
+        restoreCommand.setVmExists(null);
+        restoreCommand.setVmState(vmNameAndState.second());
+        restoreCommand.setRestoreVolumeUUID(volumeUuid);
+
+        BackupAnswer answer = null;
+        try {
+            answer = (BackupAnswer) agentManager.send(hostVO.getId(), restoreCommand);
+        } catch (AgentUnavailableException e) {
+            throw new CloudRuntimeException("Unable to contact backend control plane to initiate backup");
+        } catch (OperationTimedoutException e) {
+            throw new CloudRuntimeException("Operation to initiate backup timed out, please try again");
+        }
+
+        if (answer.getResult()) {
+            try {
+                volumeDao.persist(restoredVolume);
+            } catch (Exception e) {
+                throw new CloudRuntimeException("Unable to create restored volume due to: " + e);
+            }
+        }
+
+        return new Pair<>(answer.getResult(), answer.getDetails());
+    }
+
+    private BackupRepository getBackupRepository(VirtualMachine vm, Backup backup) {
+        BackupRepository backupRepository = backupRepositoryDao.findByBackupOfferingId(vm.getBackupOfferingId());
+        final String errorMessage = "No valid backup repository found for the VM, please check the attached backup offering";
+        if (backupRepository == null) {
+            logger.warn(errorMessage + "Re-attempting with the backup offering associated with the backup");
+        }
+        backupRepository = backupRepositoryDao.findByBackupOfferingId(backup.getBackupOfferingId());
+        if (backupRepository == null) {
+            throw new CloudRuntimeException(errorMessage);
+        }
+        return backupRepository;
+    }
+
+    private Optional<Backup.VolumeInfo> getBackedUpVolumeInfo(List<Backup.VolumeInfo> backedUpVolumes, String volumeUuid) {
+        return backedUpVolumes.stream()
+                .filter(v -> v.getUuid().equals(volumeUuid))
+                .findFirst();
+    }
+
+    @Override
+    public boolean deleteBackup(Backup backup, boolean forced) {
+        final BackupRepository backupRepository = backupRepositoryDao.findByBackupOfferingId(backup.getBackupOfferingId());
+        if (backupRepository == null) {
+            throw new CloudRuntimeException("No valid backup repository found for the VM, please check the attached backup offering");
+        }
+
+        final VirtualMachine vm  = vmInstanceDao.findByIdIncludingRemoved(backup.getVmId());
+        final Host host = getLastVMHypervisorHost(vm);
+
+        DeleteBackupCommand command = new DeleteBackupCommand(backup.getExternalId(), backupRepository.getType(),
+                backupRepository.getAddress(), backupRepository.getMountOptions());
+
+        BackupAnswer answer = null;
+        try {
+            answer = (BackupAnswer) agentManager.send(host.getId(), command);
+        } catch (AgentUnavailableException e) {
+            throw new CloudRuntimeException("Unable to contact backend control plane to initiate backup");
+        } catch (OperationTimedoutException e) {
+            throw new CloudRuntimeException("Operation to initiate backup timed out, please try again");
+        }
+
+        if (answer != null && answer.getResult()) {
+            return backupDao.remove(backup.getId());
+        }
+
+        return false;
+    }
+
+    @Override
+    public Map<VirtualMachine, Backup.Metric> getBackupMetrics(Long zoneId, List<VirtualMachine> vms) {
+        final Map<VirtualMachine, Backup.Metric> metrics = new HashMap<>();
+        if (CollectionUtils.isEmpty(vms)) {
+            LOG.warn("Unable to get VM Backup Metrics because the list of VMs is empty.");
+            return metrics;
+        }
+
+        for (final VirtualMachine vm : vms) {
+            Long vmBackupSize = 0L;
+            Long vmBackupProtectedSize = 0L;
+            for (final Backup backup: backupDao.listByVmId(null, vm.getId())) {
+                vmBackupSize += backup.getSize();
+                vmBackupProtectedSize += backup.getProtectedSize();
+            }
+            Backup.Metric vmBackupMetric = new Backup.Metric(vmBackupSize,vmBackupProtectedSize);
+            LOG.debug(String.format("Metrics for VM [uuid: %s, name: %s] is [backup size: %s, data size: %s].", vm.getUuid(),
+                    vm.getInstanceName(), vmBackupMetric.getBackupSize(), vmBackupMetric.getDataSize()));
+            metrics.put(vm, vmBackupMetric);
+        }
+        return metrics;
+    }
+
+    @Override
+    public boolean assignVMToBackupOffering(VirtualMachine vm, BackupOffering backupOffering) {
+        return Hypervisor.HypervisorType.KVM.equals(vm.getHypervisorType());
+    }
+
+    @Override
+    public boolean removeVMFromBackupOffering(VirtualMachine vm) {
+        return true;
+    }
+
+    @Override
+    public boolean willDeleteBackupsOnOfferingRemoval() {
+        return false;
+    }
+
+    @Override
+    public void syncBackups(VirtualMachine vm, Backup.Metric metric) {
+        // TODO: check and sum/return backups metrics on per VM basis
+    }
+
+    @Override
+    public List<BackupOffering> listBackupOfferings(Long zoneId) {
+        final List<BackupRepository> repositories = backupRepositoryDao.listByZoneAndProvider(zoneId, getName());
+        final List<BackupOffering> offerings = new ArrayList<>();
+        for (final BackupRepository repository : repositories) {
+            offerings.add(new NasBackupOffering(repository.getName(), repository.getUuid()));
+        }
+        return offerings;
+    }
+
+    @Override
+    public boolean isValidProviderOffering(Long zoneId, String uuid) {
+        return true;
+    }
+
+    @Override
+    public ConfigKey<?>[] getConfigKeys() {
+        return new ConfigKey[]{
+        };
+    }
+
+    @Override
+    public String getName() {
+        return "nas";

Review Comment:
   use string constant, also use the same in NasBackupOffering.java , etc



-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM (experimental) [cloudstack]

["blueorangutan (via GitHub)" <gi...@apache.org>]

blueorangutan commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2320068066

   @shwstppr a [SL] Jenkins job has been kicked to build packages. It will be bundled with  KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM (experimental) [cloudstack]

["Pearl1594 (via GitHub)" <gi...@apache.org>]

Pearl1594 commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2321672568

   @blueorangutan package


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM (experimental) [cloudstack]

["blueorangutan (via GitHub)" <gi...@apache.org>]

blueorangutan commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2322030889

   Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 10888


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM (experimental) [cloudstack]

["rohityadavcloud (via GitHub)" <gi...@apache.org>]

rohityadavcloud commented on code in PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#discussion_r1734604737


##########
ui/src/views/AutogenView.vue:
##########
@@ -1174,6 +1174,7 @@ export default {
 
       this.showAction = true
       const listIconForFillValues = ['copy-outlined', 'CopyOutlined', 'edit-outlined', 'EditOutlined', 'share-alt-outlined', 'ShareAltOutlined']
+      console.log(this.currentAction.paramFields)

Review Comment:
   ```suggestion
   ```



-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM (experimental) [cloudstack]

["blueorangutan (via GitHub)" <gi...@apache.org>]

blueorangutan commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2325353882

   @Pearl1594 a [SL] Jenkins job has been kicked to build packages. It will be bundled with  KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM (experimental) [cloudstack]

["Pearl1594 (via GitHub)" <gi...@apache.org>]

Pearl1594 commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2325396976

   @blueorangutan test


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM (experimental) [cloudstack]

["rohityadavcloud (via GitHub)" <gi...@apache.org>]

rohityadavcloud commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2316927237

   There isn't a design doc or spec, I'll submit a doc PR which can be used for notes. The plugin simply uses existing B&R framework and follows on the footsteps of `networker` plugin, so its well isolated as yet another plugin - all supported B&R functionality work as-is, doc PR will make it further clear. 
   
   @blueorangutan test


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM (experimental) [cloudstack]

["blueorangutan (via GitHub)" <gi...@apache.org>]

blueorangutan commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2320058230

   @rajujith a [SL] Jenkins job has been kicked to build packages. It will be bundled with  KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM (experimental) [cloudstack]

["shwstppr (via GitHub)" <gi...@apache.org>]

shwstppr commented on code in PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#discussion_r1737934022


##########
server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java:
##########
@@ -23,6 +23,7 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.TimeZone;

Review Comment:
   ```suggestion
   import java.util.TimeZone;
   ```



-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM (experimental) [cloudstack]

["rajujith (via GitHub)" <gi...@apache.org>]

rajujith commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2320055448

   @blueorangutan package


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM (experimental) [cloudstack]

["blueorangutan (via GitHub)" <gi...@apache.org>]

blueorangutan commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2320178656

   Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 10876


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["shwstppr (via GitHub)" <gi...@apache.org>]

shwstppr commented on code in PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#discussion_r1743469075


##########
api/src/main/java/org/apache/cloudstack/api/command/user/backup/ListBackupScheduleCmd.java:
##########
@@ -74,9 +79,14 @@ public Long getVmId() {
     @Override
     public void execute() throws ResourceUnavailableException, InsufficientCapacityException, ServerApiException, ConcurrentOperationException, ResourceAllocationException, NetworkRuleConflictException {
         try{
-            BackupSchedule schedule = backupManager.listBackupSchedule(getVmId());
-            if (schedule != null) {
-                BackupScheduleResponse response = _responseGenerator.createBackupScheduleResponse(schedule);
+            List<BackupSchedule> schedules = backupManager.listBackupSchedule(getVmId());
+            ListResponse<BackupScheduleResponse> response = new ListResponse<>();
+            List<BackupScheduleResponse> scheduleResponses = new ArrayList<>();
+            if (Objects.nonNull(schedules) && !schedules.isEmpty()) {

Review Comment:
   CollectionUtils?



##########
engine/schema/src/main/java/org/apache/cloudstack/backup/BackupVO.java:
##########
@@ -82,6 +87,9 @@ public class BackupVO implements Backup {
     @Column(name = "zone_id")
     private long zoneId;
 
+    @Column(name = "backed_volumes", length = 65535)
+    protected String backedVolumes;

Review Comment:
   What does this store? UUID or path? Would it have been better to use a details or backup_volume_map table?



##########
api/src/main/java/org/apache/cloudstack/backup/Backup.java:
##########
@@ -141,5 +142,6 @@ public String toString() {
     Backup.Status getStatus();
     Long getSize();
     Long getProtectedSize();
+    List<VolumeInfo> getBackedVolumes();

Review Comment:
   minor - getBackedUpVolumes?



##########
engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupDaoImpl.java:
##########
@@ -161,7 +162,14 @@ public BackupResponse newBackupResponse(Backup backup) {
         response.setSize(backup.getSize());
         response.setProtectedSize(backup.getProtectedSize());
         response.setStatus(backup.getStatus());
-        response.setVolumes(new Gson().toJson(vm.getBackupVolumeList().toArray(), Backup.VolumeInfo[].class));
+        // ACS 4.20: For backups taken prior this release the backup.backed_volumes column would be empty hence use vm_instance.backup_volumes

Review Comment:
   Understood the use `backup.backed_volumes` wrt my preivous comment.



-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] [WIP] backup: Simple NAS backup plugin for KVM (experimental) [cloudstack]

["blueorangutan (via GitHub)" <gi...@apache.org>]

blueorangutan commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2265138232

   Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 10548


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] [WIP] backup: Simple NAS backup plugin for KVM (experimental) [cloudstack]

["rohityadavcloud (via GitHub)" <gi...@apache.org>]

rohityadavcloud commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2265030665

   @blueorangutan package


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM (experimental) [cloudstack]

["blueorangutan (via GitHub)" <gi...@apache.org>]

blueorangutan commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2325397299

   @Pearl1594 a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["blueorangutan (via GitHub)" <gi...@apache.org>]

blueorangutan commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2331620894

   @rohityadavcloud a [SL] Jenkins job has been kicked to build packages. It will be bundled with  KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["rohityadavcloud (via GitHub)" <gi...@apache.org>]

rohityadavcloud commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2332204039

   Merging this based on reviews, manual QA and GitHub Actions tests. Will continue further test matrix on health check PR. 


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM (experimental) [cloudstack]

["blueorangutan (via GitHub)" <gi...@apache.org>]

blueorangutan commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2322373713

   <b>[SF] Trillian test result (tid-11248)</b>
   Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
   Total time taken: 49729 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9451-t11248-kvm-ol8.zip
   Smoke tests completed. 139 look OK, 0 have errors, 0 did not run
   Only failed and skipped tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["sureshanaparti (via GitHub)" <gi...@apache.org>]

sureshanaparti commented on code in PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#discussion_r1745226517


##########
engine/schema/src/main/resources/META-INF/db/schema-41910to42000.sql:
##########
@@ -141,6 +141,26 @@ CREATE TABLE `cloud`.`webhook_delivery` (
   CONSTRAINT `fk_webhook__webhook_id` FOREIGN KEY (`webhook_id`) REFERENCES `webhook`(`id`) ON DELETE CASCADE
 ) ENGINE=InnoDB DEFAULT CHARSET=utf8;
 
+-- Backup Repository NAS feature
+DROP TABLE IF EXISTS `cloud`.`backup_repository`;
+CREATE TABLE `cloud`.`backup_repository` (
+  `id` bigint unsigned NOT NULL AUTO_INCREMENT COMMENT 'id of the backup repository',
+  `uuid` varchar(255) NOT NULL COMMENT 'uuid of the backup repository',
+  `name` varchar(255) NOT NULL COMMENT 'name of the backup repository',

Review Comment:
   ```suggestion
     `name` varchar(255) CHARACTER SET utf8mb4 NOT NULL COMMENT 'name of the backup repository',
   ```
   
   to support unicode chars in the name



-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["blueorangutan (via GitHub)" <gi...@apache.org>]

blueorangutan commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2330474166

   Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 10972


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["blueorangutan (via GitHub)" <gi...@apache.org>]

blueorangutan commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2333363661

   <b>[SF] Trillian test result (tid-11377)</b>
   Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
   Total time taken: 54143 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9451-t11377-kvm-ol8.zip
   Smoke tests completed. 139 look OK, 1 have errors, 0 did not run
   Only failed and skipped tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_mount_shared_fs | `Failure` | 84.98 | test_sharedfs_lifecycle.py
   test_resize_shared_fs | `Error` | 0.44 | test_sharedfs_lifecycle.py
   


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["rajujith (via GitHub)" <gi...@apache.org>]

rajujith commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2330754403

   @blueorangutan  package


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["rohityadavcloud (via GitHub)" <gi...@apache.org>]

rohityadavcloud commented on code in PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#discussion_r1745489431


##########
server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java:
##########
@@ -945,6 +985,9 @@ public List<Class<?>> getCommands() {
         cmdList.add(RestoreBackupCmd.class);
         cmdList.add(DeleteBackupCmd.class);
         cmdList.add(RestoreVolumeFromBackupAndAttachToVMCmd.class);
+        cmdList.add(AddBackupRepositoryCmd.class);
+        cmdList.add(DeleteBackupRepositoryCmd.class);
+        cmdList.add(ListBackupRepositoriesCmd.class);

Review Comment:
   Actually I double checked, plugins are configureable even on a per zone level - so we'll need to export all the APIs when the feature is enabled.



-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["rohityadavcloud (via GitHub)" <gi...@apache.org>]

rohityadavcloud commented on code in PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#discussion_r1745268513


##########
server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java:
##########
@@ -945,6 +985,9 @@ public List<Class<?>> getCommands() {
         cmdList.add(RestoreBackupCmd.class);
         cmdList.add(DeleteBackupCmd.class);
         cmdList.add(RestoreVolumeFromBackupAndAttachToVMCmd.class);
+        cmdList.add(AddBackupRepositoryCmd.class);
+        cmdList.add(DeleteBackupRepositoryCmd.class);
+        cmdList.add(ListBackupRepositoriesCmd.class);

Review Comment:
   @Pearl1594 maybe we can export/enable the APIs only when nas plugin is enabled?



-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM (experimental) [cloudstack]

["blueorangutan (via GitHub)" <gi...@apache.org>]

blueorangutan commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2315224264

   @rohityadavcloud a [SL] Jenkins job has been kicked to build packages. It will be bundled with  KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM (experimental) [cloudstack]

["rohityadavcloud (via GitHub)" <gi...@apache.org>]

rohityadavcloud commented on code in PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#discussion_r1736163976


##########
plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java:
##########
@@ -0,0 +1,434 @@
+// 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.backup;
+
+import com.cloud.agent.AgentManager;
+import com.cloud.dc.dao.ClusterDao;
+import com.cloud.exception.AgentUnavailableException;
+import com.cloud.exception.OperationTimedoutException;
+import com.cloud.host.Host;
+import com.cloud.host.HostVO;
+import com.cloud.host.Status;
+import com.cloud.host.dao.HostDao;
+import com.cloud.hypervisor.Hypervisor;
+import com.cloud.storage.ScopeType;
+import com.cloud.storage.StoragePoolHostVO;
+import com.cloud.storage.Volume;
+import com.cloud.storage.VolumeVO;
+import com.cloud.storage.dao.StoragePoolHostDao;
+import com.cloud.storage.dao.VolumeDao;
+import com.cloud.utils.Pair;
+import com.cloud.utils.component.AdapterBase;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.vm.VirtualMachine;
+import com.cloud.vm.dao.VMInstanceDao;
+import org.apache.cloudstack.backup.dao.BackupDao;
+import org.apache.cloudstack.backup.dao.BackupOfferingDao;
+import org.apache.cloudstack.backup.dao.BackupRepositoryDao;
+import org.apache.cloudstack.framework.config.ConfigKey;
+import org.apache.cloudstack.framework.config.Configurable;
+import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
+import org.apache.commons.collections.CollectionUtils;
+import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.LogManager;
+import javax.inject.Inject;
+import java.text.SimpleDateFormat;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.Date;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import java.util.HashMap;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.UUID;
+
+public class NASBackupProvider extends AdapterBase implements BackupProvider, Configurable {
+    private static final Logger LOG = LogManager.getLogger(NASBackupProvider.class);
+
+    @Inject
+    private BackupDao backupDao;
+
+    @Inject
+    private BackupRepositoryDao backupRepositoryDao;
+
+    @Inject
+    private BackupOfferingDao backupOfferingDao;
+
+    @Inject
+    private HostDao hostDao;
+
+    @Inject
+    private ClusterDao clusterDao;
+
+    @Inject
+    private VolumeDao volumeDao;
+
+    @Inject
+    private StoragePoolHostDao storagePoolHostDao;
+
+    @Inject
+    private VMInstanceDao vmInstanceDao;
+
+    @Inject
+    private PrimaryDataStoreDao primaryDataStoreDao;
+
+    @Inject
+    private AgentManager agentManager;
+
+    protected Host getLastVMHypervisorHost(VirtualMachine vm) {
+        Long hostId = vm.getLastHostId();
+        if (hostId == null) {
+            LOG.debug("Cannot find last host for vm. This should never happen, please check your database.");
+            return null;
+        }
+        Host host = hostDao.findById(hostId);
+
+        if (host.getStatus() == Status.Up) {
+            return host;
+        } else {
+            // Try to find any Up host in the same cluster
+            for (final Host hostInCluster : hostDao.findHypervisorHostInCluster(host.getClusterId())) {
+                if (hostInCluster.getStatus() == Status.Up) {
+                    LOG.debug("Found Host " + hostInCluster.getName());
+                    return hostInCluster;
+                }
+            }
+        }
+        // Try to find any Host in the zone
+        for (final HostVO hostInZone : hostDao.listByDataCenterIdAndHypervisorType(host.getDataCenterId(), Hypervisor.HypervisorType.KVM)) {
+            if (hostInZone.getStatus() == Status.Up) {
+                LOG.debug("Found Host " + hostInZone.getName());
+                return hostInZone;
+            }
+        }
+        return null;
+    }
+
+    protected Host getVMHypervisorHost(VirtualMachine vm) {
+        Long hostId = vm.getHostId();
+        if (hostId == null && VirtualMachine.State.Running.equals(vm.getState())) {
+            throw new CloudRuntimeException(String.format("Unable to find the hypervisor host for %s. Make sure the virtual machine is running", vm.getName()));
+        }
+        if (VirtualMachine.State.Stopped.equals(vm.getState())) {
+            hostId = vm.getLastHostId();
+        }
+        if (hostId == null) {
+            throw new CloudRuntimeException(String.format("Unable to find the hypervisor host for stopped VM: %s", vm));
+        }
+        final Host host = hostDao.findById(hostId);
+        if (host == null || !Status.Up.equals(host.getStatus()) || !Hypervisor.HypervisorType.KVM.equals(host.getHypervisorType())) {
+            throw new CloudRuntimeException("Unable to contact backend control plane to initiate backup");
+        }
+        return host;
+    }
+
+    @Override
+    public boolean takeBackup(final VirtualMachine vm) {
+        final Host host = getVMHypervisorHost(vm);
+
+        final BackupRepository backupRepository = backupRepositoryDao.findByBackupOfferingId(vm.getBackupOfferingId());
+        if (backupRepository == null) {
+            throw new CloudRuntimeException("No valid backup repository found for the VM, please check the attached backup offering");
+        }
+
+        final Date creationDate = new Date();
+        final String backupPath = String.format("%s/%s", vm.getInstanceName(),
+                new SimpleDateFormat("yyyy.MM.dd.HH.mm.ss").format(creationDate));
+
+        TakeBackupCommand command = new TakeBackupCommand(vm.getInstanceName(), backupPath);
+        command.setBackupRepoType(backupRepository.getType());
+        command.setBackupRepoAddress(backupRepository.getAddress());
+        command.setMountOptions(backupRepository.getMountOptions());
+
+        if (VirtualMachine.State.Stopped.equals(vm.getState())) {
+            List<VolumeVO> vmVolumes = volumeDao.findByInstance(vm.getId());
+            List<String> volumePaths = getVolumePaths(vmVolumes);
+            command.setVolumePaths(volumePaths);
+        }

Review Comment:
   ISO backups are not considered.



-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM (experimental) [cloudstack]

["rajujith (via GitHub)" <gi...@apache.org>]

rajujith commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2320006440

   @blueorangutan  package
   


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["blueorangutan (via GitHub)" <gi...@apache.org>]

blueorangutan commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2328361322

   @rohityadavcloud a [SL] Jenkins job has been kicked to build packages. It will be bundled with  KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["rohityadavcloud (via GitHub)" <gi...@apache.org>]

rohityadavcloud commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2328360037

   @blueorangutan package


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["blueorangutan (via GitHub)" <gi...@apache.org>]

blueorangutan commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2329009542

   <b>[SF] Trillian test result (tid-11318)</b>
   Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
   Total time taken: 54306 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9451-t11318-kvm-ol8.zip
   Smoke tests completed. 138 look OK, 1 have errors, 0 did not run
   Only failed and skipped tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_01_secure_vm_migration | `Error` | 306.60 | test_vm_life_cycle.py
   test_02_unsecure_vm_migration | `Error` | 476.94 | test_vm_life_cycle.py
   test_03_secured_to_nonsecured_vm_migration | `Error` | 379.11 | test_vm_life_cycle.py
   


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["blueorangutan (via GitHub)" <gi...@apache.org>]

blueorangutan commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2330252362

   @Pearl1594 a [SL] Jenkins job has been kicked to build packages. It will be bundled with  KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] [WIP] backup: Simple NAS backup plugin for KVM (experimental) [cloudstack]

["blueorangutan (via GitHub)" <gi...@apache.org>]

blueorangutan commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2284568355

   @Pearl1594 a [SL] Jenkins job has been kicked to build packages. It will be bundled with  KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM (experimental) [cloudstack]

["Pearl1594 (via GitHub)" <gi...@apache.org>]

Pearl1594 commented on code in PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#discussion_r1741234264


##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java:
##########
@@ -0,0 +1,181 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package com.cloud.hypervisor.kvm.resource.wrapper;
+
+import com.cloud.agent.api.Answer;
+import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
+import com.cloud.resource.CommandWrapper;
+import com.cloud.resource.ResourceWrapper;
+import com.cloud.utils.Pair;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.utils.script.Script;
+import org.apache.cloudstack.backup.BackupAnswer;
+import org.apache.cloudstack.backup.RestoreBackupCommand;
+import org.apache.commons.lang3.RandomStringUtils;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.nio.file.StandardCopyOption;
+import java.util.List;
+import java.util.Locale;
+import java.util.Objects;
+
+@ResourceWrapper(handles = RestoreBackupCommand.class)
+public class LibvirtRestoreBackupCommandWrapper extends CommandWrapper<RestoreBackupCommand, Answer, LibvirtComputingResource> {
+    private static final String BACKUP_TEMP_FILE_PREFIx = "csbackup";
+    private static final String MOUNT_COMMAND = "sudo mount -t %s %s %s";
+    private static final String UMOUNT_COMMAND = "sudo umount %s";
+    private static final String FILE_PATH_PLACEHOLDER = "%s/%s";
+
+    @Override
+    public Answer execute(RestoreBackupCommand command, LibvirtComputingResource serverResource) {
+        String vmName = command.getVmName();
+        String backupPath = command.getBackupPath();
+        String backupRepoAddress = command.getBackupRepoAddress();
+        String backupRepoType = command.getBackupRepoType();
+        String mountOptions = command.getMountOptions();
+        Boolean vmExists = command.isVmExists();
+        String diskType = command.getDiskType();
+        Long deviceId = command.getDeviceId();
+        List<String> volumePaths = command.getVolumePaths();
+        String restoreVolumeUuid = command.getRestoreVolumeUUID();
+
+        String newVolumeId = null;
+        if (Objects.isNull(vmExists)) {
+            String volumePath = volumePaths.get(0);
+            int lastIndex = volumePath.lastIndexOf("/");
+            newVolumeId = volumePath.substring(lastIndex + 1);
+            restoreVolume(backupPath, backupRepoType, backupRepoAddress, volumePath, diskType, deviceId, restoreVolumeUuid);
+        } else if (vmExists) {
+            restoreVolumesOfExistingVM(volumePaths, backupPath, backupRepoType, backupRepoAddress, mountOptions);
+        } else {
+            restoreVolumesOfDestroyedVMs(volumePaths, vmName, backupPath, backupRepoType, backupRepoAddress, mountOptions);
+        }
+
+        return new BackupAnswer(command, true, newVolumeId);
+    }
+
+    private void restoreVolumesOfExistingVM(List<String> volumePaths, String backupPath,
+                                             String backupRepoType, String backupRepoAddress, String mountOptions) {
+        String diskType = "root";
+        String mountDirectory = mountBackupDirectory(backupRepoAddress, backupRepoType);
+        try {
+            for (int idx = 0; idx < volumePaths.size(); idx++) {
+                String volumePath = volumePaths.get(idx);
+                Pair<String, String> bkpPathAndVolUuid = getBackupPath(mountDirectory, volumePath, backupPath, diskType, idx, null);
+                diskType = "datadisk";
+                try {
+                    replaceVolumeWithBackup(volumePath, bkpPathAndVolUuid.first());
+                } catch (IOException e) {
+                    throw new CloudRuntimeException(String.format("Unable to revert backup for volume [%s] due to [%s].", bkpPathAndVolUuid.second(), e.getMessage()), e);

Review Comment:
   Nothing as of now. One can re-attempt again.



-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM (experimental) [cloudstack]

["blueorangutan (via GitHub)" <gi...@apache.org>]

blueorangutan commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2327229621

   @Pearl1594 a [SL] Jenkins job has been kicked to build packages. It will be bundled with  KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM (experimental) [cloudstack]

["Pearl1594 (via GitHub)" <gi...@apache.org>]

Pearl1594 commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2327470181

   @blueorangutan test


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM (experimental) [cloudstack]

["shwstppr (via GitHub)" <gi...@apache.org>]

shwstppr commented on code in PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#discussion_r1737941916


##########
core/src/main/java/org/apache/cloudstack/backup/DeleteBackupCommand.java:
##########
@@ -0,0 +1,76 @@
+//
+// 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.backup;
+
+import com.cloud.agent.api.Command;
+import com.cloud.agent.api.LogLevel;
+
+public class DeleteBackupCommand extends Command {
+    private String backupPath;
+    private String backupRepoType;
+    private String backupRepoAddress;
+    @LogLevel(LogLevel.Log4jLevel.Off)
+    private String mountOptions;
+
+    public DeleteBackupCommand(String backupPath, String backupRepoType, String backupRepoAddress, String mountOptions) {
+        super();
+        this.backupPath = backupPath;
+        this.backupRepoType = backupRepoType;
+        this.backupRepoAddress = backupRepoAddress;
+        this.mountOptions = mountOptions;
+    }
+
+    public String getBackupPath() {
+        return backupPath;
+    }
+
+    public void setBackupPath(String backupPath) {
+        this.backupPath = backupPath;
+    }
+
+    public String getBackupRepoType() {
+        return backupRepoType;
+    }
+
+    public void setBackupRepoType(String backupRepoType) {
+        this.backupRepoType = backupRepoType;
+    }
+
+    public String getBackupRepoAddress() {
+        return backupRepoAddress;
+    }
+
+    public void setBackupRepoAddress(String backupRepoAddress) {
+        this.backupRepoAddress = backupRepoAddress;
+    }
+
+    public String getMountOptions() {
+        return mountOptions == null ? "" : mountOptions;
+    }
+
+    public void setMountOptions(String mountOptions) {
+        this.mountOptions = mountOptions;
+    }
+
+    @Override
+    public boolean executeInSequence() {
+        return true;
+    }
+}

Review Comment:
   GH action Lint error
   ```suggestion
   }
   
   ```



-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["blueorangutan (via GitHub)" <gi...@apache.org>]

blueorangutan commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2331162746

   @rohityadavcloud a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["rohityadavcloud (via GitHub)" <gi...@apache.org>]

rohityadavcloud commented on code in PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#discussion_r1743420787


##########
plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java:
##########
@@ -0,0 +1,434 @@
+// 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.backup;
+
+import com.cloud.agent.AgentManager;
+import com.cloud.dc.dao.ClusterDao;
+import com.cloud.exception.AgentUnavailableException;
+import com.cloud.exception.OperationTimedoutException;
+import com.cloud.host.Host;
+import com.cloud.host.HostVO;
+import com.cloud.host.Status;
+import com.cloud.host.dao.HostDao;
+import com.cloud.hypervisor.Hypervisor;
+import com.cloud.storage.ScopeType;
+import com.cloud.storage.StoragePoolHostVO;
+import com.cloud.storage.Volume;
+import com.cloud.storage.VolumeVO;
+import com.cloud.storage.dao.StoragePoolHostDao;
+import com.cloud.storage.dao.VolumeDao;
+import com.cloud.utils.Pair;
+import com.cloud.utils.component.AdapterBase;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.vm.VirtualMachine;
+import com.cloud.vm.dao.VMInstanceDao;
+import org.apache.cloudstack.backup.dao.BackupDao;
+import org.apache.cloudstack.backup.dao.BackupOfferingDao;
+import org.apache.cloudstack.backup.dao.BackupRepositoryDao;
+import org.apache.cloudstack.framework.config.ConfigKey;
+import org.apache.cloudstack.framework.config.Configurable;
+import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
+import org.apache.commons.collections.CollectionUtils;
+import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.LogManager;
+import javax.inject.Inject;
+import java.text.SimpleDateFormat;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.Date;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import java.util.HashMap;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.UUID;
+
+public class NASBackupProvider extends AdapterBase implements BackupProvider, Configurable {
+    private static final Logger LOG = LogManager.getLogger(NASBackupProvider.class);
+
+    @Inject
+    private BackupDao backupDao;
+
+    @Inject
+    private BackupRepositoryDao backupRepositoryDao;
+
+    @Inject
+    private BackupOfferingDao backupOfferingDao;
+
+    @Inject
+    private HostDao hostDao;
+
+    @Inject
+    private ClusterDao clusterDao;
+
+    @Inject
+    private VolumeDao volumeDao;
+
+    @Inject
+    private StoragePoolHostDao storagePoolHostDao;
+
+    @Inject
+    private VMInstanceDao vmInstanceDao;
+
+    @Inject
+    private PrimaryDataStoreDao primaryDataStoreDao;
+
+    @Inject
+    private AgentManager agentManager;
+
+    protected Host getLastVMHypervisorHost(VirtualMachine vm) {
+        Long hostId = vm.getLastHostId();
+        if (hostId == null) {
+            LOG.debug("Cannot find last host for vm. This should never happen, please check your database.");

Review Comment:
   @Pearl1594 to check & advise.



##########
plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java:
##########
@@ -0,0 +1,434 @@
+// 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.backup;
+
+import com.cloud.agent.AgentManager;
+import com.cloud.dc.dao.ClusterDao;
+import com.cloud.exception.AgentUnavailableException;
+import com.cloud.exception.OperationTimedoutException;
+import com.cloud.host.Host;
+import com.cloud.host.HostVO;
+import com.cloud.host.Status;
+import com.cloud.host.dao.HostDao;
+import com.cloud.hypervisor.Hypervisor;
+import com.cloud.storage.ScopeType;
+import com.cloud.storage.StoragePoolHostVO;
+import com.cloud.storage.Volume;
+import com.cloud.storage.VolumeVO;
+import com.cloud.storage.dao.StoragePoolHostDao;
+import com.cloud.storage.dao.VolumeDao;
+import com.cloud.utils.Pair;
+import com.cloud.utils.component.AdapterBase;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.vm.VirtualMachine;
+import com.cloud.vm.dao.VMInstanceDao;
+import org.apache.cloudstack.backup.dao.BackupDao;
+import org.apache.cloudstack.backup.dao.BackupOfferingDao;
+import org.apache.cloudstack.backup.dao.BackupRepositoryDao;
+import org.apache.cloudstack.framework.config.ConfigKey;
+import org.apache.cloudstack.framework.config.Configurable;
+import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
+import org.apache.commons.collections.CollectionUtils;
+import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.LogManager;
+import javax.inject.Inject;
+import java.text.SimpleDateFormat;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.Date;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import java.util.HashMap;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.UUID;
+
+public class NASBackupProvider extends AdapterBase implements BackupProvider, Configurable {
+    private static final Logger LOG = LogManager.getLogger(NASBackupProvider.class);
+
+    @Inject
+    private BackupDao backupDao;
+
+    @Inject
+    private BackupRepositoryDao backupRepositoryDao;
+
+    @Inject
+    private BackupOfferingDao backupOfferingDao;
+
+    @Inject
+    private HostDao hostDao;
+
+    @Inject
+    private ClusterDao clusterDao;
+
+    @Inject
+    private VolumeDao volumeDao;
+
+    @Inject
+    private StoragePoolHostDao storagePoolHostDao;
+
+    @Inject
+    private VMInstanceDao vmInstanceDao;
+
+    @Inject
+    private PrimaryDataStoreDao primaryDataStoreDao;
+
+    @Inject
+    private AgentManager agentManager;
+
+    protected Host getLastVMHypervisorHost(VirtualMachine vm) {
+        Long hostId = vm.getLastHostId();
+        if (hostId == null) {
+            LOG.debug("Cannot find last host for vm. This should never happen, please check your database.");
+            return null;
+        }
+        Host host = hostDao.findById(hostId);
+
+        if (host.getStatus() == Status.Up) {
+            return host;
+        } else {
+            // Try to find any Up host in the same cluster
+            for (final Host hostInCluster : hostDao.findHypervisorHostInCluster(host.getClusterId())) {
+                if (hostInCluster.getStatus() == Status.Up) {
+                    LOG.debug("Found Host " + hostInCluster.getName());
+                    return hostInCluster;
+                }
+            }
+        }
+        // Try to find any Host in the zone
+        for (final HostVO hostInZone : hostDao.listByDataCenterIdAndHypervisorType(host.getDataCenterId(), Hypervisor.HypervisorType.KVM)) {
+            if (hostInZone.getStatus() == Status.Up) {
+                LOG.debug("Found Host " + hostInZone.getName());
+                return hostInZone;
+            }
+        }
+        return null;
+    }
+
+    protected Host getVMHypervisorHost(VirtualMachine vm) {
+        Long hostId = vm.getHostId();
+        if (hostId == null && VirtualMachine.State.Running.equals(vm.getState())) {
+            throw new CloudRuntimeException(String.format("Unable to find the hypervisor host for %s. Make sure the virtual machine is running", vm.getName()));
+        }
+        if (VirtualMachine.State.Stopped.equals(vm.getState())) {
+            hostId = vm.getLastHostId();
+        }
+        if (hostId == null) {
+            throw new CloudRuntimeException(String.format("Unable to find the hypervisor host for stopped VM: %s", vm));
+        }
+        final Host host = hostDao.findById(hostId);
+        if (host == null || !Status.Up.equals(host.getStatus()) || !Hypervisor.HypervisorType.KVM.equals(host.getHypervisorType())) {
+            throw new CloudRuntimeException("Unable to contact backend control plane to initiate backup");
+        }
+        return host;
+    }
+
+    @Override
+    public boolean takeBackup(final VirtualMachine vm) {
+        final Host host = getVMHypervisorHost(vm);
+
+        final BackupRepository backupRepository = backupRepositoryDao.findByBackupOfferingId(vm.getBackupOfferingId());
+        if (backupRepository == null) {
+            throw new CloudRuntimeException("No valid backup repository found for the VM, please check the attached backup offering");
+        }
+
+        final Date creationDate = new Date();
+        final String backupPath = String.format("%s/%s", vm.getInstanceName(),
+                new SimpleDateFormat("yyyy.MM.dd.HH.mm.ss").format(creationDate));
+
+        TakeBackupCommand command = new TakeBackupCommand(vm.getInstanceName(), backupPath);
+        command.setBackupRepoType(backupRepository.getType());
+        command.setBackupRepoAddress(backupRepository.getAddress());
+        command.setMountOptions(backupRepository.getMountOptions());
+
+        if (VirtualMachine.State.Stopped.equals(vm.getState())) {
+            List<VolumeVO> vmVolumes = volumeDao.findByInstance(vm.getId());
+            List<String> volumePaths = getVolumePaths(vmVolumes);
+            command.setVolumePaths(volumePaths);
+        }
+
+        BackupAnswer answer = null;
+        try {
+            answer = (BackupAnswer) agentManager.send(host.getId(), command);
+        } catch (AgentUnavailableException e) {
+            throw new CloudRuntimeException("Unable to contact backend control plane to initiate backup");
+        } catch (OperationTimedoutException e) {
+            throw new CloudRuntimeException("Operation to initiate backup timed out, please try again");
+        }
+
+        if (answer != null && answer.getResult()) {
+            BackupVO backup = new BackupVO();
+            backup.setVmId(vm.getId());
+            backup.setExternalId(backupPath);
+            backup.setType("FULL");
+            backup.setDate(creationDate);
+            backup.setSize(answer.getSize());
+            long virtualSize = 0L;
+            for (final Volume volume: volumeDao.findByInstance(vm.getId())) {
+                if (Volume.State.Ready.equals(volume.getState())) {
+                    virtualSize += volume.getSize();
+                }
+            }
+            backup.setProtectedSize(Long.valueOf(virtualSize));
+            backup.setStatus(Backup.Status.BackedUp);
+            backup.setBackupOfferingId(vm.getBackupOfferingId());
+            backup.setAccountId(vm.getAccountId());
+            backup.setDomainId(vm.getDomainId());
+            backup.setZoneId(vm.getDataCenterId());
+            return backupDao.persist(backup) != null;
+        }
+
+        return answer.getResult();
+    }
+
+    @Override
+    public boolean restoreVMFromBackup(VirtualMachine vm, Backup backup) {
+        List<VolumeVO> volumes = volumeDao.findByInstance(vm.getId());
+        volumes.sort(Comparator.comparing(VolumeVO::getDeviceId));
+
+        LOG.debug(String.format("Restoring vm %s from backup %s on the NAS Backup Provider", vm.getUuid(), backup.getUuid()));
+        BackupRepository backupRepository = getBackupRepository(vm, backup);
+
+        final Host host = getLastVMHypervisorHost(vm);
+        RestoreBackupCommand restoreCommand = new RestoreBackupCommand();
+        restoreCommand.setBackupPath(backup.getExternalId());
+        restoreCommand.setBackupRepoType(backupRepository.getType());
+        restoreCommand.setBackupRepoAddress(backupRepository.getAddress());
+        restoreCommand.setVmName(vm.getName());
+        restoreCommand.setVolumePaths(getVolumePaths(volumes));
+        restoreCommand.setVmExists(vm.getRemoved() == null);
+
+        BackupAnswer answer = null;
+        try {
+            answer = (BackupAnswer) agentManager.send(host.getId(), restoreCommand);
+        } catch (AgentUnavailableException e) {
+            throw new CloudRuntimeException("Unable to contact backend control plane to initiate backup");
+        } catch (OperationTimedoutException e) {
+            throw new CloudRuntimeException("Operation to initiate backup timed out, please try again");
+        }
+        return answer.getResult();
+    }
+
+    private List<String> getVolumePaths(List<VolumeVO> volumes) {
+        List<String> volumePaths = new ArrayList<>();
+        for (VolumeVO volume : volumes) {
+            StoragePoolVO storagePool = primaryDataStoreDao.findById(volume.getPoolId());
+            if (Objects.isNull(storagePool)) {
+                throw new CloudRuntimeException("Unable to find storage pool associated to the volume");
+            }
+            String volumePathPrefix = String.format("/mnt/%s", storagePool.getUuid());
+            if (ScopeType.HOST.equals(storagePool.getScope())) {
+                volumePathPrefix = storagePool.getPath();
+            }
+            volumePaths.add(String.format("%s/%s", volumePathPrefix, volume.getPath()));
+        }
+        return volumePaths;
+    }
+
+    @Override
+    public Pair<Boolean, String> restoreBackedUpVolume(Backup backup, String volumeUuid, String hostIp, String dataStoreUuid) {
+        final VolumeVO volume = volumeDao.findByUuid(volumeUuid);
+        final VirtualMachine backupSourceVm = vmInstanceDao.findById(backup.getVmId());
+        final StoragePoolHostVO dataStore = storagePoolHostDao.findByUuid(dataStoreUuid);
+        final HostVO hostVO = hostDao.findByIp(hostIp);
+        final Long zoneId = backup.getZoneId();
+
+        // TODO: Find volume from backup volumes
+        Optional<Backup.VolumeInfo> matchingVolume = getBackedUpVolumeInfo(backupSourceVm.getBackupVolumeList(), volumeUuid);
+        Long backedUpVolumeSize = matchingVolume.isPresent() ? matchingVolume.get().getSize() : 0L;
+
+        LOG.debug("Restoring vm volume" + volumeUuid + "from backup " + backup.getUuid() + " on the NAS Backup Provider");
+        BackupRepository backupRepository = getBackupRepository(backupSourceVm, backup);
+
+        VolumeVO restoredVolume = new VolumeVO(Volume.Type.DATADISK, null, backup.getZoneId(),
+                backup.getDomainId(), backup.getAccountId(), 0, null,
+                backup.getSize(), null, null, null);
+        String volumeUUID = UUID.randomUUID().toString();
+        restoredVolume.setName("RestoredVol-"+volume.getName());
+        restoredVolume.setProvisioningType(volume.getProvisioningType());
+        restoredVolume.setUpdated(new Date());
+        restoredVolume.setUuid(volumeUUID);
+        restoredVolume.setRemoved(null);
+        restoredVolume.setDisplayVolume(true);
+        restoredVolume.setPoolId(dataStore.getPoolId());
+        restoredVolume.setPath(restoredVolume.getUuid());
+        restoredVolume.setState(Volume.State.Copying);
+        restoredVolume.setSize(backedUpVolumeSize);
+        restoredVolume.setDiskOfferingId(volume.getDiskOfferingId());
+
+        RestoreBackupCommand restoreCommand = new RestoreBackupCommand();
+        restoreCommand.setBackupPath(backup.getExternalId());
+        restoreCommand.setBackupRepoType(backupRepository.getType());
+        restoreCommand.setBackupRepoAddress(backupRepository.getAddress());
+        restoreCommand.setVmName(backupSourceVm.getName());
+        restoreCommand.setVolumePaths(Collections.singletonList(String.format("%s/%s", dataStore.getLocalPath(), volumeUUID)));
+        restoreCommand.setDiskType(volume.getVolumeType().name().toLowerCase(Locale.ROOT));
+        restoreCommand.setDeviceId(volume.getDeviceId());
+        restoreCommand.setVmExists(null);
+        restoreCommand.setRestoreVolumeUUID(volumeUuid);
+
+        BackupAnswer answer = null;
+        try {
+            answer = (BackupAnswer) agentManager.send(hostVO.getId(), restoreCommand);
+        } catch (AgentUnavailableException e) {
+            throw new CloudRuntimeException("Unable to contact backend control plane to initiate backup");
+        } catch (OperationTimedoutException e) {
+            throw new CloudRuntimeException("Operation to initiate backup timed out, please try again");
+        }
+
+        if (answer.getResult()) {
+            try {
+                volumeDao.persist(restoredVolume);
+            } catch (Exception e) {
+                throw new CloudRuntimeException("Unable to create restored volume due to: " + e);
+            }
+        }
+
+        return new Pair<>(answer.getResult(), answer.getDetails());
+    }
+
+    private BackupRepository getBackupRepository(VirtualMachine vm, Backup backup) {
+        BackupRepository backupRepository = backupRepositoryDao.findByBackupOfferingId(vm.getBackupOfferingId());
+        final String errorMessage = "No valid backup repository found for the VM, please check the attached backup offering";
+        if (backupRepository == null) {
+            logger.warn(errorMessage + "Re-attempting with the backup offering associated with the backup");
+        }
+        backupRepository = backupRepositoryDao.findByBackupOfferingId(backup.getBackupOfferingId());
+        if (backupRepository == null) {
+            throw new CloudRuntimeException(errorMessage);
+        }
+        return backupRepository;
+    }
+
+    private Optional<Backup.VolumeInfo> getBackedUpVolumeInfo(List<Backup.VolumeInfo> backedUpVolumes, String volumeUuid) {
+        return backedUpVolumes.stream()
+                .filter(v -> v.getUuid().equals(volumeUuid))
+                .findFirst();
+    }
+
+    @Override
+    public boolean deleteBackup(Backup backup, boolean forced) {
+        final BackupRepository backupRepository = backupRepositoryDao.findByBackupOfferingId(backup.getBackupOfferingId());
+        if (backupRepository == null) {
+            throw new CloudRuntimeException("No valid backup repository found for the VM, please check the attached backup offering");
+        }
+
+        // TODO: this can be any host in the cluster or last host

Review Comment:
   @Pearl1594 to check & advise.



-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM (experimental) [cloudstack]

["blueorangutan (via GitHub)" <gi...@apache.org>]

blueorangutan commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2327279805

   @Pearl1594 a [SL] Jenkins job has been kicked to build packages. It will be bundled with  KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["Pearl1594 (via GitHub)" <gi...@apache.org>]

Pearl1594 commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2330250240

   @blueorangutan package


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["sureshanaparti (via GitHub)" <gi...@apache.org>]

sureshanaparti commented on code in PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#discussion_r1745236345


##########
server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java:
##########
@@ -945,6 +985,9 @@ public List<Class<?>> getCommands() {
         cmdList.add(RestoreBackupCmd.class);
         cmdList.add(DeleteBackupCmd.class);
         cmdList.add(RestoreVolumeFromBackupAndAttachToVMCmd.class);
+        cmdList.add(AddBackupRepositoryCmd.class);
+        cmdList.add(DeleteBackupRepositoryCmd.class);
+        cmdList.add(ListBackupRepositoriesCmd.class);

Review Comment:
   Update cmd required? to update name or mount options?



-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["Pearl1594 (via GitHub)" <gi...@apache.org>]

Pearl1594 commented on code in PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#discussion_r1745687905


##########
plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java:
##########
@@ -0,0 +1,434 @@
+// 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.backup;
+
+import com.cloud.agent.AgentManager;
+import com.cloud.dc.dao.ClusterDao;
+import com.cloud.exception.AgentUnavailableException;
+import com.cloud.exception.OperationTimedoutException;
+import com.cloud.host.Host;
+import com.cloud.host.HostVO;
+import com.cloud.host.Status;
+import com.cloud.host.dao.HostDao;
+import com.cloud.hypervisor.Hypervisor;
+import com.cloud.storage.ScopeType;
+import com.cloud.storage.StoragePoolHostVO;
+import com.cloud.storage.Volume;
+import com.cloud.storage.VolumeVO;
+import com.cloud.storage.dao.StoragePoolHostDao;
+import com.cloud.storage.dao.VolumeDao;
+import com.cloud.utils.Pair;
+import com.cloud.utils.component.AdapterBase;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.vm.VirtualMachine;
+import com.cloud.vm.dao.VMInstanceDao;
+import org.apache.cloudstack.backup.dao.BackupDao;
+import org.apache.cloudstack.backup.dao.BackupOfferingDao;
+import org.apache.cloudstack.backup.dao.BackupRepositoryDao;
+import org.apache.cloudstack.framework.config.ConfigKey;
+import org.apache.cloudstack.framework.config.Configurable;
+import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
+import org.apache.commons.collections.CollectionUtils;
+import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.LogManager;
+import javax.inject.Inject;
+import java.text.SimpleDateFormat;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.Date;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import java.util.HashMap;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.UUID;
+
+public class NASBackupProvider extends AdapterBase implements BackupProvider, Configurable {
+    private static final Logger LOG = LogManager.getLogger(NASBackupProvider.class);
+
+    @Inject
+    private BackupDao backupDao;
+
+    @Inject
+    private BackupRepositoryDao backupRepositoryDao;
+
+    @Inject
+    private BackupOfferingDao backupOfferingDao;
+
+    @Inject
+    private HostDao hostDao;
+
+    @Inject
+    private ClusterDao clusterDao;
+
+    @Inject
+    private VolumeDao volumeDao;
+
+    @Inject
+    private StoragePoolHostDao storagePoolHostDao;
+
+    @Inject
+    private VMInstanceDao vmInstanceDao;
+
+    @Inject
+    private PrimaryDataStoreDao primaryDataStoreDao;
+
+    @Inject
+    private AgentManager agentManager;
+
+    protected Host getLastVMHypervisorHost(VirtualMachine vm) {
+        Long hostId = vm.getLastHostId();
+        if (hostId == null) {
+            LOG.debug("Cannot find last host for vm. This should never happen, please check your database.");

Review Comment:
   It could be considered a limitation currently with this experimental feature, and most likely would be an issue seen with the networker plugin too. May be worth improving this bit in a subsequent PR



-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["blueorangutan (via GitHub)" <gi...@apache.org>]

blueorangutan commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2333034642

   <b>[SF] Trillian test result (tid-11369)</b>
   Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
   Total time taken: 53228 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9451-t11369-kvm-ol8.zip
   Smoke tests completed. 138 look OK, 1 have errors, 0 did not run
   Only failed and skipped tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_01_secure_vm_migration | `Error` | 135.16 | test_vm_life_cycle.py
   test_01_secure_vm_migration | `Error` | 135.16 | test_vm_life_cycle.py
   


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["blueorangutan (via GitHub)" <gi...@apache.org>]

blueorangutan commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2330628058

   Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 10978


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["blueorangutan (via GitHub)" <gi...@apache.org>]

blueorangutan commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2331244472

   @rohityadavcloud a [SL] Jenkins job has been kicked to build packages. It will be bundled with  KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["rohityadavcloud (via GitHub)" <gi...@apache.org>]

rohityadavcloud commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2330571841

   @blueorangutan package


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] [WIP] backup: Simple NAS backup plugin for KVM (experimental) [cloudstack]

["Pearl1594 (via GitHub)" <gi...@apache.org>]

Pearl1594 commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2271591140

   @blueorangutan package


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] [WIP] backup: Simple NAS backup plugin for KVM (experimental) [cloudstack]

["blueorangutan (via GitHub)" <gi...@apache.org>]

blueorangutan commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2271916095

   Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 10580


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] [WIP] backup: Simple NAS backup plugin for KVM (experimental) [cloudstack]

["Pearl1594 (via GitHub)" <gi...@apache.org>]

Pearl1594 commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2291626124

   @blueorangutan package


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] [WIP] backup: Simple NAS backup plugin for KVM (experimental) [cloudstack]

["Pearl1594 (via GitHub)" <gi...@apache.org>]

Pearl1594 commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2284566270

   @blueorangutan package


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["rohityadavcloud (via GitHub)" <gi...@apache.org>]

rohityadavcloud commented on code in PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#discussion_r1745491178


##########
plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java:
##########
@@ -0,0 +1,442 @@
+// 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.backup;
+
+import com.cloud.agent.AgentManager;
+import com.cloud.dc.dao.ClusterDao;
+import com.cloud.exception.AgentUnavailableException;
+import com.cloud.exception.OperationTimedoutException;
+import com.cloud.host.Host;
+import com.cloud.host.HostVO;
+import com.cloud.host.Status;
+import com.cloud.host.dao.HostDao;
+import com.cloud.hypervisor.Hypervisor;
+import com.cloud.storage.ScopeType;
+import com.cloud.storage.StoragePoolHostVO;
+import com.cloud.storage.Volume;
+import com.cloud.storage.VolumeVO;
+import com.cloud.storage.dao.StoragePoolHostDao;
+import com.cloud.storage.dao.VolumeDao;
+import com.cloud.utils.Pair;
+import com.cloud.utils.component.AdapterBase;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.vm.VirtualMachine;
+import com.cloud.vm.dao.VMInstanceDao;
+import org.apache.cloudstack.backup.dao.BackupDao;
+import org.apache.cloudstack.backup.dao.BackupOfferingDao;
+import org.apache.cloudstack.backup.dao.BackupRepositoryDao;
+import org.apache.cloudstack.framework.config.ConfigKey;
+import org.apache.cloudstack.framework.config.Configurable;
+import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
+import org.apache.commons.collections.CollectionUtils;
+import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.LogManager;
+import javax.inject.Inject;
+import java.text.SimpleDateFormat;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Date;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import java.util.HashMap;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.UUID;
+import java.util.stream.Collectors;
+
+public class NASBackupProvider extends AdapterBase implements BackupProvider, Configurable {
+    private static final Logger LOG = LogManager.getLogger(NASBackupProvider.class);
+
+    @Inject
+    private BackupDao backupDao;
+
+    @Inject
+    private BackupRepositoryDao backupRepositoryDao;
+
+    @Inject
+    private BackupOfferingDao backupOfferingDao;
+
+    @Inject
+    private HostDao hostDao;
+
+    @Inject
+    private ClusterDao clusterDao;
+
+    @Inject
+    private VolumeDao volumeDao;
+
+    @Inject
+    private StoragePoolHostDao storagePoolHostDao;
+
+    @Inject
+    private VMInstanceDao vmInstanceDao;
+
+    @Inject
+    private PrimaryDataStoreDao primaryDataStoreDao;
+
+    @Inject
+    private AgentManager agentManager;
+
+    protected Host getLastVMHypervisorHost(VirtualMachine vm) {
+        Long hostId = vm.getLastHostId();
+        if (hostId == null) {
+            LOG.debug("Cannot find last host for vm. This should never happen, please check your database.");
+            return null;
+        }
+        Host host = hostDao.findById(hostId);
+
+        if (host.getStatus() == Status.Up) {
+            return host;
+        } else {
+            // Try to find any Up host in the same cluster
+            for (final Host hostInCluster : hostDao.findHypervisorHostInCluster(host.getClusterId())) {
+                if (hostInCluster.getStatus() == Status.Up) {
+                    LOG.debug("Found Host " + hostInCluster.getName());
+                    return hostInCluster;
+                }
+            }
+        }
+        // Try to find any Host in the zone
+        for (final HostVO hostInZone : hostDao.listByDataCenterIdAndHypervisorType(host.getDataCenterId(), Hypervisor.HypervisorType.KVM)) {
+            if (hostInZone.getStatus() == Status.Up) {
+                LOG.debug("Found Host " + hostInZone.getName());
+                return hostInZone;
+            }
+        }
+        return null;
+    }
+
+    protected Host getVMHypervisorHost(VirtualMachine vm) {
+        Long hostId = vm.getHostId();
+        if (hostId == null && VirtualMachine.State.Running.equals(vm.getState())) {
+            throw new CloudRuntimeException(String.format("Unable to find the hypervisor host for %s. Make sure the virtual machine is running", vm.getName()));
+        }
+        if (VirtualMachine.State.Stopped.equals(vm.getState())) {
+            hostId = vm.getLastHostId();
+        }
+        if (hostId == null) {
+            throw new CloudRuntimeException(String.format("Unable to find the hypervisor host for stopped VM: %s", vm));
+        }
+        final Host host = hostDao.findById(hostId);
+        if (host == null || !Status.Up.equals(host.getStatus()) || !Hypervisor.HypervisorType.KVM.equals(host.getHypervisorType())) {
+            throw new CloudRuntimeException("Unable to contact backend control plane to initiate backup");
+        }
+        return host;
+    }
+
+    @Override
+    public boolean takeBackup(final VirtualMachine vm) {
+        final Host host = getVMHypervisorHost(vm);
+
+        final BackupRepository backupRepository = backupRepositoryDao.findByBackupOfferingId(vm.getBackupOfferingId());
+        if (backupRepository == null) {
+            throw new CloudRuntimeException("No valid backup repository found for the VM, please check the attached backup offering");
+        }
+
+        final Date creationDate = new Date();
+        final String backupPath = String.format("%s/%s", vm.getInstanceName(),
+                new SimpleDateFormat("yyyy.MM.dd.HH.mm.ss").format(creationDate));
+
+        BackupVO backupVO = createBackupObject(vm, backupPath);
+        TakeBackupCommand command = new TakeBackupCommand(vm.getInstanceName(), backupPath);
+        command.setBackupRepoType(backupRepository.getType());
+        command.setBackupRepoAddress(backupRepository.getAddress());
+        command.setMountOptions(backupRepository.getMountOptions());
+
+        if (VirtualMachine.State.Stopped.equals(vm.getState())) {
+            List<VolumeVO> vmVolumes = volumeDao.findByInstance(vm.getId());
+            List<String> volumePaths = getVolumePaths(vmVolumes);
+            command.setVolumePaths(volumePaths);
+        }
+
+        BackupAnswer answer = null;
+        try {
+            answer = (BackupAnswer) agentManager.send(host.getId(), command);
+        } catch (AgentUnavailableException e) {
+            throw new CloudRuntimeException("Unable to contact backend control plane to initiate backup");
+        } catch (OperationTimedoutException e) {
+            throw new CloudRuntimeException("Operation to initiate backup timed out, please try again");
+        }
+
+        if (answer != null && answer.getResult()) {
+            backupVO.setDate(new Date());
+            backupVO.setSize(answer.getSize());
+            backupVO.setStatus(Backup.Status.BackedUp);
+            backupVO.setBackedUpVolumes(BackupManagerImpl.createVolumeInfoFromVolumes(volumeDao.findByInstance(vm.getId())));
+            return backupDao.update(backupVO.getId(), backupVO);
+        } else {
+            backupVO.setStatus(Backup.Status.Failed);
+            backupDao.remove(backupVO.getId());
+        }
+        return Objects.nonNull(answer) && answer.getResult();
+    }
+
+    private BackupVO createBackupObject(VirtualMachine vm, String backupPath) {
+        BackupVO backup = new BackupVO();
+        backup.setVmId(vm.getId());
+        backup.setExternalId(backupPath);
+        backup.setType("FULL");
+        backup.setDate(new Date());
+        long virtualSize = 0L;
+        for (final Volume volume: volumeDao.findByInstance(vm.getId())) {
+            if (Volume.State.Ready.equals(volume.getState())) {
+                virtualSize += volume.getSize();
+            }
+        }
+        backup.setProtectedSize(Long.valueOf(virtualSize));
+        backup.setStatus(Backup.Status.BackingUp);
+        backup.setBackupOfferingId(vm.getBackupOfferingId());
+        backup.setAccountId(vm.getAccountId());
+        backup.setDomainId(vm.getDomainId());
+        backup.setZoneId(vm.getDataCenterId());
+        return backupDao.persist(backup);
+    }
+
+    @Override
+    public boolean restoreVMFromBackup(VirtualMachine vm, Backup backup) {
+        List<Backup.VolumeInfo> backedVolumes = backup.getBackedUpVolumes();
+        List<VolumeVO> volumes = backedVolumes.stream().map(volume -> volumeDao.findByUuid(volume.getUuid())).collect(Collectors.toList());
+
+        LOG.debug("Restoring vm {} from backup {} on the NAS Backup Provider", vm.getUuid(), backup.getUuid());
+        BackupRepository backupRepository = getBackupRepository(vm, backup);
+
+        final Host host = getLastVMHypervisorHost(vm);
+        RestoreBackupCommand restoreCommand = new RestoreBackupCommand();
+        restoreCommand.setBackupPath(backup.getExternalId());
+        restoreCommand.setBackupRepoType(backupRepository.getType());
+        restoreCommand.setBackupRepoAddress(backupRepository.getAddress());
+        restoreCommand.setVmName(vm.getName());
+        restoreCommand.setVolumePaths(getVolumePaths(volumes));
+        restoreCommand.setVmExists(vm.getRemoved() == null);
+        restoreCommand.setVmState(vm.getState());
+
+        BackupAnswer answer = null;
+        try {
+            answer = (BackupAnswer) agentManager.send(host.getId(), restoreCommand);
+        } catch (AgentUnavailableException e) {
+            throw new CloudRuntimeException("Unable to contact backend control plane to initiate backup");
+        } catch (OperationTimedoutException e) {
+            throw new CloudRuntimeException("Operation to initiate backup timed out, please try again");
+        }
+        return answer.getResult();
+    }
+
+    private List<String> getVolumePaths(List<VolumeVO> volumes) {
+        List<String> volumePaths = new ArrayList<>();
+        for (VolumeVO volume : volumes) {
+            StoragePoolVO storagePool = primaryDataStoreDao.findById(volume.getPoolId());
+            if (Objects.isNull(storagePool)) {
+                throw new CloudRuntimeException("Unable to find storage pool associated to the volume");
+            }
+            String volumePathPrefix = String.format("/mnt/%s", storagePool.getUuid());
+            if (ScopeType.HOST.equals(storagePool.getScope())) {
+                volumePathPrefix = storagePool.getPath();
+            }
+            volumePaths.add(String.format("%s/%s", volumePathPrefix, volume.getPath()));
+        }
+        return volumePaths;
+    }
+
+    @Override
+    public Pair<Boolean, String> restoreBackedUpVolume(Backup backup, String volumeUuid, String hostIp, String dataStoreUuid, Pair<String, VirtualMachine.State> vmNameAndState) {
+        final VolumeVO volume = volumeDao.findByUuid(volumeUuid);
+        final VirtualMachine backupSourceVm = vmInstanceDao.findById(backup.getVmId());
+        final StoragePoolHostVO dataStore = storagePoolHostDao.findByUuid(dataStoreUuid);
+        final HostVO hostVO = hostDao.findByIp(hostIp);
+
+        Optional<Backup.VolumeInfo> matchingVolume = getBackedUpVolumeInfo(backupSourceVm.getBackupVolumeList(), volumeUuid);
+        Long backedUpVolumeSize = matchingVolume.isPresent() ? matchingVolume.get().getSize() : 0L;
+
+        LOG.debug("Restoring vm volume" + volumeUuid + "from backup " + backup.getUuid() + " on the NAS Backup Provider");
+        BackupRepository backupRepository = getBackupRepository(backupSourceVm, backup);
+
+        VolumeVO restoredVolume = new VolumeVO(Volume.Type.DATADISK, null, backup.getZoneId(),
+                backup.getDomainId(), backup.getAccountId(), 0, null,
+                backup.getSize(), null, null, null);
+        String volumeUUID = UUID.randomUUID().toString();
+        restoredVolume.setName("RestoredVol-"+volume.getName());
+        restoredVolume.setProvisioningType(volume.getProvisioningType());
+        restoredVolume.setUpdated(new Date());
+        restoredVolume.setUuid(volumeUUID);
+        restoredVolume.setRemoved(null);
+        restoredVolume.setDisplayVolume(true);
+        restoredVolume.setPoolId(dataStore.getPoolId());
+        restoredVolume.setPath(restoredVolume.getUuid());
+        restoredVolume.setState(Volume.State.Copying);
+        restoredVolume.setSize(backedUpVolumeSize);
+        restoredVolume.setDiskOfferingId(volume.getDiskOfferingId());
+
+        RestoreBackupCommand restoreCommand = new RestoreBackupCommand();
+        restoreCommand.setBackupPath(backup.getExternalId());
+        restoreCommand.setBackupRepoType(backupRepository.getType());
+        restoreCommand.setBackupRepoAddress(backupRepository.getAddress());
+        restoreCommand.setVmName(vmNameAndState.first());
+        restoreCommand.setVolumePaths(Collections.singletonList(String.format("%s/%s", dataStore.getLocalPath(), volumeUUID)));
+        restoreCommand.setDiskType(volume.getVolumeType().name().toLowerCase(Locale.ROOT));
+        restoreCommand.setVmExists(null);
+        restoreCommand.setVmState(vmNameAndState.second());
+        restoreCommand.setRestoreVolumeUUID(volumeUuid);
+
+        BackupAnswer answer = null;
+        try {
+            answer = (BackupAnswer) agentManager.send(hostVO.getId(), restoreCommand);
+        } catch (AgentUnavailableException e) {
+            throw new CloudRuntimeException("Unable to contact backend control plane to initiate backup");
+        } catch (OperationTimedoutException e) {
+            throw new CloudRuntimeException("Operation to initiate backup timed out, please try again");
+        }
+
+        if (answer.getResult()) {
+            try {
+                volumeDao.persist(restoredVolume);
+            } catch (Exception e) {
+                throw new CloudRuntimeException("Unable to create restored volume due to: " + e);
+            }
+        }
+
+        return new Pair<>(answer.getResult(), answer.getDetails());
+    }
+
+    private BackupRepository getBackupRepository(VirtualMachine vm, Backup backup) {
+        BackupRepository backupRepository = backupRepositoryDao.findByBackupOfferingId(vm.getBackupOfferingId());
+        final String errorMessage = "No valid backup repository found for the VM, please check the attached backup offering";
+        if (backupRepository == null) {
+            logger.warn(errorMessage + "Re-attempting with the backup offering associated with the backup");
+        }
+        backupRepository = backupRepositoryDao.findByBackupOfferingId(backup.getBackupOfferingId());
+        if (backupRepository == null) {
+            throw new CloudRuntimeException(errorMessage);
+        }
+        return backupRepository;
+    }
+
+    private Optional<Backup.VolumeInfo> getBackedUpVolumeInfo(List<Backup.VolumeInfo> backedUpVolumes, String volumeUuid) {
+        return backedUpVolumes.stream()
+                .filter(v -> v.getUuid().equals(volumeUuid))
+                .findFirst();
+    }
+
+    @Override
+    public boolean deleteBackup(Backup backup, boolean forced) {
+        final BackupRepository backupRepository = backupRepositoryDao.findByBackupOfferingId(backup.getBackupOfferingId());
+        if (backupRepository == null) {
+            throw new CloudRuntimeException("No valid backup repository found for the VM, please check the attached backup offering");
+        }
+
+        final VirtualMachine vm  = vmInstanceDao.findByIdIncludingRemoved(backup.getVmId());
+        final Host host = getLastVMHypervisorHost(vm);
+
+        DeleteBackupCommand command = new DeleteBackupCommand(backup.getExternalId(), backupRepository.getType(),
+                backupRepository.getAddress(), backupRepository.getMountOptions());
+
+        BackupAnswer answer = null;
+        try {
+            answer = (BackupAnswer) agentManager.send(host.getId(), command);
+        } catch (AgentUnavailableException e) {
+            throw new CloudRuntimeException("Unable to contact backend control plane to initiate backup");
+        } catch (OperationTimedoutException e) {
+            throw new CloudRuntimeException("Operation to initiate backup timed out, please try again");
+        }
+
+        if (answer != null && answer.getResult()) {
+            return backupDao.remove(backup.getId());
+        }
+
+        return false;
+    }
+
+    @Override
+    public Map<VirtualMachine, Backup.Metric> getBackupMetrics(Long zoneId, List<VirtualMachine> vms) {
+        final Map<VirtualMachine, Backup.Metric> metrics = new HashMap<>();
+        if (CollectionUtils.isEmpty(vms)) {
+            LOG.warn("Unable to get VM Backup Metrics because the list of VMs is empty.");
+            return metrics;
+        }
+
+        for (final VirtualMachine vm : vms) {
+            Long vmBackupSize = 0L;
+            Long vmBackupProtectedSize = 0L;
+            for (final Backup backup: backupDao.listByVmId(null, vm.getId())) {
+                vmBackupSize += backup.getSize();
+                vmBackupProtectedSize += backup.getProtectedSize();
+            }
+            Backup.Metric vmBackupMetric = new Backup.Metric(vmBackupSize,vmBackupProtectedSize);
+            LOG.debug(String.format("Metrics for VM [uuid: %s, name: %s] is [backup size: %s, data size: %s].", vm.getUuid(),
+                    vm.getInstanceName(), vmBackupMetric.getBackupSize(), vmBackupMetric.getDataSize()));
+            metrics.put(vm, vmBackupMetric);
+        }
+        return metrics;
+    }
+
+    @Override
+    public boolean assignVMToBackupOffering(VirtualMachine vm, BackupOffering backupOffering) {
+        return Hypervisor.HypervisorType.KVM.equals(vm.getHypervisorType());
+    }
+
+    @Override
+    public boolean removeVMFromBackupOffering(VirtualMachine vm) {
+        return true;
+    }
+
+    @Override
+    public boolean willDeleteBackupsOnOfferingRemoval() {
+        return false;
+    }
+
+    @Override
+    public void syncBackups(VirtualMachine vm, Backup.Metric metric) {
+        // TODO: check and sum/return backups metrics on per VM basis
+    }
+
+    @Override
+    public List<BackupOffering> listBackupOfferings(Long zoneId) {
+        final List<BackupRepository> repositories = backupRepositoryDao.listByZoneAndProvider(zoneId, getName());
+        final List<BackupOffering> offerings = new ArrayList<>();
+        for (final BackupRepository repository : repositories) {
+            offerings.add(new NasBackupOffering(repository.getName(), repository.getUuid()));
+        }
+        return offerings;
+    }
+
+    @Override
+    public boolean isValidProviderOffering(Long zoneId, String uuid) {
+        return true;
+    }
+
+    @Override
+    public ConfigKey<?>[] getConfigKeys() {
+        return new ConfigKey[]{
+        };
+    }
+
+    @Override
+    public String getName() {
+        return "nas";

Review Comment:
   All other providers export string name, constants aren't required because of this. We can check maybe introduce them like enums if necessary. (same pattern used across several other types of plugins)



-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["blueorangutan (via GitHub)" <gi...@apache.org>]

blueorangutan commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2331956549

   @rohityadavcloud a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["blueorangutan (via GitHub)" <gi...@apache.org>]

blueorangutan commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2330784938

   @rajujith a [SL] Jenkins job has been kicked to build packages. It will be bundled with  KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["blueorangutan (via GitHub)" <gi...@apache.org>]

blueorangutan commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2331812354

   Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 11006


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["Pearl1594 (via GitHub)" <gi...@apache.org>]

Pearl1594 commented on code in PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#discussion_r1745690369


##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java:
##########
@@ -0,0 +1,181 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package com.cloud.hypervisor.kvm.resource.wrapper;
+
+import com.cloud.agent.api.Answer;
+import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
+import com.cloud.resource.CommandWrapper;
+import com.cloud.resource.ResourceWrapper;
+import com.cloud.utils.Pair;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.utils.script.Script;
+import org.apache.cloudstack.backup.BackupAnswer;
+import org.apache.cloudstack.backup.RestoreBackupCommand;
+import org.apache.commons.lang3.RandomStringUtils;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.nio.file.StandardCopyOption;
+import java.util.List;
+import java.util.Locale;
+import java.util.Objects;
+
+@ResourceWrapper(handles = RestoreBackupCommand.class)
+public class LibvirtRestoreBackupCommandWrapper extends CommandWrapper<RestoreBackupCommand, Answer, LibvirtComputingResource> {
+    private static final String BACKUP_TEMP_FILE_PREFIx = "csbackup";
+    private static final String MOUNT_COMMAND = "sudo mount -t %s %s %s";
+    private static final String UMOUNT_COMMAND = "sudo umount %s";
+    private static final String FILE_PATH_PLACEHOLDER = "%s/%s";
+
+    @Override
+    public Answer execute(RestoreBackupCommand command, LibvirtComputingResource serverResource) {
+        String vmName = command.getVmName();
+        String backupPath = command.getBackupPath();
+        String backupRepoAddress = command.getBackupRepoAddress();
+        String backupRepoType = command.getBackupRepoType();
+        String mountOptions = command.getMountOptions();
+        Boolean vmExists = command.isVmExists();
+        String diskType = command.getDiskType();
+        Long deviceId = command.getDeviceId();
+        List<String> volumePaths = command.getVolumePaths();
+        String restoreVolumeUuid = command.getRestoreVolumeUUID();
+
+        String newVolumeId = null;
+        if (Objects.isNull(vmExists)) {
+            String volumePath = volumePaths.get(0);
+            int lastIndex = volumePath.lastIndexOf("/");
+            newVolumeId = volumePath.substring(lastIndex + 1);
+            restoreVolume(backupPath, backupRepoType, backupRepoAddress, volumePath, diskType, deviceId, restoreVolumeUuid);
+        } else if (vmExists) {
+            restoreVolumesOfExistingVM(volumePaths, backupPath, backupRepoType, backupRepoAddress, mountOptions);
+        } else {
+            restoreVolumesOfDestroyedVMs(volumePaths, vmName, backupPath, backupRepoType, backupRepoAddress, mountOptions);
+        }
+
+        return new BackupAnswer(command, true, newVolumeId);
+    }
+
+    private void restoreVolumesOfExistingVM(List<String> volumePaths, String backupPath,
+                                             String backupRepoType, String backupRepoAddress, String mountOptions) {
+        String diskType = "root";
+        String mountDirectory = mountBackupDirectory(backupRepoAddress, backupRepoType);
+        try {
+            for (int idx = 0; idx < volumePaths.size(); idx++) {
+                String volumePath = volumePaths.get(idx);
+                Pair<String, String> bkpPathAndVolUuid = getBackupPath(mountDirectory, volumePath, backupPath, diskType, idx, null);
+                diskType = "datadisk";
+                try {
+                    replaceVolumeWithBackup(volumePath, bkpPathAndVolUuid.first());
+                } catch (IOException e) {
+                    throw new CloudRuntimeException(String.format("Unable to revert backup for volume [%s] due to [%s].", bkpPathAndVolUuid.second(), e.getMessage()), e);
+                }
+            }
+        } finally {
+            unmountBackupDirectory(mountDirectory);
+            deleteTemporaryDirectory(mountDirectory);
+        }
+
+    }
+
+    private void restoreVolumesOfDestroyedVMs(List<String> volumePaths, String vmName, String backupPath,
+                                              String backupRepoType, String backupRepoAddress, String mountOptions) {
+        String mountDirectory = mountBackupDirectory(backupRepoAddress, backupRepoType);
+        String diskType = "root";
+        try {
+            for (int i = 0; i < volumePaths.size(); i++) {
+                String volumePath = volumePaths.get(i);
+                Pair<String, String> bkpPathAndVolUuid = getBackupPath(mountDirectory, volumePath, backupPath, diskType, i, null);
+                diskType = "datadisk";

Review Comment:
   will be addressed as an improvement.



-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["rohityadavcloud (via GitHub)" <gi...@apache.org>]

rohityadavcloud commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2331160186

   @blueorangutan test


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["rohityadavcloud (via GitHub)" <gi...@apache.org>]

rohityadavcloud commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2331951979

   @blueorangutan test


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["rohityadavcloud (via GitHub)" <gi...@apache.org>]

rohityadavcloud commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2331320871

   @blueorangutan package


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] [WIP] backup: Simple NAS backup plugin for KVM (experimental) [cloudstack]

["blueorangutan (via GitHub)" <gi...@apache.org>]

blueorangutan commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2271839266

   @Pearl1594 a [SL] Jenkins job has been kicked to build packages. It will be bundled with  KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] [WIP] backup: Simple NAS backup plugin for KVM (experimental) [cloudstack]

["rohityadavcloud (via GitHub)" <gi...@apache.org>]

rohityadavcloud commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2270865931

   Shared storage mount/umount tested:
   - EL8 & EL9: NFS & CephFS works (ceph would require rbd utils, otherwise uses Linux kernel's cephfs driver)
   - Ubuntu: NFS, CephFS & CIFS/SMB3 works (the latter two need cifs-utils & ceph utils)


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM (experimental) [cloudstack]

["blueorangutan (via GitHub)" <gi...@apache.org>]

blueorangutan commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2320297460

   @rajujith a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM (experimental) [cloudstack]

["Pearl1594 (via GitHub)" <gi...@apache.org>]

Pearl1594 commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2321862820

   @blueorangutan package
   


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM (experimental) [cloudstack]

["rohityadavcloud (via GitHub)" <gi...@apache.org>]

rohityadavcloud commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2320767402

   Thanks @shwstppr @DaanHoogland for the review, let me try & address as much as I could.
   
   @DaanHoogland to answer your comments;
   
   1. yes, this is mostly for KVM; however, we think this may be in future expanded to other hypervisors; as most hypervisors support NFS (at least) among the supported NAS types; the method to take/restore backup could use full VM snapshots of non-KVM hypervisors.
   2. yes, backup repository concept for now is specific to only this plugin - however, if the scope expands to a general purpose B&R plugin, then this becomes non-specific to the plugin. Also I think the networkers & veeam plugins have some concept of backup repository or backup storage which can reuse this concept. That said, I'm happy to move the dao/vo logic to the plugin if you really insist (and refactor in future).


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM (experimental) [cloudstack]

["Pearl1594 (via GitHub)" <gi...@apache.org>]

Pearl1594 commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2325353304

   @blueorangutan package


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM (experimental) [cloudstack]

["blueorangutan (via GitHub)" <gi...@apache.org>]

blueorangutan commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2325385840

   Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 10916


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM (experimental) [cloudstack]

["Pearl1594 (via GitHub)" <gi...@apache.org>]

Pearl1594 commented on PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#issuecomment-2327276830

   @blueorangutan package


-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["rohityadavcloud (via GitHub)" <gi...@apache.org>]

rohityadavcloud commented on code in PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#discussion_r1743421548


##########
ui/src/components/view/ListResourceTable.vue:
##########
@@ -50,6 +50,10 @@
             {{ $toLocaleDate(text) }}
           </template>
 
+          <template v-else-if="column.key === 'size' || column.key === 'virtualsize'">
+            {{ parseFloat(text / (1024.0 * 1024.0 * 1024.0)).toFixed(2) }} GB
+          </template>
+

Review Comment:
   @Pearl1594 to check & advise.



##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtTakeBackupCommandWrapper.java:
##########
@@ -0,0 +1,83 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package com.cloud.hypervisor.kvm.resource.wrapper;
+
+import com.cloud.agent.api.Answer;
+import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
+import com.cloud.resource.CommandWrapper;
+import com.cloud.resource.ResourceWrapper;
+import com.cloud.utils.Pair;
+import com.cloud.utils.script.Script;
+import org.apache.cloudstack.backup.BackupAnswer;
+import org.apache.cloudstack.backup.TakeBackupCommand;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+
+@ResourceWrapper(handles = TakeBackupCommand.class)
+public class LibvirtTakeBackupCommandWrapper extends CommandWrapper<TakeBackupCommand, Answer, LibvirtComputingResource> {
+    @Override
+    public Answer execute(TakeBackupCommand command, LibvirtComputingResource libvirtComputingResource) {
+        final String vmName = command.getVmName();
+        final String backupPath = command.getBackupPath();
+        final String backupRepoType = command.getBackupRepoType();
+        final String backupRepoAddress = command.getBackupRepoAddress();
+        final String mountOptions = command.getMountOptions();
+        final List<String> diskPaths = command.getVolumePaths();
+
+        List<String[]> commands = new ArrayList<>();
+        commands.add(new String[]{
+                libvirtComputingResource.getNasBackupPath(),
+                "-o", "backup",
+                "-v", vmName,
+                "-t", backupRepoType,
+                "-s", backupRepoAddress,
+                "-m", Objects.nonNull(mountOptions) ? mountOptions : "",
+                "-p", backupPath,
+                "-d", (Objects.nonNull(diskPaths) && !diskPaths.isEmpty()) ? String.join(",", diskPaths) : ""
+        });
+
+        Pair<Integer, String> result = Script.executePipedCommands(commands, libvirtComputingResource.getCmdsTimeout());
+
+        if (result.first() != 0) {
+            logger.debug("Failed to take VM backup: " + result.second());
+            return new BackupAnswer(command, false, result.second().trim());
+        }
+
+        long backupSize = 0L;
+        if (Objects.isNull(diskPaths) || diskPaths.isEmpty()) {

Review Comment:
   @Pearl1594 to check & advise.



##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java:
##########
@@ -0,0 +1,181 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package com.cloud.hypervisor.kvm.resource.wrapper;
+
+import com.cloud.agent.api.Answer;
+import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
+import com.cloud.resource.CommandWrapper;
+import com.cloud.resource.ResourceWrapper;
+import com.cloud.utils.Pair;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.utils.script.Script;
+import org.apache.cloudstack.backup.BackupAnswer;
+import org.apache.cloudstack.backup.RestoreBackupCommand;
+import org.apache.commons.lang3.RandomStringUtils;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.nio.file.StandardCopyOption;
+import java.util.List;
+import java.util.Locale;
+import java.util.Objects;
+
+@ResourceWrapper(handles = RestoreBackupCommand.class)
+public class LibvirtRestoreBackupCommandWrapper extends CommandWrapper<RestoreBackupCommand, Answer, LibvirtComputingResource> {
+    private static final String BACKUP_TEMP_FILE_PREFIx = "csbackup";
+    private static final String MOUNT_COMMAND = "sudo mount -t %s %s %s";
+    private static final String UMOUNT_COMMAND = "sudo umount %s";
+    private static final String FILE_PATH_PLACEHOLDER = "%s/%s";
+
+    @Override
+    public Answer execute(RestoreBackupCommand command, LibvirtComputingResource serverResource) {
+        String vmName = command.getVmName();
+        String backupPath = command.getBackupPath();
+        String backupRepoAddress = command.getBackupRepoAddress();
+        String backupRepoType = command.getBackupRepoType();
+        String mountOptions = command.getMountOptions();
+        Boolean vmExists = command.isVmExists();
+        String diskType = command.getDiskType();
+        Long deviceId = command.getDeviceId();
+        List<String> volumePaths = command.getVolumePaths();
+        String restoreVolumeUuid = command.getRestoreVolumeUUID();
+
+        String newVolumeId = null;
+        if (Objects.isNull(vmExists)) {
+            String volumePath = volumePaths.get(0);
+            int lastIndex = volumePath.lastIndexOf("/");
+            newVolumeId = volumePath.substring(lastIndex + 1);
+            restoreVolume(backupPath, backupRepoType, backupRepoAddress, volumePath, diskType, deviceId, restoreVolumeUuid);
+        } else if (vmExists) {
+            restoreVolumesOfExistingVM(volumePaths, backupPath, backupRepoType, backupRepoAddress, mountOptions);
+        } else {
+            restoreVolumesOfDestroyedVMs(volumePaths, vmName, backupPath, backupRepoType, backupRepoAddress, mountOptions);
+        }
+
+        return new BackupAnswer(command, true, newVolumeId);
+    }
+
+    private void restoreVolumesOfExistingVM(List<String> volumePaths, String backupPath,
+                                             String backupRepoType, String backupRepoAddress, String mountOptions) {
+        String diskType = "root";
+        String mountDirectory = mountBackupDirectory(backupRepoAddress, backupRepoType);
+        try {
+            for (int idx = 0; idx < volumePaths.size(); idx++) {
+                String volumePath = volumePaths.get(idx);
+                Pair<String, String> bkpPathAndVolUuid = getBackupPath(mountDirectory, volumePath, backupPath, diskType, idx, null);
+                diskType = "datadisk";
+                try {
+                    replaceVolumeWithBackup(volumePath, bkpPathAndVolUuid.first());
+                } catch (IOException e) {
+                    throw new CloudRuntimeException(String.format("Unable to revert backup for volume [%s] due to [%s].", bkpPathAndVolUuid.second(), e.getMessage()), e);
+                }
+            }
+        } finally {
+            unmountBackupDirectory(mountDirectory);
+            deleteTemporaryDirectory(mountDirectory);
+        }
+
+    }
+
+    private void restoreVolumesOfDestroyedVMs(List<String> volumePaths, String vmName, String backupPath,
+                                              String backupRepoType, String backupRepoAddress, String mountOptions) {
+        String mountDirectory = mountBackupDirectory(backupRepoAddress, backupRepoType);
+        String diskType = "root";
+        try {
+            for (int i = 0; i < volumePaths.size(); i++) {
+                String volumePath = volumePaths.get(i);
+                Pair<String, String> bkpPathAndVolUuid = getBackupPath(mountDirectory, volumePath, backupPath, diskType, i, null);
+                diskType = "datadisk";
+                try {
+                    replaceVolumeWithBackup(volumePath, bkpPathAndVolUuid.first());
+                } catch (IOException e) {
+                    throw new CloudRuntimeException(String.format("Unable to revert backup for volume [%s] due to [%s].", bkpPathAndVolUuid.second(), e.getMessage()), e);
+                }
+            }
+        } finally {
+            unmountBackupDirectory(mountDirectory);
+            deleteTemporaryDirectory(mountDirectory);
+        }
+    }
+
+    private void restoreVolume(String backupPath, String backupRepoType, String backupRepoAddress, String volumePath,
+                               String diskType, Long deviceId, String volumeUUID) {
+        String mountDirectory = mountBackupDirectory(backupRepoAddress, backupRepoType);
+        Pair<String, String> bkpPathAndVolUuid;
+        try {
+            bkpPathAndVolUuid = getBackupPath(mountDirectory, volumePath, backupPath, diskType, deviceId.intValue(), volumeUUID);
+            try {
+                replaceVolumeWithBackup(volumePath, bkpPathAndVolUuid.first());
+            } catch (IOException e) {
+                throw new CloudRuntimeException(String.format("Unable to revert backup for volume [%s] due to [%s].", bkpPathAndVolUuid.second(), e.getMessage()), e);
+            }
+        } catch (Exception e) {
+            throw new CloudRuntimeException("Failed to restore volume", e);
+        } finally {
+            unmountBackupDirectory(mountDirectory);
+            deleteTemporaryDirectory(mountDirectory);
+        }
+    }
+
+
+    private String mountBackupDirectory(String backupRepoAddress, String backupRepoType) {
+        String randomChars = RandomStringUtils.random(5, true, false);
+        String mountDirectory = String.format("%s.%s",BACKUP_TEMP_FILE_PREFIx , randomChars);
+        try {
+            mountDirectory = Files.createTempDirectory(mountDirectory).toString();
+            String mount = String.format(MOUNT_COMMAND, backupRepoType, backupRepoAddress, mountDirectory);
+            Script.runSimpleBashScript(mount);
+        } catch (Exception e) {
+            throw new CloudRuntimeException(String.format("Failed to mount %s to %s", backupRepoType, backupRepoAddress), e);
+        }
+        return mountDirectory;
+    }
+
+    private void unmountBackupDirectory(String backupDirectory) {
+        try {
+            String umountCmd = String.format(UMOUNT_COMMAND, backupDirectory);
+            Script.runSimpleBashScript(umountCmd);

Review Comment:
   @Pearl1594 to check & advise.



##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java:
##########
@@ -0,0 +1,181 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package com.cloud.hypervisor.kvm.resource.wrapper;
+
+import com.cloud.agent.api.Answer;
+import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
+import com.cloud.resource.CommandWrapper;
+import com.cloud.resource.ResourceWrapper;
+import com.cloud.utils.Pair;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.utils.script.Script;
+import org.apache.cloudstack.backup.BackupAnswer;
+import org.apache.cloudstack.backup.RestoreBackupCommand;
+import org.apache.commons.lang3.RandomStringUtils;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.nio.file.StandardCopyOption;
+import java.util.List;
+import java.util.Locale;
+import java.util.Objects;
+
+@ResourceWrapper(handles = RestoreBackupCommand.class)
+public class LibvirtRestoreBackupCommandWrapper extends CommandWrapper<RestoreBackupCommand, Answer, LibvirtComputingResource> {
+    private static final String BACKUP_TEMP_FILE_PREFIx = "csbackup";
+    private static final String MOUNT_COMMAND = "sudo mount -t %s %s %s";
+    private static final String UMOUNT_COMMAND = "sudo umount %s";
+    private static final String FILE_PATH_PLACEHOLDER = "%s/%s";
+
+    @Override
+    public Answer execute(RestoreBackupCommand command, LibvirtComputingResource serverResource) {
+        String vmName = command.getVmName();
+        String backupPath = command.getBackupPath();
+        String backupRepoAddress = command.getBackupRepoAddress();
+        String backupRepoType = command.getBackupRepoType();
+        String mountOptions = command.getMountOptions();
+        Boolean vmExists = command.isVmExists();
+        String diskType = command.getDiskType();
+        Long deviceId = command.getDeviceId();
+        List<String> volumePaths = command.getVolumePaths();
+        String restoreVolumeUuid = command.getRestoreVolumeUUID();
+
+        String newVolumeId = null;
+        if (Objects.isNull(vmExists)) {
+            String volumePath = volumePaths.get(0);
+            int lastIndex = volumePath.lastIndexOf("/");
+            newVolumeId = volumePath.substring(lastIndex + 1);
+            restoreVolume(backupPath, backupRepoType, backupRepoAddress, volumePath, diskType, deviceId, restoreVolumeUuid);
+        } else if (vmExists) {
+            restoreVolumesOfExistingVM(volumePaths, backupPath, backupRepoType, backupRepoAddress, mountOptions);
+        } else {
+            restoreVolumesOfDestroyedVMs(volumePaths, vmName, backupPath, backupRepoType, backupRepoAddress, mountOptions);
+        }
+
+        return new BackupAnswer(command, true, newVolumeId);
+    }
+
+    private void restoreVolumesOfExistingVM(List<String> volumePaths, String backupPath,
+                                             String backupRepoType, String backupRepoAddress, String mountOptions) {
+        String diskType = "root";
+        String mountDirectory = mountBackupDirectory(backupRepoAddress, backupRepoType);
+        try {
+            for (int idx = 0; idx < volumePaths.size(); idx++) {
+                String volumePath = volumePaths.get(idx);
+                Pair<String, String> bkpPathAndVolUuid = getBackupPath(mountDirectory, volumePath, backupPath, diskType, idx, null);
+                diskType = "datadisk";
+                try {
+                    replaceVolumeWithBackup(volumePath, bkpPathAndVolUuid.first());
+                } catch (IOException e) {
+                    throw new CloudRuntimeException(String.format("Unable to revert backup for volume [%s] due to [%s].", bkpPathAndVolUuid.second(), e.getMessage()), e);
+                }
+            }
+        } finally {
+            unmountBackupDirectory(mountDirectory);
+            deleteTemporaryDirectory(mountDirectory);
+        }
+
+    }
+
+    private void restoreVolumesOfDestroyedVMs(List<String> volumePaths, String vmName, String backupPath,
+                                              String backupRepoType, String backupRepoAddress, String mountOptions) {
+        String mountDirectory = mountBackupDirectory(backupRepoAddress, backupRepoType);
+        String diskType = "root";
+        try {
+            for (int i = 0; i < volumePaths.size(); i++) {
+                String volumePath = volumePaths.get(i);
+                Pair<String, String> bkpPathAndVolUuid = getBackupPath(mountDirectory, volumePath, backupPath, diskType, i, null);
+                diskType = "datadisk";

Review Comment:
   @Pearl1594 to check & advise.



-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["rohityadavcloud (via GitHub)" <gi...@apache.org>]

rohityadavcloud commented on code in PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#discussion_r1743421112


##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java:
##########
@@ -0,0 +1,181 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package com.cloud.hypervisor.kvm.resource.wrapper;
+
+import com.cloud.agent.api.Answer;
+import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
+import com.cloud.resource.CommandWrapper;
+import com.cloud.resource.ResourceWrapper;
+import com.cloud.utils.Pair;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.utils.script.Script;
+import org.apache.cloudstack.backup.BackupAnswer;
+import org.apache.cloudstack.backup.RestoreBackupCommand;
+import org.apache.commons.lang3.RandomStringUtils;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.nio.file.StandardCopyOption;
+import java.util.List;
+import java.util.Locale;
+import java.util.Objects;
+
+@ResourceWrapper(handles = RestoreBackupCommand.class)
+public class LibvirtRestoreBackupCommandWrapper extends CommandWrapper<RestoreBackupCommand, Answer, LibvirtComputingResource> {
+    private static final String BACKUP_TEMP_FILE_PREFIx = "csbackup";
+    private static final String MOUNT_COMMAND = "sudo mount -t %s %s %s";
+    private static final String UMOUNT_COMMAND = "sudo umount %s";
+    private static final String FILE_PATH_PLACEHOLDER = "%s/%s";
+
+    @Override
+    public Answer execute(RestoreBackupCommand command, LibvirtComputingResource serverResource) {
+        String vmName = command.getVmName();
+        String backupPath = command.getBackupPath();
+        String backupRepoAddress = command.getBackupRepoAddress();
+        String backupRepoType = command.getBackupRepoType();
+        String mountOptions = command.getMountOptions();
+        Boolean vmExists = command.isVmExists();
+        String diskType = command.getDiskType();
+        Long deviceId = command.getDeviceId();
+        List<String> volumePaths = command.getVolumePaths();
+        String restoreVolumeUuid = command.getRestoreVolumeUUID();
+
+        String newVolumeId = null;
+        if (Objects.isNull(vmExists)) {
+            String volumePath = volumePaths.get(0);
+            int lastIndex = volumePath.lastIndexOf("/");
+            newVolumeId = volumePath.substring(lastIndex + 1);
+            restoreVolume(backupPath, backupRepoType, backupRepoAddress, volumePath, diskType, deviceId, restoreVolumeUuid);
+        } else if (vmExists) {
+            restoreVolumesOfExistingVM(volumePaths, backupPath, backupRepoType, backupRepoAddress, mountOptions);
+        } else {
+            restoreVolumesOfDestroyedVMs(volumePaths, vmName, backupPath, backupRepoType, backupRepoAddress, mountOptions);
+        }
+
+        return new BackupAnswer(command, true, newVolumeId);
+    }
+
+    private void restoreVolumesOfExistingVM(List<String> volumePaths, String backupPath,
+                                             String backupRepoType, String backupRepoAddress, String mountOptions) {
+        String diskType = "root";
+        String mountDirectory = mountBackupDirectory(backupRepoAddress, backupRepoType);
+        try {
+            for (int idx = 0; idx < volumePaths.size(); idx++) {
+                String volumePath = volumePaths.get(idx);
+                Pair<String, String> bkpPathAndVolUuid = getBackupPath(mountDirectory, volumePath, backupPath, diskType, idx, null);
+                diskType = "datadisk";
+                try {
+                    replaceVolumeWithBackup(volumePath, bkpPathAndVolUuid.first());
+                } catch (IOException e) {
+                    throw new CloudRuntimeException(String.format("Unable to revert backup for volume [%s] due to [%s].", bkpPathAndVolUuid.second(), e.getMessage()), e);
+                }
+            }
+        } finally {
+            unmountBackupDirectory(mountDirectory);
+            deleteTemporaryDirectory(mountDirectory);
+        }
+
+    }
+
+    private void restoreVolumesOfDestroyedVMs(List<String> volumePaths, String vmName, String backupPath,
+                                              String backupRepoType, String backupRepoAddress, String mountOptions) {
+        String mountDirectory = mountBackupDirectory(backupRepoAddress, backupRepoType);
+        String diskType = "root";
+        try {
+            for (int i = 0; i < volumePaths.size(); i++) {
+                String volumePath = volumePaths.get(i);
+                Pair<String, String> bkpPathAndVolUuid = getBackupPath(mountDirectory, volumePath, backupPath, diskType, i, null);
+                diskType = "datadisk";
+                try {
+                    replaceVolumeWithBackup(volumePath, bkpPathAndVolUuid.first());
+                } catch (IOException e) {
+                    throw new CloudRuntimeException(String.format("Unable to revert backup for volume [%s] due to [%s].", bkpPathAndVolUuid.second(), e.getMessage()), e);
+                }
+            }
+        } finally {
+            unmountBackupDirectory(mountDirectory);
+            deleteTemporaryDirectory(mountDirectory);
+        }
+    }
+
+    private void restoreVolume(String backupPath, String backupRepoType, String backupRepoAddress, String volumePath,
+                               String diskType, Long deviceId, String volumeUUID) {
+        String mountDirectory = mountBackupDirectory(backupRepoAddress, backupRepoType);
+        Pair<String, String> bkpPathAndVolUuid;
+        try {
+            bkpPathAndVolUuid = getBackupPath(mountDirectory, volumePath, backupPath, diskType, deviceId.intValue(), volumeUUID);
+            try {
+                replaceVolumeWithBackup(volumePath, bkpPathAndVolUuid.first());
+            } catch (IOException e) {
+                throw new CloudRuntimeException(String.format("Unable to revert backup for volume [%s] due to [%s].", bkpPathAndVolUuid.second(), e.getMessage()), e);
+            }
+        } catch (Exception e) {
+            throw new CloudRuntimeException("Failed to restore volume", e);
+        } finally {
+            unmountBackupDirectory(mountDirectory);
+            deleteTemporaryDirectory(mountDirectory);
+        }
+    }
+
+
+    private String mountBackupDirectory(String backupRepoAddress, String backupRepoType) {
+        String randomChars = RandomStringUtils.random(5, true, false);
+        String mountDirectory = String.format("%s.%s",BACKUP_TEMP_FILE_PREFIx , randomChars);
+        try {
+            mountDirectory = Files.createTempDirectory(mountDirectory).toString();
+            String mount = String.format(MOUNT_COMMAND, backupRepoType, backupRepoAddress, mountDirectory);
+            Script.runSimpleBashScript(mount);

Review Comment:
   @Pearl1594 can you check, if that should be used?



-- 
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-unsubscribe@cloudstack.apache.org

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

Re: [PR] backup: simple NAS backup plugin for KVM [cloudstack]

["Pearl1594 (via GitHub)" <gi...@apache.org>]

Pearl1594 commented on code in PR #9451:
URL: https://github.com/apache/cloudstack/pull/9451#discussion_r1743743079


##########
engine/schema/src/main/java/org/apache/cloudstack/backup/BackupVO.java:
##########
@@ -82,6 +87,9 @@ public class BackupVO implements Backup {
     @Column(name = "zone_id")
     private long zoneId;
 
+    @Column(name = "backed_volumes", length = 65535)
+    protected String backedVolumes;

Review Comment:
   I've followed the convention of the backup framework. A similar column is present in the vm_instance table. 



-- 
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-unsubscribe@cloudstack.apache.org

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