sureshanaparti commented on code in PR #13877:
URL: https://github.com/apache/cloudstack/pull/13877#discussion_r4033613757


##########
plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java:
##########
@@ -168,6 +208,330 @@ protected Host getVMHypervisorHost(VirtualMachine vm) {
         return 
resourceManager.findOneRandomRunningHostByHypervisor(Hypervisor.HypervisorType.KVM,
 vm.getDataCenterId());
     }
 
+    /**
+     * 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.
+     */
+    static final class ChainDecision {
+        final String mode;            // "full" or "incremental"
+        final String bitmapNew;
+        final String bitmapParent;    // null for full
+        // Per-volume parent backup file paths, one per current VM volume in 
deviceId order.
+        // null/empty for full. Each volume needs its own parent file because 
backup files
+        // are named after each volume's own UUID (root.<uuid>.qcow2 / 
datadisk.<uuid>.qcow2).
+        final List<String> parentPaths;
+        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 bitmapNew, String 
bitmapParent, List<String> parentPaths,
+                              String chainId, int chainPosition) {
+            this.mode = mode;
+            this.bitmapNew = bitmapNew;
+            this.bitmapParent = bitmapParent;
+            this.parentPaths = parentPaths;
+            this.chainId = chainId;
+            this.chainPosition = chainPosition;
+        }
+
+        static ChainDecision fullStart(String bitmapName) {
+            return new ChainDecision(NASBackupChainKeys.TYPE_FULL, bitmapName, 
null, null,
+                    UUID.randomUUID().toString(), 0);
+        }
+
+        /**
+         * Decision used when the incremental feature is disabled: a plain 
full backup that
+         * creates no bitmap and carries no chain identity, so nothing 
chain/checkpoint-related
+         * 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, 
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, 
bitmapNew, bitmapParent,
+                    parentPaths, chainId, chainPosition);
+        }
+
+        boolean isIncremental() {
+            return NASBackupChainKeys.TYPE_INCREMENTAL.equals(mode);
+        }
+
+        boolean isLegacyFull() {
+            return NASBackupChainKeys.TYPE_LEGACY_FULL.equals(mode);
+        }
+    }
+
+    /**
+     * Decides whether the next backup for {@code vm} should be a fresh full 
or an incremental
+     * appended to the existing chain. Stopped VMs are always full (libvirt 
{@code backup-begin}
+     * requires a running QEMU process). The {@code nas.backup.full.every} 
ConfigKey controls
+     * how many backups (full + incrementals) form one chain before a new full 
is forced.
+     *
+     * <p>The decision is anchored on the VM's {@code 
nas.active_checkpoint_id} detail, which
+     * records the bitmap that currently exists on the running QEMU. After a 
restore that
+     * detail is cleared, so the next backup is automatically full — even 
though there may be
+     * a more recent "last backup taken" row in the database. The decision 
deliberately avoids
+     * relying on "last backup taken", because that row is misleading after a 
restore.</p>
+     */
+    protected ChainDecision decideChain(VirtualMachine vm) {
+        // Master switch — when the operator disables incrementals at the zone 
level the backup
+        // behaves exactly like the pre-incremental full-only path: no bitmap 
is generated and no
+        // chain/checkpoint metadata is created, sent to the agent, or 
persisted (legacy-full).
+        Boolean incrementalEnabled = 
NASBackupIncrementalEnabled.valueIn(vm.getDataCenterId());
+        if (incrementalEnabled == null || !incrementalEnabled) {
+            return ChainDecision.legacyFull();
+        }
+
+        // Incremental backups rely on QEMU dirty bitmaps / libvirt 
checkpoints, which only exist
+        // on file-based qcow2 storage. Storage such as Ceph-RBD and Linstor 
cannot carry per-disk
+        // checkpoints, so a VM with any volume on such a pool must stay on 
the full-only (legacy)
+        // path — otherwise an incremental attempt would fail or regress those 
storages.
+        if (!allVolumesOnCheckpointCapableStorage(vm)) {
+            return ChainDecision.legacyFull();
+        }
+
+        final String newBitmap = "backup-" + System.currentTimeMillis() / 
1000L;
+
+        // Stopped VMs cannot do incrementals — script will also fall back, 
but we make the
+        // decision here so we register the right type up-front.
+        if (VirtualMachine.State.Stopped.equals(vm.getState())) {
+            return ChainDecision.fullStart(newBitmap);
+        }
+
+        Integer fullEvery = NASBackupFullEvery.valueIn(vm.getDataCenterId());
+        if (fullEvery == null || fullEvery <= 1) {
+            // Disabled or every-backup-is-full mode.
+            return ChainDecision.fullStart(newBitmap);
+        }
+
+        // 1. If the VM has no active_checkpoint_id, there is no bitmap on the 
host to use as
+        //    a parent. This is the case after restore (we clear it), after VM 
was just assigned
+        //    to the offering, or on the very first backup.
+        String activeCheckpoint = readVmActiveCheckpoint(vm.getId());
+        if (activeCheckpoint == null) {
+            return ChainDecision.fullStart(newBitmap);
+        }
+
+        // 2. The most-recent BackedUp backup is the only safe parent — after 
restore the
+        //    next backup is always a fresh full, so anything older has a 
rotated-out bitmap.
+        //    If the latest backup's bitmap doesn't match the VM's 
active_checkpoint_id, the
+        //    chain is broken: force a full.
+        Backup parent = findLatestBackedUpBackup(vm.getId());
+        if (parent == null || !activeCheckpoint.equals(readDetail(parent, 
NASBackupChainKeys.BITMAP_NAME))) {
+            LOG.debug("VM {} latest backup does not match 
active_checkpoint_id={} — forcing full",
+                    vm.getInstanceName(), activeCheckpoint);
+            return ChainDecision.fullStart(newBitmap);
+        }
+
+        String parentChainId = readDetail(parent, NASBackupChainKeys.CHAIN_ID);
+        int parentChainPosition = chainPosition(parent);
+        if (parentChainId == null || parentChainPosition == Integer.MAX_VALUE) 
{
+            return ChainDecision.fullStart(newBitmap);
+        }
+
+        // Force a fresh full when the chain has reached the configured length.
+        if (parentChainPosition + 1 >= fullEvery) {
+            return ChainDecision.fullStart(newBitmap);
+        }
+
+        // The script needs the parent backup's on-NAS file path PER VOLUME so 
it can rebase
+        // each new qcow2 onto the matching parent. The paths are stored 
relative to the NAS
+        // mount root — the script resolves them inside its mount session. 
When alignment
+        // fails (volume count changed, etc.) compose returns null and we fall 
back to full
+        // so we don't risk corrupting the chain.
+        List<String> parentPaths = composeParentBackupPaths(parent, 
vm.getId());
+        if (parentPaths == null) {
+            LOG.debug("VM {} parent backup {} volume layout no longer matches 
current VM — forcing full",
+                    vm.getInstanceName(), parent.getUuid());
+            return ChainDecision.fullStart(newBitmap);
+        }
+        return ChainDecision.incremental(newBitmap, activeCheckpoint, 
parentPaths,
+                parentChainId, parentChainPosition + 1);
+    }
+
+    /**
+     * Incremental backups require QEMU dirty bitmaps / libvirt checkpoints, 
which are only
+     * possible on file-based qcow2 storage. Returns {@code true} only when 
EVERY volume of the
+     * VM sits on HOST-scope local, {@code SharedMountPoint}, or {@code 
NetworkFilesystem} (NFS)
+     * storage. Ceph-RBD, Linstor, and any other pool that cannot carry a 
per-disk checkpoint
+     * make this return {@code false} so the caller falls back to the legacy 
full-only path. A
+     * volume whose pool can no longer be resolved is treated as incapable 
(safe default).
+     */
+    protected boolean allVolumesOnCheckpointCapableStorage(VirtualMachine vm) {
+        List<VolumeVO> volumes = volumeDao.findByInstance(vm.getId());
+        if (volumes == null) {
+            return true;
+        }
+        for (VolumeVO volume : volumes) {
+            StoragePoolVO pool = 
primaryDataStoreDao.findById(volume.getPoolId());
+            if (pool == null) {
+                LOG.debug("VM {} volume {} has no resolvable storage pool — 
forcing legacy full",
+                        vm.getInstanceName(), volume.getUuid());
+                return false;
+            }
+            boolean checkpointCapable = ScopeType.HOST.equals(pool.getScope())
+                    || 
Storage.StoragePoolType.SharedMountPoint.equals(pool.getPoolType())
+                    || 
Storage.StoragePoolType.NetworkFilesystem.equals(pool.getPoolType());
+            if (!checkpointCapable) {
+                LOG.debug("VM {} volume {} is on {} (scope {}) which cannot 
carry checkpoints — forcing legacy full",
+                        vm.getInstanceName(), volume.getUuid(), 
pool.getPoolType(), pool.getScope());
+                return false;
+            }
+        }
+        return true;
+    }
+
+    /**
+     * Read the {@code nas.active_checkpoint_id} VM detail. Returns {@code 
null} when no detail
+     * exists (post-restore, first backup, or after explicit reset).
+     */
+    private String readVmActiveCheckpoint(long vmId) {
+        VMInstanceDetailVO d = vmInstanceDetailsDao.findDetail(vmId, 
NASBackupChainKeys.VM_ACTIVE_CHECKPOINT_ID);
+        if (d == null) {
+            return null;
+        }
+        String v = d.getValue();
+        return (v == null || v.isEmpty()) ? null : v;

Review Comment:
   ```suggestion
           return (StringUtils.isBlank(v)) ? null : v;
   ```



-- 
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]

Reply via email to