nasbackup: carry the backup method separately from the mode

The RBD path overloaded ChainDecision.mode with rbd-full and rbd-incremental,
the same way the content-based path upstream overloaded it with content-full
and content-incremental. Split the two concerns as that path already did: mode
stays full or incremental, and a new method field says how the backup is taken
-- qcow2 (push mode with dirty bitmaps) or rbd (Ceph-native, anchored on an rbd
snapshot and exported with rbd export-diff).

TakeBackupCommand gets a method field and nasbackup.sh a --method option. Both
default to qcow2 when absent, so a management server that does not send a method
keeps working against an upgraded agent. legacy-full is unchanged: rewriting it
as full plus a "none" method would break incremental-disabled backups on agents
that have not been upgraded yet.

CHAIN_FORMAT stays as it is. The method says how a backup is taken and lives
only for the duration of the request; the chain format is persisted and says
what is on the repository, which is what restore and delete read later. The two
coincide for the rbd method today but answer different questions.

This mirrors c144d09 from #14194, which
introduced the method for the content path and named an RBD diff as the next
one to adopt it. Kept as close to that commit as the branch allows so the two
merge cleanly: this branch carries the RBD path but not the content one, so the
METHOD_CONTENT token is defined without a user here.

Author: weizhouapache

core/src/main/java/org/apache/cloudstack/backup/TakeBackupCommand.java

