CLOUDSTACK-1510: NPE when primary storage is added with wrong path

Description:

	a) Fixing NPE when wrong path is provided for primary datastore.
	b) No error dialog shows up in GUI when wrong path is provided,
	   after NPE fix - propagating exception upward.
	c) If the KVM agent is down, an invalid datastore gets logged in
	   storage_pool table and doesn't get removed, so it shows up
	   in the GUI in the list of datastores - fixing this as well.

api/src/org/apache/cloudstack/api/command/admin/storage/CreateStoragePoolCmd.java

@@ -182,6 +182,8 @@ public class CreateStoragePoolCmd extends BaseCmd {
         } catch (UnknownHostException ex3) {
             s_logger.warn("Exception: ", ex3);
             throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, ex3.getMessage());
+        } catch (Exception ex4) {
+            throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, ex4.getMessage());
         }
     }
 }

plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java

@@ -128,7 +128,7 @@ public class LibvirtStorageAdaptor implements StorageAdaptor {
     }
 
     private StoragePool createNfsStoragePool(Connect conn, String uuid,
-            String host, String path) {
+            String host, String path) throws LibvirtException {
         String targetPath = _mountPoint + File.separator + uuid;
         LibvirtStoragePoolDef spd = new LibvirtStoragePoolDef(poolType.NETFS,
                 uuid, uuid, host, path, targetPath);
@@ -156,6 +156,9 @@ public class LibvirtStorageAdaptor implements StorageAdaptor {
                 } else {
                     s_logger.error("Failed in unmounting and redefining storage");
                 }
+            } else {
+                s_logger.error("Internal error occurred when attempting to mount: specified path may be invalid");
+                throw e;
             }
             if (sp != null) {
                 try {
@@ -496,7 +499,13 @@ public class LibvirtStorageAdaptor implements StorageAdaptor {
             s_logger.debug("Attempting to create storage pool " + name);
 
             if (type == StoragePoolType.NetworkFilesystem) {
-                sp = createNfsStoragePool(conn, name, host, path);
+                try {
+                        sp = createNfsStoragePool(conn, name, host, path);
+                } catch (LibvirtException e) {
+                        s_logger.error("Failed to create mount");
+                        s_logger.error(e.getStackTrace());
+                        throw new CloudRuntimeException(e.toString());
+                }
             } else if (type == StoragePoolType.SharedMountPoint
                     || type == StoragePoolType.Filesystem) {
                 sp = createSharedStoragePool(conn, name, host, path);

plugins/storage/volume/default/src/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java

@@ -60,6 +60,7 @@ import com.cloud.storage.StoragePool;
 import com.cloud.storage.StoragePoolAutomation;
 import com.cloud.storage.StoragePoolDiscoverer;
 import com.cloud.storage.StoragePoolHostVO;
+import com.cloud.storage.StoragePoolStatus;
 import com.cloud.storage.dao.StoragePoolHostDao;
 import com.cloud.storage.dao.StoragePoolWorkDao;
 import com.cloud.storage.dao.VolumeDao;
@@ -396,7 +397,7 @@ public class CloudStackPrimaryDataStoreLifeCycleImpl implements PrimaryDataStore
             s_logger.warn("No host can access storage pool " + primarystore + " on cluster "
                     + primarystore.getClusterId());
             primaryDataStoreDao.expunge(primarystore.getId());
-            return false;
+            throw new CloudRuntimeException("Failed to access storage pool");
         }
 
         this.dataStoreHelper.attachCluster(store);
@@ -437,6 +438,10 @@ public class CloudStackPrimaryDataStoreLifeCycleImpl implements PrimaryDataStore
         List<StoragePoolHostVO> hostPoolRecords = this._storagePoolHostDao.listByPoolId(store.getId());
         StoragePool pool = (StoragePool) store;
         boolean deleteFlag = false;
+        // If datastore is not in ready state, simply delete its db entry.
+        if (pool.getStatus() != StoragePoolStatus.Up) {
+            return this.dataStoreHelper.deletePrimaryDataStore(store);
+        }
         // Remove the SR associated with the Xenserver
         for (StoragePoolHostVO host : hostPoolRecords) {
             DeleteStoragePoolCommand deleteCmd = new DeleteStoragePoolCommand(pool);

server/src/com/cloud/storage/StorageManagerImpl.java

@@ -717,7 +717,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C
         params.put("capacityIops", cmd.getCapacityIops());
 
         DataStoreLifeCycle lifeCycle = storeProvider.getDataStoreLifeCycle();
-        DataStore store;
+        DataStore store = null;
         try {
             store = lifeCycle.initialize(params);
             if (scopeType == ScopeType.CLUSTER) {
@@ -729,6 +729,10 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C
             }
         } catch (Exception e) {
             s_logger.debug("Failed to add data store", e);
+            // clean up the db
+            if (store != null) {
+                lifeCycle.deleteDataStore(store);
+            }
             throw new CloudRuntimeException("Failed to add data store", e);
         }