You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by pe...@apache.org on 2026/02/09 19:15:10 UTC

(cloudstack) branch clvm-enhancements created (now 1eeeeea892a)

This is an automated email from the ASF dual-hosted git repository.

pearl11594 pushed a change to branch clvm-enhancements
in repository https://gitbox.apache.org/repos/asf/cloudstack.git


      at 1eeeeea892a CLVM enhancements and fixes

This branch includes the following new commits:

     new 1eeeeea892a CLVM enhancements and fixes

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



(cloudstack) 01/01: CLVM enhancements and fixes

Posted by pe...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

pearl11594 pushed a commit to branch clvm-enhancements
in repository https://gitbox.apache.org/repos/asf/cloudstack.git

commit 1eeeeea892a148a8d1b9ab66f77eb348d31da1d8
Author: Pearl Dsilva <pe...@gmail.com>
AuthorDate: Mon Feb 9 14:14:48 2026 -0500

    CLVM enhancements and fixes
---
 .../kvm/storage/KVMStorageProcessor.java           | 107 ++++++++++++-
 .../kvm/storage/LibvirtStorageAdaptor.java         | 173 ++++++++++++++++++++-
 scripts/storage/qcow2/managesnapshot.sh            |  16 +-
 3 files changed, 281 insertions(+), 15 deletions(-)

diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
index 030d9747d6c..7c38ec5a247 100644
--- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
+++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
@@ -1115,7 +1115,18 @@ public class KVMStorageProcessor implements StorageProcessor {
                 }
             } else {
                 final Script command = new Script(_manageSnapshotPath, cmd.getWaitInMillSeconds(), logger);
-                command.add("-b", isCreatedFromVmSnapshot ? snapshotDisk.getPath() : snapshot.getPath());
+                // For CLVM, use snapshotDisk.getPath() which contains the correct base volume path
+                // snapshot.getPath() incorrectly has snapshot name appended for CLVM
+                String backupPath;
+                if (primaryPool.getType() == StoragePoolType.CLVM) {
+                    // Use snapshotDisk.getPath() for CLVM - it contains just the volume path
+                    backupPath = snapshotDisk.getPath();
+                    logger.debug("Using snapshotDisk path for CLVM backup: " + backupPath);
+                } else {
+                    // For file-based storage, use snapshot.getPath()
+                    backupPath = isCreatedFromVmSnapshot ? snapshotDisk.getPath() : snapshot.getPath();
+                }
+                command.add("-b", backupPath);
                 command.add(NAME_OPTION, snapshotName);
                 command.add("-p", snapshotDestPath);
 
@@ -1172,7 +1183,14 @@ public class KVMStorageProcessor implements StorageProcessor {
 
         if ((backupSnapshotAfterTakingSnapshot == null || BooleanUtils.toBoolean(backupSnapshotAfterTakingSnapshot)) && deleteSnapshotOnPrimary) {
             try {
-                Files.deleteIfExists(Paths.get(snapshotPath));
+                if (primaryPool.getType() == StoragePoolType.CLVM) {
+                    // For CLVM, snapshot path is device path like /dev/vg/volume/snapshot
+                    // We need to use lvremove to delete the LVM snapshot
+                    deleteClvmSnapshot(snapshotPath);
+                } else {
+                    // For file-based storage, delete the snapshot file
+                    Files.deleteIfExists(Paths.get(snapshotPath));
+                }
             } catch (IOException ex) {
                 logger.error("Failed to delete snapshot [{}] on primary storage [{}].", snapshot.getId(), snapshot.getName(), ex);
             }
@@ -1181,6 +1199,81 @@ public class KVMStorageProcessor implements StorageProcessor {
         }
     }
 
+    /**
+     * Delete a CLVM snapshot using lvremove command.
+     * For CLVM, the snapshot path stored in DB is: /dev/vgname/volumeuuid/snapshotuuid
+     * However, managesnapshot.sh creates the actual snapshot using MD5 hash of the snapshot UUID.
+     * The actual device is at: /dev/mapper/vgname-MD5(snapshotuuid)
+     * We need to compute the MD5 hash and remove both the snapshot LV and its COW volume.
+     */
+    private void deleteClvmSnapshot(String snapshotPath) {
+        try {
+            // Parse the snapshot path: /dev/acsvg/volume-uuid/snapshot-uuid
+            // Extract VG name and snapshot UUID
+            String[] pathParts = snapshotPath.split("/");
+            if (pathParts.length < 5) {
+                logger.warn("Invalid CLVM snapshot path format: " + snapshotPath + ", skipping deletion");
+                return;
+            }
+
+            String vgName = pathParts[2];
+            String snapshotUuid = pathParts[4];
+
+            // Compute MD5 hash of snapshot UUID (same as managesnapshot.sh does)
+            String md5Hash = computeMd5Hash(snapshotUuid);
+
+            logger.debug("Deleting CLVM snapshot for UUID: " + snapshotUuid + " (MD5: " + md5Hash + ")");
+
+            // Remove the snapshot device mapper entry
+            // The snapshot device is at: /dev/mapper/vgname-md5hash
+            String vgNameEscaped = vgName.replace("-", "--");
+            String snapshotDevice = vgNameEscaped + "-" + md5Hash;
+
+            Script dmRemoveCmd = new Script("/usr/sbin/dmsetup", 30000, logger);
+            dmRemoveCmd.add("remove");
+            dmRemoveCmd.add(snapshotDevice);
+            String dmResult = dmRemoveCmd.execute();
+            if (dmResult != null) {
+                logger.debug("dmsetup remove returned: " + dmResult + " (may already be removed)");
+            }
+
+            // Remove the COW (copy-on-write) volume: /dev/vgname/md5hash-cow
+            String cowLvPath = "/dev/" + vgName + "/" + md5Hash + "-cow";
+            Script removeCowCmd = new Script("/usr/sbin/lvremove", 30000, logger);
+            removeCowCmd.add("-f");
+            removeCowCmd.add(cowLvPath);
+
+            String cowResult = removeCowCmd.execute();
+            if (cowResult != null) {
+                logger.warn("Failed to remove CLVM COW volume " + cowLvPath + ": " + cowResult);
+            } else {
+                logger.debug("Successfully deleted CLVM snapshot COW volume: " + cowLvPath);
+            }
+
+        } catch (Exception ex) {
+            logger.error("Exception while deleting CLVM snapshot " + snapshotPath, ex);
+        }
+    }
+
+    /**
+     * Compute MD5 hash of a string, matching what managesnapshot.sh does:
+     * echo "${snapshot}" | md5sum -t | awk '{ print $1 }'
+     */
+    private String computeMd5Hash(String input) {
+        try {
+            java.security.MessageDigest md = java.security.MessageDigest.getInstance("MD5");
+            byte[] array = md.digest((input + "\n").getBytes("UTF-8")); // md5sum includes newline
+            StringBuilder sb = new StringBuilder();
+            for (byte b : array) {
+                sb.append(String.format("%02x", b));
+            }
+            return sb.toString();
+        } catch (Exception e) {
+            logger.error("Failed to compute MD5 hash for: " + input, e);
+            return input; // Fallback to original
+        }
+    }
+
     protected synchronized void attachOrDetachISO(final Connect conn, final String vmName, String isoPath, final boolean isAttach, Map<String, String> params, DataStoreTO store) throws
             LibvirtException, InternalErrorException {
         DiskDef iso = new DiskDef();
@@ -1842,8 +1935,14 @@ public class KVMStorageProcessor implements StorageProcessor {
                 }
             }
 
-            if (DomainInfo.DomainState.VIR_DOMAIN_RUNNING.equals(state) && volume.requiresEncryption()) {
-                throw new CloudRuntimeException("VM is running, encrypted volume snapshots aren't supported");
+            if (DomainInfo.DomainState.VIR_DOMAIN_RUNNING.equals(state)) {
+                if (volume.requiresEncryption()) {
+                    throw new CloudRuntimeException("VM is running, encrypted volume snapshots aren't supported");
+                }
+
+                if (StoragePoolType.CLVM.name().equals(primaryStore.getType())) {
+                    throw new CloudRuntimeException("VM is running, live snapshots aren't supported with CLVM primary storage");
+                }
             }
 
             KVMStoragePool primaryPool = storagePoolMgr.getStoragePool(primaryStore.getPoolType(), primaryStore.getUuid());
diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java
index a03daeb197b..f945981030b 100644
--- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java
+++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java
@@ -34,6 +34,7 @@ import java.util.stream.Collectors;
 
 import com.cloud.agent.properties.AgentProperties;
 import com.cloud.agent.properties.AgentPropertiesFileHandler;
+import com.cloud.utils.script.OutputInterpreter;
 import org.apache.cloudstack.api.ApiConstants;
 import org.apache.cloudstack.utils.cryptsetup.KeyFile;
 import org.apache.cloudstack.utils.qemu.QemuImageOptions;
@@ -254,9 +255,12 @@ public class LibvirtStorageAdaptor implements StorageAdaptor {
 
             try {
                 vol = pool.storageVolLookupByName(volName);
-                logger.debug("Found volume " + volName + " in storage pool " + pool.getName() + " after refreshing the pool");
+                if (vol != null) {
+                    logger.debug("Found volume " + volName + " in storage pool " + pool.getName() + " after refreshing the pool");
+                }
             } catch (LibvirtException e) {
-                throw new CloudRuntimeException("Could not find volume " + volName + ": " + e.getMessage());
+                logger.debug("Volume " + volName + " still not found after pool refresh: " + e.getMessage());
+                return null;
             }
         }
 
@@ -663,6 +667,17 @@ public class LibvirtStorageAdaptor implements StorageAdaptor {
 
         try {
             StorageVol vol = getVolume(libvirtPool.getPool(), volumeUuid);
+
+            // Check if volume was found - if null, treat as not found and trigger fallback for CLVM
+            if (vol == null) {
+                logger.debug("Volume " + volumeUuid + " not found in libvirt, will check for CLVM fallback");
+                if (pool.getType() == StoragePoolType.CLVM) {
+                    return getPhysicalDiskRetry(volumeUuid, pool, libvirtPool);
+                }
+
+                throw new CloudRuntimeException("Volume " + volumeUuid + " not found in libvirt pool");
+            }
+
             KVMPhysicalDisk disk;
             LibvirtStorageVolumeDef voldef = getStorageVolumeDef(libvirtPool.getPool().getConnect(), vol);
             disk = new KVMPhysicalDisk(vol.getPath(), vol.getName(), pool);
@@ -693,11 +708,159 @@ public class LibvirtStorageAdaptor implements StorageAdaptor {
             }
             return disk;
         } catch (LibvirtException e) {
-            logger.debug("Failed to get physical disk:", e);
+            logger.debug("Failed to get volume from libvirt: " + e.getMessage());
+            // For CLVM, try direct block device access as fallback
+            if (pool.getType() == StoragePoolType.CLVM) {
+                return getPhysicalDiskRetry(volumeUuid, pool, libvirtPool);
+            }
+
             throw new CloudRuntimeException(e.toString());
         }
     }
 
+    private KVMPhysicalDisk getPhysicalDiskRetry(String volumeUuid, KVMStoragePool pool, LibvirtStoragePool libvirtPool) {
+        logger.info("CLVM volume not visible to libvirt, attempting direct block device access for volume: " + volumeUuid);
+
+        try {
+            // Refresh pool first - maybe libvirt just needs a refresh
+            logger.debug("Refreshing libvirt storage pool: " + pool.getUuid());
+            libvirtPool.getPool().refresh(0);
+
+            StorageVol vol = getVolume(libvirtPool.getPool(), volumeUuid);
+            if (vol != null) {
+                logger.info("Volume found after pool refresh: " + volumeUuid);
+                KVMPhysicalDisk disk;
+                LibvirtStorageVolumeDef voldef = getStorageVolumeDef(libvirtPool.getPool().getConnect(), vol);
+                disk = new KVMPhysicalDisk(vol.getPath(), vol.getName(), pool);
+                disk.setSize(vol.getInfo().allocation);
+                disk.setVirtualSize(vol.getInfo().capacity);
+                disk.setFormat(voldef.getFormat() == LibvirtStorageVolumeDef.VolumeFormat.QCOW2 ?
+                        PhysicalDiskFormat.QCOW2 : PhysicalDiskFormat.RAW);
+                return disk;
+            }
+        } catch (LibvirtException refreshEx) {
+            logger.debug("Pool refresh failed or volume still not found: " + refreshEx.getMessage());
+        }
+
+        // Still not found after refresh, try direct block device access
+        return getPhysicalDiskViaDirectBlockDevice(volumeUuid, pool);
+    }
+
+    /**
+     * For CLVM volumes that exist in LVM but are not visible to libvirt,
+     * access them directly via block device path.
+     */
+    private KVMPhysicalDisk getPhysicalDiskViaDirectBlockDevice(String volumeUuid, KVMStoragePool pool) {
+        try {
+            // For CLVM, pool sourceDir contains the VG path (e.g., "/dev/acsvg")
+            // Extract the VG name
+            String sourceDir = pool.getLocalPath();
+            if (sourceDir == null || sourceDir.isEmpty()) {
+                throw new CloudRuntimeException("CLVM pool sourceDir is not set, cannot determine VG name");
+            }
+
+            // Strip /dev/ prefix if present to get VG name
+            String vgName = sourceDir;
+            if (vgName.startsWith("/dev/")) {
+                vgName = vgName.substring(5); // Remove "/dev/"
+            }
+
+            logger.debug("Using VG name: " + vgName + " (from sourceDir: " + sourceDir + ")");
+
+            // First, check if the LV actually exists in LVM using lvs command
+            logger.debug("Checking if volume " + volumeUuid + " exists in VG " + vgName);
+            Script checkLvCmd = new Script("/usr/sbin/lvs", 5000, logger);
+            checkLvCmd.add("--noheadings");
+            checkLvCmd.add("--unbuffered");
+            checkLvCmd.add(vgName + "/" + volumeUuid);
+
+            String checkResult = checkLvCmd.execute();
+            if (checkResult != null) {
+                // LV doesn't exist in LVM at all
+                logger.debug("Volume " + volumeUuid + " does not exist in VG " + vgName + ": " + checkResult);
+                throw new CloudRuntimeException("Storage volume not found: no storage vol with matching name '" + volumeUuid + "'");
+            }
+
+            logger.info("Volume " + volumeUuid + " exists in LVM but not visible to libvirt, accessing directly");
+
+            // Try standard device path first
+            String lvPath = "/dev/" + vgName + "/" + volumeUuid;
+            File lvDevice = new File(lvPath);
+
+            if (!lvDevice.exists()) {
+                // Try device-mapper path with escaped hyphens
+                String vgNameEscaped = vgName.replace("-", "--");
+                String volumeUuidEscaped = volumeUuid.replace("-", "--");
+                lvPath = "/dev/mapper/" + vgNameEscaped + "-" + volumeUuidEscaped;
+                lvDevice = new File(lvPath);
+
+                if (!lvDevice.exists()) {
+                    logger.warn("Volume exists in LVM but device node not found: " + volumeUuid);
+                    throw new CloudRuntimeException("Could not find volume " + volumeUuid +
+                        " in VG " + vgName + " - volume exists in LVM but device node not accessible");
+                }
+            }
+
+            // Get size from lvs command
+            long size = 0;
+            try {
+                Script lvsCmd = new Script("/usr/sbin/lvs", 5000, logger);
+                lvsCmd.add("--noheadings");
+                lvsCmd.add("--units");
+                lvsCmd.add("b");
+                lvsCmd.add("-o");
+                lvsCmd.add("lv_size");
+                lvsCmd.add(lvPath);
+
+                OutputInterpreter.AllLinesParser parser = new OutputInterpreter.AllLinesParser();
+                String result = lvsCmd.execute(parser);
+
+                String output = null;
+                if (result == null) {
+                    // Command succeeded, get output from parser
+                    output = parser.getLines();
+                } else {
+                    // Command may have failed but could have output in result
+                    // (some commands return output even on error)
+                    output = result;
+                }
+
+                if (output != null && !output.isEmpty()) {
+                    // Output format: "  8589934592B"
+                    String sizeStr = output.trim().replaceAll("[^0-9]", "");
+                    if (!sizeStr.isEmpty()) {
+                        size = Long.parseLong(sizeStr);
+                    }
+                }
+            } catch (Exception sizeEx) {
+                logger.warn("Failed to get size for CLVM volume via lvs: " + sizeEx.getMessage());
+
+                // Try to get size from block device
+                if (lvDevice.isFile()) {
+                    size = lvDevice.length();
+                }
+            }
+
+            // Create KVMPhysicalDisk object
+            KVMPhysicalDisk disk = new KVMPhysicalDisk(lvPath, volumeUuid, pool);
+            disk.setFormat(PhysicalDiskFormat.RAW);
+            disk.setSize(size);
+            disk.setVirtualSize(size);
+
+            logger.info("Successfully accessed CLVM volume via direct block device: " + lvPath +
+                       " with size: " + size + " bytes");
+
+            return disk;
+
+        } catch (CloudRuntimeException ex) {
+            // Re-throw CloudRuntimeException as-is
+            throw ex;
+        } catch (Exception ex) {
+            logger.error("Failed to access CLVM volume via direct block device: " + volumeUuid, ex);
+            throw new CloudRuntimeException("Could not find volume " + volumeUuid + ": " + ex.getMessage());
+        }
+    }
+
     /**
      * adjust refcount
      */
@@ -1227,6 +1390,10 @@ public class LibvirtStorageAdaptor implements StorageAdaptor {
         LibvirtStoragePool libvirtPool = (LibvirtStoragePool)pool;
         try {
             StorageVol vol = getVolume(libvirtPool.getPool(), uuid);
+            if (vol == null) {
+                logger.warn("Volume " + uuid + " not found in libvirt pool " + pool.getUuid() + ", it may have been already deleted");
+                return true;  // Consider it deleted if not found
+            }
             logger.debug("Instructing libvirt to remove volume " + uuid + " from pool " + pool.getUuid());
             if(Storage.ImageFormat.DIR.equals(format)){
                 deleteDirVol(libvirtPool, vol);
diff --git a/scripts/storage/qcow2/managesnapshot.sh b/scripts/storage/qcow2/managesnapshot.sh
index 3650bdd9b6f..46e194cd3a8 100755
--- a/scripts/storage/qcow2/managesnapshot.sh
+++ b/scripts/storage/qcow2/managesnapshot.sh
@@ -212,12 +212,12 @@ backup_snapshot() {
       return 1
     fi
 
-    qemuimg_ret=$($qemu_img $forceShareFlag -f raw -O qcow2 "/dev/mapper/${vg_dm}-${snapshotname}" "${destPath}/${destName}")
+    qemuimg_ret=$($qemu_img convert $forceShareFlag -f raw -O qcow2 "/dev/mapper/${vg_dm}-${snapshotname}" "${destPath}/${destName}" 2>&1)
     ret_code=$?
-    if [ $ret_code -gt 0 ] && [[ $qemuimg_ret == *"snapshot: invalid option -- 'U'"* ]]
+    if [ $ret_code -gt 0 ] && ([[ $qemuimg_ret == *"invalid option"*"'U'"* ]] || [[ $qemuimg_ret == *"unrecognized option"*"'-U'"* ]])
     then
       forceShareFlag=""
-      $qemu_img $forceShareFlag -f raw -O qcow2 "/dev/mapper/${vg_dm}-${snapshotname}" "${destPath}/${destName}"
+      $qemu_img convert $forceShareFlag -f raw -O qcow2 "/dev/mapper/${vg_dm}-${snapshotname}" "${destPath}/${destName}"
       ret_code=$?
     fi
     if [ $ret_code -gt 0 ]
@@ -240,9 +240,9 @@ backup_snapshot() {
       # Backup VM snapshot
       qemuimg_ret=$($qemu_img snapshot $forceShareFlag -l $disk 2>&1)
       ret_code=$?
-      if [ $ret_code -gt 0 ] && [[ $qemuimg_ret == *"snapshot: invalid option -- 'U'"* ]]; then
+      if [ $ret_code -gt 0 ] && ([[ $qemuimg_ret == *"invalid option"*"'U'"* ]] || [[ $qemuimg_ret == *"unrecognized option"*"'-U'"* ]]); then
         forceShareFlag=""
-        qemuimg_ret=$($qemu_img snapshot $forceShareFlag -l $disk)
+        qemuimg_ret=$($qemu_img snapshot $forceShareFlag -l $disk 2>&1)
         ret_code=$?
       fi
 
@@ -251,11 +251,11 @@ backup_snapshot() {
         return 1
       fi
 
-      qemuimg_ret=$($qemu_img convert $forceShareFlag -f qcow2 -O qcow2 -l snapshot.name=$snapshotname $disk $destPath/$destName 2>&1 > /dev/null)
+      qemuimg_ret=$($qemu_img convert $forceShareFlag -f qcow2 -O qcow2 -l snapshot.name=$snapshotname $disk $destPath/$destName 2>&1)
       ret_code=$?
-      if [ $ret_code -gt 0 ] && [[ $qemuimg_ret == *"convert: invalid option -- 'U'"* ]]; then
+      if [ $ret_code -gt 0 ] && ([[ $qemuimg_ret == *"invalid option"*"'U'"* ]] || [[ $qemuimg_ret == *"unrecognized option"*"'-U'"* ]]); then
         forceShareFlag=""
-        qemuimg_ret=$($qemu_img convert $forceShareFlag -f qcow2 -O qcow2 -l snapshot.name=$snapshotname $disk $destPath/$destName 2>&1 > /dev/null)
+        qemuimg_ret=$($qemu_img convert $forceShareFlag -f qcow2 -O qcow2 -l snapshot.name=$snapshotname $disk $destPath/$destName 2>&1)
         ret_code=$?
       fi