You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2022/04/22 20:15:32 UTC

[GitHub] [cloudstack] GutoVeronezi commented on a diff in pull request #6005: Support to resize uploaded volume during attach

GutoVeronezi commented on code in PR #6005:
URL: https://github.com/apache/cloudstack/pull/6005#discussion_r856540990


##########
server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java:
##########
@@ -337,6 +338,20 @@ protected VolumeApiServiceImpl() {
         _gson = GsonHelper.getGsonLogger();
     }
 
+    private void validateDiskSize(DiskOfferingVO diskOffering, Long sizeInGB) {
+        if (diskOffering != null && diskOffering.isCustomized()) {

Review Comment:
   We can invert the if and add a return statement to reduce code indentation.



##########
server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java:
##########
@@ -2269,8 +2316,19 @@ public Volume attachVolumeToVM(Long vmId, Long volumeId, Long deviceId) {
                     vol = _volsDao.findById((Long)jobResult);
                 }
             }
-            return vol;
         }
+
+        Long hostId = null;
+        if (vm.getHostId() != null) {
+            hostId = vm.getHostId();
+        } else if (vm.getLastHostId() != null) {
+            hostId = vm.getLastHostId();
+        }
+
+        if (vol != null && storeSize != null && hostId != null) {

Review Comment:
   We can use `ObjectUtils#allNotNull` here.



##########
server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java:
##########
@@ -364,7 +383,31 @@ public VolumeVO uploadVolume(UploadVolumeCmd cmd) throws ResourceAllocationExcep
         RegisterVolumePayload payload = new RegisterVolumePayload(cmd.getUrl(), cmd.getChecksum(), cmd.getFormat());
         vol.addPayload(payload);
 
-        volService.registerVolume(vol, store);
+        return registerVolume(vol, store, sizeInGB);
+    }
+
+    VolumeVO registerVolume(VolumeInfo vol, DataStore store, Long sizeInGB ) {
+        VolumeVO volume = _volsDao.findById(vol.getId());
+        try {
+            AsyncCallFuture<VolumeApiResult> future = volService.registerVolume(vol, store);
+            VolumeApiResult result = future.get();
+            if (result.isFailed()) {
+                s_logger.warn("Failed to register the volume");
+                String details = "";
+                if (result.getResult() != null && !result.getResult().isEmpty()) {
+                    details = result.getResult();
+                }
+                throw new CloudRuntimeException(details);
+            }
+            if (!Objects.equals(sizeInGB, vol.getSize())) {
+                if (sizeInGB !=null) {
+                    volume.setSize(sizeInGB * GiB_TO_BYTES);
+                    _volsDao.persist(volume);
+                }
+            }
+        } catch (Exception e) {
+            throw new CloudRuntimeException(String.format("Failed to register volume due to - %s", e.getMessage()), e);

Review Comment:
   We can add some info about the volume here.



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