Github user rodrigo93 commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1410#discussion_r52655615
  
    --- Diff: 
engine/orchestration/src/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java
 ---
    @@ -1054,9 +1075,28 @@ public void 
prepareForMigration(VirtualMachineProfile vm, DeployDestination dest
             }
     
             for (VolumeVO vol : vols) {
    -            DataTO volTO = volFactory.getVolume(vol.getId()).getTO();
    -            DiskTO disk = new DiskTO(volTO, vol.getDeviceId(), 
vol.getPath(), vol.getVolumeType());
                 VolumeInfo volumeInfo = volFactory.getVolume(vol.getId());
    +            DataTO volTO = volumeInfo.getTO();
    --- End diff --
    
    Hi @ustcweizhou,
    Isn't it better to create a single method for lines from 1079 to 1099 seem 
that you use it more than once?
    I can see that you replicated these lines from 1381 to 1401 and I can see 
them in the following file 
server/src/com/cloud/storage/VolumeApiServiceImpl.java as well.
    Maybe you could transform those lines in a method in a common superclass if 
possible. This would save some replicated code.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---

Reply via email to