[GitHub] [cloudstack] JoaoJandre commented on a diff in pull request #6522: Volume encryption feature

[GitBox <gi...@apache.org>]

JoaoJandre commented on code in PR #6522:
URL: https://github.com/apache/cloudstack/pull/6522#discussion_r938878861


##########
core/src/main/java/org/apache/cloudstack/storage/to/VolumeObjectTO.java:
##########
@@ -357,4 +366,22 @@ public String getvSphereStoragePolicyId() {
     public void setvSphereStoragePolicyId(String vSphereStoragePolicyId) {
         this.vSphereStoragePolicyId = vSphereStoragePolicyId;
     }
+
+    public String getEncryptFormat() { return encryptFormat; }
+
+    public void setEncryptFormat(String encryptFormat) { this.encryptFormat = encryptFormat; }
+
+    public byte[] getPassphrase() { return passphrase; }
+
+    public void setPassphrase(byte[] passphrase) { this.passphrase = passphrase; }

Review Comment:
   These getters and setters do not comply with cloudstack's [coding conventions](https://cwiki.apache.org/confluence/display/CLOUDSTACK/Coding+conventions). Namely, the block layout convention.



##########
engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java:
##########
@@ -623,8 +639,16 @@ public VolumeInfo createVolume(VolumeInfo volume, VirtualMachine vm, VirtualMach
             storageMgr.setDiskProfileThrottling(dskCh, null, diskOffering);
         }
 
-        if (diskOffering != null && diskOffering.isCustomized()) {
-            dskCh.setSize(size);
+        if (diskOffering != null) {
+            if (diskOffering.isCustomized()) {
+                dskCh.setSize(size);
+            }
+
+            VolumeVO vol = _volsDao.findById(volume.getId());
+            if (diskOffering.getEncrypt()) {
+                setPassphraseForVolumeEncryption(vol);
+                volume = volFactory.getVolume(volume.getId());
+            }

Review Comment:
   You could find the `vol` inside the `if`. I see no reason to get it before checking the condition.
   ```suggestion
               if (diskOffering.getEncrypt()) {
                   VolumeVO vol = _volsDao.findById(volume.getId());
                   setPassphraseForVolumeEncryption(vol);
                   volume = volFactory.getVolume(volume.getId());
               }
   ```



-- 
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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org