-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13877/
-----------------------------------------------------------

(Updated Aug. 28, 2013, 8:58 p.m.)


Review request for cloudstack, Chiradeep Vittal, Devdeep Singh, and edison su.


Changes
-------

Respond to reviewer's comments.


Repository: cloudstack-git


Description
-------

CIFS support for secondary storage is documented here: 
https://cwiki.apache.org/confluence/display/CLOUDSTACK/CIFS+Support

The feature was implemented by extending the NFS provider.  Its validation was 
updated so that you can pass it a URL containing the details of a CIFS share.  
The code that mounts NFS shares was extended to allow it do the same for CIFS 
shares.  Otherwise, the secondary storage code is left unchanged.

The source is in the cifs_support branch of 
https://github.com/lafferty/cloudstack

NB:  not able to submit using git format-patch due to issue described here: 
http://stackoverflow.com/questions/14594051/upload-from-git-format-patch-to-reviewboard-fails-on-file-not-found-in-the-repo


Diffs
-----

  
engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java
 d5e8a84516f4f37f23610a3d0315aa0a84991d46 
  
engine/storage/src/org/apache/cloudstack/storage/datastore/protocol/DataStoreProtocol.java
 b27c96edbb72751c72c3963d6945eaeaeb42c74b 
  
plugins/storage/image/default/src/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackImageStoreLifeCycleImpl.java
 21a5e0a7194449d8b72f4537e14586c2b65b0bb5 
  
plugins/storage/volume/default/src/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java
 b9b74243eddd2008227f8ea2b88832e6e276811d 
  server/src/com/cloud/resource/ResourceManagerImpl.java 
44ee8e3bd51be38acd6a90fd2fdd2383dad0fd15 
  server/src/com/cloud/storage/StorageManagerImpl.java 
b5d6ff953ef5d099fbe4dc56866e5948a46f1c8a 
  server/src/com/cloud/test/DatabaseConfig.java 
63f77b66520d5f27736d62a67eeb16af808c7dd5 
  services/secondary-storage/conf/agent.properties 
aab82b6337474b52ae26c472b321f2c93ac57e81 
  services/secondary-storage/conf/log4j.xml PRE-CREATION 
  services/secondary-storage/pom.xml ea4bfcaf477af6be6a01113ac99ef262ae789071 
  
services/secondary-storage/src/org/apache/cloudstack/storage/resource/LocalNfsSecondaryStorageResource.java
 c55c2361bd5551eb51ca1b14340ab4c2e1563865 
  
services/secondary-storage/src/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java
 8b9de39b70981b7fb4458b5a337b1b577c45aa3b 
  
services/secondary-storage/src/org/apache/cloudstack/storage/resource/SecondaryStorageDiscoverer.java
 5d6d61f740ac69acc228b0712633190b654b8f27 
  
services/secondary-storage/src/org/apache/cloudstack/storage/template/DownloadManagerImpl.java
 bf68b299c5c233f420ed1f78667f0f9522cf64de 
  
services/secondary-storage/test/org/apache/cloudstack/storage/resource/LocalNfsSecondaryStorageResourceTest.java
 0c355ec884d50c530d2c77bec85e14550cc57352 
  
services/secondary-storage/test/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResourceTest.java
 PRE-CREATION 
  utils/src/com/cloud/utils/UriUtils.java 
1ff4d729cbf50154688bcea79bb8f48d1a972674 

Diff: https://reviews.apache.org/r/13877/diff/


Testing
-------

Testing involved running the CIFS-specific and existing NFS tests for the 
secondary-storage service.  E.g. 

cd ./services/secondary-storage
mvn clean install -P developer,systemvm, -DskipTests=false

Notice that the pom.xml was updated to allow tests to be turned on using a 
conditional.  I ran the tests on the Citrix network, which allowed the existing 
NFS tests to run without modification.

In addition, an integration test was done exercise the code in the SystemVM 
environment and to exercise updated validation code.  The NFS secondary storage 
integration test was done using a XenServer deployment in which a template was 
registered and a VM created from the template.  The CIFS secondary storage test 
was done on a Hyper-V hypervisor a template was registered.


Thanks,

Donal Lafferty

Reply via email to