cloudstack-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From wid...@apache.org
Subject [08/50] [abbrv] git commit: updated refs/heads/disk-cache to c3569db
Date Tue, 17 Sep 2013 14:01:29 GMT
CIFS support for secondary storage is documented here: https://cwiki.apache.org/confluence/display/CLOUDSTACK/CIFS+Support
It 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.


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

Branch: refs/heads/disk-cache
Commit: e958f22ff34e089b4214ee5be68688540db5291a
Parents: a7c91fe
Author: Donal Lafferty <donal.lafferty@citrix.com>
Authored: Wed Sep 11 14:20:33 2013 +0530
Committer: Devdeep Singh <devdeep@gmail.com>
Committed: Wed Sep 11 14:23:29 2013 +0530

----------------------------------------------------------------------
 .../motion/AncientDataMotionStrategy.java       |  12 +-
 .../datastore/protocol/DataStoreProtocol.java   |   2 +-
 .../CloudStackImageStoreLifeCycleImpl.java      |  43 ++-
 ...CloudStackPrimaryDataStoreLifeCycleImpl.java |  20 +-
 .../com/cloud/resource/ResourceManagerImpl.java |  12 +-
 .../com/cloud/storage/StorageManagerImpl.java   |  10 +-
 server/src/com/cloud/test/DatabaseConfig.java   |   2 +-
 .../secondary-storage/conf/agent.properties     |   2 +
 services/secondary-storage/conf/log4j.xml       | 102 ++++++
 services/secondary-storage/pom.xml              |  10 +-
 .../LocalNfsSecondaryStorageResource.java       |  87 ++----
 .../resource/NfsSecondaryStorageResource.java   | 308 ++++++++++++++-----
 .../resource/SecondaryStorageDiscoverer.java    |   9 +-
 .../storage/template/DownloadManagerImpl.java   |   2 +-
 .../LocalNfsSecondaryStorageResourceTest.java   |  75 ++++-
 .../NfsSecondaryStorageResourceTest.java        | 120 ++++++++
 utils/src/com/cloud/utils/UriUtils.java         |  46 ++-
 17 files changed, 666 insertions(+), 196 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cloudstack/blob/e958f22f/engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java
