Re: [PR] Powerflex fix [cloudstack]

2026-03-25 Thread via GitHub 


blueorangutan commented on PR #12880:
URL: https://github.com/apache/cloudstack/pull/12880#issuecomment-4126105570

   @nvazquez a [SL] Jenkins job has been kicked to build packages. It will be 
bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted 
as I make progress.


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

Re: [PR] Powerflex fix [cloudstack]

2026-03-25 Thread via GitHub 


nvazquez commented on PR #12880:
URL: https://github.com/apache/cloudstack/pull/12880#issuecomment-4126100923

   @blueorangutan package


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

Re: [PR] Powerflex fix [cloudstack]

2026-03-25 Thread via GitHub 


owsferraro commented on code in PR #12880:
URL: https://github.com/apache/cloudstack/pull/12880#discussion_r2987336639


##
plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/client/ScaleIOGatewayClientImpl.java:
##
@@ -621,15 +624,27 @@ public boolean revertSnapshot(final String 
sourceSnapshotVolumeId, final String
 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);}

Review Comment:
   this parameter isn't required, but only have only tested on Powerflex 4.6



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

Re: [PR] Powerflex fix [cloudstack]

2026-03-25 Thread via GitHub 


sureshanaparti commented on code in PR #12880:
URL: https://github.com/apache/cloudstack/pull/12880#discussion_r2987316681


##
plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/client/ScaleIOGatewayClientImpl.java:
##
@@ -621,15 +624,27 @@ public boolean revertSnapshot(final String 
sourceSnapshotVolumeId, final String
 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);}

Review Comment:
   @owsferraro is `volumeClass` not required for 4.x call?



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

Re: [PR] Powerflex fix [cloudstack]

2026-03-25 Thread via GitHub 


sureshanaparti commented on code in PR #12880:
URL: https://github.com/apache/cloudstack/pull/12880#discussion_r2987311564


##
plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/client/ScaleIOGatewayClientImpl.java:
##
@@ -621,15 +624,27 @@ public boolean revertSnapshot(final String 
sourceSnapshotVolumeId, final String
 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);}

Review Comment:
   Ref: 
   4.0 - 
https://developer.dell.com/apis/4008/versions/4.0/PowerFlex_REST_API.json/paths/~1api~1instances~1Volume::%7Bid%7D~1action~1overwriteVolumeContent/post
   
   3..6 - 
https://developer.dell.com/apis/4008/versions/3.6/PowerFlex_3.6_RESTAPI.v1_149.json/paths/~1api~1instances~1Volume::%7Bid%7D~1action~1overwriteVolumeContent/post



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

Re: [PR] Powerflex fix [cloudstack]

2026-03-25 Thread via GitHub 


owsferraro commented on code in PR #12880:
URL: https://github.com/apache/cloudstack/pull/12880#discussion_r2987184629


##
plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/client/ScaleIOGatewayClientImpl.java:
##
@@ -621,15 +624,27 @@ public boolean revertSnapshot(final String 
sourceSnapshotVolumeId, final String
 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);}

Review Comment:
   based on public Dell Rest API documentation, the parameter was removed 
starting on version 4.0, the previous version 3.6 contains the parameter. 



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

Re: [PR] Powerflex fix [cloudstack]

2026-03-25 Thread via GitHub 


sureshanaparti commented on code in PR #12880:
URL: https://github.com/apache/cloudstack/pull/12880#discussion_r2987017562


##
plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/client/ScaleIOGatewayClientImpl.java:
##
@@ -621,15 +624,27 @@ public boolean revertSnapshot(final String 
sourceSnapshotVolumeId, final String
 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);}

Review Comment:
   is _allowOnExtManagedVol_ parameter removed from 4.5 onwards?



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

Re: [PR] Powerflex fix [cloudstack]

2026-03-24 Thread via GitHub 


sureshanaparti commented on code in PR #12880:
URL: https://github.com/apache/cloudstack/pull/12880#discussion_r2986163652


##
plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/client/ScaleIOGatewayClientImpl.java:
##
@@ -621,15 +624,27 @@ public boolean revertSnapshot(final String 
sourceSnapshotVolumeId, final String
 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);

Review Comment:
   ```suggestion
   requestBody, Boolean.class);
   ```



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

Re: [PR] Powerflex fix [cloudstack]

2026-03-24 Thread via GitHub 


owsferraro commented on code in PR #12880:
URL: https://github.com/apache/cloudstack/pull/12880#discussion_r2983132527


##
plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/client/ScaleIOGatewayClientImpl.java:
##
@@ -623,7 +623,7 @@ public boolean revertSnapshot(final String 
sourceSnapshotVolumeId, final String
 
 Boolean overwriteVolumeContentStatus = post(
 "/instances/Volume::" + destVolumeId + 
"/action/overwriteVolumeContent",
-
String.format("{\"srcVolumeId\":\"%s\",\"allowOnExtManagedVol\":\"TRUE\"}", 
sourceSnapshotVolumeId), Boolean.class);
+
String.format("{\"srcVolumeId\":\"%s\"}",sourceSnapshotVolumeId),Boolean.class);

