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 <[email protected]> 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
