weizhouapache commented on PR #10457: URL: https://github.com/apache/cloudstack/pull/10457#issuecomment-2681664584
> > @DaanHoogland the title says the PR aims to fix the merge problems, but more than 90% of this PR are related to logging > > yes, you are right, but these are rather safe. > > > my suggestion is, revert VeeamBackupProvider.java and then cherry-pick the commit #9898 > > ``` > > git checkout c80b8860e49c61252588c8cf8f40277dcf2c4ee8 plugins/backup/veeam/src/main/java/org/apache/cloudstack/backup/VeeamBackupProvider.java > > git cherry-pick 21b5e4dcae5b3cb73637fc82410bd8e4fd55e99c > > ``` > > What you suggest will not lead to a compiling version without further fixes either, neither starting on main or starting from this branch. > > The question is if https://github.com/apache/cloudstack/pull/10457/files#diff-35507d9957e6b1b12096c22153fc877ea347a7dd2316a0c670a0c80a6c3da3a4R330-R405 does the job. oh, sorry, the commit I pasted was wrong, the correct commit is a8b18a5394212454e2b2fde2bc08da45d344cc7f which is the last commit before merge forward. ``` git checkout c80b8860e49c61252588c8cf8f40277dcf2c4ee8 plugins/backup/veeam/src/main/java/org/apache/cloudstack/backup/VeeamBackupProvider.java wget https://github.com/apache/cloudstack/pull/9898.patch patch -p1 <9898.patch # fix conflicts ``` I tried locally, the difference is ``` diff --git a/plugins/backup/veeam/src/main/java/org/apache/cloudstack/backup/VeeamBackupProvider.java b/plugins/backup/veeam/src/main/java/org/apache/cloudstack/backup/VeeamBackupProvider.java index a7ce0c09cc6..c6b93568828 100644 --- a/plugins/backup/veeam/src/main/java/org/apache/cloudstack/backup/VeeamBackupProvider.java +++ b/plugins/backup/veeam/src/main/java/org/apache/cloudstack/backup/VeeamBackupProvider.java @@ -334,6 +334,7 @@ public class VeeamBackupProvider extends AdapterBase implements BackupProvider, backup.setAccountId(vm.getAccountId()); backup.setDomainId(vm.getDomainId()); backup.setZoneId(vm.getDataCenterId()); + backup.setBackedUpVolumes(BackupManagerImpl.createVolumeInfoFromVolumes(volumeDao.findByInstance(vm.getId()))); backupDao.persist(backup); return backup; } @@ -344,59 +345,6 @@ public class VeeamBackupProvider extends AdapterBase implements BackupProvider, return getClient(vm.getDataCenterId()).listRestorePoints(backupName, vm.getInstanceName()); } - @Override - public void syncBackups(VirtualMachine vm, Backup.Metric metric) { - List<Backup.RestorePoint> restorePoints = listRestorePoints(vm); - if (CollectionUtils.isEmpty(restorePoints)) { - logger.debug("Can't find any restore point to VM: {}", vm); - return; - } - Transaction.execute(new TransactionCallbackNoReturn() { - @Override - public void doInTransactionWithoutResult(TransactionStatus status) { - final List<Backup> backupsInDb = backupDao.listByVmId(null, vm.getId()); - final List<Long> removeList = backupsInDb.stream().map(InternalIdentity::getId).collect(Collectors.toList()); - for (final Backup.RestorePoint restorePoint : restorePoints) { - if (!(restorePoint.getId() == null || restorePoint.getType() == null || restorePoint.getCreated() == null)) { - Backup existingBackupEntry = checkAndUpdateIfBackupEntryExistsForRestorePoint(backupsInDb, restorePoint, metric); - if (existingBackupEntry != null) { - removeList.remove(existingBackupEntry.getId()); - continue; - } - - BackupVO backup = new BackupVO(); - backup.setVmId(vm.getId()); - backup.setExternalId(restorePoint.getId()); - backup.setType(restorePoint.getType()); - backup.setDate(restorePoint.getCreated()); - backup.setStatus(Backup.Status.BackedUp); - if (metric != null) { - backup.setSize(metric.getBackupSize()); - backup.setProtectedSize(metric.getDataSize()); - } - backup.setBackupOfferingId(vm.getBackupOfferingId()); - backup.setAccountId(vm.getAccountId()); - backup.setDomainId(vm.getDomainId()); - backup.setZoneId(vm.getDataCenterId()); - backup.setBackedUpVolumes(BackupManagerImpl.createVolumeInfoFromVolumes(volumeDao.findByInstance(vm.getId()))); - - logger.debug("Creating a new entry in backups: [id: {}, uuid: {}, name: {}, vm_id: {}, external_id: {}, type: {}, date: {}, backup_offering_id: {}, account_id: {}, " - + "domain_id: {}, zone_id: {}].", backup.getId(), backup.getUuid(), backup.getName(), backup.getVmId(), backup.getExternalId(), backup.getType(), backup.getDate(), backup.getBackupOfferingId(), backup.getAccountId(), backup.getDomainId(), backup.getZoneId()); - backupDao.persist(backup); - - ActionEventUtils.onCompletedActionEvent(User.UID_SYSTEM, vm.getAccountId(), EventVO.LEVEL_INFO, EventTypes.EVENT_VM_BACKUP_CREATE, - String.format("Created backup %s for VM ID: %s", backup.getUuid(), vm.getUuid()), - vm.getId(), ApiCommandResourceType.VirtualMachine.toString(),0); - } - } - for (final Long backupIdToRemove : removeList) { - logger.warn(String.format("Removing backup with ID: [%s].", backupIdToRemove)); - backupDao.remove(backupIdToRemove); - } - } - }); - } - ``` -- 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]
