address review comments

Author: abh1sar

api/src/main/java/org/apache/cloudstack/api/command/admin/backup/CreateImageTransferCmd.java

@@ -65,7 +65,8 @@ public class CreateImageTransferCmd extends BaseCmd implements AdminCmd {
 
     @Parameter(name = ApiConstants.FORMAT,
             type = CommandType.STRING,
-            description = "Format of the image: cow/raw. Currently only raw is supported for download. Defaults to raw if not provided")
+            description = "Format for the image transfer: raw/cow. 'raw' will create an NBD backend. 'cow' will use the File backend." +
+                    "For download, only the 'raw' format is supported. Default: raw")
     private String format;
 
     public Long getBackupId() {

api/src/main/java/org/apache/cloudstack/api/command/admin/backup/FinalizeImageTransferCmd.java

@@ -32,7 +32,7 @@
 import org.apache.cloudstack.context.CallContext;
 
 @APICommand(name = "finalizeImageTransfer",
-        description = "Finalize an image transfe. This API is intended for testing only and is disabled by default.r",
+        description = "Finalize an image transfer. This API is intended for testing only and is disabled by default.",
         responseObject = SuccessResponse.class,
         since = "4.23.0",
         authorized = {RoleType.Admin})

api/src/main/java/org/apache/cloudstack/api/command/admin/backup/StartBackupCmd.java

@@ -37,7 +37,7 @@
 import com.cloud.event.EventTypes;
 
 @APICommand(name = "startBackup",
-        description = "Start a VM backup session. This API is intended for testing only and is disabled by default.",
+        description = "Start a VM backup session using pull mode backup-begin on the KVM host. This API is intended for testing only and is disabled by default.",
         responseObject = BackupResponse.class,
         since = "4.23.0",
         authorized = {RoleType.Admin})

api/src/main/java/org/apache/cloudstack/backup/Backup.java

@@ -39,7 +39,7 @@ public interface Backup extends ControlledEntity, InternalIdentity, Identity {
     Long getHostId();
 
     enum Status {
-        Allocated, Queued, BackingUp, ReadyForTransfer, FinalizingTransfer, BackedUp, Error, Failed, Restoring, Removed, Expunged
+        Allocated, Queued, BackingUp, ReadyForImageTransfer, FinalizingImageTransfer, BackedUp, Error, Failed, Restoring, Removed, Expunged
     }
 
     class Metric {

api/src/main/java/org/apache/cloudstack/backup/BackupManager.java

@@ -58,7 +58,7 @@ public interface BackupManager extends BackupService, Configurable, PluggableSer
     ConfigKey<String> BackupProviderPlugin = new ValidatedConfigKey<>("Advanced", String.class,
             "backup.framework.provider.plugin",
             "dummy",
-            "The backup and recovery provider plugin. Valid plugin values: dummy, veeam, networker, nas",
+            "The backup and recovery provider plugin. Valid plugin values: dummy, veeam, networker and nas",
             true, ConfigKey.Scope.Zone, BackupFrameworkEnabled.key(), value -> validateBackupProviderConfig((String)value));
 
     ConfigKey<Long> BackupSyncPollingInterval = new ConfigKey<>("Advanced", Long.class,

plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/api/converter/BackupVOToBackupConverter.java

@@ -86,9 +86,9 @@ private static String mapStatusToPhase(final BackupVO.Status status) {
                 return "initializing";
             case BackingUp:
                 return "starting";
-            case ReadyForTransfer:
+            case ReadyForImageTransfer:
                 return "ready";
-            case FinalizingTransfer:
+            case FinalizingImageTransfer:
                 return "finalizing";
             case Restoring:
             case BackedUp:

server/src/main/java/org/apache/cloudstack/backup/KVMBackupExportServiceImpl.java

@@ -138,8 +138,11 @@ public class KVMBackupExportServiceImpl extends ManagerBase implements KVMBackup
 
     VmWorkJobHandlerProxy jobHandlerProxy = new VmWorkJobHandlerProxy(this);
 
-    private boolean isKVMBackupExportServiceSupported(Long zoneId) {
-        return !BackupFrameworkEnabled.value() || StringUtils.equals("dummy", BackupProviderPlugin.valueIn(zoneId));
+    private void verifyKVMBackupExportServiceSupported(Long zoneId) {
+        if (BackupFrameworkEnabled.value() && !StringUtils.equals("dummy", BackupProviderPlugin.valueIn(zoneId))) {
+            throw new CloudRuntimeException("Veeam-KVM integration can not be used along with the " + BackupProviderPlugin.valueIn(zoneId) +
+                    " backup provider. Either set backup.framework.enabled to false or set the Zone level config backup.framework.provider.plugin to \"dummy\".");
+        }
     }
 
     @Override
@@ -151,10 +154,7 @@ public Backup createBackup(StartBackupCmd cmd) {
             throw new CloudRuntimeException("VM not found: " + vmId);
         }
 
-        if (!isKVMBackupExportServiceSupported(vm.getDataCenterId())) {
-            throw new CloudRuntimeException("Veeam-KVM integration can not be used along with the " + BackupProviderPlugin.valueIn(vm.getDataCenterId()) +
-                    " backup provider. Either set backup.framework.enabled to false or set the Zone level config backup.framework.provider.plugin to \"dummy\".");
-        }
+        verifyKVMBackupExportServiceSupported(vm.getDataCenterId());
 
         if (vm.getState() != State.Running && vm.getState() != State.Stopped) {
             throw new CloudRuntimeException("VM must be running or stopped to start backup");
@@ -281,7 +281,7 @@ public Backup startBackup(StartBackupCmd cmd) {
 
         // Update backup with checkpoint creation time
         backup.setCheckpointCreateTime(answer.getCheckpointCreateTime());
-        updateBackupState(backup, Backup.Status.ReadyForTransfer);
+        updateBackupState(backup, Backup.Status.ReadyForImageTransfer);
         queueBackupFinalizeWaitWorkJob(vm, backup);
         return backup;
     }
@@ -329,7 +329,7 @@ public Backup finalizeBackup(FinalizeBackupCmd cmd) {
             throw new CloudRuntimeException("VM not found: " + vmId);
         }
 
-        updateBackupState(backup, Backup.Status.FinalizingTransfer);
+        updateBackupState(backup, Backup.Status.FinalizingImageTransfer);
 
         List<ImageTransferVO> transfers = imageTransferDao.listByBackupId(backupId);
         for (ImageTransferVO transfer : transfers) {
@@ -607,10 +607,7 @@ public ImageTransfer createImageTransfer(long volumeId, Long backupId, ImageTran
             throw new CloudRuntimeException("Volume not found with the specified Id");
         }
 
-        if (!isKVMBackupExportServiceSupported(volume.getDataCenterId())) {
-            throw new CloudRuntimeException("Veeam-KVM integration can not be used along with the " + BackupProviderPlugin.valueIn(volume.getDataCenterId()) +
-                    " backup provider. Either set backup.framework.enabled to false or set the Zone level config backup.framework.provider.plugin to \"dummy\".");
-        }
+        verifyKVMBackupExportServiceSupported(volume.getDataCenterId());
 
         ImageTransferVO existingTransfer = imageTransferDao.findByVolume(volume.getId());
         if (existingTransfer != null) {
@@ -805,10 +802,8 @@ public boolean deleteVmCheckpoint(DeleteVmCheckpointCmd cmd) {
         if (vm == null) {
             throw new CloudRuntimeException("VM not found: " + cmd.getVmId());
         }
-        if (!isKVMBackupExportServiceSupported(vm.getDataCenterId())) {
-            throw new CloudRuntimeException("Veeam-KVM integration can not be used along with the " + BackupProviderPlugin.valueIn(vm.getDataCenterId()) +
-                    " backup provider. Either set backup.framework.enabled to false or set the Zone level config backup.framework.provider.plugin to \"dummy\".");
-        }
+
+        verifyKVMBackupExportServiceSupported(vm.getDataCenterId());
 
         if (vm.getState() != State.Running && vm.getState() != State.Stopped) {
             throw new CloudRuntimeException("VM must be running or stopped to delete checkpoint");