DaanHoogland commented on code in PR #8271:
URL: https://github.com/apache/cloudstack/pull/8271#discussion_r1406467754


##########
plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/util/LinstorConfigurationManager.java:
##########
@@ -21,8 +21,8 @@
 
 public class LinstorConfigurationManager implements Configurable
 {
-    public static final ConfigKey<Boolean> BackupSnapshots = new 
ConfigKey<>(Boolean.class, "lin.backup.snapshots", "Advanced", "false",
-        "Backup Linstor primary storage snapshots to secondary storage 
(deleting ps snapshot), only works on hyperconverged setups.", true, 
ConfigKey.Scope.Global, null);
+    public static final ConfigKey<Boolean> BackupSnapshots = new 
ConfigKey<>(Boolean.class, "lin.backup.snapshots", "Advanced", "true",

Review Comment:
   I'm a bit wairy of changing the default values for settings. Can you justify 
this?



##########
plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/driver/LinstorPrimaryDataStoreDriverImpl.java:
##########
@@ -956,13 +993,30 @@ protected Answer copySnapshot(DataObject srcData, 
DataObject destData) {
                 VirtualMachineManager.ExecuteInSequence.value());
             cmd.setOptions(options);
 
-            Optional<RemoteHostEndPoint> optEP = getDiskfullEP(
-                api, LinstorUtil.RSC_PREFIX + 
snapshotInfo.getBaseVolume().getUuid());
+            String rscName = LinstorUtil.RSC_PREFIX + 
snapshotInfo.getBaseVolume().getUuid();
+            Optional<RemoteHostEndPoint> optEP = getDiskfullEP(api, rscName);
             Answer answer;
             if (optEP.isPresent()) {
                 answer = optEP.get().sendMessage(cmd);
             } else {
-                answer = new Answer(cmd, false, "Unable to get matching 
Linstor endpoint.");
+                s_logger.debug("No diskfull endpoint found to copy image, 
creating diskless endpoint");
+                // create a temporary resource from the snapshot to backup, so 
we can copy the data on a diskless agent

Review Comment:
   sounds like this comment could be the title of a new comment and the code 
below factored out.



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