JoaoJandre commented on code in PR #10140:
URL: https://github.com/apache/cloudstack/pull/10140#discussion_r2225869526
##########
engine/schema/src/main/java/org/apache/cloudstack/backup/BackupVO.java:
##########
@@ -240,10 +268,32 @@ public void setBackedUpVolumes(String backedUpVolumes) {
this.backedUpVolumes = backedUpVolumes;
}
+ @Override
+ public Map<String, String> getDetails() {
+ return details;
+ }
+
+ public void setDetail(String name, String value) {
+ assert (details != null) : "Did you forget to load the details?";
Review Comment:
@abh1sar Two wrongs do not make a right. It is not advisable to use asserts
in production code, these can be disabled when running the process, for
example.
I also think it would be better to throw a CloudRuntimeException instead of
an AssertionError if this condition is true.
##########
engine/schema/src/main/resources/META-INF/db/schema-42010to42100.sql:
##########
@@ -203,3 +199,91 @@ SET `sort_key` = CASE
ELSE `sort_key`
END;
-- End: Changes for Guest OS category cleanup
+
+-- 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"');
+
+-- Make the column vm_id in backups table nullable to handle orphan backups
+ALTER TABLE `cloud`.`backups` MODIFY COLUMN `vm_id` BIGINT UNSIGNED NULL;
Review Comment:
After a VM is expunged, its record is still on the DB, it just set as
removed. I think it would be better to keep the record to the VM.
##########
engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupDaoImpl.java:
##########
@@ -193,27 +260,61 @@ 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);
+ }
+
+ @Override
+ public BackupResponse newBackupResponse(Backup backup, Boolean
listVmDetails) {
VMInstanceVO vm =
vmInstanceDao.findByIdIncludingRemoved(backup.getVmId());
- AccountVO account =
accountDao.findByIdIncludingRemoved(vm.getAccountId());
- DomainVO domain = domainDao.findByIdIncludingRemoved(vm.getDomainId());
- DataCenterVO zone =
dataCenterDao.findByIdIncludingRemoved(vm.getDataCenterId());
+ AccountVO account =
accountDao.findByIdIncludingRemoved(backup.getAccountId());
+ DomainVO domain =
domainDao.findByIdIncludingRemoved(backup.getDomainId());
+ DataCenterVO zone =
dataCenterDao.findByIdIncludingRemoved(backup.getZoneId());
Long offeringId = backup.getBackupOfferingId();
- if (offeringId == null) {
- offeringId = vm.getBackupOfferingId();
- }
BackupOffering offering =
backupOfferingDao.findByIdIncludingRemoved(offeringId);
BackupResponse response = new BackupResponse();
response.setId(backup.getUuid());
- response.setVmId(vm.getUuid());
+ response.setName(backup.getName());
+ response.setDescription(backup.getDescription());
response.setVmName(vm.getHostName());
+ if (vm.getState() != VirtualMachine.State.Expunging) {
Review Comment:
Why only not in expunging?
--
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
