CLOUDSTACK-9539: Refactor based on reviews

Author: nvazquez

api/src/com/cloud/vm/snapshot/VMSnapshot.java

@@ -102,4 +102,6 @@ enum Event {
 
     @Override
     public long getAccountId();
+
+    public long getServiceOfferingId();
 }

core/src/com/cloud/agent/api/VMSnapshotTO.java

@@ -35,7 +35,6 @@ public class VMSnapshotTO {
     private VMSnapshotTO parent;
     private List<VolumeObjectTO> volumes;
     private boolean quiescevm;
-    private Long serviceOfferingId;
 
     public Long getId() {
         return id;
@@ -45,7 +44,7 @@ public void setId(Long id) {
         this.id = id;
     }
 
-    public VMSnapshotTO(Long id, String snapshotName, VMSnapshot.Type type, Long createTime, String description, Boolean current, VMSnapshotTO parent, boolean quiescevm, Long serviceOfferingId) {
+    public VMSnapshotTO(Long id, String snapshotName, VMSnapshot.Type type, Long createTime, String description, Boolean current, VMSnapshotTO parent, boolean quiescevm) {
         super();
         this.id = id;
         this.snapshotName = snapshotName;
@@ -55,7 +54,6 @@ public VMSnapshotTO(Long id, String snapshotName, VMSnapshot.Type type, Long cre
         this.description = description;
         this.parent = parent;
         this.quiescevm = quiescevm;
-        this.serviceOfferingId = serviceOfferingId;
     }
 
     public VMSnapshotTO() {
@@ -126,11 +124,4 @@ public void setQuiescevm(boolean quiescevm) {
         this.quiescevm = quiescevm;
     }
 
-    public Long getServiceOfferingId() {
-        return serviceOfferingId;
-    }
-
-    public void setServiceOfferingId(Long serviceOfferingId) {
-        this.serviceOfferingId = serviceOfferingId;
-    }
 }

engine/schema/src/com/cloud/vm/snapshot/VMSnapshotVO.java

@@ -253,6 +253,7 @@ public Class<?> getEntityType() {
         return VMSnapshot.class;
     }
 
+    @Override
     public long getServiceOfferingId() {
         return serviceOfferingId;
     }

engine/storage/snapshot/src/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java

@@ -128,7 +128,7 @@ public VMSnapshot takeVMSnapshot(VMSnapshot vmSnapshot) {
             if (options != null)
                 quiescevm = options.needQuiesceVM();
             VMSnapshotTO target =
-                new VMSnapshotTO(vmSnapshot.getId(), vmSnapshot.getName(), vmSnapshot.getType(), null, vmSnapshot.getDescription(), false, current, quiescevm, userVm.getServiceOfferingId());
+                new VMSnapshotTO(vmSnapshot.getId(), vmSnapshot.getName(), vmSnapshot.getType(), null, vmSnapshot.getDescription(), false, current, quiescevm);
             if (current == null)
                 vmSnapshotVO.setParent(null);
             else
@@ -199,7 +199,7 @@ public boolean deleteVMSnapshot(VMSnapshot vmSnapshot) {
             VMSnapshotTO parent = vmSnapshotHelper.getSnapshotWithParents(vmSnapshotVO).getParent();
             VMSnapshotTO vmSnapshotTO =
                 new VMSnapshotTO(vmSnapshot.getId(), vmSnapshot.getName(), vmSnapshot.getType(), vmSnapshot.getCreated().getTime(), vmSnapshot.getDescription(),
-                    vmSnapshot.getCurrent(), parent, true, vmSnapshotVO.getServiceOfferingId());
+                    vmSnapshot.getCurrent(), parent, true);
             GuestOSVO guestOS = guestOSDao.findById(userVm.getGuestOSId());
             DeleteVMSnapshotCommand deleteSnapshotCommand = new DeleteVMSnapshotCommand(vmInstanceName, vmSnapshotTO, volumeTOs, guestOS.getDisplayName());
 
@@ -347,7 +347,7 @@ public boolean revertVMSnapshot(VMSnapshot vmSnapshot) {
 
             VMSnapshotTO vmSnapshotTO =
                 new VMSnapshotTO(snapshot.getId(), snapshot.getName(), snapshot.getType(), snapshot.getCreated().getTime(), snapshot.getDescription(),
-                    snapshot.getCurrent(), parent, true, snapshot.getServiceOfferingId());
+                    snapshot.getCurrent(), parent, true);
             Long hostId = vmSnapshotHelper.pickRunningHost(vmSnapshot.getVmId());
             GuestOSVO guestOS = guestOSDao.findById(userVm.getGuestOSId());
             RevertToVMSnapshotCommand revertToSnapshotCommand = new RevertToVMSnapshotCommand(vmInstanceName, userVm.getUuid(), vmSnapshotTO, volumeTOs, guestOS.getDisplayName());

engine/storage/src/org/apache/cloudstack/storage/helper/HypervisorHelperImpl.java

@@ -120,7 +120,7 @@ public VMSnapshotTO quiesceVm(VirtualMachine virtualMachine) {
         int wait = NumbersUtil.parseInt(value, 1800);
         Long hostId = vmSnapshotHelper.pickRunningHost(virtualMachine.getId());
         VMSnapshotTO vmSnapshotTO = new VMSnapshotTO(1L,  UUID.randomUUID().toString(), VMSnapshot.Type.Disk, null, null, false,
-                null, true, virtualMachine.getServiceOfferingId());
+                null, true);
         GuestOSVO guestOS = guestOSDao.findById(virtualMachine.getGuestOSId());
         List<VolumeObjectTO> volumeTOs = vmSnapshotHelper.getVolumeTOList(virtualMachine.getId());
         CreateVMSnapshotCommand ccmd =

engine/storage/src/org/apache/cloudstack/storage/helper/VMSnapshotHelperImpl.java

@@ -122,7 +122,7 @@ public List<VolumeObjectTO> getVolumeTOList(Long vmId) {
     }
 
     private VMSnapshotTO convert2VMSnapshotTO(VMSnapshotVO vo) {
-        return new VMSnapshotTO(vo.getId(), vo.getName(), vo.getType(), vo.getCreated().getTime(), vo.getDescription(), vo.getCurrent(), null, true, vo.getServiceOfferingId());
+        return new VMSnapshotTO(vo.getId(), vo.getName(), vo.getType(), vo.getCreated().getTime(), vo.getDescription(), vo.getCurrent(), null, true);
     }
 
     @Override

server/src/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java

@@ -615,11 +615,7 @@ public UserVm revertToSnapshot(Long vmSnapshotId) throws InsufficientCapacityExc
             VmWorkJobVO placeHolder = null;
             placeHolder = createPlaceHolderWork(vmSnapshotVo.getVmId());
             try {
-                UserVm revertedVM = orchestrateRevertToVMSnapshot(vmSnapshotId);
-
-                updateUserVmServiceOffering(revertedVM, vmSnapshotVo);
-
-                return revertedVM;
+                return orchestrateRevertToVMSnapshot(vmSnapshotId);
             } finally {
                 _workJobDao.expunge(placeHolder.getId());
             }
@@ -648,8 +644,6 @@ else if (jobResult instanceof Throwable)
                     throw new RuntimeException("Unexpected exception", (Throwable)jobResult);
             }
 
-            updateUserVmServiceOffering(userVm, vmSnapshotVo);
-
             return userVm;
         }
     }
@@ -766,6 +760,7 @@ private UserVm orchestrateRevertToVMSnapshot(Long vmSnapshotId) throws Insuffici
         try {
             VMSnapshotStrategy strategy = findVMSnapshotStrategy(vmSnapshotVo);
             strategy.revertVMSnapshot(vmSnapshotVo);
+            updateUserVmServiceOffering(userVm, vmSnapshotVo);
             return userVm;
         } catch (Exception e) {
             s_logger.debug("Failed to revert vmsnapshot: " + vmSnapshotId, e);