----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/11984/#review22205 -----------------------------------------------------------
engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/DataStoreLifeCycle.java <https://reviews.apache.org/r/11984/#comment45627> If stoagemanager wants to delete datastore, then just call deletedatastore, don't need add another api. In implementation of deletedatastore method, you can add a conditional check, if the datastore's state is not in ready state, then don't need to send DeleteStoragePoolCommand to hypervisor, delete the db entry instead. plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java <https://reviews.apache.org/r/11984/#comment45628> Don't need to check exeception error message here, if adding storage pool failed, then just failed, rethrow the exception to upper layer. plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java <https://reviews.apache.org/r/11984/#comment45629> How about log the exception stack trace also? server/src/com/cloud/storage/StorageManagerImpl.java <https://reviews.apache.org/r/11984/#comment45630> Call lifecycle->deletedatastore - edison su On June 20, 2013, 2:42 a.m., Venkata Siva Vijayendra Bhamidipati wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/11984/ > ----------------------------------------------------------- > > (Updated June 20, 2013, 2:42 a.m.) > > > Review request for cloudstack, Chip Childers, edison su, and Min Chen. > > > Description > ------- > > Patch for fixes for issues detected while working on bug CLOUDSTACK-1510 > (https://issues.apache.org/jira/browse/CLOUDSTACK-1510). > > > This addresses bug CLOUDSTACK-1510. > > > Diffs > ----- > > > api/src/org/apache/cloudstack/api/command/admin/storage/CreateStoragePoolCmd.java > 74eb2b9 > > engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/DataStoreLifeCycle.java > cb46709 > > plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java > 89e22c8 > > plugins/storage/volume/default/src/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java > fb37e8f > server/src/com/cloud/storage/StorageManagerImpl.java 20b435c > > Diff: https://reviews.apache.org/r/11984/diff/ > > > Testing > ------- > > Deploy KVM cluster in cloudstack. Attempt to add a primary NFS datastore > using an invalid path. NPE is not encountered anymore. If KVM host is down or > the cloud-agent on the KVM host is down, the primary datastore (whether valid > or otherwise) is not logged to the db's storage_pool table. So invalid > datastores do not show up in the GUI when listing the primary datastores > available. Also, exception is propagated to GUI. > > > Thanks, > > Venkata Siva Vijayendra Bhamidipati > >
