cloudstack-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From edi...@apache.org
Subject [2/2] git commit: updated refs/heads/4.2 to 2a8e113
Date Wed, 10 Jul 2013 21:59:01 GMT
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.


Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo
Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/2a8e113a
Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/2a8e113a
Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/2a8e113a

Branch: refs/heads/4.2
Commit: 2a8e113aa1b0bb0d7e95069d8300c894dca9804a
Parents: 1bde0f3
Author: Vijayendra Bhamidipati <vijayendra.bhamidipati@citrix.com>
Authored: Wed Jun 19 13:01:18 2013 -0700
Committer: Edison Su <sudison@gmail.com>
Committed: Wed Jul 10 14:58:41 2013 -0700

----------------------------------------------------------------------
 .../command/admin/storage/CreateStoragePoolCmd.java    |  2 ++
 .../hypervisor/kvm/storage/LibvirtStorageAdaptor.java  | 13 +++++++++++--
 .../CloudStackPrimaryDataStoreLifeCycleImpl.java       |  7 ++++++-
 server/src/com/cloud/storage/StorageManagerImpl.java   |  6 +++++-
 4 files changed, 24 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cloudstack/blob/2a8e113a/api/src/org/apache/cloudstack/api/command/admin/storage/CreateStoragePoolCmd.java
----------------------------------------------------------------------
diff --git a/api/src/org/apache/cloudstack/api/command/admin/storage/CreateStoragePoolCmd.java
b/api/src/org/apache/cloudstack/api/command/admin/storage/CreateStoragePoolCmd.java
index f5750b9..c26d199 100644
--- a/api/src/org/apache/cloudstack/api/command/admin/storage/CreateStoragePoolCmd.java
+++ b/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());
         }
     }
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/2a8e113a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java
----------------------------------------------------------------------
diff --git a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java
b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java
index 89e22c8..db1811e 100644
--- a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java
+++ b/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);

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/2a8e113a/plugins/storage/volume/default/src/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java
----------------------------------------------------------------------
diff --git a/plugins/storage/volume/default/src/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java
b/plugins/storage/volume/default/src/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java
index 2e0ff66..26733d4 100644
--- a/plugins/storage/volume/default/src/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java
+++ b/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);

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/2a8e113a/server/src/com/cloud/storage/StorageManagerImpl.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/storage/StorageManagerImpl.java b/server/src/com/cloud/storage/StorageManagerImpl.java
index dc48d3a..b290a57 100755
--- a/server/src/com/cloud/storage/StorageManagerImpl.java
+++ b/server/src/com/cloud/storage/StorageManagerImpl.java
@@ -716,7 +716,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) {
@@ -728,6 +728,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);
         }
 


Mime
View raw message