winterhazel commented on code in PR #12758:
URL: https://github.com/apache/cloudstack/pull/12758#discussion_r3568081487
##########
server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java:
##########
@@ -1782,6 +1949,29 @@ private Pair<HostVO, StoragePoolVO>
getRestoreVolumeHostAndDatastoreForNas(VMIns
return new Pair<>(hostVO, storagePoolVO);
}
+ private Pair<HostVO, StoragePoolVO>
getRestoreVolumeHostAndDatastoreForKboss(VMInstanceVO vm, VolumeVO
backedVolume, boolean quickRestore, Long hostId) {
+ StoragePoolVO storagePool =
primaryDataStoreDao.findById(backedVolume.getPoolId());
+ if (vm.getHostId() != null) {
+ hostId = vm.getHostId();
+ } else if (hostId == null || !quickRestore) {
+ if (vm.getLastHostId() != null) {
+ hostId = vm.getLastHostId();
+ } else {
+ if (storagePool == null) {
+ throw new
InvalidParameterValueException(String.format("Storage pool of volume [%s] was
not found.", backedVolume.getUuid()));
+ }
+ List<HostVO> listHost =
+
hostDao.listAllUpAndEnabledNonHAHosts(Host.Type.Routing,
storagePool.getClusterId(), storagePool.getPodId(),
storagePool.getDataCenterId(), null);
+ return new Pair<>(listHost.stream().findFirst().orElseThrow(),
null);
Review Comment:
In `orElseThrow`, throw an exception that contains a message identifying
what was the problem (no hosts found for restore).
##########
server/src/main/java/org/apache/cloudstack/backup/InternalBackupServiceImpl.java:
##########
@@ -0,0 +1,366 @@
+//
+// 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.to.DataTO;
+import com.cloud.hypervisor.Hypervisor;
+import com.cloud.storage.DataStoreRole;
+import com.cloud.storage.Storage;
+import com.cloud.storage.Upload;
+import com.cloud.storage.Volume;
+import com.cloud.storage.VolumeVO;
+import com.cloud.storage.dao.VolumeDao;
+import com.cloud.uservm.UserVm;
+import com.cloud.utils.Pair;
+import com.cloud.utils.ReflectionUse;
+import com.cloud.utils.component.ComponentLifecycleBase;
+import com.cloud.utils.db.Transaction;
+import com.cloud.utils.db.TransactionCallback;
+import com.cloud.utils.db.TransactionLegacy;
+import com.cloud.vm.VirtualMachine;
+import com.cloud.vm.VirtualMachineManager;
+import com.cloud.vm.VmWork;
+import com.cloud.vm.VmWorkDeleteBackup;
+import com.cloud.vm.VmWorkJobHandler;
+import com.cloud.vm.VmWorkJobHandlerProxy;
+import com.cloud.vm.VmWorkRestoreBackup;
+import com.cloud.vm.VmWorkRestoreVolumeBackupAndAttach;
+import com.cloud.vm.VmWorkTakeBackup;
+import com.cloud.vm.dao.UserVmDao;
+import com.cloud.vm.snapshot.VMSnapshot;
+import org.apache.cloudstack.api.response.ExtractResponse;
+import org.apache.cloudstack.backup.dao.BackupDao;
+import org.apache.cloudstack.backup.dao.BackupDetailsDao;
+import org.apache.cloudstack.backup.dao.InternalBackupJoinDao;
+import org.apache.cloudstack.backup.dao.InternalBackupStoragePoolDao;
+import org.apache.cloudstack.backup.to.BackupScreenshotObject;
+import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
+import org.apache.cloudstack.framework.jobs.AsyncJobManager;
+import org.apache.cloudstack.jobs.JobInfo;
+import org.apache.cloudstack.storage.command.DeleteCommand;
+import org.apache.cloudstack.storage.command.RevertSnapshotCommand;
+import org.apache.cloudstack.storage.datastore.db.ImageStoreObjectDownloadDao;
+import org.apache.cloudstack.storage.datastore.db.ImageStoreObjectDownloadVO;
+import org.apache.cloudstack.storage.image.datastore.ImageStoreEntity;
+import org.apache.cloudstack.backup.to.BackupScreenshotTO;
+import org.apache.cloudstack.storage.to.VolumeObjectTO;
+import org.apache.commons.collections4.CollectionUtils;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
+import javax.inject.Inject;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+public class InternalBackupServiceImpl extends ComponentLifecycleBase
implements InternalBackupService, VmWorkJobHandler {
+ protected Logger logger = LogManager.getLogger(getClass());
+
+ @Inject
+ private InternalBackupStoragePoolDao internalBackupStoragePoolDao;
+
+ @Inject
+ private BackupManager backupManager;
+ @Inject
+ private BackupDao backupDao;
+ @Inject
+ private AsyncJobManager jobManager;
+ @Inject
+ private UserVmDao userVmDao;
+ @Inject
+ private VirtualMachineManager virtualMachineManager;
+ @Inject
+ private VolumeDao volumeDao;
+ @Inject
+ private InternalBackupJoinDao internalBackupJoinDao;
+ @Inject
+ private BackupDetailsDao backupDetailDao;
+
+ @Inject
+ private ImageStoreObjectDownloadDao imageStoreObjectDownloadDao;
+
+ @Inject
+ private DataStoreManager dataStoreMgr;
+
+ private VmWorkJobHandlerProxy jobHandlerProxy = new
VmWorkJobHandlerProxy(this);
+ private HashMap<String, InternalBackupProvider> internalBackupProviderMap
= new HashMap<>();
+ private List<InternalBackupProvider> internalBackupProviders;
+
+ public void setInternalBackupProviders(final List<InternalBackupProvider>
internalBackupProviders) {
+ this.internalBackupProviders = internalBackupProviders;
+ }
+
+ @Override
+ public boolean start() {
+ super.start();
+
+ if (internalBackupProviders != null) {
+ for (InternalBackupProvider internalBackupProvider :
internalBackupProviders) {
+
internalBackupProviderMap.put(internalBackupProvider.getName().toLowerCase(),
internalBackupProvider);
+ }
+ }
+ return true;
+ }
+
+ @Override
+ public void configureChainInfo(DataTO volumeTo, Command cmd) {
+ if (!(volumeTo instanceof VolumeObjectTO)) {
+ return;
+ }
+ VolumeObjectTO volumeObjectTO = (VolumeObjectTO) volumeTo;
+ List<InternalBackupStoragePoolVO> backupDeltas =
internalBackupStoragePoolDao.listByVolumeId(volumeObjectTO.getVolumeId());
+ if (backupDeltas.isEmpty()) {
+ return;
+ }
+
volumeObjectTO.setDeltasToRemove(backupDeltas.stream().map(InternalBackupStoragePoolVO::getBackupDeltaParentPath).collect(Collectors.toSet()));
+ if (cmd instanceof DeleteCommand) {
+ ((DeleteCommand) cmd).setDeleteChain(true);
+ } else if (cmd instanceof RevertSnapshotCommand) {
+ ((RevertSnapshotCommand) cmd).setDeleteChain(true);
+ } else {
+ return;
+ }
+ logger.debug("Configured chain info for volume [{}]. Set it as [{}].",
volumeObjectTO.getUuid(), volumeObjectTO.getChainInfo());
+ }
+
+ @Override
+ public void cleanupBackupMetadata(long volumeId) {
+ logger.debug("Cleaning up backup metadata for volume [{}].", volumeId);
+ List<InternalBackupJoinVO> currents =
internalBackupJoinDao.listCurrentsByVolumeIdDesc(volumeId);
+ if (currents.isEmpty()) {
+ return;
+ }
+ internalBackupStoragePoolDao.expungeByVolumeId(volumeId);
+ for (InternalBackupJoinVO current : currents) {
+ if
(CollectionUtils.isNotEmpty(internalBackupStoragePoolDao.listByBackupId(current.getId())))
{
+ continue;
+ }
+
+ logger.debug("Volume [{}] was the last volume with deltas in
backup [{}]. Setting the backup as END_OF_CHAIN and not current.", volumeId,
current.getUuid());
+ backupDetailDao.removeDetail(current.getId(),
BackupDetailsDao.CURRENT);
+ if (!current.getEndOfChain()) {
+ backupDetailDao.persist(new BackupDetailVO(current.getId(),
BackupDetailsDao.END_OF_CHAIN, Boolean.TRUE.toString(), true));
+ }
+ }
+
+ }
+
+
+ @Override
+ public void prepareVolumeForDetach(Volume volume, VirtualMachine
virtualMachine) {
+ if (isBackupFrameworkDisabled(virtualMachine)) {
+ return;
+ }
+
+ InternalBackupProvider internalBackupProvider =
getInternalBackupProviderForZone(virtualMachine.getDataCenterId());
+ if (internalBackupProvider == null) {
+ return;
+ }
+ internalBackupProvider.prepareVolumeForDetach(volume, virtualMachine);
+ }
+
+ @Override
+ public void prepareVolumeForMigration(Volume volume) {
+ if (volume.getInstanceId() == null) {
+ return;
+ }
+ VirtualMachine virtualMachine =
virtualMachineManager.findById(volume.getInstanceId());
+ if (isBackupFrameworkDisabled(virtualMachine)) {
+ return;
+ }
+ InternalBackupProvider internalBackupProvider =
getInternalBackupProviderForZone(volume.getDataCenterId());
+ if (internalBackupProvider == null) {
+ return;
+ }
+ internalBackupProvider.prepareVolumeForMigration(volume,
virtualMachine);
+ }
+
+ @Override
+ public void updateVolumeId(long oldVolumeId, long newVolumeId) {
+ VolumeVO volumeVO = volumeDao.findById(newVolumeId);
+ if (volumeVO.getInstanceId() == null) {
+ return;
+ }
+ VirtualMachine virtualMachine =
virtualMachineManager.findById(volumeVO.getInstanceId());
+ if (isBackupFrameworkDisabled(virtualMachine)) {
+ return;
+ }
+
+ InternalBackupProvider internalBackupProvider =
getInternalBackupProviderForZone(virtualMachine.getDataCenterId());
+ if (internalBackupProvider == null) {
+ return;
+ }
+ internalBackupProvider.updateVolumeId(virtualMachine, oldVolumeId,
newVolumeId);
+ }
+
+ @Override
+ public void prepareVmForSnapshotRevert(VMSnapshot vmSnapshot) {
+ VirtualMachine virtualMachine =
virtualMachineManager.findById(vmSnapshot.getVmId());
+ if (isBackupFrameworkDisabled(virtualMachine)) {
+ return;
+ }
+
+ InternalBackupProvider internalBackupProvider =
getInternalBackupProviderForZone(virtualMachine.getDataCenterId());
+ if (internalBackupProvider == null) {
+ return;
+ }
+ internalBackupProvider.prepareVmForSnapshotRevert(vmSnapshot,
virtualMachine);
+ }
+
+ /**
+ * Ask the backup provider to get the necessary secondary storages that
must be mounted at VM start.
+ * <br/>
+ * Note: This is currently only used for Backup Validation VMs. As they
are created with backing files that are on secondary storage.
+ * */
+ @Override
+ public Set<String> getSecondaryStorageUrls(UserVm userVm) {
+ InternalBackupProvider internalBackupProvider =
getInternalBackupProviderForZone(userVm.getDataCenterId());
+ if (internalBackupProvider == null) {
+ return Set.of();
+ }
+ return internalBackupProvider.getSecondaryStorageUrls(userVm);
+ }
+
+ @Override
+ public boolean startBackupCompression(long backupId, long hostId, long
zoneId) {
+ InternalBackupProvider internalBackupProvider =
getInternalBackupProviderForZone(zoneId);
+ if (internalBackupProvider == null) {
+ return false;
+ }
+ return internalBackupProvider.startBackupCompression(backupId, hostId);
+ }
+
+ @Override
+ public boolean finalizeBackupCompression(long backupId, long hostId, long
zoneId) {
+ InternalBackupProvider internalBackupProvider =
getInternalBackupProviderForZone(zoneId);
+ if (internalBackupProvider == null) {
+ return false;
+ }
+ return internalBackupProvider.finalizeBackupCompression(backupId,
hostId);
+ }
+
+ @Override
+ public boolean validateBackup(long backupId, long hostId, long zoneId) {
+ InternalBackupProvider internalBackupProvider =
getInternalBackupProviderForZone(zoneId);
+ if (internalBackupProvider == null) {
+ return false;
+ }
+ return internalBackupProvider.validateBackup(backupId, hostId);
+ }
+
+ @Override
+ public ExtractResponse downloadScreenshot(long backupId) {
+ BackupDetailVO screenshotPathDetail =
backupDetailDao.findDetail(backupId, BackupDetailsDao.SCREENSHOT_PATH);
+ ExtractResponse response = new ExtractResponse();
+ if (screenshotPathDetail == null) {
+
response.setState(Upload.Status.DOWNLOAD_URL_NOT_CREATED.toString());
+ return response;
+ }
+ BackupDetailVO imageStoreId = backupDetailDao.findDetail(backupId,
BackupDetailsDao.IMAGE_STORE_ID);
+ ImageStoreEntity imageStore = (ImageStoreEntity)
dataStoreMgr.getDataStore(Long.parseLong(imageStoreId.getValue()),
DataStoreRole.Image);
+ String screenshotPath = screenshotPathDetail.getValue();
+ ImageStoreObjectDownloadVO imageStoreObj =
imageStoreObjectDownloadDao.findByStoreIdAndPath(Long.parseLong(imageStoreId.getValue()),
screenshotPath);
+
+ if (imageStoreObj == null) {
+ BackupScreenshotTO dataTo = new
BackupScreenshotTO(imageStore.getTO(), Hypervisor.HypervisorType.KVM,
screenshotPath);
+ BackupScreenshotObject objectTo = new
BackupScreenshotObject(dataTo, imageStore);
+ String downloadUrl =
imageStore.createEntityExtractUrl(screenshotPath, Storage.ImageFormat.PNG,
objectTo);
+ imageStoreObj = imageStoreObjectDownloadDao.persist(new
ImageStoreObjectDownloadVO(imageStore.getId(), screenshotPath, downloadUrl));
+ }
+
+ if (imageStoreObj != null) {
+ response.setUrl(imageStoreObj.getDownloadUrl());
+
response.setName(screenshotPath.substring(screenshotPath.lastIndexOf("/") + 1));
+ response.setState(Upload.Status.DOWNLOAD_URL_CREATED.toString());
+ } else {
+
response.setState(Upload.Status.DOWNLOAD_URL_NOT_CREATED.toString());
+ }
+ return response;
+ }
+
+ @Override
+ public boolean finishBackupChain(long vmId) {
+ VirtualMachine vm = virtualMachineManager.findById(vmId);
Review Comment:
Passing the ID of a VM that does not exist results in a NPE in the line
below. It would be nice to check if `vm` is null and throw another exception
instead.
##########
api/src/main/java/org/apache/cloudstack/backup/BackupManager.java:
##########
@@ -138,6 +139,12 @@ public interface BackupManager extends BackupService,
Configurable, PluggableSer
*/
BackupOffering importBackupOffering(final ImportBackupOfferingCmd cmd);
+ /**
+ * Create a new Backup and Recovery policy to CloudStack. Currently only
supported for KBOSS.
+ * @param cmd import backup offering cmd
Review Comment:
```suggestion
* @param cmd create backup offering cmd
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]