@@ -38,6 +38,7 @@ public class TakeBackupCommand extends Command {
 
     // Incremental backup fields (NAS provider; null/empty for legacy full-only callers).
     private String mode;          // "full" or "incremental"; null => legacy behaviour (script default)
+    private String method;        // "qcow2" (bitmaps, push mode) or "content" (pull mode); null => "qcow2"
     private String bitmapNew;     // Checkpoint/bitmap name to create with this backup (timestamp-based)
     private String bitmapParent;  // Incremental: parent bitmap to read changes since
 
@@ -125,6 +126,14 @@ public void setMode(String mode) {
         this.mode = mode;
     }
 
+    public String getMethod() {
+        return method;
+    }
+
+    public void setMethod(String method) {
+        this.method = method;
+    }
+
     public String getBitmapNew() {
         return bitmapNew;
     }

plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupChainKeys.java

@@ -55,18 +55,36 @@ public final class NASBackupChainKeys {
      */
     public static final String TYPE_FULL = "full";
     public static final String TYPE_INCREMENTAL = "incremental";
-    /**
-     * Ceph/RBD equivalents of {@link #TYPE_FULL} / {@link #TYPE_INCREMENTAL}. RBD images are raw
-     * and cannot carry a QEMU dirty bitmap, so these anchor each cycle on an RBD snapshot and
-     * export the delta with {@code rbd export-diff} instead of a libvirt checkpoint + push backup.
-     */
-    public static final String TYPE_RBD_FULL = "rbd-full";
-    public static final String TYPE_RBD_INCREMENTAL = "rbd-incremental";
     // Feature disabled: behave exactly like the pre-incremental full-only backup — no QEMU
     // bitmap/checkpoint is created and no chain metadata is persisted. Matches nasbackup.sh's
     // "legacy-full" mode token (which sets make_checkpoint=0).
     public static final String TYPE_LEGACY_FULL = "legacy-full";
 
+    /**
+     * In-memory backup-method sentinels used by {@code ChainDecision.method}: how a backup is
+     * taken, independent of whether it is a full or an incremental ({@code ChainDecision.mode}).
+     *
+     * <p>{@link #METHOD_QCOW2}: libvirt push-mode backup; incrementals come from QEMU persistent
+     * dirty bitmaps (libvirt checkpoints), which only qcow2 volumes can carry.</p>
+     */
+    public static final String METHOD_QCOW2 = "qcow2";
+
+    /**
+     * Content-based libvirt pull-mode backup, used for raw block-device storage such as
+     * LINSTOR/DRBD which cannot carry persistent dirty bitmaps. A full is written as a sparse
+     * qcow2 in a single pass; an incremental is a qcow2 overlay on the disk's NBD export,
+     * safe-rebased onto the parent so it holds only the clusters that differ.
+     */
+    public static final String METHOD_CONTENT = "content";
+
+    /**
+     * Ceph-native backup, used for RBD volumes. They are raw and cannot carry a persistent dirty
+     * bitmap, so each cycle anchors on an RBD snapshot and exports the delta with
+     * {@code rbd export-diff}. Unlike the other methods this one writes {@link #FORMAT_RBD_DIFF}
+     * streams rather than qcow2 files, which is what {@link #CHAIN_FORMAT} records for restore.
+     */
+    public static final String METHOD_RBD = "rbd";
+
     /**
      * VM-scoped detail (stored in {@code vm_instance_details}) holding the QEMU dirty-bitmap
      * name that currently exists on the running VM and is therefore the only valid parent

plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java

@@ -217,11 +217,12 @@ protected Host getVMHypervisorHost(VirtualMachine vm) {
 
     /**
      * Returned by {@link #decideChain(VirtualMachine)} to describe the next backup's place in
-     * the chain: full vs incremental, the bitmap name to create, and (for incrementals) the
-     * parent bitmap and parent file path.
+     * the chain: full vs incremental, how it is taken, the bitmap name to create, and (for
+     * incrementals) the parent bitmap and parent file path.
      */
     static final class ChainDecision {
-        final String mode;            // "full"/"incremental", or their "rbd-" prefixed equivalents
+        final String mode;            // "full" or "incremental"
+        final String method;          // NASBackupChainKeys.METHOD_QCOW2, METHOD_CONTENT or METHOD_RBD
         // On-NAS format of the chain this backup belongs to: NASBackupChainKeys.FORMAT_QCOW2 for
         // the libvirt-checkpoint path, FORMAT_RBD_DIFF for the Ceph-native rbd export-diff path.
         final String format;
@@ -234,9 +235,10 @@ static final class ChainDecision {
         final String chainId;         // chain identifier this backup belongs to
         final int chainPosition;      // 0 for full, N for the Nth incremental in the chain
 
-        private ChainDecision(String mode, String format, String bitmapNew, String bitmapParent, List<String> parentPaths,
-                              String chainId, int chainPosition) {
+        private ChainDecision(String mode, String method, String format, String bitmapNew, String bitmapParent,
+                              List<String> parentPaths, String chainId, int chainPosition) {
             this.mode = mode;
+            this.method = method;
             this.format = format;
             this.bitmapNew = bitmapNew;
             this.bitmapParent = bitmapParent;
@@ -246,8 +248,8 @@ private ChainDecision(String mode, String format, String bitmapNew, String bitma
         }
 
         static ChainDecision fullStart(String bitmapName) {
-            return new ChainDecision(NASBackupChainKeys.TYPE_FULL, NASBackupChainKeys.FORMAT_QCOW2, bitmapName,
-                    null, null, UUID.randomUUID().toString(), 0);
+            return new ChainDecision(NASBackupChainKeys.TYPE_FULL, NASBackupChainKeys.METHOD_QCOW2,
+                    NASBackupChainKeys.FORMAT_QCOW2, bitmapName, null, null, UUID.randomUUID().toString(), 0);
         }
 
         /**
@@ -257,8 +259,8 @@ static ChainDecision fullStart(String bitmapName) {
          * deleted backup, an unassigned offering, a restore) cannot leave snapshots behind.
          */
         static ChainDecision rbdFullStart(String snapshotName) {
-            return new ChainDecision(NASBackupChainKeys.TYPE_RBD_FULL, NASBackupChainKeys.FORMAT_RBD_DIFF,
-                    snapshotName, null, null, UUID.randomUUID().toString(), 0);
+            return new ChainDecision(NASBackupChainKeys.TYPE_FULL, NASBackupChainKeys.METHOD_RBD,
+                    NASBackupChainKeys.FORMAT_RBD_DIFF, snapshotName, null, null, UUID.randomUUID().toString(), 0);
         }
 
         /**
@@ -267,8 +269,8 @@ static ChainDecision rbdFullStart(String snapshotName) {
          * diff stream is not rebased onto its parent, the chain is replayed in order on restore.
          */
         static ChainDecision rbdIncremental(String snapshotName, String parentSnapshot, String chainId, int chainPosition) {
-            return new ChainDecision(NASBackupChainKeys.TYPE_RBD_INCREMENTAL, NASBackupChainKeys.FORMAT_RBD_DIFF,
-                    snapshotName, parentSnapshot, null, chainId, chainPosition);
+            return new ChainDecision(NASBackupChainKeys.TYPE_INCREMENTAL, NASBackupChainKeys.METHOD_RBD,
+                    NASBackupChainKeys.FORMAT_RBD_DIFF, snapshotName, parentSnapshot, null, chainId, chainPosition);
         }
 
         /**
@@ -277,27 +279,26 @@ static ChainDecision rbdIncremental(String snapshotName, String parentSnapshot,
          * is sent to the agent or persisted. Keeps the feature-off path byte-for-byte legacy.
          */
         static ChainDecision legacyFull() {
-            return new ChainDecision(NASBackupChainKeys.TYPE_LEGACY_FULL, NASBackupChainKeys.FORMAT_QCOW2,
-                    null, null, null, null, 0);
+            return new ChainDecision(NASBackupChainKeys.TYPE_LEGACY_FULL, NASBackupChainKeys.METHOD_QCOW2,
+                    NASBackupChainKeys.FORMAT_QCOW2, null, null, null, null, 0);
         }
 
         static ChainDecision incremental(String bitmapNew, String bitmapParent, List<String> parentPaths,
                                          String chainId, int chainPosition) {
-            return new ChainDecision(NASBackupChainKeys.TYPE_INCREMENTAL, NASBackupChainKeys.FORMAT_QCOW2,
-                    bitmapNew, bitmapParent, parentPaths, chainId, chainPosition);
+            return new ChainDecision(NASBackupChainKeys.TYPE_INCREMENTAL, NASBackupChainKeys.METHOD_QCOW2,
+                    NASBackupChainKeys.FORMAT_QCOW2, bitmapNew, bitmapParent, parentPaths, chainId, chainPosition);
         }
 
         boolean isIncremental() {
-            return NASBackupChainKeys.TYPE_INCREMENTAL.equals(mode)
-                    || NASBackupChainKeys.TYPE_RBD_INCREMENTAL.equals(mode);
+            return NASBackupChainKeys.TYPE_INCREMENTAL.equals(mode);
         }
 
         boolean isLegacyFull() {
             return NASBackupChainKeys.TYPE_LEGACY_FULL.equals(mode);
         }
 
         boolean isRbd() {
-            return NASBackupChainKeys.FORMAT_RBD_DIFF.equals(format);
+            return NASBackupChainKeys.METHOD_RBD.equals(method);
         }
 
         /**
@@ -701,6 +702,7 @@ public Pair<Boolean, Backup> takeBackup(final VirtualMachine vm, Boolean quiesce
         command.setMountOptions(backupRepository.getMountOptions());
         command.setQuiesce(quiesceVM);
         command.setMode(decision.mode);
+        command.setMethod(decision.method);
         command.setBitmapNew(decision.bitmapNew);
         command.setBitmapParent(decision.bitmapParent);
         command.setParentPaths(decision.parentPaths);

plugins/backup/nas/src/test/java/org/apache/cloudstack/backup/NASBackupProviderTest.java

@@ -486,7 +486,8 @@ public void decideChainStartsAnRbdChainForRbdBackedVm() {
 
         NASBackupProvider.ChainDecision decision = nasBackupProvider.decideChain(vm);
 
-        Assert.assertEquals(NASBackupChainKeys.TYPE_RBD_FULL, decision.mode);
+        Assert.assertEquals(NASBackupChainKeys.TYPE_FULL, decision.mode);
+        Assert.assertEquals(NASBackupChainKeys.METHOD_RBD, decision.method);
         Assert.assertEquals(NASBackupChainKeys.FORMAT_RBD_DIFF, decision.format);
         Assert.assertTrue("RBD chains anchor on an RBD snapshot, not a QEMU bitmap",
                 decision.bitmapNew.startsWith("csbackup-"));
@@ -513,7 +514,8 @@ public void decideChainKeepsStoppedRbdVmOnTheRbdPath() {
 
         NASBackupProvider.ChainDecision decision = nasBackupProvider.decideChain(vm);
 
-        Assert.assertEquals(NASBackupChainKeys.TYPE_RBD_FULL, decision.mode);
+        Assert.assertEquals(NASBackupChainKeys.TYPE_FULL, decision.mode);
+        Assert.assertEquals(NASBackupChainKeys.METHOD_RBD, decision.method);
         Assert.assertEquals(NASBackupChainKeys.FORMAT_RBD_DIFF, decision.format);
     }
 
@@ -529,7 +531,8 @@ public void asFullRestartKeepsTheRbdFlavour() {
 
         NASBackupProvider.ChainDecision restarted = incremental.asFullRestart();
 
-        Assert.assertEquals(NASBackupChainKeys.TYPE_RBD_FULL, restarted.mode);
+        Assert.assertEquals(NASBackupChainKeys.TYPE_FULL, restarted.mode);
+        Assert.assertEquals(NASBackupChainKeys.METHOD_RBD, restarted.method);
         Assert.assertEquals(NASBackupChainKeys.FORMAT_RBD_DIFF, restarted.format);
         Assert.assertEquals("csbackup-2", restarted.bitmapNew);
         Assert.assertEquals(0, restarted.chainPosition);

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtTakeBackupCommandWrapper.java

@@ -52,11 +52,13 @@ public class LibvirtTakeBackupCommandWrapper extends CommandWrapper<TakeBackupCo
     // Incremental feature disabled: plain full backup with no QEMU bitmap/checkpoint and no
     // chain metadata. Matches nasbackup.sh's "legacy-full" mode (make_checkpoint=0).
     private static final String MODE_LEGACY_FULL = "legacy-full";
-    // Ceph/RBD chains: anchored on an `rbd snap` per cycle and exported with `rbd export-diff`
-    // instead of a libvirt checkpoint + push backup. bitmapNew/bitmapParent carry RBD snapshot
-    // names rather than QEMU bitmap names; no parent file paths, nothing is rebased.
-    private static final String MODE_RBD_FULL = "rbd-full";
-    private static final String MODE_RBD_INCREMENTAL = "rbd-incremental";
+
+    // Backup methods, orthogonal to the mode. qcow2 is the push-mode path with QEMU dirty bitmaps,
+    // also assumed when the management server sends none. rbd is the Ceph-native path: it anchors
+    // each cycle on an `rbd snap` and exports the delta with `rbd export-diff`, so bitmapNew and
+    // bitmapParent carry RBD snapshot names rather than QEMU bitmap names, and nothing is rebased.
+    private static final String METHOD_QCOW2 = "qcow2";
+    private static final String METHOD_RBD = "rbd";
 
     @Override
     public Answer execute(TakeBackupCommand command, LibvirtComputingResource libvirtComputingResource) {
@@ -163,6 +165,10 @@ private Pair<Integer, String> runBackupScript(LibvirtComputingResource libvirtCo
             argv.add("-M");
             argv.add(mode);
         }
+        if (StringUtils.isNotBlank(command.getMethod())) {
+            argv.add("--method");
+            argv.add(command.getMethod());
+        }
         if (StringUtils.isNotBlank(bitmapNew)) {
             argv.add("--bitmap-new");
             argv.add(bitmapNew);
@@ -187,6 +193,13 @@ private Pair<Integer, String> runBackupScript(LibvirtComputingResource libvirtCo
      */
     private String validateBackupArgs(TakeBackupCommand command) {
         String mode = command.getMode();
+        String method = StringUtils.defaultIfBlank(command.getMethod(), METHOD_QCOW2);
+        if (METHOD_RBD.equals(method)) {
+            return validateRbdBackupArgs(command, mode);
+        }
+        if (!METHOD_QCOW2.equals(method)) {
+            return "Unknown backup method: " + method;
+        }
         if (StringUtils.isBlank(mode)) {
             return null; // legacy full-only, no extra args expected
         }
@@ -211,22 +224,31 @@ private String validateBackupArgs(TakeBackupCommand command) {
         if (MODE_LEGACY_FULL.equals(mode)) {
             return null; // feature-off full backup, no bitmap or chain args expected
         }
-        if (MODE_RBD_FULL.equals(mode) || MODE_RBD_INCREMENTAL.equals(mode)) {
-            if (command.getBitmapNew() == null || command.getBitmapNew().isEmpty()) {
-                return mode + " mode requires bitmapNew (the RBD snapshot to create as the chain anchor)";
-            }
-            if (MODE_RBD_INCREMENTAL.equals(mode)
-                    && (command.getBitmapParent() == null || command.getBitmapParent().isEmpty())) {
-                return "rbd-incremental mode requires bitmapParent (the RBD snapshot to diff against)";
-            }
-            if (command.getVolumePaths() == null || command.getVolumePaths().isEmpty()) {
-                return mode + " mode requires the VM's volume paths";
-            }
-            return null;
-        }
         return "Unknown backup mode: " + mode;
     }
 
+    /**
+     * An RBD chain anchors each cycle on an {@code rbd snap} whose name travels in bitmapNew, and
+     * an incremental diffs it against the previous anchor in bitmapParent. No parent file paths:
+     * the diff stream is replayed on restore rather than rebased onto its parent.
+     */
+    private String validateRbdBackupArgs(TakeBackupCommand command, String mode) {
+        if (!MODE_FULL.equals(mode) && !MODE_INCREMENTAL.equals(mode)) {
+            return "Unknown backup mode for the rbd method: " + mode;
+        }
+        if (command.getBitmapNew() == null || command.getBitmapNew().isEmpty()) {
+            return mode + " mode requires bitmapNew (the RBD snapshot to create as the chain anchor)";
+        }
+        if (MODE_INCREMENTAL.equals(mode)
+                && (command.getBitmapParent() == null || command.getBitmapParent().isEmpty())) {
+            return "incremental mode requires bitmapParent (the RBD snapshot to diff against)";
+        }
+        if (command.getVolumePaths() == null || command.getVolumePaths().isEmpty()) {
+            return mode + " mode requires the VM's volume paths";
+        }
+        return null;
+    }
+
     /**
      * Sum the per-disk size lines emitted by nasbackup.sh. Single-volume mode emits one
      * line containing just the byte count; multi-volume mode emits one line per disk