Review Comment:
   changed the code to check the Powerflex version and if >= 4.0 the 
allowOnExtManagedVol is removed. I have tested this change on Powerflex version 
4.6, I cannot test on version 3.5



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

Re: [PR] Powerflex fix [cloudstack]

2026-03-24 Thread via GitHub 


owsferraro commented on code in PR #12880:
URL: https://github.com/apache/cloudstack/pull/12880#discussion_r2983117680


##
engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/ScaleIOVMSnapshotStrategy.java:
##
@@ -201,11 +202,44 @@ public VMSnapshot takeVMSnapshot(VMSnapshot vmSnapshot) {
 List vmSnapshotDetails = new 
ArrayList();
 vmSnapshotDetails.add(new 
VMSnapshotDetailsVO(vmSnapshot.getId(), "SnapshotGroupId", snapshotGroupId, 
false));
 
-for (int index = 0; index < volumeIds.size(); index++) {
+   /* 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));
 }
+   */

Review Comment:
   comments removed



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

Re: [PR] Powerflex fix [cloudstack]

2026-03-24 Thread via GitHub 


owsferraro commented on code in PR #12880:
URL: https://github.com/apache/cloudstack/pull/12880#discussion_r2983119069


##
engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/ScaleIOVMSnapshotStrategy.java:
##
@@ -201,11 +202,44 @@ public VMSnapshot takeVMSnapshot(VMSnapshot vmSnapshot) {
 List vmSnapshotDetails = new 
ArrayList();
 vmSnapshotDetails.add(new 
VMSnapshotDetailsVO(vmSnapshot.getId(), "SnapshotGroupId", snapshotGroupId, 
false));
 
-for (int index = 0; index < volumeIds.size(); index++) {
+   /* 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));
 }
+   */
+ // Invert the srcVolumeDestSnapshotMap: snapshotName -> 
srcVolumePath

Review Comment:
   comment removed



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

Re: [PR] Powerflex fix [cloudstack]

2026-03-24 Thread via GitHub 


codecov[bot] commented on PR #12880:
URL: https://github.com/apache/cloudstack/pull/12880#issuecomment-4117680893

   ## 
[Codecov](https://app.codecov.io/gh/apache/cloudstack/pull/12880?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
 Report
   :white_check_mark: All modified and coverable lines are covered by tests.
   :white_check_mark: Project coverage is 3.53%. Comparing base 
([`b744824`](https://app.codecov.io/gh/apache/cloudstack/commit/b744824f65813e01786d8a69ed82a4f49fe45e39?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache))
 to head 
([`ed4bb4c`](https://app.codecov.io/gh/apache/cloudstack/commit/ed4bb4cee55a188fa13df725b7450cccd5ecc3c9?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   > :exclamation:  There is a different number of reports uploaded between 
BASE (b744824) and HEAD (ed4bb4c). Click for more details.
   > 
   > HEAD has 1 upload less than BASE
   >
   >| Flag | BASE (b744824) | HEAD (ed4bb4c) |
   >|--|--|--|
   >|unittests|1|0|
   >
   
   Additional details and impacted files
   
   
   
   ```diff
   @@  Coverage Diff  @@
   ##   main   #12880   +/-   ##
   =
   - Coverage 18.02%3.53%   -14.50% 
   =
 Files  5968  464 -5504 
 Lines53708640020   -497066 
 Branches  65961 7530-58431 
   =
   - Hits  96819 1414-95405 
   + Misses   42934738418   -390929 
   + Partials  10920  188-10732 
   ```
   
   | 
[Flag](https://app.codecov.io/gh/apache/cloudstack/pull/12880/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
 | Coverage Δ | |
   |---|---|---|
   | 
[uitests](https://app.codecov.io/gh/apache/cloudstack/pull/12880/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
 | `3.53% <ø> (ø)` | |
   | 
[unittests](https://app.codecov.io/gh/apache/cloudstack/pull/12880/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
 | `?` | |
   
   Flags with carried forward coverage won't be shown. [Click 
here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment)
 to find out more.
   
   
   [:umbrella: View full report in Codecov by 
Sentry](https://app.codecov.io/gh/apache/cloudstack/pull/12880?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   
   :loudspeaker: Have feedback on the report? [Share it 
here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:rocket: New features to boost your workflow: 
   
   - :snowflake: [Test 
Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, 
report on failures, and find test suite problems.
   - :package: [JS Bundle 
Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save 
yourself from yourself by tracking and limiting bundle sizes in JS merges.
   


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

Re: [PR] Powerflex fix [cloudstack]

2026-03-24 Thread via GitHub 


sureshanaparti commented on code in PR #12880:
URL: https://github.com/apache/cloudstack/pull/12880#discussion_r2980998558


##
plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/client/ScaleIOGatewayClientImpl.java:
##
@@ -623,7 +623,7 @@ public boolean revertSnapshot(final String 
sourceSnapshotVolumeId, final String
 
 Boolean overwriteVolumeContentStatus = post(
 "/instances/Volume::" + destVolumeId + 
"/action/overwriteVolumeContent",
-
String.format("{\"srcVolumeId\":\"%s\",\"allowOnExtManagedVol\":\"TRUE\"}", 
sourceSnapshotVolumeId), Boolean.class);
+
String.format("{\"srcVolumeId\":\"%s\"}",sourceSnapshotVolumeId),Boolean.class);

Review Comment:
   this will break the func with 3.x (and < 4.5) version 



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

Re: [PR] Powerflex fix [cloudstack]

2026-03-24 Thread via GitHub 


sureshanaparti commented on code in PR #12880:
URL: https://github.com/apache/cloudstack/pull/12880#discussion_r2981001836


##
engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/ScaleIOVMSnapshotStrategy.java:
##
@@ -201,11 +202,44 @@ public VMSnapshot takeVMSnapshot(VMSnapshot vmSnapshot) {
 List vmSnapshotDetails = new 
ArrayList();
 vmSnapshotDetails.add(new 
VMSnapshotDetailsVO(vmSnapshot.getId(), "SnapshotGroupId", snapshotGroupId, 
false));
 
-for (int index = 0; index < volumeIds.size(); index++) {
+   /* 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));
 }
+   */
+ // Invert the srcVolumeDestSnapshotMap: snapshotName -> 
srcVolumePath

Review Comment:
   ```suggestion
   ```



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

Re: [PR] Powerflex fix [cloudstack]

2026-03-24 Thread via GitHub 


sureshanaparti commented on code in PR #12880:
URL: https://github.com/apache/cloudstack/pull/12880#discussion_r2981000459


##
engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/ScaleIOVMSnapshotStrategy.java:
##
@@ -201,11 +202,44 @@ public VMSnapshot takeVMSnapshot(VMSnapshot vmSnapshot) {
 List vmSnapshotDetails = new 
ArrayList();
 vmSnapshotDetails.add(new 
VMSnapshotDetailsVO(vmSnapshot.getId(), "SnapshotGroupId", snapshotGroupId, 
false));
 
-for (int index = 0; index < volumeIds.size(); index++) {
+   /* 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));
 }
+   */

Review Comment:
   remove the comment is not needed



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

Re: [PR] Powerflex fix [cloudstack]

2026-03-24 Thread via GitHub 


sureshanaparti commented on code in PR #12880:
URL: https://github.com/apache/cloudstack/pull/12880#discussion_r2980998558


##
plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/client/ScaleIOGatewayClientImpl.java:
##
@@ -623,7 +623,7 @@ public boolean revertSnapshot(final String 
sourceSnapshotVolumeId, final String
 
 Boolean overwriteVolumeContentStatus = post(
 "/instances/Volume::" + destVolumeId + 
"/action/overwriteVolumeContent",
-
String.format("{\"srcVolumeId\":\"%s\",\"allowOnExtManagedVol\":\"TRUE\"}", 
sourceSnapshotVolumeId), Boolean.class);
+
String.format("{\"srcVolumeId\":\"%s\"}",sourceSnapshotVolumeId),Boolean.class);

Review Comment:
   this will break the func with 3.x version



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

Re: [PR] Powerflex fix [cloudstack]

2026-03-24 Thread via GitHub 


boring-cyborg[bot] commented on PR #12880:
URL: https://github.com/apache/cloudstack/pull/12880#issuecomment-4117083642

   Congratulations on your first Pull Request and welcome to the Apache 
CloudStack community! If you have any issues or are unsure about any anything 
please check our Contribution Guide 
(https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md)
   Here are some useful points:
   - In case of a new feature add useful documentation (raise doc PR at 
https://github.com/apache/cloudstack-documentation)
   - Be patient and persistent. It might take some time to get a review or get 
the final approval from the committers.
   - Pay attention to the quality of your code, ensure tests are passing and 
your PR doesn't have conflicts.
   - Please follow [ASF Code of 
Conduct](https://github.com/apache/.github/blob/main/.github/CODE_OF_CONDUCT.md)
 for all communication including (but not limited to) comments on Pull 
Requests, Issues, Mailing list and Slack.
   - Be sure to read the [CloudStack Coding 
Conventions](https://cwiki.apache.org/confluence/display/CLOUDSTACK/Coding+conventions).
   Apache CloudStack is a community-driven project and together we are making 
it better 🚀.
   In case of doubts contact the developers at:
   Mailing List: [email protected] 
(https://cloudstack.apache.org/mailing-lists.html)
   Slack: https://apachecloudstack.slack.com/
   


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