Damans227 commented on code in PR #14203:
URL: https://github.com/apache/cloudstack/pull/14203#discussion_r4258286157


##########
scripts/vm/hypervisor/kvm/nasbackup.sh:
##########
@@ -135,6 +138,135 @@ get_linstor_uuid_from_device() {
   return 1
 }
 
+# StorPool volume name (with the "~globalId" form the storpool/storpool_req 
CLIs expect) from
+# /dev/storpool-byid/<globalId>.
+sp_volume_name_from_path() {
+  local fullpath="$1"
+  local name
+  if [[ "$fullpath" == /dev/storpool-byid/* ]]; then
+    name="${fullpath#/dev/storpool-byid/}"
+    name="~${name%%/*}"
+  else
+    return 1
+  fi
+  echo "$name"
+}
+
+# Poll for the device node to actually appear after a successful attach: udev 
creates the
+# symlink asynchronously, so a successful "attach" CLI call doesn't guarantee 
the device path
+# is usable yet.
+sp_wait_for_device_symlink() {
+  local name="$1"
+  local devpath="/dev/storpool-byid/${name#\~}"
+  local tries=10 i
+  for ((i = 0; i < tries; i++)); do
+    if [[ -e "$devpath" ]] && [[ "$(blockdev --getsize64 "$devpath" 
2>/dev/null || echo 0)" -gt 0 ]]; then
+      return 0
+    fi
+    sleep 0.1
+  done
+  return 1
+}
+
+# detach is retried (a live volume's detach can transiently fail while still 
in use), attach is
+# not. A successful attach also waits for the device symlink.
+sp_attach_detach_volume() {
+  local cmd="$1" name="$2"
+  local tries=10 i
+  for ((i = 0; i < tries; i++)); do
+    if [[ "$cmd" == "attach" ]]; then
+      storpool -M -B attach volume "$name" here onRemoteAttached export 
>>"$logFile" 2>&1 && { sp_wait_for_device_symlink "$name"; return $?; }
+      break
+    else
+      storpool -M -B detach volume "$name" here >>"$logFile" 2>&1 && return 0
+      sleep 1
+    fi
+  done
+  return 1
+}
+
+# Clones $1 (a live StorPool volume path) into a new, point-in-time StorPool 
volume via the
+# VolumeCreate API's baseOn option, attaches it, and echoes the clone's device 
path. Only called
+# once this script's own (fresh, right-here) VM liveness check has already 
routed to the cold
+# path — see backup_stopped_vm.
+sp_create_backup_source_disk() {
+  local volume_path="$1"
+  local base_on
+  if ! base_on=$(sp_volume_name_from_path "$volume_path"); then
+    echo "Could not resolve a StorPool volume name from path $volume_path to 
create a backup source volume" >&2
+    return 1
+  fi
+
+  log -ne "StorPool: creating backup source volume based on $base_on (from 
$volume_path)"
+
+  local resp
+  if ! resp=$(storpool_req -P -M --json 
"{\"baseOn\":\"$base_on\",\"tags\":{\"cs\":\"backup\"}}" VolumeCreate 
2>>"$logFile"); then
+    log -ne "StorPool: VolumeCreate baseOn=$base_on failed, see above for 
storpool_req output"
+    echo "Could not create a backup source volume based on $base_on" >&2
+    return 1
+  fi
+
+  local global_id
+  global_id=$(python3 -c '
+import sys, json
+try:
+    data = json.load(sys.stdin)
+    gid = data.get("globalId")
+except Exception:
+    gid = None
+if not gid:
+    sys.exit(1)
+print(gid)
+' <<< "$resp" 2>>"$logFile") || {
+    log -ne "StorPool: VolumeCreate baseOn=$base_on returned no globalId, 
response: $resp -- the volume was created but its name could not be parsed; it 
must be found (tag cs=backup, baseOn=$base_on) and deleted manually to avoid an 
orphaned volume"
+    echo "StorPool did not return a volume name when cloning $base_on for 
backup" >&2
+    return 1
+  }
+
+  local clone_path="/dev/storpool-byid/$global_id"
+  local clone_name="~$global_id"
+  log -ne "StorPool: created backup source volume $clone_name ($clone_path), 
attaching"
+  if ! sp_attach_detach_volume "attach" "$clone_name"; then
+    log -ne "StorPool: failed to attach backup source volume $clone_name, 
deleting it"
+    storpool_req -P -M VolumeDelete "$clone_name" >>"$logFile" 2>&1 || true

Review Comment:
   if the attach works but the device doesnt show up in time, wont this delete 
fail on a still attached clone? it isnt in the cleanup list yet so it stays 
behind



##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java:
##########
@@ -173,23 +179,30 @@ private void 
restoreVolumesOfDestroyedVMs(KVMStoragePoolManager storagePoolMgr,
         }
     }
 
-    private void restoreVolume(KVMStoragePoolManager storagePoolMgr, String 
backupPath, PrimaryDataStoreTO volumePool, String volumePath, String diskType, 
String backupFile,
+    /**
+     * @return the volume path the backup was actually written to. Equal to 
{@code volumePath}
+     * except when the pool assigns the created volume's identity itself 
(StorPool), in which
+     * case it reflects the real device path.
+     */
+    private String restoreVolume(KVMStoragePoolManager storagePoolMgr, String 
backupPath, PrimaryDataStoreTO volumePool, String volumePath, String diskType, 
String backupFile,
                                Long size, Pair<String, VirtualMachine.State> 
vmNameAndState, String mountDirectory, int timeout, Integer mountTimeout) {
         String bkpPath;
         String volumeUuid;
         try {
             bkpPath = getBackupPath(mountDirectory, backupPath, backupFile, 
diskType);
             volumeUuid = getVolumeUuidFromPath(volumePath, volumePool);
             verifyBackupFile(bkpPath, volumeUuid);
-            if (!replaceVolumeWithBackup(storagePoolMgr, volumePool, 
volumePath, bkpPath, timeout, true, size)) {
+            Pair<Boolean, String> restoreResult = 
replaceVolumeWithBackup(storagePoolMgr, volumePool, volumePath, bkpPath, 
timeout, true, size);
+            if (!restoreResult.first()) {
                 throw new CloudRuntimeException(String.format("Unable to 
restore contents from the backup volume [%s].", volumeUuid));

Review Comment:
   if the copy fails here, does anything delete the new storpool volume? 
cloudstack never gets its id back so it looks like it stays behind



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