JoaoJandre commented on code in PR #10140:
URL: https://github.com/apache/cloudstack/pull/10140#discussion_r2228507025
##########
server/src/main/java/com/cloud/server/ManagementServerImpl.java:
##########
@@ -1036,6 +1037,8 @@ public class ManagementServerImpl extends ManagerBase
implements ManagementServe
@Inject
StoragePoolTagsDao storagePoolTagsDao;
@Inject
+ BackupManager backupManager;
Review Comment:
```suggestion
private BackupManager backupManager;
```
##########
server/src/main/java/com/cloud/vm/UserVmManagerImpl.java:
##########
@@ -9317,6 +9391,234 @@ public boolean unmanageUserVM(Long vmId) {
return true;
}
+ private void updateDetailsWithRootDiskAttributes(Map<String, String>
details, VmDiskInfo rootVmDiskInfo) {
+ details.put(VmDetailConstants.ROOT_DISK_SIZE,
rootVmDiskInfo.getSize().toString());
+ if (rootVmDiskInfo.getMinIops() != null) {
+ details.put(MIN_IOPS, rootVmDiskInfo.getMinIops().toString());
+ }
+ if (rootVmDiskInfo.getMaxIops() != null) {
+ details.put(MAX_IOPS, rootVmDiskInfo.getMaxIops().toString());
+ }
+ }
+
+ private void checkRootDiskSizeAgainstBackup(Long
instanceVolumeSize,DiskOffering rootDiskOffering, Long backupVolumeSize) {
+ Long instanceRootDiskSize = rootDiskOffering.isCustomized() ?
instanceVolumeSize : rootDiskOffering.getDiskSize() / GiB_TO_BYTES;
+ if (instanceRootDiskSize < backupVolumeSize) {
+ throw new InvalidParameterValueException(
+ String.format("Instance volume root disk size %d[GiB]
cannot be less than the backed-up volume size %d[GiB].",
+ instanceVolumeSize, backupVolumeSize));
+ }
+ }
+
+ @Override
+ public UserVm allocateVMFromBackup(CreateVMFromBackupCmd cmd) throws
InsufficientCapacityException, ResourceAllocationException,
ResourceUnavailableException {
+ if (!backupManager.canCreateInstanceFromBackup(cmd.getBackupId())) {
+ throw new CloudRuntimeException("Create instance from backup is
not supported for this provider.");
+ }
+ DataCenter zone = _dcDao.findById(cmd.getZoneId());
+ if (zone == null) {
+ throw new InvalidParameterValueException("Unable to find zone by
id=" + cmd.getZoneId());
+ }
+
+ BackupVO backup = backupDao.findById(cmd.getBackupId());
+ if (backup == null) {
+ throw new InvalidParameterValueException("Backup " +
cmd.getBackupId() + " does not exist");
+ }
+ if (backup.getZoneId() != cmd.getZoneId()) {
+ throw new InvalidParameterValueException("Instance should be
created in the same zone as the backup");
+ }
+ backupManager.validateBackupForZone(backup.getZoneId());
+ backupDao.loadDetails(backup);
+
+ verifyDetails(cmd.getDetails());
+
+ UserVmVO backupVm = _vmDao.findByIdIncludingRemoved(backup.getVmId());
+ HypervisorType hypervisorType = backupVm.getHypervisorType();
+
+ Long serviceOfferingId = cmd.getServiceOfferingId();
+ ServiceOffering serviceOffering;
+ if (serviceOfferingId != null) {
+ serviceOffering = serviceOfferingDao.findById(serviceOfferingId);
+ if (serviceOffering == null) {
+ throw new InvalidParameterValueException("Unable to find
service offering: " + serviceOffering.getId());
+ }
+ } else {
+ String serviceOfferingUuid =
backup.getDetail(ApiConstants.SERVICE_OFFERING_ID);
+ if (serviceOfferingUuid == null) {
+ throw new CloudRuntimeException("Backup doesn't contain
service offering uuid. Please specify a valid service offering id while
creating the instance");
+ }
+ serviceOffering =
serviceOfferingDao.findByUuid(serviceOfferingUuid);
+ if (serviceOffering == null) {
+ throw new CloudRuntimeException("Unable to find service
offering with the uuid stored in backup. Please specify a valid service
offering id while creating instance");
+ }
+ }
+ verifyServiceOffering(cmd, serviceOffering);
+
+ VirtualMachineTemplate template;
+ if (cmd.getTemplateId() != null) {
+ Long templateId = cmd.getTemplateId();
+ template = _templateDao.findById(templateId);
+ if (template == null) {
+ throw new InvalidParameterValueException("Unable to use
template " + templateId);
+ }
+ } else {
+ String templateUuid = backup.getDetail(ApiConstants.TEMPLATE_ID);
+ if (templateUuid == null) {
+ throw new CloudRuntimeException("Backup doesn't contain
Template uuid. Please specify a valid Template/ISO while creating the
instance");
+ }
+ template = _templateDao.findByUuid(templateUuid);
+ if (template == null) {
+ throw new CloudRuntimeException("Unable to find template
associated with the backup. Please specify a valid Template/ISO while creating
instance");
+ }
+ }
+ verifyTemplate(cmd, template, serviceOffering.getId());
+
+ Long size = cmd.getSize();
+
+ Long diskOfferingId = cmd.getDiskOfferingId();
+ Boolean isIso = template.getFormat().equals(ImageFormat.ISO);
+ if (diskOfferingId != null) {
+ if (!isIso) {
+ throw new
InvalidParameterValueException(ApiConstants.DISK_OFFERING_ID + " parameter is
supported for creating instance from backup only for ISO. For creating VMs with
templates, please use the parameter " + ApiConstants.DATADISKS_DETAILS);
+ }
+ DiskOffering diskOffering =
_diskOfferingDao.findById(diskOfferingId);
+ if (diskOffering == null) {
+ throw new InvalidParameterValueException("Unable to find disk
offering " + diskOfferingId);
+ }
+ if (diskOffering.isComputeOnly()) {
+ throw new InvalidParameterValueException(String.format("The
disk offering %s provided is directly mapped to a service offering, please
provide an individual disk offering", diskOffering));
+ }
+ }
+
+ Long overrideDiskOfferingId = cmd.getOverrideDiskOfferingId();
+
+ VmDiskInfo rootVmDiskInfoFromBackup =
backupManager.getRootDiskInfoFromBackup(backup);
+
+ if (isIso) {
+ if (diskOfferingId == null) {
+ diskOfferingId =
rootVmDiskInfoFromBackup.getDiskOffering().getId();
+ updateDetailsWithRootDiskAttributes(cmd.getDetails(),
rootVmDiskInfoFromBackup);
+ size = rootVmDiskInfoFromBackup.getSize();
+ } else {
+ DiskOffering rootDiskOffering =
_diskOfferingDao.findById(diskOfferingId);
+ checkRootDiskSizeAgainstBackup(size, rootDiskOffering,
rootVmDiskInfoFromBackup.getSize());
+ }
+ } else {
+ if (overrideDiskOfferingId == null) {
+ overrideDiskOfferingId = serviceOffering.getDiskOfferingId();
+ updateDetailsWithRootDiskAttributes(cmd.getDetails(),
rootVmDiskInfoFromBackup);
+ } else {
+ DiskOffering overrideDiskOffering =
_diskOfferingDao.findById(overrideDiskOfferingId);
+ if (overrideDiskOffering.isComputeOnly()) {
+ updateDetailsWithRootDiskAttributes(cmd.getDetails(),
rootVmDiskInfoFromBackup);
+ } else {
+ String diskSizeFromDetails =
cmd.getDetails().get(VmDetailConstants.ROOT_DISK_SIZE);
+ Long rootDiskSize = diskSizeFromDetails == null ? null :
Long.parseLong(diskSizeFromDetails);
+ checkRootDiskSizeAgainstBackup(rootDiskSize,
overrideDiskOffering, rootVmDiskInfoFromBackup.getSize());
+ }
+ }
+ }
+
+ List<VmDiskInfo> dataDiskInfoList = cmd.getDataDiskInfoList();
+ if (dataDiskInfoList != null) {
+ backupManager.checkVmDisksSizeAgainstBackup(dataDiskInfoList,
backup);
+ } else {
+ dataDiskInfoList =
backupManager.getDataDiskInfoListFromBackup(backup);
+ }
+
+ List<Long> networkIds = cmd.getNetworkIds();
+ Account owner =
_accountService.getActiveAccountById(cmd.getEntityOwnerId());
+ LinkedHashMap<Integer, Long> userVmNetworkMap =
getVmOvfNetworkMapping(zone, owner, template, cmd.getVmNetworkMap());
+ if (MapUtils.isNotEmpty(userVmNetworkMap)) {
+ networkIds = new ArrayList<>(userVmNetworkMap.values());
+ }
+
+ Map<Long, IpAddresses> ipToNetworkMap = cmd.getIpToNetworkMap();
+ if (networkIds == null && ipToNetworkMap == null) {
+ networkIds = new ArrayList<Long>();
+ ipToNetworkMap = backupManager.getIpToNetworkMapFromBackup(backup,
cmd.getPreserveIp(), networkIds);
+ }
+
+ UserVm vm = createVirtualMachine(cmd, zone, owner, serviceOffering,
template, hypervisorType, diskOfferingId, size, overrideDiskOfferingId,
dataDiskInfoList, networkIds, ipToNetworkMap, null, null);
+
+ String vmSettingsFromBackup =
backup.getDetail(ApiConstants.VM_SETTINGS);
+ if (vm != null && vmSettingsFromBackup != null) {
+ UserVmVO vmVO = _vmDao.findById(vm.getId());
+ Map<String, String> details =
vmInstanceDetailsDao.listDetailsKeyPairs(vm.getId());
+ vmVO.setDetails(details);
+
+ Type type = new TypeToken<Map<String, String>>(){}.getType();
+ Map<String, String> vmDetailsFromBackup = new
Gson().fromJson(vmSettingsFromBackup, type);
+ for (Entry<String, String> entry : vmDetailsFromBackup.entrySet())
{
+ if (!details.containsKey(entry.getKey())) {
+ vmVO.setDetail(entry.getKey(), entry.getValue());
+ }
+ }
+ _vmDao.saveDetails(vmVO);
+ }
+
+ return vm;
+ }
+
+ @Override
+ public UserVm restoreVMFromBackup(CreateVMFromBackupCmd cmd) throws
ResourceUnavailableException, InsufficientCapacityException,
ResourceAllocationException {
+ long vmId = cmd.getEntityId();
+ Map<Long, DiskOffering> diskOfferingMap =
cmd.getDataDiskTemplateToDiskOfferingMap();
+ Map<VirtualMachineProfile.Param, Object> additonalParams = new
HashMap<>();
+ UserVm vm;
+
+ try {
+ vm = startVirtualMachine(vmId, null, null, null, diskOfferingMap,
additonalParams, null);
+
+ boolean status =
stopVirtualMachine(CallContext.current().getCallingUserId(), vm.getId()) ;
Review Comment:
Why start then stop the VM?
##########
engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupDaoImpl.java:
##########
@@ -193,27 +272,97 @@ public List<BackupVO> listBackupsByVMandIntervalType(Long
vmId, Backup.Type back
}
@Override
- public BackupResponse newBackupResponse(Backup backup) {
+ public void loadDetails(BackupVO backup) {
+ Map<String, String> details =
backupDetailsDao.listDetailsKeyPairs(backup.getId());
+ backup.setDetails(details);
+ }
+
+ @Override
+ public void saveDetails(BackupVO backup) {
+ Map<String, String> detailsStr = backup.getDetails();
+ if (detailsStr == null) {
+ return;
+ }
+ List<BackupDetailVO> details = new ArrayList<BackupDetailVO>();
+ for (String key : detailsStr.keySet()) {
+ BackupDetailVO detail = new BackupDetailVO(backup.getId(), key,
detailsStr.get(key), true);
+ details.add(detail);
+ }
+ backupDetailsDao.saveDetails(details);
+ }
+
+ @Override
+ public List<Long> listVmIdsWithBackupsInZone(Long zoneId) {
+ SearchCriteria<Long> sc = backupVmSearchInZone.create();
+ sc.setParameters("zone_id", zoneId);
+ return customSearchIncludingRemoved(sc, null);
+ }
+
+ Map<String, String> getDetailsFromBackupDetails(Long backupId) {
Review Comment:
Considering that this method does not do a single query in the `backups`
table (instead querying multiple separate tables), don't you think it should
not be somewhere else? I think that it should not be on the BackupDaoImpl.
##########
server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java:
##########
@@ -406,9 +501,12 @@ public boolean removeVMFromBackupOffering(final Long vmId,
final boolean forced)
}
}
if ((result || forced) && vmInstanceDao.update(vm.getId(), vm)) {
-
UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VM_BACKUP_OFFERING_REMOVE,
vm.getAccountId(), vm.getDataCenterId(), vm.getId(),
- "Backup-" + vm.getHostName() + "-" + vm.getUuid(),
vm.getBackupOfferingId(), null, null,
- Backup.class.getSimpleName(), vm.getUuid());
+ final List<Backup> backups = backupDao.listByVmId(null,
vm.getId());
+ if (backups.size() == 0) {
+
UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VM_BACKUP_OFFERING_REMOVE,
vm.getAccountId(), vm.getDataCenterId(), vm.getId(),
+ "Backup-" + vm.getHostName() + "-" + vm.getUuid(),
backupOfferingId, null, null,
+ Backup.class.getSimpleName(), vm.getUuid());
+ }
Review Comment:
If this event is not published here, when will it be published?
##########
server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java:
##########
@@ -926,6 +1058,224 @@ private Backup.VolumeInfo
getVolumeInfo(List<Backup.VolumeInfo> backedUpVolumes,
return null;
}
+ @Override
+ public void checkVmDisksSizeAgainstBackup(List<VmDiskInfo> vmDiskInfoList,
Backup backup) {
+ List<VmDiskInfo> vmDiskInfoListFromBackup =
getDataDiskInfoListFromBackup(backup);
+ int index = 0;
+ if (vmDiskInfoList.size() != vmDiskInfoListFromBackup.size()) {
+ throw new InvalidParameterValueException("Unable to create
Instance from Backup " +
+ "as the backup has a different number of disks than the
Instance.");
+ }
+ for (VmDiskInfo vmDiskInfo : vmDiskInfoList) {
+ if (index < vmDiskInfoListFromBackup.size()) {
+ if (vmDiskInfo.getSize() <
vmDiskInfoListFromBackup.get(index).getSize()) {
+ throw new InvalidParameterValueException(
+ String.format("Instance volume size %d[GiB] cannot
be less than the backed-up volume size %d[GiB].",
+ vmDiskInfo.getSize(),
vmDiskInfoListFromBackup.get(index).getSize()));
+ }
+ }
+ index++;
+ }
+ }
+
+ @Override
+ public VmDiskInfo getRootDiskInfoFromBackup(Backup backup) {
+ List<Backup.VolumeInfo> volumes = backup.getBackedUpVolumes();
+ VmDiskInfo rootDiskOffering = null;
+ if (volumes == null || volumes.isEmpty()) {
+ throw new CloudRuntimeException("Failed to get backed-up volumes
info from backup");
+ }
+ for (Backup.VolumeInfo volume : volumes) {
+ if (volume.getType() == Volume.Type.ROOT) {
+ DiskOfferingVO diskOffering =
diskOfferingDao.findByUuid(volume.getDiskOfferingId());
+ if (diskOffering == null) {
+ throw new CloudRuntimeException(String.format("Unable to
find the root disk offering with uuid (%s) " +
+ "stored in backup. Please specify a valid root
disk offering id while creating the instance",
+ volume.getDiskOfferingId()));
+ }
+ Long size = volume.getSize() / (1024 * 1024 * 1024);
+ rootDiskOffering = new VmDiskInfo(diskOffering, size,
volume.getMinIops(), volume.getMaxIops());
+ }
+ }
+ if (rootDiskOffering == null) {
+ throw new CloudRuntimeException("Failed to get the root disk in
backed-up volumes info from backup");
+ }
+ return rootDiskOffering;
+ }
+
+ @Override
+ public List<VmDiskInfo> getDataDiskInfoListFromBackup(Backup backup) {
+ List<VmDiskInfo> vmDiskInfoList = new ArrayList<>();
+ List<Backup.VolumeInfo> volumes = backup.getBackedUpVolumes();
+ if (volumes == null || volumes.isEmpty()) {
+ throw new CloudRuntimeException("Failed to get backed-up Volumes
info from backup");
+ }
+ for (Backup.VolumeInfo volume : volumes) {
+ if (volume.getType() == Volume.Type.DATADISK) {
+ DiskOfferingVO diskOffering =
diskOfferingDao.findByUuid(volume.getDiskOfferingId());
+ if (diskOffering == null ||
diskOffering.getState().equals(DiskOffering.State.Inactive)) {
+ throw new CloudRuntimeException("Unable to find the disk
offering with uuid (" + volume.getDiskOfferingId() + ") stored in backup. " +
+ "Please specify a valid disk offering id while
creating the instance");
+ }
+ Long size = volume.getSize() / (1024 * 1024 * 1024);
+ vmDiskInfoList.add(new VmDiskInfo(diskOffering, size,
volume.getMinIops(), volume.getMaxIops(), volume.getDeviceId()));
+ }
+ }
+ return vmDiskInfoList;
+ }
+
+ private List<String> parseAddressString(String input) {
+ if (input == null) return null;
+ return Arrays.stream(input.split(","))
+ .map(s -> "null".equalsIgnoreCase(s.trim()) ? null : s.trim())
+ .collect(Collectors.toList());
+ }
+
+ @Override
+ public Map<Long, Network.IpAddresses> getIpToNetworkMapFromBackup(Backup
backup, boolean preserveIps, List<Long> networkIds)
+ {
+ Map<Long, Network.IpAddresses> ipToNetworkMap = new
LinkedHashMap<Long, Network.IpAddresses>();
+
+ String nicsJson = backup.getDetail(ApiConstants.NICS);
+ if (nicsJson == null) {
+ throw new CloudRuntimeException("Backup doesn't contain network
information. " +
+ "Please specify at least one valid network while creating
instance");
+ }
+
+ Type type = new TypeToken<List<Map<String, String>>>(){}.getType();
+ List<Map<String, String>> nics = new Gson().fromJson(nicsJson, type);
+
+ for (Map<String, String> nic : nics) {
+ String networkUuid = nic.get(ApiConstants.NETWORK_ID);
+ if (networkUuid == null) {
+ throw new CloudRuntimeException("Backup doesn't contain
network information. " +
+ "Please specify at least one valid network while
creating instance");
+ }
+
+ Network network = networkDao.findByUuid(networkUuid);
+ if (network == null) {
+ throw new CloudRuntimeException("Unable to find network with
the uuid " + networkUuid + " stored in backup. " +
+ "Please specify a valid network id while creating the
instance");
+ }
+
+ Long networkId = network.getId();
+ Network.IpAddresses ipAddresses = null;
+
+ if (preserveIps) {
+ String ip = nic.get(ApiConstants.IP_ADDRESS);
+ String ipv6 = nic.get(ApiConstants.IP6_ADDRESS);
+ String mac = nic.get(ApiConstants.MAC_ADDRESS);
+ ipAddresses = networkService.getIpAddressesFromIps(ip, ipv6,
mac);
+ }
+
+ ipToNetworkMap.put(networkId, ipAddresses);
+ networkIds.add(networkId);
+ }
+ return ipToNetworkMap;
+ }
+
+ @Override
+ public Boolean canCreateInstanceFromBackup(final Long backupId) {
+ final BackupVO backup = backupDao.findById(backupId);
+ BackupOffering offering =
backupOfferingDao.findByIdIncludingRemoved(backup.getBackupOfferingId());
+ if (offering == null) {
+ throw new CloudRuntimeException("Failed to find backup offering");
+ }
+ final BackupProvider backupProvider =
getBackupProvider(offering.getProvider());
+ return backupProvider.supportsInstanceFromBackup();
+ }
+
+ @Override
+ public boolean restoreBackupToVM(final Long backupId, final Long vmId)
throws ResourceUnavailableException {
+ final BackupVO backup = backupDao.findById(backupId);
+ if (backup == null) {
+ throw new CloudRuntimeException("Backup " + backupId + " does not
exist");
+ }
+ if (backup.getStatus() != Backup.Status.BackedUp) {
+ throw new CloudRuntimeException("Backup should be in BackedUp
state");
+ }
+ validateBackupForZone(backup.getZoneId());
+
+ VMInstanceVO vm = vmInstanceDao.findByIdIncludingRemoved(vmId);
+ if (vm == null) {
+ throw new CloudRuntimeException("Instance with ID " +
backup.getVmId() + " couldn't be found.");
+ }
+ accountManager.checkAccess(CallContext.current().getCallingAccount(),
null, true, vm);
+
+ if (vm.getRemoved() != null) {
+ throw new CloudRuntimeException("Instance with ID " +
backup.getVmId() + " couldn't be found.");
+ }
+ if (!vm.getState().equals(VirtualMachine.State.Stopped)) {
+ throw new CloudRuntimeException("The VM should be in stopped
state");
+ }
+
+ List<Backup.VolumeInfo> backupVolumes = backup.getBackedUpVolumes();
+ if (backupVolumes == null) {
+ throw new CloudRuntimeException("Backed up volumes info not found
in the backup");
+ }
+
+ List<VolumeVO> vmVolumes = volumeDao.findByInstance(vmId);
+ if (vmVolumes.size() != backupVolumes.size()) {
+ throw new CloudRuntimeException("Unable to create Instance from
backup as the backup has a different number of disks than the Instance");
+ }
+
+ int index = 0;
+ for (VolumeVO vmVolume: vmVolumes) {
+ Backup.VolumeInfo backupVolume = backupVolumes.get(index);
+ if (vmVolume.getSize() < backupVolume.getSize()) {
+ throw new CloudRuntimeException(String.format(
+ "Instance volume size %d[GiB] for volume (%s) is less than
the backed-up volume size %d[GiB] for backed-up volume (%s).",
+ vmVolume.getSize(), vmVolume.getUuid(),
backupVolume.getSize(), backupVolume.getUuid()));
+ }
+ index++;
+ }
+
+ BackupOffering offering =
backupOfferingDao.findByIdIncludingRemoved(backup.getBackupOfferingId());
+ if (offering == null) {
+ throw new CloudRuntimeException("Failed to find backup offering");
+ }
+ final BackupProvider backupProvider =
getBackupProvider(offering.getProvider());
+ if (!backupProvider.supportsInstanceFromBackup()) {
+ throw new CloudRuntimeException("Create instance from backup is
not supported by the " + offering.getProvider() + " provider.");
+ }
+
+ String backupDetailsInMessage =
ReflectionToStringBuilderUtils.reflectOnlySelectedFields(backup, "uuid",
"externalId", "newVMId", "type", "status", "date");
+ Long eventId = null;
+ try {
+ updateVmState(vm, VirtualMachine.Event.RestoringRequested,
VirtualMachine.State.Restoring);
+ updateVolumeState(vm, Volume.Event.RestoreRequested,
Volume.State.Restoring);
+ eventId = ActionEventUtils.onStartedActionEvent(User.UID_SYSTEM,
vm.getAccountId(), EventTypes.EVENT_VM_CREATE_FROM_BACKUP,
+ String.format("Creating Instance %s from backup %s",
vm.getInstanceName(), backup.getUuid()),
+ vm.getId(),
ApiCommandResourceType.VirtualMachine.toString(),
+ true, 0);
+
+ String host = null;
+ String dataStore = null;
+ if (!"nas".equals(offering.getProvider())) {
+ Pair<HostVO, StoragePoolVO> restoreInfo =
getRestoreVolumeHostAndDatastore(vm);
+ host = restoreInfo.first().getPrivateIpAddress();
+ dataStore = restoreInfo.second().getUuid();
+ }
+ if (!backupProvider.restoreBackupToVM(vm, backup, host,
dataStore)) {
+ throw new CloudRuntimeException(String.format("Error restoring
backup [%s] to VM %s.", backupDetailsInMessage, vm.getUuid()));
+ }
+ } catch (Exception e) {
+ updateVolumeState(vm, Volume.Event.RestoreFailed,
Volume.State.Ready);
+ updateVmState(vm, VirtualMachine.Event.RestoringFailed,
VirtualMachine.State.Stopped);
Review Comment:
If we are creating a VM from a backup, this is a new VM correct?
If the process failed, shouldn't the VM and the volumes go into an error
state? If not, we might have a bunch of volumes that are marked as `Ready` but
are inconsistent.
##########
server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java:
##########
@@ -282,18 +319,75 @@ public boolean deleteBackupOffering(final Long
offeringId) {
throw new CloudRuntimeException("Could not find a backup offering
with id: " + offeringId);
}
- if (vmInstanceDao.listByZoneWithBackups(offering.getZoneId(),
offering.getId()).size() > 0) {
+ if (backupDao.listByOfferingId(offering.getId()).size() > 0) {
+ throw new CloudRuntimeException("Backup Offering cannot be removed
as it has backups associated with it.");
+ }
+
+ if (vmInstanceDao.listByZoneAndBackupOffering(offering.getZoneId(),
offering.getId()).size() > 0) {
throw new CloudRuntimeException("Backup offering is assigned to
VMs, remove the assignment(s) in order to remove the offering.");
}
- validateForZone(offering.getZoneId());
+ validateBackupForZone(offering.getZoneId());
return backupOfferingDao.remove(offering.getId());
}
- public static String createVolumeInfoFromVolumes(List<VolumeVO> vmVolumes)
{
+ @Override
+ public Map<String, String> getBackupDetailsFromVM(VirtualMachine vm) {
+ HashMap<String, String> details = new HashMap<>();
+
+ ServiceOffering serviceOffering =
serviceOfferingDao.findById(vm.getServiceOfferingId());
+ details.put(ApiConstants.SERVICE_OFFERING_ID,
serviceOffering.getUuid());
+ VirtualMachineTemplate template =
vmTemplateDao.findById(vm.getTemplateId());
+ details.put(ApiConstants.TEMPLATE_ID, template.getUuid());
+
+ List<VMInstanceDetailVO> vmDetails =
vmInstanceDetailsDao.listDetails(vm.getId());
+ HashMap<String, String> settings = new HashMap<>();
+ for (VMInstanceDetailVO detail : vmDetails) {
+ settings.put(detail.getName(), detail.getValue());
+ }
+ if (!settings.isEmpty()) {
+ details.put(ApiConstants.VM_SETTINGS, new Gson().toJson(settings));
+ }
+
+ List<UserVmJoinVO> userVmJoinVOs =
userVmJoinDao.searchByIds(vm.getId());
+ if (userVmJoinVOs != null && !userVmJoinVOs.isEmpty()) {
+ List<Map<String, String>> nics = new ArrayList<>();
+ Set<String> seen = new HashSet<>();
+
+ for (UserVmJoinVO userVmJoinVO : userVmJoinVOs) {
+ Map<String, String> nicInfo = new HashMap<>();
+ String key = userVmJoinVO.getNetworkUuid();
+ if (seen.add(key)) {
+ nicInfo.put(ApiConstants.NETWORK_ID,
userVmJoinVO.getNetworkUuid());
+ nicInfo.put(ApiConstants.IP_ADDRESS,
userVmJoinVO.getIpAddress());
+ nicInfo.put(ApiConstants.IP6_ADDRESS,
userVmJoinVO.getIp6Address());
+ nicInfo.put(ApiConstants.MAC_ADDRESS,
userVmJoinVO.getMacAddress());
+ nics.add(nicInfo);
+ }
+ }
+
+ if (!nics.isEmpty()) {
+ String nicJson = new Gson().toJson(nics);
+ details.put("nics", nicJson);
+ }
Review Comment:
We could extract this to a method
##########
engine/schema/src/main/java/org/apache/cloudstack/backup/BackupVO.java:
##########
@@ -240,10 +268,27 @@ public void setBackedUpVolumes(String backedUpVolumes) {
this.backedUpVolumes = backedUpVolumes;
}
+ @Override
+ public Map<String, String> getDetails() {
+ return details;
+ }
+
+ @Override
+ public String getDetail(String name) {
+ return this.details.get(name);
+ }
+
+ public void setDetails(Map<String, String> details) {
+ this.details = details;
+ }
+
+ public void addDetails(Map<String, String> details) {
+ this.details.putAll(details);
+ }
Review Comment:
It seems like this is never used in the code
##########
server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java:
##########
@@ -599,10 +705,19 @@ public boolean createBackup(final Long vmId, final Long
scheduleId) throws Resou
throw new CloudRuntimeException("VM backup offering not found");
}
+ final BackupProvider backupProvider =
getBackupProvider(offering.getProvider());
+ if (offering == null) {
+ throw new CloudRuntimeException("VM backup provider not found for
the offering");
+ }
Review Comment:
If offering is null, a NPE will be thrown in the previous line.
##########
plugins/backup/networker/src/main/java/org/apache/cloudstack/backup/NetworkerBackupProvider.java:
##########
@@ -116,6 +121,15 @@ public class NetworkerBackupProvider extends AdapterBase
implements BackupProvid
@Inject
private VMInstanceDao vmInstanceDao;
+ @Inject
+ private EntityManager entityManager;
+
+ @Inject
+ BackupManager backupManager;
Review Comment:
```suggestion
private BackupManager backupManager;
```
##########
engine/schema/src/main/resources/META-INF/db/schema-42010to42100.sql:
##########
@@ -241,3 +237,88 @@ ALTER TABLE `cloud`.`vm_instance_details` ADD CONSTRAINT
`fk_vm_instance_details
CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.backup_schedule', 'uuid',
'VARCHAR(40) NOT NULL');
UPDATE `cloud`.`backup_schedule` SET uuid = UUID();
+
+-- Add column max_backup to backup_schedule table
+CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.backup_schedule', 'max_backups',
'int(8) default NULL COMMENT "maximum number of backups to maintain"');
+CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.backup_schedule', 'quiescevm',
'tinyint(1) default NULL COMMENT "Quiesce VM before taking backup"');
+
+-- Add columns name, description and backup_interval_type to backup table
+CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.backups', 'name', 'VARCHAR(255)
NULL COMMENT "name of the backup"');
+UPDATE `cloud`.`backups` backup INNER JOIN `cloud`.`vm_instance` vm ON
backup.vm_id = vm.id SET backup.name = vm.name;
+ALTER TABLE `cloud`.`backups` MODIFY COLUMN `name` VARCHAR(255) NOT NULL;
+CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.backups', 'description',
'VARCHAR(1024) COMMENT "description for the backup"');
+CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.backups', 'backup_interval_type',
'int(5) COMMENT "type of backup, e.g. manual, recurring - hourly, daily, weekly
or monthly"');
+
+-- Create backup details table
+CREATE TABLE IF NOT EXISTS `cloud`.`backup_details` (
+ `id` bigint unsigned NOT NULL auto_increment,
+ `backup_id` bigint unsigned NOT NULL COMMENT 'backup id',
+ `name` varchar(255) NOT NULL,
+ `value` TEXT NOT NULL,
+ `display` tinyint(1) NOT NULL DEFAULT 1 COMMENT 'Should detail be displayed
to the end user',
+ PRIMARY KEY (`id`),
+ CONSTRAINT `fk_backup_details__backup_id` FOREIGN KEY
`fk_backup_details__backup_id`(`backup_id`) REFERENCES `backups`(`id`) ON
DELETE CASCADE
+) ENGINE=InnoDB DEFAULT CHARSET=utf8;
+
+-- Add diskOfferingId, deviceId, minIops and maxIops to backed_volumes in
backups table
+UPDATE `cloud`.`backups` b
+INNER JOIN `cloud`.`vm_instance` vm ON b.vm_id = vm.id
+SET b.backed_volumes = (
+ SELECT CONCAT("[",
+ GROUP_CONCAT(
+ CONCAT(
+ "{\"uuid\":\"", v.uuid, "\",",
+ "\"type\":\"", v.volume_type, "\",",
+ "\"size\":", v.`size`, ",",
+ "\"path\":\"", IFNULL(v.path, 'null'), "\",",
+ "\"deviceId\":", IFNULL(v.device_id, 'null'), ",",
+ "\"diskOfferingId\":\"", doff.uuid, "\",",
+ "\"minIops\":", IFNULL(v.min_iops, 'null'), ",",
+ "\"maxIops\":", IFNULL(v.max_iops, 'null'),
+ "}"
+ )
+ SEPARATOR ","
+ ),
+ "]")
+ FROM `cloud`.`volumes` v
+ LEFT JOIN `cloud`.`disk_offering` doff ON v.disk_offering_id = doff.id
+ WHERE v.instance_id = vm.id
+);
+
+-- Add diskOfferingId, deviceId, minIops and maxIops to backup_volumes in
vm_instance table
+UPDATE `cloud`.`vm_instance` vm
+SET vm.backup_volumes = (
+ SELECT CONCAT("[",
Review Comment:
Why do we have the same info in two tables?
##########
server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java:
##########
@@ -1470,7 +1910,22 @@ private Backup
checkAndUpdateIfBackupEntryExistsForRestorePoint(Backup.RestorePo
return null;
}
- private void syncBackups(BackupProvider backupProvider, VirtualMachine
vm, Backup.Metric metric) {
+ private void processRemoveList(List<Long> removeList, VirtualMachine
vm) {
+ for (final Long backupIdToRemove : removeList) {
+ logger.warn(String.format("Removing backup with ID: [%s].",
backupIdToRemove));
+ Backup backup = backupDao.findById(backupIdToRemove);
+ resourceLimitMgr.decrementResourceCount(backup.getAccountId(),
Resource.ResourceType.backup);
+ resourceLimitMgr.decrementResourceCount(backup.getAccountId(),
Resource.ResourceType.backup_storage, backup.getSize());
+ boolean result = backupDao.remove(backupIdToRemove);
+ if (result) {
+
checkAndGenerateUsageForLastBackupDeletedAfterOfferingRemove(vm, backup);
+ } else {
+ logger.error("Failed to remove backup db wentry ith ID: {}
during sync backups", backupIdToRemove);
Review Comment:
```suggestion
logger.error("Failed to remove backup db entry with ID:
{} during sync backups", backupIdToRemove);
```
##########
server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java:
##########
@@ -926,6 +1058,224 @@ private Backup.VolumeInfo
getVolumeInfo(List<Backup.VolumeInfo> backedUpVolumes,
return null;
}
+ @Override
+ public void checkVmDisksSizeAgainstBackup(List<VmDiskInfo> vmDiskInfoList,
Backup backup) {
+ List<VmDiskInfo> vmDiskInfoListFromBackup =
getDataDiskInfoListFromBackup(backup);
+ int index = 0;
+ if (vmDiskInfoList.size() != vmDiskInfoListFromBackup.size()) {
+ throw new InvalidParameterValueException("Unable to create
Instance from Backup " +
+ "as the backup has a different number of disks than the
Instance.");
+ }
+ for (VmDiskInfo vmDiskInfo : vmDiskInfoList) {
+ if (index < vmDiskInfoListFromBackup.size()) {
+ if (vmDiskInfo.getSize() <
vmDiskInfoListFromBackup.get(index).getSize()) {
+ throw new InvalidParameterValueException(
+ String.format("Instance volume size %d[GiB] cannot
be less than the backed-up volume size %d[GiB].",
+ vmDiskInfo.getSize(),
vmDiskInfoListFromBackup.get(index).getSize()));
+ }
+ }
+ index++;
+ }
+ }
+
+ @Override
+ public VmDiskInfo getRootDiskInfoFromBackup(Backup backup) {
+ List<Backup.VolumeInfo> volumes = backup.getBackedUpVolumes();
+ VmDiskInfo rootDiskOffering = null;
+ if (volumes == null || volumes.isEmpty()) {
+ throw new CloudRuntimeException("Failed to get backed-up volumes
info from backup");
+ }
+ for (Backup.VolumeInfo volume : volumes) {
+ if (volume.getType() == Volume.Type.ROOT) {
+ DiskOfferingVO diskOffering =
diskOfferingDao.findByUuid(volume.getDiskOfferingId());
+ if (diskOffering == null) {
+ throw new CloudRuntimeException(String.format("Unable to
find the root disk offering with uuid (%s) " +
+ "stored in backup. Please specify a valid root
disk offering id while creating the instance",
+ volume.getDiskOfferingId()));
+ }
+ Long size = volume.getSize() / (1024 * 1024 * 1024);
+ rootDiskOffering = new VmDiskInfo(diskOffering, size,
volume.getMinIops(), volume.getMaxIops());
+ }
+ }
+ if (rootDiskOffering == null) {
+ throw new CloudRuntimeException("Failed to get the root disk in
backed-up volumes info from backup");
+ }
+ return rootDiskOffering;
+ }
+
+ @Override
+ public List<VmDiskInfo> getDataDiskInfoListFromBackup(Backup backup) {
+ List<VmDiskInfo> vmDiskInfoList = new ArrayList<>();
+ List<Backup.VolumeInfo> volumes = backup.getBackedUpVolumes();
+ if (volumes == null || volumes.isEmpty()) {
+ throw new CloudRuntimeException("Failed to get backed-up Volumes
info from backup");
+ }
+ for (Backup.VolumeInfo volume : volumes) {
+ if (volume.getType() == Volume.Type.DATADISK) {
+ DiskOfferingVO diskOffering =
diskOfferingDao.findByUuid(volume.getDiskOfferingId());
+ if (diskOffering == null ||
diskOffering.getState().equals(DiskOffering.State.Inactive)) {
+ throw new CloudRuntimeException("Unable to find the disk
offering with uuid (" + volume.getDiskOfferingId() + ") stored in backup. " +
+ "Please specify a valid disk offering id while
creating the instance");
+ }
+ Long size = volume.getSize() / (1024 * 1024 * 1024);
+ vmDiskInfoList.add(new VmDiskInfo(diskOffering, size,
volume.getMinIops(), volume.getMaxIops(), volume.getDeviceId()));
+ }
+ }
+ return vmDiskInfoList;
+ }
+
+ private List<String> parseAddressString(String input) {
+ if (input == null) return null;
+ return Arrays.stream(input.split(","))
+ .map(s -> "null".equalsIgnoreCase(s.trim()) ? null : s.trim())
+ .collect(Collectors.toList());
+ }
Review Comment:
Is this used anywhere? Also, what is its purpose?
##########
server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java:
##########
@@ -1959,8 +1959,7 @@ private Pair<String, String>
handleCheckAndRepairVolumeJob(Long vmId, Long volum
} else if (jobResult instanceof ResourceAllocationException) {
throw (ResourceAllocationException)jobResult;
} else if (jobResult instanceof Throwable) {
- Throwable throwable = (Throwable) jobResult;
- throw new RuntimeException(String.format("Unexpected
exception: %s", throwable.getMessage()), throwable);
+ throw new RuntimeException("Unexpected exception",
(Throwable) jobResult);
Review Comment:
CloudRuntimeException
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