----------------------------------------------------------------------
diff --git a/engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java b/engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java
index c8a24c6..96d1f5a 100644
--- a/engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java
+++ b/engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java
@@ -148,12 +148,6 @@ public class
 
         if (srcData.getType() == DataObjectType.TEMPLATE) {
             TemplateInfo template = (TemplateInfo)srcData;
-            if (template.getHypervisorType() == HypervisorType.Hyperv) {
-                if (s_logger.isDebugEnabled()) {
-                    s_logger.debug("needCacheStorage false due to src TemplateInfo, which is DataObjectType.TEMPLATE of HypervisorType.Hyperv");
-                }
-                return false;
-            }
         }
 
         if (s_logger.isDebugEnabled()) {
@@ -304,11 +298,11 @@ public class
         Scope destScope = getZoneScope(destData.getDataStore().getScope());
         DataStore cacheStore = cacheMgr.getCacheStorage(destScope);
         if (cacheStore == null) {
-            // need to find a nfs image store, assuming that can't copy volume
+            // need to find a nfs or cifs image store, assuming that can't copy volume
             // directly to s3
             ImageStoreEntity imageStore = (ImageStoreEntity) this.dataStoreMgr.getImageStore(destScope.getScopeId());
-            if (!imageStore.getProtocol().equalsIgnoreCase("nfs")) {
-                s_logger.debug("can't find a nfs image store");
+            if (!imageStore.getProtocol().equalsIgnoreCase("nfs") && !imageStore.getProtocol().equalsIgnoreCase("cifs")) {
+                s_logger.debug("can't find a nfs (or cifs) image store to satisfy the need for a staging store");
                 return null;
             }
 

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/e958f22f/engine/storage/src/org/apache/cloudstack/storage/datastore/protocol/DataStoreProtocol.java
----------------------------------------------------------------------
diff --git a/engine/storage/src/org/apache/cloudstack/storage/datastore/protocol/DataStoreProtocol.java b/engine/storage/src/org/apache/cloudstack/storage/datastore/protocol/DataStoreProtocol.java
index b27c96e..ee9f0fb 100644
--- a/engine/storage/src/org/apache/cloudstack/storage/datastore/protocol/DataStoreProtocol.java
+++ b/engine/storage/src/org/apache/cloudstack/storage/datastore/protocol/DataStoreProtocol.java
@@ -17,7 +17,7 @@
 package org.apache.cloudstack.storage.datastore.protocol;
 
 public enum DataStoreProtocol {
-    NFS("nfs"), ISCSI("iscsi");
+    NFS("nfs"), CIFS("cifs"), ISCSI("iscsi");
 
     private String name;
 

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/e958f22f/plugins/storage/image/default/src/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackImageStoreLifeCycleImpl.java
----------------------------------------------------------------------
diff --git a/plugins/storage/image/default/src/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackImageStoreLifeCycleImpl.java b/plugins/storage/image/default/src/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackImageStoreLifeCycleImpl.java
index 21a5e0a..ee6f47b 100644
--- a/plugins/storage/image/default/src/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackImageStoreLifeCycleImpl.java
+++ b/plugins/storage/image/default/src/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackImageStoreLifeCycleImpl.java
@@ -16,14 +16,16 @@
 // under the License.
 package org.apache.cloudstack.storage.datastore.lifecycle;
 
-import com.cloud.agent.api.StoragePoolInfo;
-import com.cloud.exception.InvalidParameterValueException;
-import com.cloud.hypervisor.Hypervisor.HypervisorType;
-import com.cloud.resource.Discoverer;
-import com.cloud.resource.ResourceManager;
-import com.cloud.storage.DataStoreRole;
-import com.cloud.storage.ScopeType;
-import com.cloud.utils.UriUtils;
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import javax.inject.Inject;
+
+import org.apache.log4j.Logger;
+
 import org.apache.cloudstack.engine.subsystem.api.storage.ClusterScope;
 import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
 import org.apache.cloudstack.engine.subsystem.api.storage.HostScope;
@@ -33,14 +35,15 @@ import org.apache.cloudstack.storage.datastore.db.ImageStoreVO;
 import org.apache.cloudstack.storage.image.datastore.ImageStoreHelper;
 import org.apache.cloudstack.storage.image.datastore.ImageStoreProviderManager;
 import org.apache.cloudstack.storage.image.store.lifecycle.ImageStoreLifeCycle;
-import org.apache.log4j.Logger;
 
-import javax.inject.Inject;
-import java.net.URI;
-import java.net.URISyntaxException;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
+import com.cloud.agent.api.StoragePoolInfo;
+import com.cloud.exception.InvalidParameterValueException;
+import com.cloud.hypervisor.Hypervisor.HypervisorType;
+import com.cloud.resource.Discoverer;
+import com.cloud.resource.ResourceManager;
+import com.cloud.storage.DataStoreRole;
+import com.cloud.storage.ScopeType;
+import com.cloud.utils.UriUtils;
 
 public class CloudStackImageStoreLifeCycleImpl implements ImageStoreLifeCycle {
 
@@ -87,13 +90,21 @@ public class CloudStackImageStoreLifeCycleImpl implements ImageStoreLifeCycle {
         try {
             uri = new URI(UriUtils.encodeURIComponent(url));
             if (uri.getScheme() == null) {
-                throw new InvalidParameterValueException("uri.scheme is null " + url + ", add nfs:// as a prefix");
+                throw new InvalidParameterValueException("uri.scheme is null " + url + ", add nfs:// (or cifs://) as a prefix");
             } else if (uri.getScheme().equalsIgnoreCase("nfs")) {
                 if (uri.getHost() == null || uri.getHost().equalsIgnoreCase("") || uri.getPath() == null
                         || uri.getPath().equalsIgnoreCase("")) {
                     throw new InvalidParameterValueException(
                             "Your host and/or path is wrong.  Make sure it's of the format nfs://hostname/path");
                 }
+            } else if (uri.getScheme().equalsIgnoreCase("cifs")) {
+                // Don't validate against a URI encoded URI.
+                URI cifsUri = new URI(url);
+                String warnMsg = UriUtils.getCifsUriParametersProblems(cifsUri);
+                if (warnMsg != null)
+                {
+                    throw new InvalidParameterValueException(warnMsg);
+                }
             }
         } catch (URISyntaxException e) {
             throw new InvalidParameterValueException(url + " is not a valid uri");

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/e958f22f/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 b9b7424..9a70124 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
@@ -143,13 +143,21 @@ public class CloudStackPrimaryDataStoreLifeCycleImpl implements PrimaryDataStore
         try {
             uri = new URI(UriUtils.encodeURIComponent(url));
             if (uri.getScheme() == null) {
-                throw new InvalidParameterValueException("scheme is null " + url + ", add nfs:// as a prefix");
+                throw new InvalidParameterValueException("scheme is null " + url + ", add nfs:// (or cifs://) as a prefix");
             } else if (uri.getScheme().equalsIgnoreCase("nfs")) {
                 String uriHost = uri.getHost();
                 String uriPath = uri.getPath();
                 if (uriHost == null || uriPath == null || uriHost.trim().isEmpty() || uriPath.trim().isEmpty()) {
                     throw new InvalidParameterValueException("host or path is null, should be nfs://hostname/path");
                 }
+            } else if (uri.getScheme().equalsIgnoreCase("cifs")) {
+                // Don't validate against a URI encoded URI.
+                URI cifsUri = new URI(url);
+                String warnMsg = UriUtils.getCifsUriParametersProblems(cifsUri);
+                if (warnMsg != null)
+                {
+                    throw new InvalidParameterValueException(warnMsg);
+                }
             } else if (uri.getScheme().equalsIgnoreCase("sharedMountPoint")) {
                 String uriPath = uri.getPath();
                 if (uriPath == null) {
@@ -193,6 +201,16 @@ public class CloudStackPrimaryDataStoreLifeCycleImpl implements PrimaryDataStore
             parameters.setHost(storageHost);
             parameters.setPort(port);
             parameters.setPath(hostPath);
+        } else if (scheme.equalsIgnoreCase("cifs")) {
+            if (port == -1) {
+                port = 445;
+            }
+            parameters.setType(StoragePoolType.NetworkFilesystem);
+            parameters.setHost(storageHost);
+            parameters.setPort(port);
+            parameters.setPath(hostPath);
+            parameters.setUserInfo(uri.getQuery());
+
         } else if (scheme.equalsIgnoreCase("file")) {
             if (port == -1) {
                 port = 0;

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/e958f22f/server/src/com/cloud/resource/ResourceManagerImpl.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/resource/ResourceManagerImpl.java b/server/src/com/cloud/resource/ResourceManagerImpl.java
index 69cbd5b..76d7cf9 100755
--- a/server/src/com/cloud/resource/ResourceManagerImpl.java
+++ b/server/src/com/cloud/resource/ResourceManagerImpl.java
@@ -685,12 +685,20 @@ public class ResourceManagerImpl extends ManagerBase implements ResourceManager,
         try {
             uri = new URI(UriUtils.encodeURIComponent(url));
             if (uri.getScheme() == null) {
-                throw new InvalidParameterValueException("uri.scheme is null " + url + ", add nfs:// as a prefix");
+                throw new InvalidParameterValueException("uri.scheme is null " + url + ", add nfs:// (or cifs://) as a prefix");
             } else if (uri.getScheme().equalsIgnoreCase("nfs")) {
                 if (uri.getHost() == null || uri.getHost().equalsIgnoreCase("") || uri.getPath() == null || uri.getPath().equalsIgnoreCase("")) {
                     throw new InvalidParameterValueException("Your host and/or path is wrong.  Make sure it's of the format nfs://hostname/path");
                 }
-            }
+            } else if (uri.getScheme().equalsIgnoreCase("cifs")) {
+                // Don't validate against a URI encoded URI.
+                URI cifsUri = new URI(url);
+                String warnMsg = UriUtils.getCifsUriParametersProblems(cifsUri);
+                if (warnMsg != null)
+                {
+                    throw new InvalidParameterValueException(warnMsg);
+                }
+	        }
         } catch (URISyntaxException e) {
             throw new InvalidParameterValueException(url + " is not a valid uri");
         }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/e958f22f/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 b380692..8417066 100755
--- a/server/src/com/cloud/storage/StorageManagerImpl.java
+++ b/server/src/com/cloud/storage/StorageManagerImpl.java
@@ -1406,11 +1406,19 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C
         try {
             uri = new URI(UriUtils.encodeURIComponent(newUrl));
             if (uri.getScheme() == null) {
-                throw new InvalidParameterValueException("uri.scheme is null " + newUrl + ", add nfs:// as a prefix");
+                throw new InvalidParameterValueException("uri.scheme is null " + newUrl + ", add nfs:// (or cifs://) as a prefix");
             } else if (uri.getScheme().equalsIgnoreCase("nfs")) {
                 if (uri.getHost() == null || uri.getHost().equalsIgnoreCase("") || uri.getPath() == null || uri.getPath().equalsIgnoreCase("")) {
                     throw new InvalidParameterValueException("Your host and/or path is wrong.  Make sure it's of the format nfs://hostname/path");
                 }
+            } else if (uri.getScheme().equalsIgnoreCase("cifs")) {
+                // Don't validate against a URI encoded URI.
+                URI cifsUri = new URI(newUrl);
+                String warnMsg = UriUtils.getCifsUriParametersProblems(cifsUri);
+                if (warnMsg != null)
+                {
+                    throw new InvalidParameterValueException(warnMsg);
+                }
             }
         } catch (URISyntaxException e) {
             throw new InvalidParameterValueException(newUrl + " is not a valid uri");

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/e958f22f/server/src/com/cloud/test/DatabaseConfig.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/test/DatabaseConfig.java b/server/src/com/cloud/test/DatabaseConfig.java
index 63f77b6..38a1abf 100755
--- a/server/src/com/cloud/test/DatabaseConfig.java
+++ b/server/src/com/cloud/test/DatabaseConfig.java
@@ -505,7 +505,7 @@ public class DatabaseConfig {
             stmt.setLong(14, 1238425896);
 
             boolean nfs = false;
-            if (url.startsWith("nfs")) {
+            if (url.startsWith("nfs") || url.startsWith("cifs")) {
                 nfs = true;
             }
             if (nfs) {

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/e958f22f/services/secondary-storage/conf/agent.properties
----------------------------------------------------------------------
diff --git a/services/secondary-storage/conf/agent.properties b/services/secondary-storage/conf/agent.properties
index aab82b6..507ea4d 100644
--- a/services/secondary-storage/conf/agent.properties
+++ b/services/secondary-storage/conf/agent.properties
@@ -1,2 +1,4 @@
 #mount.path=~/secondary-storage/
 resource=org.apache.cloudstack.storage.resource.NfsSecondaryStorageResource
+testCifsMount=cifs://192.168.1.1/CSHV3?user=administrator&password=1pass%40word1
+#testLocalRoot=test

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/e958f22f/services/secondary-storage/conf/log4j.xml
----------------------------------------------------------------------
diff --git a/services/secondary-storage/conf/log4j.xml b/services/secondary-storage/conf/log4j.xml
new file mode 100644
index 0000000..9511f30
--- /dev/null
+++ b/services/secondary-storage/conf/log4j.xml
@@ -0,0 +1,102 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements.  See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership.  The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License.  You may obtain a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied.  See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+<!DOCTYPE log4j:configuration SYSTEM "log4j.dtd">
+
+<log4j:configuration xmlns:log4j="http://jakarta.apache.org/log4j/" debug="false">
+
+   <!-- ================================= -->
+   <!-- Preserve messages in a local file -->
+   <!-- ================================= -->
+
+   <!-- A time/date based rolling appender -->
+   <appender name="FILE" class="org.apache.log4j.DailyRollingFileAppender">
+      <param name="File" value="${ss.log.home}systemvm.log"/>
+      <param name="Append" value="true"/>
+      <param name="Threshold" value="DEBUG"/>
+
+      <!-- Rollover at midnight each day -->
+      <param name="DatePattern" value="'.'yyyy-MM-dd"/>
+
+      <layout class="org.apache.log4j.PatternLayout">
+         <param name="ConversionPattern" value="%d %-5p [%c{3}] (%t:%x) %m%n"/>
+      </layout>
+   </appender>
+   
+   <!-- ============================== -->
+   <!-- Append messages to the console -->
+   <!-- ============================== -->
+
+   <appender name="CONSOLE" class="org.apache.log4j.ConsoleAppender">
+      <param name="Target" value="System.out"/>
+      <param name="Threshold" value="DEBUG"/>
+
+      <layout class="org.apache.log4j.PatternLayout">
+         <param name="ConversionPattern" value="%d{ABSOLUTE} %5p %c{1}:%L - %m%n"/>
+      </layout>
+   </appender>
+
+   <!-- ================ -->
+   <!-- Limit categories -->
+   <!-- ================ -->
+
+   <category name="com.cloud.console.ConsoleCanvas">
+     <priority value="WARN"/>
+   </category>
+   
+   <category name="com.cloud.consoleproxy.ConsoleProxyAjaxImageHandler">
+     <priority value="WARN"/>
+   </category>
+   
+   <category name="com.cloud.consoleproxy.ConsoleProxyViewer">
+     <priority value="WARN"/>
+   </category>
+
+   <category name="com.cloud.consoleproxy">
+     <priority value="INFO"/>
+   </category>
+
+   <category name="com.cloud">
+     <priority value="DEBUG"/>
+   </category>
+   
+   <!-- Limit the org.apache category to INFO as its DEBUG is verbose -->
+   <category name="org.apache">
+      <priority value="DEBUG"/>
+   </category>
+
+   <category name="org">
+      <priority value="INFO"/>
+   </category>
+   
+   <category name="net">
+     <priority value="INFO"/>
+   </category>
+
+   <!-- ======================= -->
+   <!-- Setup the Root category -->
+   <!-- ======================= -->
+
+   <root>
+      <level value="DEBUG"/>
+      <appender-ref ref="CONSOLE"/>
+      <appender-ref ref="FILE"/>
+   </root>
+
+</log4j:configuration>

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/e958f22f/services/secondary-storage/pom.xml
----------------------------------------------------------------------
diff --git a/services/secondary-storage/pom.xml b/services/secondary-storage/pom.xml
index ea4bfca..9fe8da3 100644
--- a/services/secondary-storage/pom.xml
+++ b/services/secondary-storage/pom.xml
@@ -26,7 +26,10 @@
     <version>4.3.0-SNAPSHOT</version>
     <relativePath>../pom.xml</relativePath>
   </parent>
-  <dependencies>
+    <properties>
+    <skipTests>true</skipTests>
+  </properties>
+    <dependencies>
     <dependency>
       <groupId>log4j</groupId>
       <artifactId>log4j</artifactId>
@@ -64,7 +67,10 @@
         <artifactId>maven-surefire-plugin</artifactId>
         <version>2.14</version>
         <configuration>
-         <skipTests>true</skipTests>
+         <skipTests>${skipTests}</skipTests>
+         <systemPropertyVariables>
+            <log4j.configuration>file:${project.build.testSourceDirectory}/../conf/log4j.xml</log4j.configuration>
+        </systemPropertyVariables>
         </configuration>
       </plugin>
       <plugin>

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/e958f22f/services/secondary-storage/src/org/apache/cloudstack/storage/resource/LocalNfsSecondaryStorageResource.java
----------------------------------------------------------------------
diff --git a/services/secondary-storage/src/org/apache/cloudstack/storage/resource/LocalNfsSecondaryStorageResource.java b/services/secondary-storage/src/org/apache/cloudstack/storage/resource/LocalNfsSecondaryStorageResource.java
index c55c236..926a1e8 100644
--- a/services/secondary-storage/src/org/apache/cloudstack/storage/resource/LocalNfsSecondaryStorageResource.java
+++ b/services/secondary-storage/src/org/apache/cloudstack/storage/resource/LocalNfsSecondaryStorageResource.java
@@ -82,15 +82,8 @@ public class LocalNfsSecondaryStorageResource extends NfsSecondaryStorageResourc
     synchronized public String getRootDir(String secUrl) {
         try {
             URI uri = new URI(secUrl);
-            String nfsHost = uri.getHost();
-
-            InetAddress nfsHostAddr = InetAddress.getByName(nfsHost);
-            String nfsHostIp = nfsHostAddr.getHostAddress();
-            String nfsPath = nfsHostIp + ":" + uri.getPath();
-            String dir = UUID.nameUUIDFromBytes(nfsPath.getBytes()).toString();
-            String root = _parent + "/" + dir;
-            mount(root, nfsPath);
-            return root;
+            String dir = mountUri(uri);
+            return _parent + "/" + dir;
         } catch (Exception e) {
             String msg = "GetRootDir for " + secUrl + " failed due to " + e.toString();
             s_logger.error(msg, e);
@@ -99,74 +92,30 @@ public class LocalNfsSecondaryStorageResource extends NfsSecondaryStorageResourc
     }
 
     @Override
-    protected String mount(String root, String nfsPath) {
-        File file = new File(root);
-        if (!file.exists()) {
-            if (_storage.mkdir(root)) {
-                s_logger.debug("create mount point: " + root);
-            } else {
-                s_logger.debug("Unable to create mount point: " + root);
-                return null;
-            }
+    protected void mount(String localRootPath, String remoteDevice, URI uri) {
+        ensureLocalRootPathExists(localRootPath, uri);
+        
+        if (mountExists(localRootPath, uri)) {
+            return;
         }
 
-        Script script = null;
-        String result = null;
-        script = new Script(!_inSystemVM, "mount", _timeout, s_logger);
-        List<String> res = new ArrayList<String>();
-        ZfsPathParser parser = new ZfsPathParser(root);
-        script.execute(parser);
-        res.addAll(parser.getPaths());
-        for (String s : res) {
-            if (s.contains(root)) {
-                s_logger.debug("mount point " + root + " already exists");
-                return root;
-            }
-        }
-
-        Script command = new Script(!_inSystemVM, "mount", _timeout, s_logger);
-        command.add("-t", "nfs");
-        if ("Mac OS X".equalsIgnoreCase(System.getProperty("os.name"))) {
-            command.add("-o", "resvport");
-        }
-        if (_inSystemVM) {
-            // Fedora Core 12 errors out with any -o option executed from java
-            command.add("-o", "soft,timeo=133,retrans=2147483647,tcp,acdirmax=0,acdirmin=0");
-        }
-        command.add(nfsPath);
-        command.add(root);
-        result = command.execute();
-        if (result != null) {
-            s_logger.warn("Unable to mount " + nfsPath + " due to " + result);
-            file = new File(root);
-            if (file.exists())
-                file.delete();
-            return null;
-        }
-        s_logger.debug("Successfully mount " + nfsPath);
+        attemptMount(localRootPath, remoteDevice, uri);
 
-        // Change permissions for the mountpoint
-        script = new Script(true, "chmod", _timeout, s_logger);
-        script.add("777", root);
-        result = script.execute();
+        // Change permissions for the mountpoint - seems to bypass authentication
+        Script script = new Script(true, "chmod", _timeout, s_logger);
+        script.add("777", localRootPath);
+        String result = script.execute();
         if (result != null) {
-            s_logger.warn("Unable to set permissions for " + root + " due to " + result);
-            return null;
+            String errMsg = "Unable to set permissions for " + localRootPath + " due to " + result;
+            s_logger.error(errMsg);
+            throw new CloudRuntimeException(errMsg);
         }
-        s_logger.debug("Successfully set 777 permission for " + root);
+        s_logger.debug("Successfully set 777 permission for " + localRootPath);
 
         // XXX: Adding the check for creation of snapshots dir here. Might have
         // to move it somewhere more logical later.
-        if (!checkForSnapshotsDir(root)) {
-            return null;
-        }
-
-        // Create the volumes dir
-        if (!checkForVolumesDir(root)) {
-            return null;
-        }
-
-        return root;
+        checkForSnapshotsDir(localRootPath);
+        checkForVolumesDir(localRootPath);
     }
 
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/e958f22f/services/secondary-storage/src/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java
----------------------------------------------------------------------
diff --git a/services/secondary-storage/src/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java b/services/secondary-storage/src/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java
index 3a563cb..3ef950b 100755
--- a/services/secondary-storage/src/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java
+++ b/services/secondary-storage/src/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java
@@ -27,6 +27,7 @@ import java.io.*;
 import java.math.BigInteger;
 import java.net.InetAddress;
 import java.net.URI;
+import java.net.UnknownHostException;
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
 import java.util.ArrayList;
@@ -59,8 +60,10 @@ import org.apache.commons.codec.digest.DigestUtils;
 import org.apache.commons.lang.StringUtils;
 import org.apache.http.HttpEntity;
 import org.apache.http.HttpResponse;
+import org.apache.http.NameValuePair;
 import org.apache.http.client.HttpClient;
 import org.apache.http.client.methods.HttpGet;
+import org.apache.http.client.utils.URLEncodedUtils;
 import org.apache.http.impl.client.DefaultHttpClient;
 import org.apache.log4j.Logger;
 
@@ -156,6 +159,14 @@ public class NfsSecondaryStorageResource extends ServerResourceBase implements S
     final private String _tmpltpp = "template.properties";
     protected String createTemplateFromSnapshotXenScript;
 
+    public void setParentPath(String path) {
+        _parent = path;
+    }
+
+    public String getMountingRoot() {
+        return _parent;
+    }
+
     @Override
     public void disconnected() {
     }
@@ -1205,16 +1216,10 @@ public class NfsSecondaryStorageResource extends ServerResourceBase implements S
             String secUrl = cmd.getSecUrl();
             try {
                 URI uri = new URI(secUrl);
-                String nfsHost = uri.getHost();
-
-                InetAddress nfsHostAddr = InetAddress.getByName(nfsHost);
-                String nfsHostIp = nfsHostAddr.getHostAddress();
+                String nfsHostIp = getUriHostIp(uri);
 
                 addRouteToInternalIpOrCidr(_storageGateway, _storageIp, _storageNetmask, nfsHostIp);
-                String nfsPath = nfsHostIp + ":" + uri.getPath();
-                String dir = UUID.nameUUIDFromBytes(nfsPath.getBytes()).toString();
-                String root = _parent + "/" + dir;
-                mount(root, nfsPath);
+                String dir = mountUri(uri);
 
                 configCerts(cmd.getCerts());
 
@@ -1893,15 +1898,8 @@ public class NfsSecondaryStorageResource extends ServerResourceBase implements S
         }
         try {
             URI uri = new URI(secUrl);
-            String nfsHost = uri.getHost();
-
-            InetAddress nfsHostAddr = InetAddress.getByName(nfsHost);
-            String nfsHostIp = nfsHostAddr.getHostAddress();
-            String nfsPath = nfsHostIp + ":" + uri.getPath();
-            String dir = UUID.nameUUIDFromBytes(nfsPath.getBytes()).toString();
-            String root = _parent + "/" + dir;
-            mount(root, nfsPath);
-            return root;
+            String dir = mountUri(uri);
+            return _parent + "/" + dir;
         } catch (Exception e) {
             String msg = "GetRootDir for " + secUrl + " failed due to " + e.toString();
             s_logger.error(msg, e);
@@ -1968,6 +1966,7 @@ public class NfsSecondaryStorageResource extends ServerResourceBase implements S
 
         String inSystemVM = (String) params.get("secondary.storage.vm");
         if (inSystemVM == null || "true".equalsIgnoreCase(inSystemVM)) {
+            s_logger.debug("conf secondary.storage.vm is true, act as if executing in SSVM");
             _inSystemVM = true;
         }
 
@@ -1984,24 +1983,7 @@ public class NfsSecondaryStorageResource extends ServerResourceBase implements S
         _timeout = NumbersUtil.parseInt(value, 1440) * 1000;
 
         _storage = (StorageLayer) params.get(StorageLayer.InstanceConfigKey);
-        if (_storage == null) {
-            value = (String) params.get(StorageLayer.ClassConfigKey);
-            if (value == null) {
-                value = "com.cloud.storage.JavaStorageLayer";
-            }
-
-            try {
-                Class<?> clazz = Class.forName(value);
-                _storage = (StorageLayer) clazz.newInstance();
-                _storage.configure("StorageLayer", params);
-            } catch (ClassNotFoundException e) {
-                throw new ConfigurationException("Unable to find class " + value);
-            } catch (InstantiationException e) {
-                throw new ConfigurationException("Unable to find class " + value);
-            } catch (IllegalAccessException e) {
-                throw new ConfigurationException("Unable to find class " + value);
-            }
-        }
+        configureStorageLayerClass(params);
 
         if (_inSystemVM) {
             _storage.mkdirs(_parent);
@@ -2090,6 +2072,28 @@ public class NfsSecondaryStorageResource extends ServerResourceBase implements S
         return true;
     }
 
+    protected void configureStorageLayerClass(Map<String, Object> params) throws ConfigurationException {
+        String value;
+        if (_storage == null) {
+            value = (String) params.get(StorageLayer.ClassConfigKey);
+            if (value == null) {
+                value = "com.cloud.storage.JavaStorageLayer";
+            }
+
+            try {
+                Class<?> clazz = Class.forName(value);
+                _storage = (StorageLayer) clazz.newInstance();
+                _storage.configure("StorageLayer", params);
+            } catch (ClassNotFoundException e) {
+                throw new ConfigurationException("Unable to find class " + value);
+            } catch (InstantiationException e) {
+                throw new ConfigurationException("Unable to find class " + value);
+            } catch (IllegalAccessException e) {
+                throw new ConfigurationException("Unable to find class " + value);
+            }
+        }
+    }
+
     private void startAdditionalServices() {
         if (!_inSystemVM) {
             return;
@@ -2212,60 +2216,222 @@ public class NfsSecondaryStorageResource extends ServerResourceBase implements S
         return result;
     }
 
-    protected String mount(String root, String nfsPath) {
-        File file = new File(root);
-        if (!file.exists()) {
-            if (_storage.mkdir(root)) {
-                s_logger.debug("create mount point: " + root);
-            } else {
-                s_logger.debug("Unable to create mount point: " + root);
-                return null;
-            }
+    /**
+     * Mount remote device named on local file system on subfolder of _parent
+     * field.
+     * <p>
+     * 
+     * Supported schemes are "nfs" and "cifs".
+     * <p>
+     * 
+     * CIFS parameters are documented with mount.cifs at
+     * http://linux.die.net/man/8/mount.cifs
+	 * For simplicity, when a URI is used to specify a CIFS share,
+	 * options such as domain,user,password are passed as query parameters.
+     * 
+     * @param uri
+     *            crresponding to the remote device. Will throw for unsupported
+     *            scheme.
+     * @return name of folder in _parent that device was mounted.
+     * @throws UnknownHostException
+     */
+    protected String mountUri(URI uri) throws UnknownHostException {
+        String uriHostIp = getUriHostIp(uri);
+        String nfsPath = uriHostIp + ":" + uri.getPath();
+
+        // Single means of calculating mount directory regardless of scheme
+        String dir = UUID.nameUUIDFromBytes(nfsPath.getBytes()).toString();
+        String localRootPath = _parent + "/" + dir;
+
+        // remote device syntax varies by scheme.
+        String remoteDevice;
+        if (uri.getScheme().equals("cifs")) {
+            remoteDevice = "//" + uriHostIp + uri.getPath();
+            s_logger.debug("Mounting device with cifs-style path of "
+                    + remoteDevice);
+        }
+        else
+        {
+            remoteDevice = nfsPath;
+            s_logger.debug("Mounting device with nfs-style path of "
+                    + remoteDevice);
+        }
+
+        mount(localRootPath, remoteDevice, uri);
+
+        return dir;
+    }
+
+    
+    protected void umount(String localRootPath, URI uri) {
+        ensureLocalRootPathExists(localRootPath, uri);
+
+        if (!mountExists(localRootPath, uri)) {
+            return;
         }
 
-        Script script = null;
-        String result = null;
-        script = new Script(!_inSystemVM, "mount", _timeout, s_logger);
-        List<String> res = new ArrayList<String>();
-        ZfsPathParser parser = new ZfsPathParser(root);
-        script.execute(parser);
-        res.addAll(parser.getPaths());
-        for (String s : res) {
-            if (s.contains(root)) {
-                return root;
+        Script command = new Script(!_inSystemVM, "mount", _timeout, s_logger);
+        command.add(localRootPath);
+        String result = command.execute();
+        if (result != null) {
+            // Fedora Core 12 errors out with any -o option executed from java
+            String errMsg = "Unable to umount " + localRootPath + " due to "
+                    + result;
+            s_logger.error(errMsg);
+            File file = new File(localRootPath);
+            if (file.exists()) {
+                file.delete();
             }
+            throw new CloudRuntimeException(errMsg);
+        }
+        s_logger.debug("Successfully umounted " + localRootPath);
+    }
+    
+    protected void mount(String localRootPath, String remoteDevice, URI uri) {
+        s_logger.debug("mount " + uri.toString() + " on " + localRootPath);
+        ensureLocalRootPathExists(localRootPath, uri);
+
+        if (mountExists(localRootPath, uri)) {
+            return;
         }
 
+        attemptMount(localRootPath, remoteDevice, uri);
+
+        // XXX: Adding the check for creation of snapshots dir here. Might have
+        // to move it somewhere more logical later.
+        checkForSnapshotsDir(localRootPath);
+        checkForVolumesDir(localRootPath);
+    }
+
+    protected void attemptMount(String localRootPath, String remoteDevice, URI uri) {
+        String result;
+        s_logger.debug("Make cmdline call to mount " + remoteDevice + " at "
+                + localRootPath + " based on uri " + uri);
         Script command = new Script(!_inSystemVM, "mount", _timeout, s_logger);
-        command.add("-t", "nfs");
-        if (_inSystemVM) {
-            // Fedora Core 12 errors out with any -o option executed from java
-            command.add("-o", "soft,timeo=133,retrans=2147483647,tcp,acdirmax=0,acdirmin=0");
+
+        String scheme = uri.getScheme().toLowerCase();
+        command.add("-t", scheme);
+
+        if (scheme.equals("nfs")) {
+            if ("Mac OS X".equalsIgnoreCase(System.getProperty("os.name"))) {
+                // See http://wiki.qnap.com/wiki/Mounting_an_NFS_share_from_OS_X
+                command.add("-o", "resvport");
+            }
+            if (_inSystemVM) {
+                command.add("-o",
+                        "soft,timeo=133,retrans=2147483647,tcp,acdirmax=0,acdirmin=0");
+            }
+        } else if (scheme.equals("cifs")) {
+            String extraOpts = parseCifsMountOptions(uri);
+
+            // nfs acdirmax / acdirmin correspoonds to CIFS actimeo (see
+            // http://linux.die.net/man/8/mount.cifs)
+            // no equivalent to nfs timeo, retrans or tcp in CIFS
+            // todo: allow security mode to be set.
+            command.add("-o", extraOpts + "soft,actimeo=0");
+        } else {
+            String errMsg = "Unsupported storage device scheme " + scheme
+                    + " in uri " + uri.toString();
+            s_logger.error(errMsg);
+            throw new CloudRuntimeException(errMsg);
         }
-        command.add(nfsPath);
-        command.add(root);
+
+        command.add(remoteDevice);
+        command.add(localRootPath);
         result = command.execute();
         if (result != null) {
-            s_logger.warn("Unable to mount " + nfsPath + " due to " + result);
-            file = new File(root);
+            // Fedora Core 12 errors out with any -o option executed from java
+            String errMsg = "Unable to mount " + remoteDevice + " at "
+                    + localRootPath + " due to " + result;
+            s_logger.error(errMsg);
+            File file = new File(localRootPath);
             if (file.exists()) {
                 file.delete();
             }
-            return null;
+            throw new CloudRuntimeException(errMsg);
+        }
+        s_logger.debug("Successfully mounted " + remoteDevice + " at " + localRootPath);
+    }
+
+    protected String parseCifsMountOptions(URI uri) {
+        List<NameValuePair> args = URLEncodedUtils.parse(uri, "UTF-8");
+        boolean foundUser = false;
+        boolean foundPswd = false;
+        String extraOpts = "";
+        for (NameValuePair nvp : args) {
+            String name = nvp.getName();
+            if (name.equals("user")) {
+                foundUser = true;
+                s_logger.debug("foundUser is" + foundUser);
+            }
+            else if (name.equals("password")) {
+                foundPswd = true;
+                s_logger.debug("foundPswd is" + foundPswd);
+            }
+
+            extraOpts = extraOpts + name + "=" + nvp.getValue() + ",";
         }
 
-        // XXX: Adding the check for creation of snapshots dir here. Might have
-        // to move it somewhere more logical later.
-        if (!checkForSnapshotsDir(root)) {
-            return null;
+        if (s_logger.isDebugEnabled())
+        {
+            s_logger.error("extraOpts now " + extraOpts);
         }
 
-        // Create the volumes dir
-        if (!checkForVolumesDir(root)) {
-            return null;
+        if (!foundUser || !foundPswd) {
+            String errMsg = "Missing user and password from URI. Make sure they"
+                    + "are in the query string and separated by '&'.  E.g. "
+                    + "cifs://example.com/some_share?user=foo&password=bar";
+            s_logger.error(errMsg);
+            throw new CloudRuntimeException(errMsg);
         }
+        return extraOpts;
+    }
+
+    protected boolean mountExists(String localRootPath, URI uri) {
+        Script script = null;
+        script = new Script(!_inSystemVM, "mount", _timeout, s_logger);
+
+        List<String> res = new ArrayList<String>();
+        ZfsPathParser parser = new ZfsPathParser(localRootPath);
+        script.execute(parser);
+        res.addAll(parser.getPaths());
+        for (String s : res) {
+            if (s.contains(localRootPath)) {
+                s_logger.debug("Some device already mounted at "
+                        + localRootPath + ", no need to mount "
+                        + uri.toString());
+                return true;
+            }
+        }
+        return false;
+    }
+
+    protected void ensureLocalRootPathExists(String localRootPath, URI uri) {
+        s_logger.debug("making available " + localRootPath + " on " + uri.toString());
+        File file = new File(localRootPath);
+        s_logger.debug("local folder for mount will be " + file.getPath());
+        if (!file.exists()) {
+            s_logger.debug("create mount point: " + file.getPath());
+            _storage.mkdir(file.getPath());
+
+            // Need to check after mkdir to allow O/S to complete operation
+            if (!file.exists()) {
+                String errMsg = "Unable to create local folder for: "
+                        + localRootPath
+                        + " in order to mount " + uri.toString();
+                s_logger.error(errMsg);
+                throw new CloudRuntimeException(errMsg);
+            }
+        }
+    }
 
-        return root;
+    protected String getUriHostIp(URI uri) throws UnknownHostException {
+        String nfsHost = uri.getHost();
+        InetAddress nfsHostAddr = InetAddress.getByName(nfsHost);
+        String nfsHostIp = nfsHostAddr.getHostAddress();
+        s_logger.info("Determined host " + nfsHost + " corresponds to IP "
+                + nfsHostIp);
+        return nfsHostIp;
     }
 
     @Override

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/e958f22f/services/secondary-storage/src/org/apache/cloudstack/storage/resource/SecondaryStorageDiscoverer.java
----------------------------------------------------------------------
diff --git a/services/secondary-storage/src/org/apache/cloudstack/storage/resource/SecondaryStorageDiscoverer.java b/services/secondary-storage/src/org/apache/cloudstack/storage/resource/SecondaryStorageDiscoverer.java
index 5d6d61f..0e5221b 100755
--- a/services/secondary-storage/src/org/apache/cloudstack/storage/resource/SecondaryStorageDiscoverer.java
+++ b/services/secondary-storage/src/org/apache/cloudstack/storage/resource/SecondaryStorageDiscoverer.java
@@ -83,13 +83,16 @@ public class SecondaryStorageDiscoverer extends DiscovererBase implements Discov
 
     @Override
     public Map<? extends ServerResource, Map<String, String>> find(long dcId, Long podId, Long clusterId, URI uri, String username, String password, List<String> hostTags) {
-        if (!uri.getScheme().equalsIgnoreCase("nfs") && !uri.getScheme().equalsIgnoreCase("file")
-                && !uri.getScheme().equalsIgnoreCase("iso") && !uri.getScheme().equalsIgnoreCase("dummy")) {
+        if (!uri.getScheme().equalsIgnoreCase("nfs") && !uri.getScheme().equalsIgnoreCase("cifs")
+        		&& !uri.getScheme().equalsIgnoreCase("file") 
+        		&& !uri.getScheme().equalsIgnoreCase("iso") 
+        		&& !uri.getScheme().equalsIgnoreCase("dummy")) {
             s_logger.debug("It's not NFS or file or ISO, so not a secondary storage server: " + uri.toString());
             return null;
         }
 
-        if (uri.getScheme().equalsIgnoreCase("nfs") || uri.getScheme().equalsIgnoreCase("iso")) {
+        if (uri.getScheme().equalsIgnoreCase("nfs")	|| uri.getScheme().equalsIgnoreCase("cifs")
+        		|| uri.getScheme().equalsIgnoreCase("iso")) {
             return createNfsSecondaryStorageResource(dcId, podId, uri);
         } else if (uri.getScheme().equalsIgnoreCase("file")) {
             return createLocalSecondaryStorageResource(dcId, podId, uri);

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/e958f22f/services/secondary-storage/src/org/apache/cloudstack/storage/template/DownloadManagerImpl.java
----------------------------------------------------------------------
diff --git a/services/secondary-storage/src/org/apache/cloudstack/storage/template/DownloadManagerImpl.java b/services/secondary-storage/src/org/apache/cloudstack/storage/template/DownloadManagerImpl.java
index bf68b29..20e39ee 100755
--- a/services/secondary-storage/src/org/apache/cloudstack/storage/template/DownloadManagerImpl.java
+++ b/services/secondary-storage/src/org/apache/cloudstack/storage/template/DownloadManagerImpl.java
@@ -566,7 +566,7 @@ public class DownloadManagerImpl extends ManagerBase implements DownloadManager
                     td = new LocalTemplateDownloader(_storage, url, tmpDir, maxTemplateSizeInBytes, new Completion(jobId));
                 } else if (uri.getScheme().equalsIgnoreCase("scp")) {
                     td = new ScpTemplateDownloader(_storage, url, tmpDir, maxTemplateSizeInBytes, new Completion(jobId));
-                } else if (uri.getScheme().equalsIgnoreCase("nfs")) {
+                } else if (uri.getScheme().equalsIgnoreCase("nfs") || uri.getScheme().equalsIgnoreCase("cifs")) {
                     td = null;
                     // TODO: implement this.
                     throw new CloudRuntimeException("Scheme is not supported " + url);

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/e958f22f/services/secondary-storage/test/org/apache/cloudstack/storage/resource/LocalNfsSecondaryStorageResourceTest.java
----------------------------------------------------------------------
diff --git a/services/secondary-storage/test/org/apache/cloudstack/storage/resource/LocalNfsSecondaryStorageResourceTest.java b/services/secondary-storage/test/org/apache/cloudstack/storage/resource/LocalNfsSecondaryStorageResourceTest.java
index 0c355ec..34d510d 100644
--- a/services/secondary-storage/test/org/apache/cloudstack/storage/resource/LocalNfsSecondaryStorageResourceTest.java
+++ b/services/secondary-storage/test/org/apache/cloudstack/storage/resource/LocalNfsSecondaryStorageResourceTest.java
@@ -18,7 +18,29 @@
  */
 package org.apache.cloudstack.storage.resource;
 
-import com.cloud.agent.api.Answer;
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileNotFoundException;
+import java.io.IOException;
+import java.util.Map;
+import java.util.Properties;
+import java.util.UUID;
+
+import javax.naming.ConfigurationException;
+
+import junit.framework.Assert;
+import junit.framework.TestCase;
+
+import org.apache.log4j.Logger;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+import org.apache.cloudstack.storage.command.CopyCmdAnswer;
+import org.apache.cloudstack.storage.command.CopyCommand;
+import org.apache.cloudstack.storage.command.DownloadCommand;
+import org.apache.cloudstack.storage.to.TemplateObjectTO;
+
 import com.cloud.agent.api.storage.DownloadAnswer;
 import com.cloud.agent.api.storage.ListTemplateAnswer;
 import com.cloud.agent.api.storage.ListTemplateCommand;
@@ -27,25 +49,15 @@ import com.cloud.agent.api.to.NfsTO;
 import com.cloud.agent.api.to.SwiftTO;
 import com.cloud.storage.DataStoreRole;
 import com.cloud.storage.Storage;
-import com.cloud.utils.SwiftUtil;
-import junit.framework.Assert;
-import junit.framework.TestCase;
-import org.apache.cloudstack.api.command.user.tag.ListTagsCmd;
-import org.apache.cloudstack.storage.command.CopyCmdAnswer;
-import org.apache.cloudstack.storage.command.CopyCommand;
-import org.apache.cloudstack.storage.command.DownloadCommand;
-import org.apache.cloudstack.storage.to.SnapshotObjectTO;
-import org.apache.cloudstack.storage.to.TemplateObjectTO;
-import org.junit.Before;
-import org.junit.Test;
-import org.mockito.Mockito;
+import com.cloud.utils.PropertiesUtil;
+import com.cloud.utils.exception.CloudRuntimeException;
 
+public class LocalNfsSecondaryStorageResourceTest extends TestCase {
+    private static Map<String, Object> testParams;
 
-import javax.naming.ConfigurationException;
-import java.util.HashMap;
-import java.util.UUID;
+    private static final Logger s_logger = Logger
+            .getLogger(LocalNfsSecondaryStorageResourceTest.class.getName());
 
-public class LocalNfsSecondaryStorageResourceTest extends TestCase {
     LocalNfsSecondaryStorageResource resource;
     @Before
     @Override
@@ -53,7 +65,15 @@ public class LocalNfsSecondaryStorageResourceTest extends TestCase {
         resource = new LocalNfsSecondaryStorageResource();
         resource.setInSystemVM(true);
 
+        testParams = PropertiesUtil.toMap(loadProperties());
+        resource.configureStorageLayerClass(testParams);
+        Object testLocalRoot = testParams.get("testLocalRoot");
         resource.setParentPath("/mnt");
+
+        if (testLocalRoot != null) {
+            resource.setParentPath((String)testLocalRoot);
+        }
+
         System.setProperty("paths.script", "/Users/edison/develop/asf-master/script");
         //resource.configure("test", new HashMap<String, Object>());
     }
@@ -101,4 +121,25 @@ public class LocalNfsSecondaryStorageResourceTest extends TestCase {
         Assert.assertTrue(listAnswer.getTemplateInfo().size() > 0);
     }
 
+    public static Properties loadProperties() throws ConfigurationException {
+        Properties properties = new Properties();
+        final File file = PropertiesUtil.findConfigFile("agent.properties");
+        if (file == null) {
+            throw new ConfigurationException(
+                    "Unable to find agent.properties.");
+        }
+
+        s_logger.info("agent.properties found at " + file.getAbsolutePath());
+
+        try {
+            properties.load(new FileInputStream(file));
+        } catch (final FileNotFoundException ex) {
+            throw new CloudRuntimeException("Cannot find the file: "
+                    + file.getAbsolutePath(), ex);
+        } catch (final IOException ex) {
+            throw new CloudRuntimeException("IOException in reading "
+                    + file.getAbsolutePath(), ex);
+        }
+        return properties;
+    }
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/e958f22f/services/secondary-storage/test/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResourceTest.java
----------------------------------------------------------------------
diff --git a/services/secondary-storage/test/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResourceTest.java b/services/secondary-storage/test/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResourceTest.java
new file mode 100644
index 0000000..a805c78
--- /dev/null
+++ b/services/secondary-storage/test/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResourceTest.java
@@ -0,0 +1,120 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.cloudstack.storage.resource;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileNotFoundException;
+import java.io.IOException;
+import java.net.URI;
+import java.util.Map;
+import java.util.Properties;
+
+import javax.naming.ConfigurationException;
+
+import junit.framework.Assert;
+import junit.framework.TestCase;
+
+import org.apache.log4j.Level;
+import org.apache.log4j.Logger;
+import org.junit.Before;
+import org.junit.Test;
+
+import com.cloud.utils.PropertiesUtil;
+import com.cloud.utils.exception.CloudRuntimeException;
+
+public class NfsSecondaryStorageResourceTest extends TestCase {
+    private static Map<String, Object> testParams;
+
+    private static final Logger s_logger = Logger
+            .getLogger(NfsSecondaryStorageResourceTest.class.getName());
+
+	NfsSecondaryStorageResource resource;
+    @Before
+    @Override
+    public void setUp() throws ConfigurationException {
+        s_logger.setLevel(Level.ALL);
+        resource = new NfsSecondaryStorageResource();
+        resource.setInSystemVM(true);
+        testParams = PropertiesUtil.toMap(loadProperties());
+        resource.configureStorageLayerClass(testParams);
+        Object testLocalRoot = testParams.get("testLocalRoot");
+        if (testLocalRoot != null) {
+            resource.setParentPath((String)testLocalRoot);
+        }
+    }
+
+    @Test
+    public void testMount() throws Exception {
+        String sampleUriStr = "cifs://192.168.1.128/CSHV3?user=administrator&password=1pass%40word1&foo=bar";
+        URI sampleUri =  new URI(sampleUriStr);
+        
+        s_logger.info("Check HostIp parsing");
+        String hostIpStr = resource.getUriHostIp(sampleUri);
+        Assert.assertEquals("Expected host IP " + sampleUri.getHost() +
+                " and actual host IP " + hostIpStr + " differ.",
+                sampleUri.getHost(), hostIpStr);
+ 
+        s_logger.info("Check option parsing");
+        String expected = "user=administrator,password=1pass@word1,foo=bar,";
+        String actualOpts = resource.parseCifsMountOptions(sampleUri);
+        Assert.assertEquals("Options should be " + expected + " and not "
+                + actualOpts, expected, actualOpts);
+
+        // attempt a configured mount
+        final Map<String, Object> params =
+                PropertiesUtil.toMap(loadProperties());
+        String sampleMount = (String)params.get("testCifsMount");
+        if ( !sampleMount.isEmpty())
+        {
+            s_logger.info("functional test, mount " + sampleMount);
+            URI realMntUri = new URI(sampleMount);
+            String mntSubDir = resource.mountUri(realMntUri);
+            s_logger.info("functional test, umount " + mntSubDir);
+            resource.umount(resource.getMountingRoot() + mntSubDir, realMntUri);
+        }
+        else {
+            s_logger.info("no entry for testCifsMount in "
+                    + "./conf/agent.properties - skip functional test");
+        }
+    }
+
+    public static Properties loadProperties() throws ConfigurationException {
+        Properties properties = new Properties();
+        final File file = PropertiesUtil.findConfigFile("agent.properties");
+        if (file == null) {
+            throw new ConfigurationException(
+                    "Unable to find agent.properties.");
+        }
+
+        s_logger.info("agent.properties found at " + file.getAbsolutePath());
+
+        try {
+            properties.load(new FileInputStream(file));
+        } catch (final FileNotFoundException ex) {
+            throw new CloudRuntimeException("Cannot find the file: "
+                    + file.getAbsolutePath(), ex);
+        } catch (final IOException ex) {
+            throw new CloudRuntimeException("IOException in reading "
+                    + file.getAbsolutePath(), ex);
+        }
+        return properties;
+    }
+
+}

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/e958f22f/utils/src/com/cloud/utils/UriUtils.java
----------------------------------------------------------------------
diff --git a/utils/src/com/cloud/utils/UriUtils.java b/utils/src/com/cloud/utils/UriUtils.java
index 1ff4d72..4759710 100644
--- a/utils/src/com/cloud/utils/UriUtils.java
+++ b/utils/src/com/cloud/utils/UriUtils.java
@@ -26,6 +26,7 @@ import java.net.URI;
 import java.net.URISyntaxException;
 import java.net.URLEncoder;
 import java.net.UnknownHostException;
+import java.util.List;
 
 import javax.net.ssl.HttpsURLConnection;
 
@@ -36,8 +37,8 @@ import org.apache.commons.httpclient.MultiThreadedHttpConnectionManager;
 import org.apache.commons.httpclient.UsernamePasswordCredentials;
 import org.apache.commons.httpclient.auth.AuthScope;
 import org.apache.commons.httpclient.methods.GetMethod;
-import org.apache.http.HttpResponse;
-import org.apache.http.client.methods.HttpGet;
+import org.apache.http.NameValuePair;
+import org.apache.http.client.utils.URLEncodedUtils;
 import org.apache.log4j.Logger;
 
 import com.cloud.utils.exception.CloudRuntimeException;
@@ -101,6 +102,47 @@ public class UriUtils {
         return url;
     }
 
+    public static String getCifsUriParametersProblems(URI uri) {
+        if (!UriUtils.hostAndPathPresent(uri)) {
+            String errMsg = "cifs URI missing host and/or path.  "
+                    + " Make sure it's of the format "
+                    + "cifs://hostname/path?user=<username>&password=<password>";
+            s_logger.warn(errMsg);
+            return errMsg;
+        }
+        if (!UriUtils.cifsCredentialsPresent(uri))
+        {
+            String errMsg = "cifs URI missing user and password details. "
+                    + "Add them as query parameters, e.g. "
+                    + "cifs://example.com/some_share?user=foo&password=bar";
+            s_logger.warn(errMsg);
+            return errMsg;
+        }
+        return null;
+    }
+
+    public static boolean hostAndPathPresent(URI uri) {
+        return !(uri.getHost() == null || uri.getHost().trim().isEmpty()
+                || uri.getPath() == null || uri.getPath().trim().isEmpty());
+    }
+
+    public static boolean cifsCredentialsPresent(URI uri) {
+        List<NameValuePair> args = URLEncodedUtils.parse(uri, "UTF-8");
+        boolean foundUser = false;
+        boolean foundPswd = false;
+        for (NameValuePair nvp : args) {
+            String name = nvp.getName();
+            if (name.equals("user")) {
+                foundUser = true;
+                s_logger.debug("foundUser is" + foundUser);
+            }
+            else if (name.equals("password")) {
+                foundPswd = true;
+                s_logger.debug("foundPswd is" + foundPswd);
+            }
+        }
+        return (foundUser && foundPswd);
+    }
     // Get the size of a file from URL response header.
     public static Long getRemoteSize(String url) {
         Long remoteSize = (long) 0;


Mime
View raw message