From 131ea9f7aceb4c12d8cbf0fe92bcc0419f40a2bc Mon Sep 17 00:00:00 2001 From: owsferraro Date: Fri, 27 Mar 2026 11:22:08 +0100 Subject: [PATCH] Fix PowerFlex 4.x issues with take & revert instance snapshots (#12880) * fixed database update on snapshot with multiple volumes and an api change * changed overwritevolumecontent based on powerflex version and removed unnecessary comments * Update plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/client/ScaleIOGatewayClientImpl.java Co-authored-by: Suresh Kumar Anaparti * Update plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/client/ScaleIOGatewayClientImpl.java Co-authored-by: Suresh Kumar Anaparti * Update plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/client/ScaleIOGatewayClientImpl.java Co-authored-by: Suresh Kumar Anaparti --------- Co-authored-by: Suresh Kumar Anaparti --- .../vmsnapshot/ScaleIOVMSnapshotStrategy.java | 33 ++++++++-- .../client/ScaleIOGatewayClientImpl.java | 61 ++++++++++++++++++- 2 files changed, 89 insertions(+), 5 deletions(-) diff --git a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/ScaleIOVMSnapshotStrategy.java b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/ScaleIOVMSnapshotStrategy.java index 7199fce1d34..aced750bd32 100644 --- a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/ScaleIOVMSnapshotStrategy.java +++ b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/ScaleIOVMSnapshotStrategy.java @@ -40,6 +40,7 @@ import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.apache.cloudstack.storage.datastore.util.ScaleIOUtil; import org.apache.cloudstack.storage.to.VolumeObjectTO; import org.apache.commons.collections.CollectionUtils; +import org.apache.cloudstack.storage.datastore.api.Volume; import com.cloud.agent.api.VMSnapshotTO; import com.cloud.alert.AlertManager; @@ -200,11 +201,35 @@ public class ScaleIOVMSnapshotStrategy extends ManagerBase implements VMSnapshot if (volumeIds != null && !volumeIds.isEmpty()) { List vmSnapshotDetails = new ArrayList(); vmSnapshotDetails.add(new VMSnapshotDetailsVO(vmSnapshot.getId(), "SnapshotGroupId", snapshotGroupId, false)); + Map snapshotNameToSrcPathMap = new HashMap<>(); + for (Map.Entry entry : srcVolumeDestSnapshotMap.entrySet()) { + snapshotNameToSrcPathMap.put(entry.getValue(), entry.getKey()); + } - for (int index = 0; index < volumeIds.size(); index++) { - String volumeSnapshotName = srcVolumeDestSnapshotMap.get(ScaleIOUtil.getVolumePath(volumeTOs.get(index).getPath())); - String pathWithScaleIOVolumeName = ScaleIOUtil.updatedPathWithVolumeName(volumeIds.get(index), volumeSnapshotName); - vmSnapshotDetails.add(new VMSnapshotDetailsVO(vmSnapshot.getId(), "Vol_" + volumeTOs.get(index).getId() + "_Snapshot", pathWithScaleIOVolumeName, false)); + for (String snapshotVolumeId : volumeIds) { + // Use getVolume() to fetch snapshot volume details and get its name + Volume snapshotVolume = client.getVolume(snapshotVolumeId); + if (snapshotVolume == null) { + throw new CloudRuntimeException("Cannot find snapshot volume with id: " + snapshotVolumeId); + } + String snapshotName = snapshotVolume.getName(); + + // Match back to source volume path + String srcVolumePath = snapshotNameToSrcPathMap.get(snapshotName); + if (srcVolumePath == null) { + throw new CloudRuntimeException("Cannot match snapshot " + snapshotName + " to a source volume"); + } + + // Find the matching VolumeObjectTO by path + VolumeObjectTO matchedVolume = volumeTOs.stream() + .filter(v -> ScaleIOUtil.getVolumePath(v.getPath()).equals(srcVolumePath)) + .findFirst() + .orElseThrow(() -> new CloudRuntimeException("Cannot find source volume for path: " + srcVolumePath)); + + String pathWithScaleIOVolumeName = ScaleIOUtil.updatedPathWithVolumeName(snapshotVolumeId, snapshotName); + vmSnapshotDetails.add(new VMSnapshotDetailsVO(vmSnapshot.getId(), + "Vol_" + matchedVolume.getId() + "_Snapshot", + pathWithScaleIOVolumeName, false)); } vmSnapshotDetailsDao.saveDetails(vmSnapshotDetails); diff --git a/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/client/ScaleIOGatewayClientImpl.java b/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/client/ScaleIOGatewayClientImpl.java index c6a61c35b8b..8e23bc159a4 100644 --- a/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/client/ScaleIOGatewayClientImpl.java +++ b/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/client/ScaleIOGatewayClientImpl.java @@ -94,6 +94,9 @@ public class ScaleIOGatewayClientImpl implements ScaleIOGatewayClient { private String password; private String sessionKey; + private String gatewayVersion = null; + private int[] parsedVersion = null; + // The session token is valid for 8 hours from the time it was created, unless there has been no activity for 10 minutes // Reference: https://cpsdocs.dellemc.com/bundle/PF_REST_API_RG/page/GUID-92430F19-9F44-42B6-B898-87D5307AE59B.html private static final long MAX_VALID_SESSION_TIME_IN_HRS = 8; @@ -621,15 +624,26 @@ public class ScaleIOGatewayClientImpl implements ScaleIOGatewayClient { throw new CloudRuntimeException("Unable to revert, source snapshot volume and destination volume doesn't belong to same volume tree"); } + String requestBody = buildOverwriteVolumeContentRequest(sourceSnapshotVolumeId); + Boolean overwriteVolumeContentStatus = post( "/instances/Volume::" + destVolumeId + "/action/overwriteVolumeContent", - String.format("{\"srcVolumeId\":\"%s\",\"allowOnExtManagedVol\":\"TRUE\"}", sourceSnapshotVolumeId), Boolean.class); + requestBody, Boolean.class); if (overwriteVolumeContentStatus != null) { return overwriteVolumeContentStatus; } return false; } + private String buildOverwriteVolumeContentRequest(final String srcVolumeId) { + if (isVersionAtLeast(4, 0)) { + logger.debug("Using PowerFlex 4.0+ overwriteVolumeContent request body"); + return String.format("{\"srcVolumeId\":\"%s\"}", srcVolumeId); + } else { + logger.debug("Using pre-4.0 overwriteVolumeContent request body"); + return String.format("{\"srcVolumeId\":\"%s\",\"allowOnExtManagedVol\":\"TRUE\"}", srcVolumeId); } + } + @Override public boolean mapVolumeToSdc(final String volumeId, final String sdcId) { Preconditions.checkArgument(StringUtils.isNotEmpty(volumeId), "Volume id cannot be null"); @@ -1168,4 +1182,49 @@ public class ScaleIOGatewayClientImpl implements ScaleIOGatewayClient { sb.append("\n"); return sb.toString(); } + + private String fetchGatewayVersion() { + try { + JsonNode node = get("/version", JsonNode.class); + if (node != null && node.isTextual()) { + return node.asText(); + } + if (node != null && node.has("version")) { + return node.get("version").asText(); + } + } catch (Exception e) { + logger.warn("Could not fetch PowerFlex gateway version: " + e.getMessage()); + } + return null; + } + + private int[] parseVersion(String version) { + if (StringUtils.isEmpty(version)) return new int[]{0, 0, 0}; + String[] parts = version.replaceAll("\"", "").split("\\."); + int[] parsed = new int[3]; + for (int i = 0; i < Math.min(parts.length, 3); i++) { + try { + parsed[i] = Integer.parseInt(parts[i].trim()); + } catch (NumberFormatException e) { + parsed[i] = 0; + } + } + return parsed; + } + + private synchronized int[] getGatewayVersion() { + if (parsedVersion == null) { + gatewayVersion = fetchGatewayVersion(); + parsedVersion = parseVersion(gatewayVersion); + logger.info("PowerFlex Gateway version detected: " + gatewayVersion + + " => parsed: " + Arrays.toString(parsedVersion)); + } + return parsedVersion; + } + + private boolean isVersionAtLeast(int major, int minor) { + int[] v = getGatewayVersion(); + if (v[0] != major) return v[0] > major; + return v[1] >= minor; + } }