abh1sar commented on code in PR #14386:
URL: https://github.com/apache/cloudstack/pull/14386#discussion_r4346619350


##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java:
##########
@@ -381,6 +457,212 @@ private boolean 
replaceBlockDeviceWithBackup(KVMStoragePoolManager storagePoolMg
         return true;
     }
 
+    /**
+     * Rebuild an RBD image by replaying a chain of {@code rbd export-diff} 
streams into it: the
+     * chain's full anchor first, then each incremental in the order it was 
taken.
+     *
+     * <p>RBD images are raw and cannot hold a backing-file reference, so — 
unlike the qcow2 path —
+     * a diff carries no pointer to the one before it and the chain cannot be 
flattened by
+     * {@code qemu-img}. Ceph applies the deltas natively instead: each diff 
ends at the snapshot
+     * the backup was taken from, {@code import-diff} recreates that snapshot 
on the target, and the
+     * next diff in the chain is validated against it. A truncated or 
out-of-order diff is therefore
+     * rejected by Ceph rather than silently producing a half-restored 
image.</p>
+     *
+     * <p>The image is recreated first. A full diff only carries the extents 
that were allocated
+     * when it was taken, so any block still allocated in the current image 
would otherwise show
+     * through the restore as stale data. Recreating the image also drops 
every RBD snapshot it
+     * carried: a restore rewinds the volume wholesale, and snapshots of 
content that is being
+     * replaced cannot be kept. The chain anchors this path takes are ours to 
drop, but a snapshot
+     * taken outside it — a CloudStack volume snapshot kept on primary 
storage, say — is not, so an
+     * image carrying one is refused rather than rewound.</p>
+     */
+    private boolean restoreRbdImageFromDiffChain(KVMStoragePoolManager 
storagePoolMgr, PrimaryDataStoreTO volumePool,
+                                                 String volumePath, 
List<String> diffFiles, Long size) {
+        if (!Storage.StoragePoolType.RBD.equals(volumePool.getPoolType())) {
+            logger.error("Backup [{}] holds RBD diffs but volume [{}] is on {} 
storage; it cannot be restored there.",
+                    diffFiles.get(0), volumePath, volumePool.getPoolType());
+            return false;
+        }
+        KVMStoragePool pool = 
storagePoolMgr.getStoragePool(volumePool.getPoolType(), volumePool.getUuid());
+        Path keyFile = null;
+        try {
+            keyFile = writeCephKeyFile(pool.getAuthSecret());
+            List<String> connection = rbdConnectionArgs(pool, keyFile);
+            // Volume paths of RBD volumes are already "<ceph pool>/<image>".
+            String image = volumePath;
+
+            // Prefer the current image's own size: it is what the VM's disk 
was sized to. Only a
+            // restore that creates a brand new volume has no image to read it 
from.
+            Long imageSize = rbdImageSize(connection, image);
+            if (imageSize == null) {
+                imageSize = size;
+            }
+            if (imageSize == null || imageSize <= 0) {
+                logger.error("Unable to determine the size to recreate RBD 
image {} with", image);
+                return false;
+            }
+
+            if (!recreateRbdImage(connection, image, imageSize, 
rbdDataPool(pool))) {
+                return false;
+            }
+
+            for (String diff : diffFiles) {
+                logger.debug("Applying backup diff {} to RBD image {}", diff, 
image);
+                if (Script.executeCommandForExitValue(rbdCommand(connection, 
"import-diff", diff, image)) != 0) {
+                    logger.error("Failed to apply backup diff {} to RBD image 
{}", diff, image);
+                    return false;
+                }
+            }
+
+            // The snapshots import-diff recreated only existed to chain the 
diffs together; the
+            // restored volume should look like any other, so drop them.
+            if (Script.executeCommandForExitValue(rbdCommand(connection, 
"snap", "purge", image)) != 0) {
+                logger.warn("Failed to purge the snapshots left behind by the 
restore of RBD image {}", image);
+            }
+            logger.debug("Successfully restored RBD image {} from {} backup 
diff(s)", image, diffFiles.size());
+            return true;
+        } catch (IOException e) {
+            logger.error("Failed to prepare the Ceph credentials needed to 
restore volume [{}]", volumePath, e);
+            return false;
+        } finally {
+            deleteCephKeyFile(keyFile);
+        }
+    }
+
+    /**
+     * Hand the cephx secret to the {@code rbd} CLI through a 0600 temporary 
file rather than an
+     * argument, so it never shows up in the host's process list.
+     */
+    private Path writeCephKeyFile(String authSecret) throws IOException {
+        if (StringUtils.isBlank(authSecret)) {
+            return null;
+        }
+        Path keyFile = Files.createTempFile("csrestore-ceph-", ".key",
+                
PosixFilePermissions.asFileAttribute(PosixFilePermissions.fromString("rw-------")));
+        Files.write(keyFile, authSecret.getBytes(StandardCharsets.UTF_8));
+        return keyFile;
+    }
+
+    private void deleteCephKeyFile(Path keyFile) {
+        if (keyFile == null) {
+            return;
+        }
+        try {
+            Files.deleteIfExists(keyFile);
+        } catch (IOException e) {
+            logger.warn("Failed to delete the temporary Ceph key file [{}].", 
keyFile, e);
+        }
+    }
+
+    private List<String> rbdConnectionArgs(KVMStoragePool pool, Path keyFile) {
+        List<String> args = new ArrayList<>();
+        if (StringUtils.isNotBlank(pool.getSourceHost())) {
+            args.add("-m");
+            args.add(RbdUtils.getRbdMonitors(pool.getSourceHost(), 
pool.getSourcePort()));
+        }
+        if (StringUtils.isNotBlank(pool.getAuthUserName())) {
+            args.add("--id");
+            args.add(pool.getAuthUserName());
+            if (keyFile != null) {
+                args.add("--keyfile");
+                args.add(keyFile.toString());
+            }
+        }
+        return args;
+    }
+
+    private String[] rbdCommand(List<String> connection, String... arguments) {
+        List<String> command = new ArrayList<>();
+        command.add(Script.getExecutableAbsolutePath("rbd"));
+        command.addAll(connection);
+        command.addAll(Arrays.asList(arguments));
+        return command.toArray(new String[0]);
+    }
+
+    /** Size of {@code image} in bytes, or {@code null} when the image does 
not exist. */
+    private Long rbdImageSize(List<String> connection, String image) {
+        Pair<Integer, String> result = Script.executePipedCommands(
+                Collections.singletonList(rbdCommand(connection, "info", 
image, "--format", "json")), 0);
+        if (result.first() == null || result.first() != 0 || result.second() 
== null) {
+            return null;
+        }
+        Matcher matcher = RBD_INFO_SIZE.matcher(result.second());
+        return matcher.find() ? Long.valueOf(matcher.group(1)) : null;
+    }
+
+    private boolean recreateRbdImage(List<String> connection, String image, 
long sizeBytes, String dataPool) {
+        // `rbd rm` refuses to delete an image that still has snapshots, and a 
restore target
+        // normally carries the anchor snapshot of the backup chain it belongs 
to. Ask first, so an
+        // image without snapshots is not handed a purge with nothing to do; 
when the listing itself
+        // fails we cannot tell, and purging is what we would have done anyway.
+        List<String> snapshots = rbdSnapshots(connection, image);
+        failIfImageCarriesForeignSnapshots(image, snapshots);

Review Comment:
   Is it possible to have per volume snapshots in ceph-cloudstack? If so, 
should we do this check for all volumes before doing `rbd rm` on any of them?



##########
scripts/vm/hypervisor/kvm/nasbackup.sh:
##########
@@ -137,6 +138,282 @@ get_linstor_uuid_from_device() {
   return 1
 }
 
+# Globals populated by parse_rbd_opts() for the disk it was last called with.
+RBD_IMAGE=""          # "<pool>/<image>"
+RBD_ARGS=()           # connection arguments to pass to every rbd invocation
+RBD_KEYFILE=""        # temporary cephx keyfile, removed by rbd_cleanup_keyfile
+# Prefix NASBackupProvider names every chain anchor with. Snapshots carrying 
it belong to this
+# tool and no other, which is what makes the stale-anchor sweep safe.
+RBD_SNAPSHOT_PREFIX="csbackup-"
+
+rbd_cleanup_keyfile() {
+  if [[ -n "$RBD_KEYFILE" ]]; then
+    rm -f "$RBD_KEYFILE"
+    RBD_KEYFILE=""
+  fi
+}
+
+# Split an RBD disk string produced by KVMPhysicalDisk.RBDStringBuilder into 
the image name and
+# the connection arguments the rbd CLI needs:
+#   
rbd:<pool>/<image>:mon_host=<h1>\;<h2>:auth_client_required=cephx:id=<user>:key=<secret>:...
+# ':' and ';' are backslash-escaped inside mon_host so that option splitting 
stays unambiguous;
+# they are masked before the split and restored afterwards.
+parse_rbd_opts() {
+  local s="${1#rbd:}"
+  s="${s//\\:/$'\x01'}"
+  s="${s//\\;/$'\x02'}"
+  local -a parts=()
+  IFS=':' read -ra parts <<< "$s"
+
+  rbd_cleanup_keyfile
+  RBD_IMAGE="${parts[0]}"
+  RBD_ARGS=()
+
+  local p monHost="" authId="" authKey=""
+  for p in "${parts[@]:1}"; do
+    case "$p" in
+      mon_host=*) monHost="${p#mon_host=}" ;;
+      id=*)       authId="${p#id=}" ;;
+      key=*)      authKey="${p#key=}" ;;
+    esac
+  done
+  monHost="${monHost//$'\x01'/:}"   # host\:port  -> host:port
+  monHost="${monHost//$'\x02'/,}"   # monitor list -> the comma-separated form 
rbd -m accepts
+
+  if [[ -n "$monHost" ]]; then
+    RBD_ARGS+=(-m "$monHost")
+  fi
+  if [[ -n "$authId" ]]; then
+    RBD_ARGS+=(--id "$authId")
+    if [[ -n "$authKey" ]]; then
+      RBD_KEYFILE=$(mktemp -t csbackup-ceph.XXXXXX)
+      chmod 600 "$RBD_KEYFILE"
+      printf '%s' "$authKey" > "$RBD_KEYFILE"
+      RBD_ARGS+=(--keyfile "$RBD_KEYFILE")
+    fi
+  fi
+}
+
+# Best-effort removal of snapshot $1 from each of the remaining disk 
arguments. Used both to roll
+# back a half-created anchor and to drop the previous anchor once its delta 
has been exported.
+rbd_remove_snapshots() {
+  local snapName="$1"
+  shift
+  local d
+  for d in "$@"; do
+    [[ -z "$d" ]] && continue
+    parse_rbd_opts "$d"
+    if ! rbd "${RBD_ARGS[@]}" snap rm "$RBD_IMAGE@$snapName" >> "$logFile" 
2>&1; then
+      log -e "cleanup: failed to remove RBD snapshot $RBD_IMAGE@$snapName 
(non-fatal)"
+    fi
+  done
+  rbd_cleanup_keyfile
+}
+
+# Remove every snapshot carrying the backup prefix except $1, across the 
remaining disk arguments.
+# Only snapshots this tool created are ever touched: user and CloudStack 
volume snapshots do not
+# use the prefix.
+rbd_sweep_stale_anchors() {
+  local keepSnap="$1"
+  shift
+  local d stale
+  for d in "$@"; do
+    [[ -z "$d" ]] && continue
+    parse_rbd_opts "$d"
+    while read -r stale; do
+      [[ -z "$stale" || "$stale" == "$keepSnap" ]] && continue
+      if ! rbd "${RBD_ARGS[@]}" snap rm "$RBD_IMAGE@$stale" >> "$logFile" 
2>&1; then
+        log -e "cleanup: failed to remove stale RBD snapshot $RBD_IMAGE@$stale 
(non-fatal)"
+      fi
+    done < <(
+      rbd "${RBD_ARGS[@]}" snap ls "$RBD_IMAGE" --format json 2>>"$logFile" | 
python3 -c '
+import sys, json
+try:
+    snaps = json.load(sys.stdin)
+except Exception:
+    sys.exit(0)
+for s in snaps or []:
+    name = (s or {}).get("name") or ""
+    if name.startswith(sys.argv[1]):
+        print(name)
+' "$RBD_SNAPSHOT_PREFIX" 2>/dev/null || true
+    )
+  done
+  rbd_cleanup_keyfile
+}
+
+rbd_snapshot_exists() {
+  local snapName="$1"
+  rbd "${RBD_ARGS[@]}" snap ls "$RBD_IMAGE" --format json 2>>"$logFile" | 
python3 -c '
+import sys, json
+try:
+    snaps = json.load(sys.stdin)
+except Exception:
+    sys.exit(1)
+sys.exit(0 if any((s or {}).get("name") == sys.argv[1] for s in snaps or []) 
else 1)
+' "$snapName"
+}
+
+# Incremental backup of Ceph/RBD volumes.
+#
+# RBD images are raw, so they have nowhere to persist a QEMU dirty bitmap and 
cannot take part in
+# the libvirt-checkpoint scheme backup_running_vm uses. Ceph provides the 
equivalent natively: an
+# `rbd snap` per cycle as the anchor, and `rbd export-diff --from-snap` to 
stream out exactly the
+# extents that changed between two anchors (computed inside Ceph from the 
image object-map).
+#
+# Because the snapshot lives in Ceph rather than in the QEMU process, this 
works identically for
+# running and stopped VMs, and survives a live migration of the VM between 
hosts.
+#
+# Only the newest anchor is kept on each image: once this run's delta is 
exported, the parent
+# snapshot is removed. A full additionally sweeps every other snapshot 
carrying the backup prefix,
+# so anchors abandoned by a deleted chain, an unassigned offering or a restore 
cannot accumulate.
+backup_rbd_vm() {
+  if [[ -z "$DISK_PATHS" ]]; then
+    echo "RBD backup mode requires the volume paths (-d|--diskpaths)"
+    exit 1
+  fi
+  if [[ -z "$BITMAP_NEW" ]]; then
+    echo "RBD backup mode requires --bitmap-new (the snapshot to create)"
+    exit 1
+  fi
+
+  local effective_mode="$MODE"
+  case "$effective_mode" in
+    full|incremental)
+      ;;
+    *)
+      echo "Mode $effective_mode is not supported by the rbd method"
+      cleanup
+      exit 1
+      ;;
+  esac
+  local -a disks=()
+  IFS=',' read -ra disks <<< "$DISK_PATHS"
+
+  local d
+  for d in "${disks[@]}"; do
+    if [[ "$d" != rbd:* ]]; then
+      echo "RBD backup mode was requested but disk $d is not an RBD volume"
+      exit 1
+    fi
+  done
+
+  # The parent anchor must still be present on EVERY disk. A volume migration 
or an out-of-band
+  # snapshot purge can drop it; fall back to a full rather than letting 
export-diff fail. Checked
+  # before anything is mounted or created so the fallback costs nothing.
+  if [[ "$effective_mode" == "incremental" ]]; then
+    for d in "${disks[@]}"; do
+      parse_rbd_opts "$d"
+      if ! rbd_snapshot_exists "$BITMAP_PARENT"; then
+        log -e "rbd incremental: parent snapshot $BITMAP_PARENT missing on 
$RBD_IMAGE — falling back to full"
+        echo "INCREMENTAL_FALLBACK=true"
+        effective_mode="full"
+        break
+      fi
+    done
+    rbd_cleanup_keyfile
+  fi
+
+  mount_operation
+  mkdir -p "$dest" || { echo "Failed to create backup directory $dest"; exit 
1; }
+
+  # Freezing the guest filesystems makes the snapshot set 
filesystem-consistent instead of merely
+  # crash-consistent. Only possible while the VM runs and its guest agent 
answers.
+  local thaw=0
+  local vm_state
+  vm_state=$(virsh -c qemu:///system domstate "$VM" 2>/dev/null || true)
+  if [[ ${QUIESCE} == "true" && "$vm_state" == "running" ]]; then
+    if virsh -c qemu:///system qemu-agent-command "$VM" 
'{"execute":"guest-fsfreeze-freeze"}' > /dev/null 2>/dev/null; then
+      thaw=1
+    fi
+  fi
+
+  # Create this cycle's anchor on every disk back to back, so the set is as 
close to a single
+  # point in time for the VM as a whole as Ceph allows.
+  local -a snapped=()
+  local snap_failed=0
+  for d in "${disks[@]}"; do

Review Comment:
   For a multidisk VM which doesn't have qemu-guest-agent installed or 
`quiesce` is not selected in the API, the snapshots will be taken one after 
another while the VM keeps getting written. This doesn't get us crash 
consistent backup like what `virsh backup-begin` gives.
   I think we should pause-resume the VM in that case using virsh 
suspend/resume until all snapshots are taken. 



##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java:
##########
@@ -381,6 +457,212 @@ private boolean 
replaceBlockDeviceWithBackup(KVMStoragePoolManager storagePoolMg
         return true;
     }
 
+    /**
+     * Rebuild an RBD image by replaying a chain of {@code rbd export-diff} 
streams into it: the
+     * chain's full anchor first, then each incremental in the order it was 
taken.
+     *
+     * <p>RBD images are raw and cannot hold a backing-file reference, so — 
unlike the qcow2 path —
+     * a diff carries no pointer to the one before it and the chain cannot be 
flattened by
+     * {@code qemu-img}. Ceph applies the deltas natively instead: each diff 
ends at the snapshot
+     * the backup was taken from, {@code import-diff} recreates that snapshot 
on the target, and the
+     * next diff in the chain is validated against it. A truncated or 
out-of-order diff is therefore
+     * rejected by Ceph rather than silently producing a half-restored 
image.</p>
+     *
+     * <p>The image is recreated first. A full diff only carries the extents 
that were allocated
+     * when it was taken, so any block still allocated in the current image 
would otherwise show
+     * through the restore as stale data. Recreating the image also drops 
every RBD snapshot it
+     * carried: a restore rewinds the volume wholesale, and snapshots of 
content that is being
+     * replaced cannot be kept. The chain anchors this path takes are ours to 
drop, but a snapshot
+     * taken outside it — a CloudStack volume snapshot kept on primary 
storage, say — is not, so an
+     * image carrying one is refused rather than rewound.</p>
+     */
+    private boolean restoreRbdImageFromDiffChain(KVMStoragePoolManager 
storagePoolMgr, PrimaryDataStoreTO volumePool,
+                                                 String volumePath, 
List<String> diffFiles, Long size) {
+        if (!Storage.StoragePoolType.RBD.equals(volumePool.getPoolType())) {
+            logger.error("Backup [{}] holds RBD diffs but volume [{}] is on {} 
storage; it cannot be restored there.",

Review Comment:
   This is a behavior change from other primary storage backups and full 
ceph-rbd backups. In other cases a Backup taken from any primary storage can be 
restored on any other primary storage.
   Can we do something about it? or we should atleast make this clear in the 
documents.
   
   



##########
scripts/vm/hypervisor/kvm/nasbackup.sh:
##########
@@ -137,6 +138,282 @@ get_linstor_uuid_from_device() {
   return 1
 }
 
+# Globals populated by parse_rbd_opts() for the disk it was last called with.
+RBD_IMAGE=""          # "<pool>/<image>"
+RBD_ARGS=()           # connection arguments to pass to every rbd invocation
+RBD_KEYFILE=""        # temporary cephx keyfile, removed by rbd_cleanup_keyfile
+# Prefix NASBackupProvider names every chain anchor with. Snapshots carrying 
it belong to this
+# tool and no other, which is what makes the stale-anchor sweep safe.
+RBD_SNAPSHOT_PREFIX="csbackup-"
+
+rbd_cleanup_keyfile() {
+  if [[ -n "$RBD_KEYFILE" ]]; then
+    rm -f "$RBD_KEYFILE"
+    RBD_KEYFILE=""
+  fi
+}
+
+# Split an RBD disk string produced by KVMPhysicalDisk.RBDStringBuilder into 
the image name and
+# the connection arguments the rbd CLI needs:
+#   
rbd:<pool>/<image>:mon_host=<h1>\;<h2>:auth_client_required=cephx:id=<user>:key=<secret>:...
+# ':' and ';' are backslash-escaped inside mon_host so that option splitting 
stays unambiguous;
+# they are masked before the split and restored afterwards.
+parse_rbd_opts() {
+  local s="${1#rbd:}"
+  s="${s//\\:/$'\x01'}"
+  s="${s//\\;/$'\x02'}"
+  local -a parts=()
+  IFS=':' read -ra parts <<< "$s"
+
+  rbd_cleanup_keyfile
+  RBD_IMAGE="${parts[0]}"
+  RBD_ARGS=()
+
+  local p monHost="" authId="" authKey=""
+  for p in "${parts[@]:1}"; do
+    case "$p" in
+      mon_host=*) monHost="${p#mon_host=}" ;;
+      id=*)       authId="${p#id=}" ;;
+      key=*)      authKey="${p#key=}" ;;
+    esac
+  done
+  monHost="${monHost//$'\x01'/:}"   # host\:port  -> host:port
+  monHost="${monHost//$'\x02'/,}"   # monitor list -> the comma-separated form 
rbd -m accepts
+
+  if [[ -n "$monHost" ]]; then
+    RBD_ARGS+=(-m "$monHost")
+  fi
+  if [[ -n "$authId" ]]; then
+    RBD_ARGS+=(--id "$authId")
+    if [[ -n "$authKey" ]]; then
+      RBD_KEYFILE=$(mktemp -t csbackup-ceph.XXXXXX)
+      chmod 600 "$RBD_KEYFILE"
+      printf '%s' "$authKey" > "$RBD_KEYFILE"
+      RBD_ARGS+=(--keyfile "$RBD_KEYFILE")
+    fi
+  fi
+}
+
+# Best-effort removal of snapshot $1 from each of the remaining disk 
arguments. Used both to roll
+# back a half-created anchor and to drop the previous anchor once its delta 
has been exported.
+rbd_remove_snapshots() {
+  local snapName="$1"
+  shift
+  local d
+  for d in "$@"; do
+    [[ -z "$d" ]] && continue
+    parse_rbd_opts "$d"
+    if ! rbd "${RBD_ARGS[@]}" snap rm "$RBD_IMAGE@$snapName" >> "$logFile" 
2>&1; then
+      log -e "cleanup: failed to remove RBD snapshot $RBD_IMAGE@$snapName 
(non-fatal)"
+    fi
+  done
+  rbd_cleanup_keyfile
+}
+
+# Remove every snapshot carrying the backup prefix except $1, across the 
remaining disk arguments.
+# Only snapshots this tool created are ever touched: user and CloudStack 
volume snapshots do not
+# use the prefix.
+rbd_sweep_stale_anchors() {
+  local keepSnap="$1"
+  shift
+  local d stale
+  for d in "$@"; do
+    [[ -z "$d" ]] && continue
+    parse_rbd_opts "$d"
+    while read -r stale; do
+      [[ -z "$stale" || "$stale" == "$keepSnap" ]] && continue
+      if ! rbd "${RBD_ARGS[@]}" snap rm "$RBD_IMAGE@$stale" >> "$logFile" 
2>&1; then
+        log -e "cleanup: failed to remove stale RBD snapshot $RBD_IMAGE@$stale 
(non-fatal)"
+      fi
+    done < <(
+      rbd "${RBD_ARGS[@]}" snap ls "$RBD_IMAGE" --format json 2>>"$logFile" | 
python3 -c '
+import sys, json
+try:
+    snaps = json.load(sys.stdin)
+except Exception:
+    sys.exit(0)
+for s in snaps or []:
+    name = (s or {}).get("name") or ""
+    if name.startswith(sys.argv[1]):
+        print(name)
+' "$RBD_SNAPSHOT_PREFIX" 2>/dev/null || true
+    )
+  done
+  rbd_cleanup_keyfile
+}
+
+rbd_snapshot_exists() {
+  local snapName="$1"
+  rbd "${RBD_ARGS[@]}" snap ls "$RBD_IMAGE" --format json 2>>"$logFile" | 
python3 -c '
+import sys, json
+try:
+    snaps = json.load(sys.stdin)
+except Exception:
+    sys.exit(1)
+sys.exit(0 if any((s or {}).get("name") == sys.argv[1] for s in snaps or []) 
else 1)
+' "$snapName"
+}
+
+# Incremental backup of Ceph/RBD volumes.
+#
+# RBD images are raw, so they have nowhere to persist a QEMU dirty bitmap and 
cannot take part in
+# the libvirt-checkpoint scheme backup_running_vm uses. Ceph provides the 
equivalent natively: an
+# `rbd snap` per cycle as the anchor, and `rbd export-diff --from-snap` to 
stream out exactly the
+# extents that changed between two anchors (computed inside Ceph from the 
image object-map).
+#
+# Because the snapshot lives in Ceph rather than in the QEMU process, this 
works identically for
+# running and stopped VMs, and survives a live migration of the VM between 
hosts.
+#
+# Only the newest anchor is kept on each image: once this run's delta is 
exported, the parent
+# snapshot is removed. A full additionally sweeps every other snapshot 
carrying the backup prefix,
+# so anchors abandoned by a deleted chain, an unassigned offering or a restore 
cannot accumulate.
+backup_rbd_vm() {
+  if [[ -z "$DISK_PATHS" ]]; then
+    echo "RBD backup mode requires the volume paths (-d|--diskpaths)"
+    exit 1
+  fi
+  if [[ -z "$BITMAP_NEW" ]]; then
+    echo "RBD backup mode requires --bitmap-new (the snapshot to create)"
+    exit 1
+  fi
+
+  local effective_mode="$MODE"
+  case "$effective_mode" in
+    full|incremental)
+      ;;
+    *)
+      echo "Mode $effective_mode is not supported by the rbd method"
+      cleanup
+      exit 1
+      ;;
+  esac
+  local -a disks=()
+  IFS=',' read -ra disks <<< "$DISK_PATHS"
+
+  local d
+  for d in "${disks[@]}"; do
+    if [[ "$d" != rbd:* ]]; then
+      echo "RBD backup mode was requested but disk $d is not an RBD volume"
+      exit 1
+    fi
+  done
+
+  # The parent anchor must still be present on EVERY disk. A volume migration 
or an out-of-band
+  # snapshot purge can drop it; fall back to a full rather than letting 
export-diff fail. Checked
+  # before anything is mounted or created so the fallback costs nothing.
+  if [[ "$effective_mode" == "incremental" ]]; then
+    for d in "${disks[@]}"; do
+      parse_rbd_opts "$d"
+      if ! rbd_snapshot_exists "$BITMAP_PARENT"; then
+        log -e "rbd incremental: parent snapshot $BITMAP_PARENT missing on 
$RBD_IMAGE — falling back to full"

