core: make VmBackupStop async using flag

Currently StopVmBackup command changed VM Backup phase in order to stop/finalize backup. This patch changes StopVmBackupCommand to set a flag on the backup entity that it is stopped (only for Hybrid backup), and then HybridBackupCommand detects that the flag was raised and stops / finalizes the Backup accordingly.
oVirt · Apr 17, 2022 · 83a2a17 · 83a2a17
1 parent b2384c6
commit 83a2a17
Show file tree

Hide file tree

Showing 7 changed files with 73 additions and 24 deletions.
diff --git a/...dules/bll/src/main/java/org/ovirt/engine/core/bll/storage/backup/HybridBackupCommand.java b/...dules/bll/src/main/java/org/ovirt/engine/core/bll/storage/backup/HybridBackupCommand.java
@@ -135,7 +135,15 @@ protected void executeCommand() {
     @Override
     public boolean performNextOperation(int completedChildCount) {
         restoreCommandState();
-        if (getParameters().getVmBackup().getPhase() == VmBackupPhase.STARTING) {
+        VmBackupPhase phase = getParameters().getVmBackup().getPhase();
+
+        if (getParameters().getVmBackup().isStopped() && !phase.isBackupCleaningUp()) {
+            removeAutoGeneratedSnapshot(createStepsContext(StepEnum.MERGE_SNAPSHOTS), getActionType(), getParameters());
+            updateVmBackupPhase(VmBackupPhase.REMOVING_SNAPSHOT);
+            return true;
+        }
+
+        if (phase == VmBackupPhase.STARTING) {
             auditLog(AuditLogType.USER_CREATE_SNAPSHOT_FINISHED_SUCCESS);
             VmBackup vmBackup = getParameters().getVmBackup();
             Map<Guid, Guid> diskIdToSnapshot =
@@ -160,13 +168,7 @@ public boolean performNextOperation(int completedChildCount) {
             return true;
         }
 
-        if (getParameters().getVmBackup().getPhase() == VmBackupPhase.READY) {
-            return true;
-        }
-
-        if (getParameters().getVmBackup().getPhase() == VmBackupPhase.FINALIZING) {
-            removeAutoGeneratedSnapshot(createStepsContext(StepEnum.MERGE_SNAPSHOTS), getActionType(), getParameters());
-            updateVmBackupPhase(VmBackupPhase.REMOVING_SNAPSHOT);
+        if (phase == VmBackupPhase.READY) {
             return true;
         }
 

diff --git a/...dules/bll/src/main/java/org/ovirt/engine/core/bll/storage/backup/StopVmBackupCommand.java b/...dules/bll/src/main/java/org/ovirt/engine/core/bll/storage/backup/StopVmBackupCommand.java
@@ -70,7 +70,7 @@ protected void executeCommand() {
             vmBackup.setDisks(vmBackupDao.getDisksByBackupId(vmBackup.getId()));
 
             VmBackupPhase phase = vmBackup.getPhase();
-            if (phase.isBackupFinalizing() || phase.isBackupFinished()) {
+            if (phase.isBackupFinalizing() || phase.isBackupFinished() || vmBackup.isStopped()) {
                 String errorMsg = String.format(
                         "VM '%s' backup '%s' is already in '%s' phase, no need to stop the backup again",
                         vmBackup.getVmId(), vmBackup.getId(), phase);
@@ -80,11 +80,15 @@ protected void executeCommand() {
             }
 
             if (stopVmBackup()) {
-                if (phase == VmBackupPhase.READY) {
+                if (isHybridBackup()) {
+                    updateVmBackupStopped();
+                    setSucceeded(true);
+                } else if (phase == VmBackupPhase.READY) {
+                    // Finalize backup for backup types prior to hybrid backup
                     updateVmBackupPhase(VmBackupPhase.FINALIZING);
                     setSucceeded(true);
-                } else {
-                    // Premature backup finish.
+                } else  {
+                    // Premature backup finish for backup types prior to hybrid backup.
                     String errorMsg = String.format(
                             "Backup '%s' is in '%s' phase, premature backup stop was initiated.",
                             vmBackup.getId(), phase);
@@ -94,7 +98,11 @@ protected void executeCommand() {
                 }
             } else {
                 log.error("Failed to stop backup '{}' in '{}' phase", vmBackup.getId(), phase);
-                updateVmBackupPhase(VmBackupPhase.FINALIZING_FAILURE);
+                if (isHybridBackup()) {
+                    updateVmBackupStopped();
+                } else {
+                    updateVmBackupPhase(VmBackupPhase.FINALIZING_FAILURE);
+                }
             }
         }
     }
@@ -130,13 +138,23 @@ private boolean isLiveBackup() {
         return vmBackup.getBackupType() == VmBackupType.Live;
     }
 
+    private boolean isHybridBackup() {
+        return vmBackup.getBackupType() == VmBackupType.Hybrid;
+    }
+
     private void updateVmBackupPhase(VmBackupPhase phase) {
         log.info("Change VM '{}' backup '{}' phase from '{}' to '{}'",
                 vmBackup.getVmId(), vmBackup.getId(), vmBackup.getPhase(), phase);
         vmBackup.setPhase(phase);
         vmBackupDao.update(vmBackup);
     }
 
+    private void updateVmBackupStopped() {
+        log.info("VM '{}' backup '{}' marked as stopped", vmBackup.getVmId(), vmBackup.getId());
+        vmBackup.setStopped(true);
+        vmBackupDao.update(vmBackup);
+    }
+
     private EngineLock getEntityUpdateLock(Guid backupId) {
         Map<String, Pair<String, String>> lockMap = Collections.singletonMap(
                 backupId.toString(),
@@ -163,6 +181,10 @@ public List<PermissionSubject> getPermissionCheckSubjects() {
     public AuditLogType getAuditLogTypeValue() {
         addCustomValue("VmName", getVm().getName());
         addCustomValue("backupId", getParameters().getVmBackup().getId().toString());
-        return getSucceeded() ? AuditLogType.VM_BACKUP_FINALIZED : AuditLogType.VM_BACKUP_FAILED_TO_FINALIZE;
+        if (getSucceeded()) {
+            return isHybridBackup() ? AuditLogType.VM_BACKUP_STOPPED : AuditLogType.VM_BACKUP_FINALIZED;
+        } else {
+            return AuditLogType.VM_BACKUP_FAILED_TO_FINALIZE;
+        }
     }
 }
diff --git a/.../modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VmBackup.java b/.../modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VmBackup.java
@@ -36,6 +36,8 @@ public class VmBackup implements Queryable, BusinessEntity<Guid> {
 
     private Guid snapshotId;
 
+    private boolean isStopped;
+
     public Guid getId() {
         return id;
     }
@@ -136,6 +138,14 @@ public void setSnapshotId(Guid snapshotId) {
         this.snapshotId = snapshotId;
     }
 
+    public boolean isStopped() {
+        return isStopped;
+    }
+
+    public void setStopped(boolean stopped) {
+        isStopped = stopped;
+    }
+
     @Override
     public int hashCode() {
         return Objects.hash(
@@ -150,7 +160,8 @@ public int hashCode() {
                 modificationDate,
                 description,
                 backupType,
-                snapshotId
+                snapshotId,
+                isStopped
         );
     }
 
@@ -174,7 +185,8 @@ public boolean equals(Object obj) {
                 && Objects.equals(modificationDate, other.modificationDate)
                 && Objects.equals(description, other.description)
                 && Objects.equals(backupType, other.backupType)
-                && Objects.equals(snapshotId, other.snapshotId);
+                && Objects.equals(snapshotId, other.snapshotId)
+                && Objects.equals(isStopped, other.isStopped());
     }
 
     @Override

diff --git a/...les/common/src/main/java/org/ovirt/engine/core/common/businessentities/VmBackupPhase.java b/...les/common/src/main/java/org/ovirt/engine/core/common/businessentities/VmBackupPhase.java
@@ -31,7 +31,11 @@ public boolean isBackupFinished() {
     }
 
     public boolean isBackupFinalizing() {
-        return this == FINALIZING || this == FINALIZING_FAILURE;
+        return this == FINALIZING || this == FINALIZING_FAILURE || this == REMOVING_SNAPSHOT;
+    }
+
+    public boolean isBackupCleaningUp() {
+        return this == REMOVING_SNAPSHOT;
     }
 
     public static VmBackupPhase forName(String name) {

diff --git a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmBackupDaoImpl.java b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmBackupDaoImpl.java
@@ -40,7 +40,8 @@ protected MapSqlParameterSource createFullParametersMapper(VmBackup entity) {
                 .addValue("_update_date", entity.getModificationDate())
                 .addValue("description", entity.getDescription())
                 .addValue("backup_type", entity.getBackupType().getName())
-                .addValue("snapshot_id", entity.getSnapshotId());
+                .addValue("snapshot_id", entity.getSnapshotId())
+                .addValue("is_stopped", entity.isStopped());
     }
 
     @Override
@@ -66,6 +67,7 @@ protected RowMapper<VmBackup> createEntityRowMapper() {
         entity.setDescription(rs.getString("description"));
         entity.setBackupType(VmBackupType.forName(rs.getString("backup_type")));
         entity.setSnapshotId(getGuid(rs, "snapshot_id"));
+        entity.setStopped(rs.getBoolean("is_stopped"));
         return entity;
     };
 
@@ -87,7 +89,8 @@ public void update(VmBackup entity) {
                 .addValue("_update_date", new Date())
                 .addValue("description", entity.getDescription())
                 .addValue("backup_type", entity.getBackupType().getName())
-                .addValue("snapshot_id", entity.getSnapshotId());
+                .addValue("snapshot_id", entity.getSnapshotId())
+                .addValue("is_stopped", entity.isStopped());
         getCallsHandler()
                 .executeModification("UpdateVmBackup", parameterSource);
     }

diff --git a/packaging/dbscripts/upgrade/04_05_0270_add_backup_is_stopped.sql b/packaging/dbscripts/upgrade/04_05_0270_add_backup_is_stopped.sql
@@ -0,0 +1 @@
+SELECT fn_db_add_column('vm_backups', 'is_stopped', 'BOOL NOT NULL DEFAULT FALSE');
diff --git a/packaging/dbscripts/vm_backups_sp.sql b/packaging/dbscripts/vm_backups_sp.sql
@@ -25,7 +25,8 @@ CREATE OR REPLACE FUNCTION InsertVmBackup (
     v__update_date TIMESTAMP WITH TIME ZONE,
     v_description VARCHAR(1024),
     v_backup_type VARCHAR(50),
-    v_snapshot_id UUID
+    v_snapshot_id UUID,
+    v_is_stopped boolean
     )
 RETURNS VOID AS $PROCEDURE$
 BEGIN
@@ -40,7 +41,8 @@ BEGIN
         _update_date,
         description,
         backup_type,
-        snapshot_id
+        snapshot_id,
+        is_stopped
         )
     VALUES (
         v_backup_id,
@@ -53,7 +55,8 @@ BEGIN
         v__update_date,
         v_description,
         v_backup_type,
-        v_snapshot_id
+        v_snapshot_id,
+        v_is_stopped
         );
 END;$PROCEDURE$
 LANGUAGE plpgsql;
@@ -68,7 +71,8 @@ CREATE OR REPLACE FUNCTION UpdateVmBackup (
     v__update_date TIMESTAMP WITH TIME ZONE,
     v_description VARCHAR(1024),
     v_backup_type VARCHAR(50),
-    v_snapshot_id UUID
+    v_snapshot_id UUID,
+    v_is_stopped boolean
     )
 RETURNS VOID AS $PROCEDURE$
 BEGIN
@@ -82,7 +86,8 @@ BEGIN
         _update_date = v__update_date,
         description = v_description,
         backup_type = v_backup_type,
-        snapshot_id = v_snapshot_id
+        snapshot_id = v_snapshot_id,
+        is_stopped = v_is_stopped
     WHERE backup_id = v_backup_id;
 END;$PROCEDURE$
 LANGUAGE plpgsql;