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


##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java:
##########
@@ -1427,7 +1427,7 @@ public KVMPhysicalDisk copyPhysicalDisk(KVMPhysicalDisk 
disk, String name, KVMSt
 
                 r.ioCtxDestroy(io);
             } catch (QemuImgException | LibvirtException e) {
-                s_logger.error("Failed to convert from " + 
srcFile.getFileName() + " to " + destFile.getFileName() + " the error was: " + 
e.getMessage());
+                s_logger.error(String.format("Failed to convert from %s to %s 
the error was: " + e.getMessage(), srcFile != null ? srcFile.getFileName() : 
null, destFile != null ? destFile.getFileName() : null));

Review Comment:
   ```suggestion
                   s_logger.error(String.format("Failed to convert from %s to 
%s the error was: %s",
                       srcFile != null ? srcFile.getFileName() : null,
                       destFile != null ? destFile.getFileName() : null,
                       e.getMessage()));
   ```



##########
server/src/main/java/com/cloud/alert/SecondaryStorageVmAlertAdapter.java:
##########
@@ -59,92 +59,100 @@ public void onSSVMAlert(Object sender, 
SecStorageVmAlertEventArgs args) {
             throw new CloudRuntimeException("Invalid alert arguments, 
secStorageVm must be set");
         }
 
+        String secStorageVmHostName = "";
+        String secStorageVmPublicIpAddress = "";
+        String secStorageVmPrivateIpAddress = "N/A";
+        Long secStorageVmPodIdToDeployIn = null;
+
+        if (secStorageVm != null) {
+            secStorageVmHostName = secStorageVm.getHostName();
+            secStorageVmPublicIpAddress = secStorageVm.getPublicIpAddress();
+            secStorageVmPrivateIpAddress = secStorageVm.getPrivateIpAddress() 
== null ? "N/A" : secStorageVm.getPrivateIpAddress();
+            secStorageVmPodIdToDeployIn = secStorageVm.getPodIdToDeployIn();
+        }

Review Comment:
   this is the exact same functionality as 
https://github.com/apache/cloudstack/pull/7140/files#diff-836e2e853d64b7a2f86c1add5aa76deea112788fe69c19a3c5eb2618d753ff2c
 lines 63-73. Can we make it a utiity method?
   
   i.e. can we use `VMInstanceVO` instead of `ConsoleProxyVO` and 
`SecondaryStorageVmVO` and make it a member method of that 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]

Reply via email to