DaanHoogland commented on code in PR #6580:
URL: https://github.com/apache/cloudstack/pull/6580#discussion_r936719762
##########
plugins/backup/veeam/src/main/java/org/apache/cloudstack/backup/VeeamBackupProvider.java:
##########
@@ -201,9 +204,29 @@ public boolean takeBackup(final VirtualMachine vm) {
}
@Override
- public boolean deleteBackup(Backup backup) {
- // Veeam does not support removal of a restore point or point-in-time
backup
- throw new CloudRuntimeException("Veeam B&R plugin does not allow
removal of backup restore point, to delete the backup chain remove VM from the
backup offering");
+ public boolean deleteBackup(Backup backup, boolean forced) {
+ VMInstanceVO vm =
vmInstanceDao.findByIdIncludingRemoved(backup.getVmId());
+ if (vm == null) {
+ throw new CloudRuntimeException(String.format("Could not find any
VM associated with the Backup [uuid: %s, externalId: %s].", backup.getUuid(),
backup.getExternalId()));
+ }
+ if (!forced) {
+ LOG.debug(String.format("Veeam backup provider does not have a
safe way to remove a single restore point, which results in all backup chain
being removed. "
+ + "More information about this limitation can be found in
the links: [%s, %s].",
"https://forums.veeam.com/powershell-f26/removing-a-single-restorepoint-t21061.html";,
+
"https://helpcenter.veeam.com/docs/backup/vsphere/retention_separate_vms.html?ver=110";));
+ throw new CloudRuntimeException("Veeam backup provider does not
have a safe way to remove a single restore point, which results in all backup
chain being removed. "
+ + "Use forced:true to skip this verification and remove
the complete backup chain.");
+ }
+ VeeamClient client = getClient(vm.getDataCenterId());
+ boolean result = client.deleteBackup(backup.getExternalId());
+ if (result) {
+ List<Backup> allBackups = backupDao.listByVmId(backup.getZoneId(),
backup.getVmId());
+ for (Backup b : allBackups) {
+ if (b.getId() != backup.getId()) {
+ backupDao.remove(b.getId());
+ }
+ }
+ }
+ return result;
Review Comment:
I´d rather not. Rather I'd have the code simplified by extracting the
validations at the start of the method. and extracting
```
List<Backup> allBackups =
backupDao.listByVmId(backup.getZoneId(), backup.getVmId());
for (Backup b : allBackups) {
if (b.getId() != backup.getId()) {
backupDao.remove(b.getId());
}
}
```
and making it
```suggestion
if (result) {
<extracted method call here>();
}
return result;
```
early extra return statements are not improving readability of code.
--
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]
