Fix primary storage count when deleting volumes (#2629)

* Primary Storage count for an account does not decrease when a Data Disk is deleted

When a data disk is created and not attached in a running VM, the "deleteVolume" will not decrement the count for used primary storage in the VMs accounting information. The property that is not being decremented is called "primarystoragetotal"; this information can be retrieved via "listAccounts" API method.

Steps to reproduce this issue:
1 - Create an account, deploy a VM in it
2 - Check the primary storage count for the account with listAccounts API
3 - Create a data disk
4 - Check the primary storage count for the account with listAccounts API
5 - Delete the Data disk
6 - Check the primary storage count for the account with listAccounts API - It is the same as before deleting the data disk (it should not be the same as the value in step 2!)

* formatting and cleanups

* fix imports that were wrongly changed during rebase

Author: rafaelweingartner

api/src/com/cloud/storage/VolumeApiService.java

@@ -18,6 +18,8 @@
  */
 package com.cloud.storage;
 
+import java.net.MalformedURLException;
+
 import org.apache.cloudstack.api.command.user.volume.AttachVolumeCmd;
 import org.apache.cloudstack.api.command.user.volume.CreateVolumeCmd;
 import org.apache.cloudstack.api.command.user.volume.DetachVolumeCmd;
@@ -26,13 +28,10 @@
 import org.apache.cloudstack.api.command.user.volume.MigrateVolumeCmd;
 import org.apache.cloudstack.api.command.user.volume.ResizeVolumeCmd;
 import org.apache.cloudstack.api.command.user.volume.UploadVolumeCmd;
+import org.apache.cloudstack.api.response.GetUploadParamsResponse;
 
-import com.cloud.exception.ConcurrentOperationException;
 import com.cloud.exception.ResourceAllocationException;
 import com.cloud.user.Account;
-import org.apache.cloudstack.api.response.GetUploadParamsResponse;
-
-import java.net.MalformedURLException;
 
 public interface VolumeApiService {
     /**
@@ -72,17 +71,18 @@ public interface VolumeApiService {
      *
      * @return Volume object
      */
-    Volume uploadVolume(UploadVolumeCmd cmd)    throws ResourceAllocationException;
+    Volume uploadVolume(UploadVolumeCmd cmd) throws ResourceAllocationException;
 
     GetUploadParamsResponse uploadVolume(GetUploadParamsForVolumeCmd cmd) throws ResourceAllocationException, MalformedURLException;
 
-    boolean deleteVolume(long volumeId, Account caller) throws ConcurrentOperationException;
+    boolean deleteVolume(long volumeId, Account caller);
 
     Volume attachVolumeToVM(AttachVolumeCmd command);
 
     Volume detachVolumeFromVM(DetachVolumeCmd cmd);
 
-    Snapshot takeSnapshot(Long volumeId, Long policyId, Long snapshotId, Account account, boolean quiescevm, Snapshot.LocationType locationType, boolean asyncBackup) throws ResourceAllocationException;
+    Snapshot takeSnapshot(Long volumeId, Long policyId, Long snapshotId, Account account, boolean quiescevm, Snapshot.LocationType locationType, boolean asyncBackup)
+            throws ResourceAllocationException;
 
     Snapshot allocSnapshot(Long volumeId, Long policyId, String snapshotName, Snapshot.LocationType locationType) throws ResourceAllocationException;
 
@@ -92,10 +92,8 @@ public interface VolumeApiService {
      * Extracts the volume to a particular location.
      *
      * @param cmd
-     *            the command specifying url (where the volume needs to be extracted to), zoneId (zone where the volume
-     *            exists),
+     *            the command specifying url (where the volume needs to be extracted to), zoneId (zone where the volume exists),
      *            id (the id of the volume)
-     *
      */
     String extractVolume(ExtractVolumeCmd cmd);
 

engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/VolumeInfo.java

@@ -25,6 +25,7 @@
 import com.cloud.vm.VirtualMachine;
 
 public interface VolumeInfo extends DataObject, Volume {
+
     boolean isAttachedVM();
 
     void addPayload(Object data);
@@ -36,6 +37,7 @@ public interface VolumeInfo extends DataObject, Volume {
     Long getLastPoolId();
 
     String getAttachedVmName();
+
     VirtualMachine getAttachedVM();
 
     void processEventOnly(ObjectInDataStoreStateMachine.Event event);

engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/VolumeService.java

@@ -20,16 +20,15 @@
 
 import java.util.Map;
 
-import com.cloud.utils.Pair;
 import org.apache.cloudstack.engine.cloud.entity.api.VolumeEntity;
 import org.apache.cloudstack.framework.async.AsyncCallFuture;
 import org.apache.cloudstack.storage.command.CommandResult;
 
 import com.cloud.agent.api.to.VirtualMachineTO;
-import com.cloud.exception.ConcurrentOperationException;
 import com.cloud.host.Host;
 import com.cloud.hypervisor.Hypervisor.HypervisorType;
 import com.cloud.offering.DiskOffering;
+import com.cloud.utils.Pair;
 
 public interface VolumeService {
     class VolumeApiResult extends CommandResult {
@@ -54,50 +53,36 @@ public VolumeInfo getVolume() {
     /**
      * Creates the volume based on the given criteria
      *
-     * @param cmd
-     *
      * @return the volume object
      */
     AsyncCallFuture<VolumeApiResult> createVolumeAsync(VolumeInfo volume, DataStore store);
 
     /**
      * Delete volume
-     *
-     * @param volumeId
-     * @return
-     * @throws ConcurrentOperationException
      */
     AsyncCallFuture<VolumeApiResult> expungeVolumeAsync(VolumeInfo volume);
 
-    /**
-     *
-     */
     boolean cloneVolume(long volumeId, long baseVolId);
 
-    /**
-     *
-     */
     AsyncCallFuture<VolumeApiResult> createVolumeFromSnapshot(VolumeInfo volume, DataStore store, SnapshotInfo snapshot);
 
     VolumeEntity getVolumeEntity(long volumeId);
 
-    AsyncCallFuture<VolumeApiResult> createManagedStorageVolumeFromTemplateAsync(VolumeInfo volumeInfo, long destDataStoreId,
-            TemplateInfo srcTemplateInfo, long destHostId);
+    AsyncCallFuture<VolumeApiResult> createManagedStorageVolumeFromTemplateAsync(VolumeInfo volumeInfo, long destDataStoreId, TemplateInfo srcTemplateInfo, long destHostId);
 
-    AsyncCallFuture<VolumeApiResult> createVolumeFromTemplateAsync(VolumeInfo volume, long dataStoreId,
-            TemplateInfo template);
+    AsyncCallFuture<VolumeApiResult> createVolumeFromTemplateAsync(VolumeInfo volume, long dataStoreId, TemplateInfo template);
 
     AsyncCallFuture<VolumeApiResult> copyVolume(VolumeInfo srcVolume, DataStore destStore);
 
     AsyncCallFuture<VolumeApiResult> migrateVolume(VolumeInfo srcVolume, DataStore destStore);
 
     AsyncCallFuture<CommandResult> migrateVolumes(Map<VolumeInfo, DataStore> volumeMap, VirtualMachineTO vmTo, Host srcHost, Host destHost);
 
-    boolean destroyVolume(long volumeId) throws ConcurrentOperationException;
+    void destroyVolume(long volumeId);
 
     AsyncCallFuture<VolumeApiResult> registerVolume(VolumeInfo volume, DataStore store);
 
-    public Pair<EndPoint,DataObject> registerVolumeForPostUpload(VolumeInfo volume, DataStore store);
+    public Pair<EndPoint, DataObject> registerVolumeForPostUpload(VolumeInfo volume, DataStore store);
 
     AsyncCallFuture<VolumeApiResult> resize(VolumeInfo volume);
 
@@ -108,5 +93,4 @@ AsyncCallFuture<VolumeApiResult> createVolumeFromTemplateAsync(VolumeInfo volume
     SnapshotInfo takeSnapshot(VolumeInfo volume);
 
     VolumeInfo updateHypervisorSnapshotReserveForVolume(DiskOffering diskOffering, long volumeId, HypervisorType hyperType);
-
-}
+}