Review Comment:
   can we remove the em-dashes from all logs and exceptions?
   ```suggestion
           log -e "rbd incremental: parent snapshot $BITMAP_PARENT missing on 
$RBD_IMAGE - falling back to full"
   ```



##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java:
##########
@@ -292,11 +361,18 @@ private boolean checkBackupPathExists(String backupPath) {
         return exitValue == 0;
     }
 
-    private boolean replaceVolumeWithBackup(KVMStoragePoolManager 
storagePoolMgr, PrimaryDataStoreTO volumePool, String volumePath, String 
backupPath, int timeout) {
-        return replaceVolumeWithBackup(storagePoolMgr, volumePool, volumePath, 
backupPath, timeout, false, null);
+    private boolean replaceVolumeWithBackup(KVMStoragePoolManager 
storagePoolMgr, PrimaryDataStoreTO volumePool, String volumePath,
+                                            List<String> backupFiles, 
BackupLayout layout, int timeout) {
+        return replaceVolumeWithBackup(storagePoolMgr, volumePool, volumePath, 
backupFiles, layout, timeout, false, null);
     }
 
-    private boolean replaceVolumeWithBackup(KVMStoragePoolManager 
storagePoolMgr, PrimaryDataStoreTO volumePool, String volumePath, String 
backupPath, int timeout, boolean createTargetVolume, Long size) {
+    private boolean replaceVolumeWithBackup(KVMStoragePoolManager 
storagePoolMgr, PrimaryDataStoreTO volumePool, String volumePath,
+                                            List<String> backupFiles, 
BackupLayout layout, int timeout, boolean createTargetVolume, Long size) {
+        if (layout.isRbdDiff()) {
+            return restoreRbdImageFromDiffChain(storagePoolMgr, volumePool, 
volumePath, backupFiles, size);

Review Comment:
   Can we pass timeout here and use it for calling 
`Script.executeCommandForExitValue(rbdCommand(connection, "import-diff", diff, 
image)) != 0`?
   
   Otherwise restore might fail after the default (1 hour) for VMs with 
multiple volumes after doing `rbd rm` on some volumes.



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