You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by ed...@apache.org on 2013/07/10 23:57:22 UTC
[2/2] git commit: updated refs/heads/master to 7ea8c5f
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/7ea8c5fd
Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/7ea8c5fd
Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/7ea8c5fd
Branch: refs/heads/master
Commit: 7ea8c5fd9a19b9f34158e2ab8db840967db6663d
Parents: 7089e1c
Author: Vijayendra Bhamidipati <vi...@citrix.com>
Authored: Wed Jun 19 13:01:18 2013 -0700
Committer: Edison Su <su...@gmail.com>
Committed: Wed Jul 10 14:55:57 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/7ea8c5fd/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/7ea8c5fd/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/7ea8c5fd/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/7ea8c5fd/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 7378708..830fd36 100755
--- a/server/src/com/cloud/storage/StorageManagerImpl.java
+++ b/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);
}