CLOUDSTACK-9368: Fix for Support configurable NFS version for Secondary Storage mounts

api/src/com/cloud/agent/api/to/NfsTO.java

@@ -24,6 +24,7 @@ public class NfsTO implements DataStoreTO {
     private DataStoreRole _role;
     private String uuid;
     private static final String pathSeparator = "/";
+    private Integer nfsVersion;
 
     public NfsTO() {
 
@@ -71,4 +72,12 @@ public class NfsTO implements DataStoreTO {
     public String getPathSeparator() {
         return pathSeparator;
     }
+
+    public Integer getNfsVersion() {
+        return nfsVersion;
+    }
+
+    public void setNfsVersion(Integer nfsVersion) {
+        this.nfsVersion = nfsVersion;
+    }
 }

core/src/com/cloud/agent/api/BackupSnapshotCommand.java

@@ -39,7 +39,6 @@ public class BackupSnapshotCommand extends SnapshotCommand {
     private S3TO s3;
     StorageFilerTO pool;
     private Long secHostId;
-    private String nfsVersion;
 
     protected BackupSnapshotCommand() {
 
@@ -109,11 +108,4 @@ public class BackupSnapshotCommand extends SnapshotCommand {
         return secHostId;
     }
 
-    public String getNfsVersion() {
-        return nfsVersion;
-    }
-
-    public void setNfsVersion(String nfsVersion) {
-        this.nfsVersion = nfsVersion;
-    }
 }

core/src/com/cloud/agent/api/CreatePrivateTemplateFromSnapshotCommand.java

@@ -28,7 +28,6 @@ public class CreatePrivateTemplateFromSnapshotCommand extends SnapshotCommand {
     private String origTemplateInstallPath;
     private Long newTemplateId;
     private String templateName;
-    private String nfsVersion;
 
     protected CreatePrivateTemplateFromSnapshotCommand() {
 
@@ -74,11 +73,4 @@ public class CreatePrivateTemplateFromSnapshotCommand extends SnapshotCommand {
         return templateName;
     }
 
-    public String getNfsVersion() {
-        return nfsVersion;
-    }
-
-    public void setNfsVersion(String nfsVersion) {
-        this.nfsVersion = nfsVersion;
-    }
 }

core/src/com/cloud/agent/api/CreatePrivateTemplateFromVolumeCommand.java

@@ -32,7 +32,6 @@ public class CreatePrivateTemplateFromVolumeCommand extends SnapshotCommand {
     StorageFilerTO _primaryPool;
     // For XenServer
     private String _secondaryStorageUrl;
-    private String nfsVersion;
 
     public CreatePrivateTemplateFromVolumeCommand() {
     }
@@ -101,11 +100,4 @@ public class CreatePrivateTemplateFromVolumeCommand extends SnapshotCommand {
         _templateId = templateId;
     }
 
-    public String getNfsVersion() {
-        return nfsVersion;
-    }
-
-    public void setNfsVersion(String nfsVersion) {
-        this.nfsVersion = nfsVersion;
-    }
 }

core/src/com/cloud/agent/api/CreateVolumeFromSnapshotCommand.java

@@ -26,8 +26,6 @@ import com.cloud.storage.StoragePool;
  */
 public class CreateVolumeFromSnapshotCommand extends SnapshotCommand {
 
-    private String nfsVersion;
-
     protected CreateVolumeFromSnapshotCommand() {
 
     }
@@ -53,11 +51,4 @@ public class CreateVolumeFromSnapshotCommand extends SnapshotCommand {
         setWait(wait);
     }
 
-    public String getNfsVersion() {
-        return nfsVersion;
-    }
-
-    public void setNfsVersion(String nfsVersion) {
-        this.nfsVersion = nfsVersion;
-    }
 }

core/src/com/cloud/agent/api/GetStorageStatsCommand.java

@@ -20,17 +20,17 @@
 package com.cloud.agent.api;
 
 import com.cloud.agent.api.LogLevel.Log4jLevel;
+import com.cloud.agent.api.storage.StorageNfsVersionCommand;
 import com.cloud.agent.api.to.DataStoreTO;
 import com.cloud.storage.Storage.StoragePoolType;
 
 @LogLevel(Log4jLevel.Trace)
-public class GetStorageStatsCommand extends Command {
+public class GetStorageStatsCommand extends StorageNfsVersionCommand {
     private String id;
     private String localPath;
     private StoragePoolType pooltype;
     private String secUrl;
     private DataStoreTO store;
-    private String nfsVersion;
 
     public String getSecUrl() {
         return secUrl;
@@ -55,9 +55,9 @@ public class GetStorageStatsCommand extends Command {
         this.store = store;
     }
 
-    public GetStorageStatsCommand(DataStoreTO store, String nfsVersion) {
+    public GetStorageStatsCommand(DataStoreTO store, Integer nfsVersion) {
+        super(nfsVersion);
         this.store = store;
-        this.nfsVersion = nfsVersion;
     }
 
     public GetStorageStatsCommand(String secUrl) {
@@ -87,14 +87,6 @@ public class GetStorageStatsCommand extends Command {
         return this.store;
     }
 
-    public String getNfsVersion() {
-        return nfsVersion;
-    }
-
-    public void setNfsVersion(String nfsVersion) {
-        this.nfsVersion = nfsVersion;
-    }
-
     @Override
     public boolean executeInSequence() {
         return false;

core/src/com/cloud/agent/api/SecStorageSetupCommand.java

@@ -21,14 +21,14 @@ package com.cloud.agent.api;
 
 import org.apache.cloudstack.framework.security.keystore.KeystoreManager;
 
+import com.cloud.agent.api.storage.StorageNfsVersionCommand;
 import com.cloud.agent.api.to.DataStoreTO;
 
-public class SecStorageSetupCommand extends Command {
+public class SecStorageSetupCommand extends StorageNfsVersionCommand {
     private DataStoreTO store;
     private String secUrl;
     private KeystoreManager.Certificates certs;
     private String postUploadKey;
-    private String nfsVersion;
 
 
     public SecStorageSetupCommand() {
@@ -76,11 +76,4 @@ public class SecStorageSetupCommand extends Command {
         this.postUploadKey = postUploadKey;
     }
 
-    public String getNfsVersion() {
-        return nfsVersion;
-    }
-
-    public void setNfsVersion(String nfsVersion) {
-        this.nfsVersion = nfsVersion;
-    }
 }

core/src/com/cloud/agent/api/SnapshotCommand.java

@@ -19,6 +19,7 @@
 
 package com.cloud.agent.api;
 
+import com.cloud.agent.api.storage.StorageNfsVersionCommand;
 import com.cloud.agent.api.to.StorageFilerTO;
 import com.cloud.storage.StoragePool;
 
@@ -26,7 +27,7 @@ import com.cloud.storage.StoragePool;
  * This currently assumes that both primary and secondary storage are mounted on
  * the XenServer.
  */
-public class SnapshotCommand extends Command {
+public class SnapshotCommand extends StorageNfsVersionCommand {
     protected String primaryStoragePoolNameLabel;
     StorageFilerTO primaryPool;
     private String snapshotUuid;

core/src/com/cloud/agent/api/storage/CopyVolumeCommand.java

@@ -19,11 +19,10 @@
 
 package com.cloud.agent.api.storage;
 
-import com.cloud.agent.api.Command;
 import com.cloud.agent.api.to.StorageFilerTO;
 import com.cloud.storage.StoragePool;
 
-public class CopyVolumeCommand extends Command {
+public class CopyVolumeCommand extends StorageNfsVersionCommand {
 
     long volumeId;
     String volumePath;
@@ -32,7 +31,6 @@ public class CopyVolumeCommand extends Command {
     boolean toSecondaryStorage;
     String vmName;
     boolean executeInSequence = false;
-    String nfsVersion;
 
     public CopyVolumeCommand() {
     }
@@ -77,11 +75,4 @@ public class CopyVolumeCommand extends Command {
         return vmName;
     }
 
-    public String getNfsVersion() {
-        return nfsVersion;
-    }
-
-    public void setNfsVersion(String nfsVersion) {
-        this.nfsVersion = nfsVersion;
-    }
 }

core/src/com/cloud/agent/api/storage/ListTemplateCommand.java

@@ -23,21 +23,17 @@ import com.cloud.agent.api.to.DataStoreTO;
 
 public class ListTemplateCommand extends StorageCommand {
     private DataStoreTO store;
-    private String nfsVersion;
-
-    //private String secUrl;
 
     public ListTemplateCommand() {
     }
 
     public ListTemplateCommand(DataStoreTO store) {
         this.store = store;
-//        this.secUrl = url;
     }
 
-    public ListTemplateCommand(DataStoreTO store, String nfsVersion) {
+    public ListTemplateCommand(DataStoreTO store, Integer nfsVersion) {
+        super(nfsVersion);
         this.store = store;
-        this.nfsVersion = nfsVersion;
     }
 
     @Override
@@ -49,12 +45,4 @@ public class ListTemplateCommand extends StorageCommand {
         return store;
     }
 
-    public String getNfsVersion() {
-        return nfsVersion;
-    }
-
-    //   public String getSecUrl() {
-    //       return secUrl;
-    //   }
-
 }

core/src/com/cloud/agent/api/storage/PrimaryStorageDownloadCommand.java

@@ -35,7 +35,6 @@ public class PrimaryStorageDownloadCommand extends AbstractDownloadCommand {
 
     String secondaryStorageUrl;
     String primaryStorageUrl;
-    String nfsVersion;
 
     protected PrimaryStorageDownloadCommand() {
     }
@@ -89,11 +88,4 @@ public class PrimaryStorageDownloadCommand extends AbstractDownloadCommand {
         return true;
     }
 
-    public String getNfsVersion() {
-        return nfsVersion;
-    }
-
-    public void setNfsVersion(String nfsVersion) {
-        this.nfsVersion = nfsVersion;
-    }
 }

core/src/com/cloud/agent/api/storage/SsCommand.java

@@ -19,9 +19,7 @@
 
 package com.cloud.agent.api.storage;
 
-import com.cloud.agent.api.Command;
+public abstract class SsCommand extends StorageNfsVersionCommand {
-
-public abstract class SsCommand extends Command {
     private String secUrl;
 
     public SsCommand() {

core/src/com/cloud/agent/api/storage/StorageCommand.java

@@ -19,11 +19,13 @@
 
 package com.cloud.agent.api.storage;
 
-import com.cloud.agent.api.Command;
+public abstract class StorageCommand extends StorageNfsVersionCommand {
-
-public abstract class StorageCommand extends Command {
     protected StorageCommand() {
         super();
     }
 
+    protected StorageCommand(Integer nfsVersion){
+        super(nfsVersion);
+    }
+
 }

core/src/com/cloud/agent/api/storage/StorageNfsVersionCommand.java

@@ -0,0 +1,44 @@
+//
+// 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 com.cloud.agent.api.storage;
+
+import com.cloud.agent.api.Command;
+
+public abstract class StorageNfsVersionCommand extends Command {
+
+    protected StorageNfsVersionCommand(){
+        super();
+    }
+
+    protected StorageNfsVersionCommand(Integer nfsVersion){
+        super();
+        this.nfsVersion = nfsVersion;
+    }
+
+    private Integer nfsVersion;
+
+    public Integer getNfsVersion() {
+        return nfsVersion;
+    }
+
+    public void setNfsVersion(Integer nfsVersion) {
+        this.nfsVersion = nfsVersion;
+    }
+
+}

core/src/org/apache/cloudstack/storage/command/TemplateOrVolumePostUploadCommand.java

@@ -53,7 +53,7 @@ public class TemplateOrVolumePostUploadCommand {
 
     private long accountId;
 
-    private String nfsVersion;
+    private Integer nfsVersion;
 
     public TemplateOrVolumePostUploadCommand(long entityId, String entityUUID, String absolutePath, String checksum, String type, String name, String imageFormat, String dataTo,
             String dataToRole) {
@@ -199,11 +199,11 @@ public class TemplateOrVolumePostUploadCommand {
         return accountId;
     }
 
-    public String getNfsVersion() {
+    public Integer getNfsVersion() {
         return nfsVersion;
     }
 
-    public void setNfsVersion(String nfsVersion) {
+    public void setNfsVersion(Integer nfsVersion) {
         this.nfsVersion = nfsVersion;
     }
 }

engine/storage/image/src/org/apache/cloudstack/storage/image/TemplateServiceImpl.java

@@ -638,7 +638,8 @@ public class TemplateServiceImpl implements TemplateService {
     }
 
     private Map<String, TemplateProp> listTemplate(DataStore ssStore) {
-        ListTemplateCommand cmd = new ListTemplateCommand(ssStore.getTO(), imageStoreDetailsUtil.getNfsVersion(ssStore.getId()));
+        Integer nfsVersion = imageStoreDetailsUtil.getNfsVersion(ssStore.getId());
+        ListTemplateCommand cmd = new ListTemplateCommand(ssStore.getTO(), nfsVersion);
         EndPoint ep = _epSelector.select(ssStore);
         Answer answer = null;
         if (ep == null) {

engine/storage/integration-test/test/org/apache/cloudstack/storage/MockLocalNfsSecondaryStorageResource.java

@@ -51,7 +51,7 @@ public class MockLocalNfsSecondaryStorageResource extends NfsSecondaryStorageRes
     }
 
     @Override
-    public String getRootDir(String secUrl, String nfsVersion) {
+    public String getRootDir(String secUrl, Integer nfsVersion) {
         return "/mnt";
     }
 

engine/storage/src/org/apache/cloudstack/storage/image/BaseImageStoreDriverImpl.java

@@ -79,6 +79,7 @@ public abstract class BaseImageStoreDriverImpl implements ImageStoreDriver {
     VMTemplateZoneDao _vmTemplateZoneDao;
     @Inject
     AlertManager _alertMgr;
+
     protected String _proxy = null;
 
     protected Proxy getHttpProxy() {

engine/storage/src/org/apache/cloudstack/storage/image/NfsImageStoreDriverImpl.java

@@ -0,0 +1,48 @@
+/*
+ * 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.image;
+
+import java.util.Map;
+
+import javax.inject.Inject;
+
+import org.apache.cloudstack.storage.datastore.db.ImageStoreDetailsDao;
+
+public abstract class NfsImageStoreDriverImpl extends BaseImageStoreDriverImpl {
+
+    @Inject
+    ImageStoreDetailsDao _imageStoreDetailsDao;
+
+    private static final String NFS_VERSION_DETAILS_KEY = "nfs.version";
+
+    /**
+     * Retrieve NFS version to be used for imgStoreId store, if provided in image_store_details table
+     * @param imgStoreId store id
+     * @return "nfs.version" associated value for imgStoreId in image_store_details table if exists, null if not
+     */
+    protected Integer getNfsVersion(long imgStoreId){
+        Map<String, String> imgStoreDetails = _imageStoreDetailsDao.getDetails(imgStoreId);
+        if (imgStoreDetails != null && imgStoreDetails.containsKey(NFS_VERSION_DETAILS_KEY)){
+            String nfsVersionParam = imgStoreDetails.get(NFS_VERSION_DETAILS_KEY);
+            return (nfsVersionParam != null ? Integer.valueOf(nfsVersionParam) : null);
+        }
+        return null;
+    }
+
+}

plugins/hypervisors/simulator/src/com/cloud/resource/AgentStorageResource.java

@@ -109,7 +109,7 @@ public class AgentStorageResource extends AgentResourceBase implements Secondary
     }
 
     @Override
-    public String getRootDir(String url, String nfsVersion) {
+    public String getRootDir(String url, Integer nfsVersion) {
         // TODO Auto-generated method stub
         return null;
     }

plugins/hypervisors/simulator/src/org/apache/cloudstack/storage/datastore/driver/SimulatorImageStoreDriverImpl.java

@@ -35,7 +35,7 @@ import org.apache.cloudstack.framework.async.AsyncCallbackDispatcher;
 import org.apache.cloudstack.framework.async.AsyncCompletionCallback;
 import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreDao;
 import org.apache.cloudstack.storage.datastore.db.VolumeDataStoreDao;
-import org.apache.cloudstack.storage.image.BaseImageStoreDriverImpl;
+import org.apache.cloudstack.storage.image.NfsImageStoreDriverImpl;
 import org.apache.cloudstack.storage.image.store.ImageStoreImpl;
 
 import com.cloud.agent.api.storage.DownloadAnswer;
@@ -47,7 +47,7 @@ import com.cloud.storage.VMTemplateStorageResourceAssoc;
 import com.cloud.storage.dao.VMTemplateDao;
 import com.cloud.storage.dao.VolumeDao;
 
-public class SimulatorImageStoreDriverImpl extends BaseImageStoreDriverImpl {
+public class SimulatorImageStoreDriverImpl extends NfsImageStoreDriverImpl {
     private static final Logger s_logger = Logger.getLogger(SimulatorImageStoreDriverImpl.class);
 
     @Inject
@@ -67,6 +67,7 @@ public class SimulatorImageStoreDriverImpl extends BaseImageStoreDriverImpl {
         NfsTO nfsTO = new NfsTO();
         nfsTO.setRole(store.getRole());
         nfsTO.setUrl(nfsStore.getUri());
+        nfsTO.setNfsVersion(getNfsVersion(nfsStore.getId()));
         return nfsTO;
     }
 

plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java

@@ -553,7 +553,8 @@ public class VmwareManagerImpl extends ManagerBase implements VmwareManager, Vmw
 
     @Override
     public void prepareSecondaryStorageStore(String storageUrl, Long storeId) {
-        String mountPoint = getMountPoint(storageUrl, imageStoreDetailsUtil.getNfsVersion(storeId));
+        Integer nfsVersion = imageStoreDetailsUtil.getNfsVersion(storeId);
+        String mountPoint = getMountPoint(storageUrl, nfsVersion);
 
         GlobalLock lock = GlobalLock.getInternLock("prepare.systemvm");
         try {
@@ -661,7 +662,7 @@ public class VmwareManagerImpl extends ManagerBase implements VmwareManager, Vmw
     }
 
     @Override
-    public String getMountPoint(String storageUrl, String nfsVersion) {
+    public String getMountPoint(String storageUrl, Integer nfsVersion) {
         String mountPoint = null;
         synchronized (_storageMounts) {
             mountPoint = _storageMounts.get(storageUrl);
@@ -752,7 +753,7 @@ public class VmwareManagerImpl extends ManagerBase implements VmwareManager, Vmw
         }
     }
 
-    protected String mount(String path, String parent, String nfsVersion) {
+    protected String mount(String path, String parent, Integer nfsVersion) {
         String mountPoint = setupMountPoint(parent);
         if (mountPoint == null) {
             s_logger.warn("Unable to create a mount point");

plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImpl.java

@@ -93,7 +93,7 @@ import com.cloud.vm.snapshot.VMSnapshot;
 
 public class VmwareStorageManagerImpl implements VmwareStorageManager {
 
-    private String _nfsVersion;
+    private Integer _nfsVersion;
 
     @Override
     public boolean execute(VmwareHostService hostService, CreateEntityDownloadURLCommand cmd) {
@@ -141,7 +141,7 @@ public class VmwareStorageManagerImpl implements VmwareStorageManager {
         _mountService = mountService;
     }
 
-    public VmwareStorageManagerImpl(VmwareStorageMount mountService, String nfsVersion) {
+    public VmwareStorageManagerImpl(VmwareStorageMount mountService, Integer nfsVersion) {
         assert (mountService != null);
         _mountService = mountService;
         _nfsVersion = nfsVersion;
@@ -555,7 +555,7 @@ public class VmwareStorageManagerImpl implements VmwareStorageManager {
     // templateName: name in secondary storage
     // templateUuid: will be used at hypervisor layer
     private void copyTemplateFromSecondaryToPrimary(VmwareHypervisorHost hyperHost, DatastoreMO datastoreMo, String secondaryStorageUrl,
-            String templatePathAtSecondaryStorage, String templateName, String templateUuid, String nfsVersion) throws Exception {
+            String templatePathAtSecondaryStorage, String templateName, String templateUuid, Integer nfsVersion) throws Exception {
 
         s_logger.info("Executing copyTemplateFromSecondaryToPrimary. secondaryStorage: " + secondaryStorageUrl + ", templatePathAtSecondaryStorage: " +
                 templatePathAtSecondaryStorage + ", templateName: " + templateName);
@@ -611,7 +611,7 @@ public class VmwareStorageManagerImpl implements VmwareStorageManager {
     }
 
     private Ternary<String, Long, Long> createTemplateFromVolume(VirtualMachineMO vmMo, long accountId, long templateId, String templateUniqueName, String secStorageUrl,
-            String volumePath, String workerVmName, String nfsVersion) throws Exception {
+            String volumePath, String workerVmName, Integer nfsVersion) throws Exception {
 
         String secondaryMountPoint = _mountService.getMountPoint(secStorageUrl, nfsVersion);
         String installPath = getTemplateRelativeDirInSecStorage(accountId, templateId);
@@ -676,7 +676,7 @@ public class VmwareStorageManagerImpl implements VmwareStorageManager {
     }
 
     private Ternary<String, Long, Long> createTemplateFromSnapshot(long accountId, long templateId, String templateUniqueName, String secStorageUrl, long volumeId,
-            String backedUpSnapshotUuid, String nfsVersion) throws Exception {
+            String backedUpSnapshotUuid, Integer nfsVersion) throws Exception {
 
         String secondaryMountPoint = _mountService.getMountPoint(secStorageUrl, nfsVersion);
         String installPath = getTemplateRelativeDirInSecStorage(accountId, templateId);
@@ -860,14 +860,14 @@ public class VmwareStorageManagerImpl implements VmwareStorageManager {
     }
 
     private String createVolumeFromSnapshot(VmwareHypervisorHost hyperHost, DatastoreMO primaryDsMo, String newVolumeName, long accountId, long volumeId,
-            String secStorageUrl, String snapshotBackupUuid, String nfsVersion) throws Exception {
+            String secStorageUrl, String snapshotBackupUuid, Integer nfsVersion) throws Exception {
 
         restoreVolumeFromSecStorage(hyperHost, primaryDsMo, newVolumeName, secStorageUrl, getSnapshotRelativeDirInSecStorage(accountId, volumeId), snapshotBackupUuid, nfsVersion);
         return null;
     }
 
     private void restoreVolumeFromSecStorage(VmwareHypervisorHost hyperHost, DatastoreMO primaryDsMo, String newVolumeName, String secStorageUrl, String secStorageDir,
-            String backupName, String nfsVersion) throws Exception {
+            String backupName, Integer nfsVersion) throws Exception {
 
         String secondaryMountPoint = _mountService.getMountPoint(secStorageUrl, nfsVersion);
         String srcOVAFileName = secondaryMountPoint + "/" + secStorageDir + "/" + backupName + "." + ImageFormat.OVA.getFileExtension();
@@ -927,7 +927,7 @@ public class VmwareStorageManagerImpl implements VmwareStorageManager {
     }
 
     private String backupSnapshotToSecondaryStorage(VirtualMachineMO vmMo, long accountId, long volumeId, String volumePath, String snapshotUuid, String secStorageUrl,
-            String prevSnapshotUuid, String prevBackupUuid, String workerVmName, String nfsVersion) throws Exception {
+            String prevSnapshotUuid, String prevBackupUuid, String workerVmName, Integer nfsVersion) throws Exception {
 
         String backupUuid = UUID.randomUUID().toString();
         exportVolumeToSecondaryStroage(vmMo, volumePath, secStorageUrl, getSnapshotRelativeDirInSecStorage(accountId, volumeId), backupUuid, workerVmName, nfsVersion);
@@ -935,7 +935,7 @@ public class VmwareStorageManagerImpl implements VmwareStorageManager {
     }
 
     private void exportVolumeToSecondaryStroage(VirtualMachineMO vmMo, String volumePath, String secStorageUrl, String secStorageDir, String exportName,
-            String workerVmName, String nfsVersion) throws Exception {
+            String workerVmName, Integer nfsVersion) throws Exception {
 
         String secondaryMountPoint = _mountService.getMountPoint(secStorageUrl, nfsVersion);
         String exportPath = secondaryMountPoint + "/" + secStorageDir + "/" + exportName;
@@ -980,7 +980,7 @@ public class VmwareStorageManagerImpl implements VmwareStorageManager {
     }
 
     private Pair<String, String> copyVolumeToSecStorage(VmwareHostService hostService, VmwareHypervisorHost hyperHost, CopyVolumeCommand cmd, String vmName,
-            long volumeId, String poolId, String volumePath, String secStorageUrl, String workerVmName, String nfsVersion) throws Exception {
+            long volumeId, String poolId, String volumePath, String secStorageUrl, String workerVmName, Integer nfsVersion) throws Exception {
 
         String volumeFolder = String.valueOf(volumeId) + "/";
         VirtualMachineMO workerVm = null;
@@ -1038,7 +1038,7 @@ public class VmwareStorageManagerImpl implements VmwareStorageManager {
         return datastoreVolumePath;
     }
 
-    private Pair<String, String> copyVolumeFromSecStorage(VmwareHypervisorHost hyperHost, long volumeId, DatastoreMO dsMo, String secStorageUrl, String exportName, String nfsVersion)
+    private Pair<String, String> copyVolumeFromSecStorage(VmwareHypervisorHost hyperHost, long volumeId, DatastoreMO dsMo, String secStorageUrl, String exportName, Integer nfsVersion)
             throws Exception {
 
         String volumeFolder = String.valueOf(volumeId) + "/";
@@ -1458,7 +1458,7 @@ public class VmwareStorageManagerImpl implements VmwareStorageManager {
         }
     }
 
-    private String deleteVolumeDirOnSecondaryStorage(long volumeId, String secStorageUrl, String nfsVersion) throws Exception {
+    private String deleteVolumeDirOnSecondaryStorage(long volumeId, String secStorageUrl, Integer nfsVersion) throws Exception {
         String secondaryMountPoint = _mountService.getMountPoint(secStorageUrl, nfsVersion);
         String volumeMountRoot = secondaryMountPoint + "/" + getVolumeRelativeDirInSecStroage(volumeId);
 

plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareStorageMount.java

@@ -17,5 +17,5 @@
 package com.cloud.hypervisor.vmware.manager;
 
 public interface VmwareStorageMount {
-    String getMountPoint(String storageUrl, String nfsVersion);
+    String getMountPoint(String storageUrl, Integer nfsVersion);
 }

plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java

@@ -97,7 +97,9 @@ import com.vmware.vim25.VirtualMachineRuntimeInfo;
 import com.vmware.vim25.VirtualMachineVideoCard;
 import com.vmware.vim25.VmwareDistributedVirtualSwitchVlanIdSpec;
 
+import org.apache.cloudstack.storage.command.CopyCommand;
 import org.apache.cloudstack.storage.command.StorageSubSystemCommand;
+import org.apache.cloudstack.storage.resource.NfsSecondaryStorageResource;
 import org.apache.cloudstack.storage.to.TemplateObjectTO;
 import org.apache.cloudstack.storage.to.VolumeObjectTO;
 import org.apache.commons.lang.math.NumberUtils;
@@ -306,6 +308,7 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa
     protected String _password;
     protected String _guid;
     protected String _vCenterAddress;
+    protected Integer storageNfsVersion;
 
     protected String _privateNetworkVSwitchName;
     protected VmwareTrafficLabel _guestTrafficInfo = new VmwareTrafficLabel(TrafficType.Guest);
@@ -475,6 +478,7 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa
             } else if (clz == UnregisterVMCommand.class) {
                 return execute((UnregisterVMCommand) cmd);
             } else if (cmd instanceof StorageSubSystemCommand) {
+                checkStorageProcessorAndHandlerNfsVersionAttribute((StorageSubSystemCommand)cmd);
                 return storageHandler.handleStorageCommands((StorageSubSystemCommand) cmd);
             } else if (clz == ScaleVmCommand.class) {
                 return execute((ScaleVmCommand)cmd);
@@ -523,6 +527,64 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa
         return answer;
     }
 
+    /**
+     * Check if storage NFS version is already set or needs to be reconfigured.<br>
+     * If _storageNfsVersion is not null -> nothing to do, version already set.<br>
+     * If _storageNfsVersion is null -> examine StorageSubSystemCommand to get NFS version and set it
+     * to the storage processor and storage handler.
+     * @param cmd command to execute
+     */
+    protected void checkStorageProcessorAndHandlerNfsVersionAttribute(StorageSubSystemCommand cmd) {
+        if (storageNfsVersion != null) return;
+        if (cmd instanceof CopyCommand){
+            examineStorageSubSystemCommandNfsVersion((CopyCommand) cmd);
+        }
+    }
+
+    /**
+     * Examine StorageSubSystem command to get storage NFS version, if provided
+     * @param cmd command to execute
+     */
+    protected void examineStorageSubSystemCommandNfsVersion(CopyCommand cmd){
+        DataStoreTO srcDataStore = cmd.getSrcTO().getDataStore();
+        boolean nfsVersionFound = false;
+
+        if (srcDataStore instanceof NfsTO){
+            nfsVersionFound = getStorageNfsVersionFromNfsTO((NfsTO) srcDataStore);
+        }
+
+        if (nfsVersionFound){
+            setCurrentNfsVersionInProcessorAndHandler();
+        }
+    }
+
+    /**
+     * Get storage NFS version from NfsTO
+     * @param nfsTO nfsTO
+     * @return true if NFS version was found and not null, false in other case
+     */
+    protected boolean getStorageNfsVersionFromNfsTO(NfsTO nfsTO){
+        if (nfsTO != null && nfsTO.getNfsVersion() != null){
+            storageNfsVersion = nfsTO.getNfsVersion();
+            return true;
+        }
+        return false;
+    }
+
+    /**
+     * Sets _storageNfsVersion into storage processor and storage handler by calling reconfigureNfsVersion on the storage handler,
+     * which will set NFS version into it and the storage processor.
+     */
+    protected void setCurrentNfsVersionInProcessorAndHandler() {
+        VmwareStorageSubsystemCommandHandler handler = (VmwareStorageSubsystemCommandHandler) storageHandler;
+        boolean success = handler.reconfigureNfsVersion(storageNfsVersion);
+        if (success){
+            s_logger.info("NFS version " + storageNfsVersion + " successfully set in VmwareStorageProcessor and VmwareStorageSubsystemCommandHandler");
+        } else {
+            s_logger.error("Error while setting NFS version " + storageNfsVersion);
+        }
+    }
+
     /**
      * Registers the vm to the inventory given the vmx file.
      */
@@ -5311,8 +5373,10 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa
 
             value = (String)params.get("scripts.timeout");
             int timeout = NumbersUtil.parseInt(value, 1440) * 1000;
-            _storageProcessor = new VmwareStorageProcessor((VmwareHostService)this, _fullCloneFlag, (VmwareStorageMount)mgr, timeout, this, _shutdownWaitMs, null, (String)params.get("nfsVersion"));
+
-            storageHandler = new VmwareStorageSubsystemCommandHandler(_storageProcessor, (String)params.get("nfsVersion"));
+            storageNfsVersion = NfsSecondaryStorageResource.retrieveNfsVersionFromParams(params);
+            _storageProcessor = new VmwareStorageProcessor((VmwareHostService)this, _fullCloneFlag, (VmwareStorageMount)mgr, timeout, this, _shutdownWaitMs, null, storageNfsVersion);
+            storageHandler = new VmwareStorageSubsystemCommandHandler(_storageProcessor, storageNfsVersion);
 
             _vrResource = new VirtualRoutingResource(this);
             if (!_vrResource.configure(name, params)) {

plugins/hypervisors/vmware/src/com/cloud/storage/resource/PremiumSecondaryStorageResource.java

@@ -102,7 +102,8 @@ public class PremiumSecondaryStorageResource extends NfsSecondaryStorageResource
             VmwareSecondaryStorageContextFactory.initFactoryEnvironment();
         }
 
-        registerHandler(Hypervisor.HypervisorType.VMware, new VmwareSecondaryStorageResourceHandler(this, (String)params.get("nfsVersion")));
+        Integer nfsVersion = NfsSecondaryStorageResource.retrieveNfsVersionFromParams(params);
+        registerHandler(Hypervisor.HypervisorType.VMware, new VmwareSecondaryStorageResourceHandler(this, nfsVersion));
         return true;
     }
 }

plugins/hypervisors/vmware/src/com/cloud/storage/resource/VmwareSecondaryStorageResourceHandler.java

@@ -66,7 +66,7 @@ public class VmwareSecondaryStorageResourceHandler implements SecondaryStorageRe
      * private Map<String, HostMO> _activeHosts = new HashMap<String, HostMO>();
      */
 
-    public VmwareSecondaryStorageResourceHandler(PremiumSecondaryStorageResource resource, String nfsVersion) {
+    public VmwareSecondaryStorageResourceHandler(PremiumSecondaryStorageResource resource, Integer nfsVersion) {
         _resource = resource;
         _storageMgr = new VmwareStorageManagerImpl(this, nfsVersion);
         _gson = GsonHelper.getGsonLogger();
@@ -304,7 +304,7 @@ public class VmwareSecondaryStorageResourceHandler implements SecondaryStorageRe
     }
 
     @Override
-    public String getMountPoint(String storageUrl, String nfsVersion) {
+    public String getMountPoint(String storageUrl, Integer nfsVersion) {
         return _resource.getRootDir(storageUrl, nfsVersion);
     }
 }

plugins/hypervisors/vmware/src/com/cloud/storage/resource/VmwareStorageProcessor.java

@@ -125,10 +125,10 @@ public class VmwareStorageProcessor implements StorageProcessor {
     protected Integer _shutdownWaitMs;
     private final Gson _gson;
     private final StorageLayer _storage = new JavaStorageLayer();
-    private String _nfsVersion;
+    private Integer _nfsVersion;
 
     public VmwareStorageProcessor(VmwareHostService hostService, boolean fullCloneFlag, VmwareStorageMount mountService, Integer timeout, VmwareResource resource,
-            Integer shutdownWaitMs, PremiumSecondaryStorageResource storageResource, String nfsVersion) {
+            Integer shutdownWaitMs, PremiumSecondaryStorageResource storageResource, Integer nfsVersion) {
         this.hostService = hostService;
         _fullCloneFlag = fullCloneFlag;
         this.mountService = mountService;
@@ -169,7 +169,7 @@ public class VmwareStorageProcessor implements StorageProcessor {
     }
 
     private VirtualMachineMO copyTemplateFromSecondaryToPrimary(VmwareHypervisorHost hyperHost, DatastoreMO datastoreMo, String secondaryStorageUrl,
-            String templatePathAtSecondaryStorage, String templateName, String templateUuid, boolean createSnapshot, String nfsVersion) throws Exception {
+            String templatePathAtSecondaryStorage, String templateName, String templateUuid, boolean createSnapshot, Integer nfsVersion) throws Exception {
 
         s_logger.info("Executing copyTemplateFromSecondaryToPrimary. secondaryStorage: " + secondaryStorageUrl + ", templatePathAtSecondaryStorage: " +
                 templatePathAtSecondaryStorage + ", templateName: " + templateName);
@@ -532,7 +532,7 @@ public class VmwareStorageProcessor implements StorageProcessor {
         }
     }
 
-    private Pair<String, String> copyVolumeFromSecStorage(VmwareHypervisorHost hyperHost, String srcVolumePath, DatastoreMO dsMo, String secStorageUrl, long wait, String nfsVersion) throws Exception {
+    private Pair<String, String> copyVolumeFromSecStorage(VmwareHypervisorHost hyperHost, String srcVolumePath, DatastoreMO dsMo, String secStorageUrl, long wait, Integer nfsVersion) throws Exception {
 
         String volumeFolder = null;
         String volumeName = null;
@@ -552,7 +552,7 @@ public class VmwareStorageProcessor implements StorageProcessor {
         return new Pair<String, String>(volumeFolder, newVolume);
     }
 
-    private String deleteVolumeDirOnSecondaryStorage(String volumeDir, String secStorageUrl, String nfsVersion) throws Exception {
+    private String deleteVolumeDirOnSecondaryStorage(String volumeDir, String secStorageUrl, Integer nfsVersion) throws Exception {
         String secondaryMountPoint = mountService.getMountPoint(secStorageUrl, nfsVersion);
         String volumeMountRoot = secondaryMountPoint + File.separator + volumeDir;
 
@@ -734,7 +734,7 @@ public class VmwareStorageProcessor implements StorageProcessor {
     }
 
     private Ternary<String, Long, Long> createTemplateFromVolume(VirtualMachineMO vmMo, String installPath, long templateId, String templateUniqueName,
-            String secStorageUrl, String volumePath, String workerVmName, String nfsVersion) throws Exception {
+            String secStorageUrl, String volumePath, String workerVmName, Integer nfsVersion) throws Exception {
 
         String secondaryMountPoint = mountService.getMountPoint(secStorageUrl, nfsVersion);
         String installFullPath = secondaryMountPoint + "/" + installPath;
@@ -899,7 +899,7 @@ public class VmwareStorageProcessor implements StorageProcessor {
     }
 
     private Ternary<String, Long, Long> createTemplateFromSnapshot(String installPath, String templateUniqueName, String secStorageUrl, String snapshotPath,
-            Long templateId, long wait, String nfsVersion) throws Exception {
+            Long templateId, long wait, Integer nfsVersion) throws Exception {
         //Snapshot path is decoded in this form: /snapshots/account/volumeId/uuid/uuid
         String backupSSUuid;
         String snapshotFolder;
@@ -1066,7 +1066,7 @@ public class VmwareStorageProcessor implements StorageProcessor {
 
     // return Pair<String(divice bus name), String[](disk chain)>
     private Pair<String, String[]> exportVolumeToSecondaryStroage(VirtualMachineMO vmMo, String volumePath, String secStorageUrl, String secStorageDir,
-            String exportName, String workerVmName, String nfsVersion) throws Exception {
+            String exportName, String workerVmName, Integer nfsVersion) throws Exception {
 
         String secondaryMountPoint = mountService.getMountPoint(secStorageUrl, nfsVersion);
         String exportPath = secondaryMountPoint + "/" + secStorageDir + "/" + exportName;
@@ -1110,7 +1110,7 @@ public class VmwareStorageProcessor implements StorageProcessor {
 
     // Ternary<String(backup uuid in secondary storage), String(device bus name), String[](original disk chain in the snapshot)>
     private Ternary<String, String, String[]> backupSnapshotToSecondaryStorage(VirtualMachineMO vmMo, String installPath, String volumePath, String snapshotUuid,
-            String secStorageUrl, String prevSnapshotUuid, String prevBackupUuid, String workerVmName, String nfsVersion) throws Exception {
+            String secStorageUrl, String prevSnapshotUuid, String prevBackupUuid, String workerVmName, Integer nfsVersion) throws Exception {
 
         String backupUuid = UUID.randomUUID().toString();
         Pair<String, String[]> snapshotInfo = exportVolumeToSecondaryStroage(vmMo, volumePath, secStorageUrl, installPath, backupUuid, workerVmName, nfsVersion);
@@ -2212,7 +2212,7 @@ public class VmwareStorageProcessor implements StorageProcessor {
     }
 
     private Long restoreVolumeFromSecStorage(VmwareHypervisorHost hyperHost, DatastoreMO primaryDsMo, String newVolumeName, String secStorageUrl, String secStorageDir,
-            String backupName, long wait, String nfsVersion) throws Exception {
+            String backupName, long wait, Integer nfsVersion) throws Exception {
 
         String secondaryMountPoint = mountService.getMountPoint(secStorageUrl, null);
         String srcOVAFileName = null;
@@ -2389,4 +2389,9 @@ public class VmwareStorageProcessor implements StorageProcessor {
     private String getLegacyVmDataDiskController() throws Exception {
         return DiskControllerType.lsilogic.toString();
     }
+
+    public void setNfsVersion(Integer nfsVersion){
+        this._nfsVersion = nfsVersion;
+        s_logger.debug("VmwareProcessor instance now using NFS version: " + nfsVersion);
+    }
 }

plugins/hypervisors/vmware/src/com/cloud/storage/resource/VmwareStorageSubsystemCommandHandler.java

@@ -43,7 +43,7 @@ public class VmwareStorageSubsystemCommandHandler extends StorageSubsystemComman
     private static final Logger s_logger = Logger.getLogger(VmwareStorageSubsystemCommandHandler.class);
     private VmwareStorageManager storageManager;
     private PremiumSecondaryStorageResource storageResource;
-    private String _nfsVersion;
+    private Integer _nfsVersion;
 
     public PremiumSecondaryStorageResource getStorageResource() {
         return storageResource;
@@ -61,11 +61,28 @@ public class VmwareStorageSubsystemCommandHandler extends StorageSubsystemComman
         this.storageManager = storageManager;
     }
 
-    public VmwareStorageSubsystemCommandHandler(StorageProcessor processor, String nfsVersion) {
+    public VmwareStorageSubsystemCommandHandler(StorageProcessor processor, Integer nfsVersion) {
         super(processor);
         this._nfsVersion = nfsVersion;
     }
 
+    /**
+     * Reconfigure NFS version for storage operations
+     * @param nfsVersion NFS version to set
+     * @return true if NFS version could be configured, false in other case
+     */
+    public boolean reconfigureNfsVersion(Integer nfsVersion){
+        try {
+            VmwareStorageProcessor processor = (VmwareStorageProcessor) this.processor;
+            processor.setNfsVersion(nfsVersion);
+            this._nfsVersion = nfsVersion;
+            return true;
+        } catch (Exception e){
+            s_logger.error("Error while reconfiguring NFS version " + nfsVersion);
+            return false;
+        }
+    }
+
     @Override
     protected Answer execute(CopyCommand cmd) {
         DataTO srcData = cmd.getSrcTO();

plugins/hypervisors/vmware/test/com/cloud/hypervisor/vmware/resource/VmwareResourceTest.java

@@ -20,17 +20,27 @@ import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 import static org.mockito.Mockito.mock;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
 import static org.mockito.Mockito.any;
+import static org.mockito.Mockito.never;
 
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.Map;
 
+import org.apache.cloudstack.storage.command.CopyCommand;
 import org.junit.Before;
 import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.InjectMocks;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
 import org.mockito.Spy;
+import org.powermock.api.mockito.PowerMockito;
+import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.powermock.modules.junit4.PowerMockRunner;
 
 import com.vmware.vim25.VirtualDevice;
 import com.vmware.vim25.VirtualMachineConfigSpec;
@@ -38,14 +48,26 @@ import com.vmware.vim25.VirtualMachineVideoCard;
 import com.cloud.agent.api.Command;
 import com.cloud.agent.api.ScaleVmAnswer;
 import com.cloud.agent.api.ScaleVmCommand;
+import com.cloud.agent.api.to.DataTO;
+import com.cloud.agent.api.to.NfsTO;
 import com.cloud.agent.api.to.VirtualMachineTO;
 import com.cloud.hypervisor.vmware.mo.VirtualMachineMO;
 import com.cloud.hypervisor.vmware.mo.VmwareHypervisorHost;
 import com.cloud.hypervisor.vmware.util.VmwareContext;
+import com.cloud.storage.resource.VmwareStorageProcessor;
+import com.cloud.storage.resource.VmwareStorageSubsystemCommandHandler;
 
+@RunWith(PowerMockRunner.class)
+@PrepareForTest(CopyCommand.class)
 public class VmwareResourceTest {
 
+    @Mock
+    VmwareStorageProcessor storageProcessor;
+    @Mock
+    VmwareStorageSubsystemCommandHandler storageHandler;
+
     @Spy
+    @InjectMocks
     VmwareResource _resource = new VmwareResource() {
 
         @Override
@@ -57,6 +79,7 @@ public class VmwareResourceTest {
         public VmwareHypervisorHost getHyperHost(VmwareContext context, Command cmd) {
             return hyperHost;
         }
+
     };
 
     @Mock
@@ -75,12 +98,25 @@ public class VmwareResourceTest {
     VirtualMachineMO vmMo3dgpu;
     @Mock
     VirtualMachineTO vmSpec3dgpu;
+    @Mock
+    DataTO srcDataTO;
+    @Mock
+    NfsTO srcDataNfsTO;
+
+    CopyCommand storageCmd;
+
+    private static final Integer NFS_VERSION = Integer.valueOf(3);
+    private static final Integer NFS_VERSION_NOT_PRESENT = null;
 
     @Before
     public void setup() {
         MockitoAnnotations.initMocks(this);
+        storageCmd = PowerMockito.mock(CopyCommand.class);
         doReturn(context).when(_resource).getServiceContext(null);
         when(cmd.getVirtualMachine()).thenReturn(vmSpec);
+        when(storageCmd.getSrcTO()).thenReturn(srcDataTO);
+        when(srcDataTO.getDataStore()).thenReturn(srcDataNfsTO);
+        when(srcDataNfsTO.getNfsVersion()).thenReturn(NFS_VERSION);
     }
 
     //Test successful scaling up the vm
@@ -117,4 +153,61 @@ public class VmwareResourceTest {
         verify(vmMo3dgpu).configureVm(any(VirtualMachineConfigSpec.class));
     }
 
-}
+    // ---------------------------------------------------------------------------------------------------
+
+    @Test
+    public void testgetNfsVersionFromNfsTONull(){
+        assertFalse(_resource.getStorageNfsVersionFromNfsTO(null));
+    }
+
+    @Test
+    public void testgetNfsVersionFromNfsTONfsVersionNull(){
+        when(srcDataNfsTO.getNfsVersion()).thenReturn(NFS_VERSION_NOT_PRESENT);
+        assertFalse(_resource.getStorageNfsVersionFromNfsTO(srcDataNfsTO));
+    }
+
+    @Test
+    public void testgetNfsVersionFromNfsTONfsVersion(){
+        assertTrue(_resource.getStorageNfsVersionFromNfsTO(srcDataNfsTO));
+    }
+
+    // ---------------------------------------------------------------------------------------------------
+
+    @Test
+    public void testSetCurrentNfsVersionInProcessorAndHandler(){
+        _resource.setCurrentNfsVersionInProcessorAndHandler();
+        verify(storageHandler).reconfigureNfsVersion(any(Integer.class));
+    }
+
+    // ---------------------------------------------------------------------------------------------------
+
+    @Test
+    public void testExamineStorageSubSystemCommandNfsVersionNotPresent(){
+        when(srcDataNfsTO.getNfsVersion()).thenReturn(NFS_VERSION_NOT_PRESENT);
+        _resource.examineStorageSubSystemCommandNfsVersion(storageCmd);
+        verify(_resource, never()).setCurrentNfsVersionInProcessorAndHandler();
+    }
+
+    @Test
+    public void testExamineStorageSubSystemCommandNfsVersion(){
+        _resource.examineStorageSubSystemCommandNfsVersion(storageCmd);
+        verify(_resource).setCurrentNfsVersionInProcessorAndHandler();
+    }
+
+    // ---------------------------------------------------------------------------------------------------
+
+    @Test
+    public void checkStorageProcessorAndHandlerNfsVersionAttributeVersionNotSet(){
+        _resource.checkStorageProcessorAndHandlerNfsVersionAttribute(storageCmd);
+        verify(_resource).examineStorageSubSystemCommandNfsVersion(storageCmd);
+        assertEquals(NFS_VERSION, _resource.storageNfsVersion);
+    }
+
+    @Test
+    public void checkStorageProcessorAndHandlerNfsVersionAttributeVersionSet(){
+        _resource.storageNfsVersion = NFS_VERSION;
+        _resource.checkStorageProcessorAndHandlerNfsVersionAttribute(storageCmd);
+        verify(_resource, never()).examineStorageSubSystemCommandNfsVersion(storageCmd);
+    }
+
+}

plugins/storage/image/default/src/org/apache/cloudstack/storage/datastore/driver/CloudStackImageStoreDriverImpl.java

@@ -32,7 +32,7 @@ import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
 import org.apache.cloudstack.engine.subsystem.api.storage.EndPoint;
 import org.apache.cloudstack.engine.subsystem.api.storage.EndPointSelector;
 import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
-import org.apache.cloudstack.storage.image.BaseImageStoreDriverImpl;
+import org.apache.cloudstack.storage.image.NfsImageStoreDriverImpl;
 import org.apache.cloudstack.storage.image.datastore.ImageStoreEntity;
 import org.apache.cloudstack.storage.image.store.ImageStoreImpl;
 
@@ -44,7 +44,7 @@ import com.cloud.configuration.Config;
 import com.cloud.storage.Storage.ImageFormat;
 import com.cloud.utils.exception.CloudRuntimeException;
 
-public class CloudStackImageStoreDriverImpl extends BaseImageStoreDriverImpl {
+public class CloudStackImageStoreDriverImpl extends NfsImageStoreDriverImpl {
     private static final Logger s_logger = Logger.getLogger(CloudStackImageStoreDriverImpl.class);
 
     @Inject
@@ -60,6 +60,7 @@ public class CloudStackImageStoreDriverImpl extends BaseImageStoreDriverImpl {
         NfsTO nfsTO = new NfsTO();
         nfsTO.setRole(store.getRole());
         nfsTO.setUrl(nfsStore.getUri());
+        nfsTO.setNfsVersion(getNfsVersion(nfsStore.getId()));
         return nfsTO;
     }
 

server/src/com/cloud/server/StatsCollector.java

@@ -770,7 +770,8 @@ public class StatsCollector extends ManagerBase implements ComponentMethodInterc
                         continue;
                     }
 
-                    GetStorageStatsCommand command = new GetStorageStatsCommand(store.getTO(), imageStoreDetailsUtil.getNfsVersion(store.getId()));
+                    Integer nfsVersion = imageStoreDetailsUtil.getNfsVersion(store.getId());
+                    GetStorageStatsCommand command = new GetStorageStatsCommand(store.getTO(), nfsVersion);
                     EndPoint ssAhost = _epSelector.select(store);
                     if (ssAhost == null) {
                         s_logger.debug("There is no secondary storage VM for secondary storage host " + store.getName());

server/src/com/cloud/storage/ImageStoreDetailsUtil.java

@@ -38,7 +38,7 @@ public class ImageStoreDetailsUtil {
      * @return {@code null} if {@code nfs.version} is not found for storeId <br/>
      * {@code X} if {@code nfs.version} is found found for storeId
      */
-    public String getNfsVersion(long storeId) {
+    public Integer getNfsVersion(long storeId) throws NumberFormatException {
         String nfsVersion = null;
         if (imageStoreDetailsDao.getDetails(storeId) != null){
             Map<String, String> storeDetails = imageStoreDetailsDao.getDetails(storeId);
@@ -46,7 +46,7 @@ public class ImageStoreDetailsUtil {
                 nfsVersion = storeDetails.get("nfs.version");
             }
         }
-        return nfsVersion;
+        return (nfsVersion != null ? Integer.valueOf(nfsVersion) : null);
     }
 
     /**
@@ -56,7 +56,7 @@ public class ImageStoreDetailsUtil {
      * @return {@code null} if {@code nfs.version} is not found for storeUuid <br/>
      * {@code X} if {@code nfs.version} is found found for storeUuid
      */
-    public String getNfsVersionByUuid(String storeUuid){
+    public Integer getNfsVersionByUuid(String storeUuid){
         ImageStoreVO imageStore = imageStoreDao.findByUuid(storeUuid);
         if (imageStore != null){
             return getNfsVersion(imageStore.getId());

server/test/com/cloud/storage/ImageStoreDetailsUtilTest.java

@@ -34,7 +34,7 @@ public class ImageStoreDetailsUtilTest {
 
     private final static long STORE_ID = 1l;
     private final static String STORE_UUID = "aaaa-aaaa-aaaa-aaaa";
-    private final static String NFS_VERSION = "3";
+    private final static Integer NFS_VERSION = 3;
 
     ImageStoreDetailsUtil imageStoreDetailsUtil = new ImageStoreDetailsUtil();
 
@@ -44,7 +44,7 @@ public class ImageStoreDetailsUtilTest {
     @Before
     public void setup() throws Exception {
         Map<String, String> imgStoreDetails = new HashMap<String, String>();
-        imgStoreDetails.put("nfs.version", NFS_VERSION);
+        imgStoreDetails.put("nfs.version", String.valueOf(NFS_VERSION));
         when(imgStoreDetailsDao.getDetails(STORE_ID)).thenReturn(imgStoreDetails);
 
         ImageStoreVO imgStoreVO = mock(ImageStoreVO.class);
@@ -57,7 +57,7 @@ public class ImageStoreDetailsUtilTest {
 
     @Test
     public void testGetNfsVersion(){
-        String nfsVersion = imageStoreDetailsUtil.getNfsVersion(STORE_ID);
+        Integer nfsVersion = imageStoreDetailsUtil.getNfsVersion(STORE_ID);
         assertEquals(NFS_VERSION, nfsVersion);
     }
 
@@ -67,7 +67,7 @@ public class ImageStoreDetailsUtilTest {
         imgStoreDetails.put("other.prop", "propValue");
         when(imgStoreDetailsDao.getDetails(STORE_ID)).thenReturn(imgStoreDetails);
 
-        String nfsVersion = imageStoreDetailsUtil.getNfsVersion(STORE_ID);
+        Integer nfsVersion = imageStoreDetailsUtil.getNfsVersion(STORE_ID);
         assertNull(nfsVersion);
     }
 
@@ -76,20 +76,20 @@ public class ImageStoreDetailsUtilTest {
         Map<String, String> imgStoreDetails = new HashMap<String, String>();
         when(imgStoreDetailsDao.getDetails(STORE_ID)).thenReturn(imgStoreDetails);
 
-        String nfsVersion = imageStoreDetailsUtil.getNfsVersion(STORE_ID);
+        Integer nfsVersion = imageStoreDetailsUtil.getNfsVersion(STORE_ID);
         assertNull(nfsVersion);
     }
 
     @Test
     public void testGetNfsVersionByUuid(){
-        String nfsVersion = imageStoreDetailsUtil.getNfsVersionByUuid(STORE_UUID);
+        Integer nfsVersion = imageStoreDetailsUtil.getNfsVersionByUuid(STORE_UUID);
         assertEquals(NFS_VERSION, nfsVersion);
     }
 
     @Test
     public void testGetNfsVersionByUuidNoImgStore(){
         when(imgStoreDao.findByUuid(STORE_UUID)).thenReturn(null);
-        String nfsVersion = imageStoreDetailsUtil.getNfsVersionByUuid(STORE_UUID);
+        Integer nfsVersion = imageStoreDetailsUtil.getNfsVersionByUuid(STORE_UUID);
         assertNull(nfsVersion);
     }
 }

services/secondary-storage/controller/src/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java

@@ -313,7 +313,8 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar
                     setupCmd = new SecStorageSetupCommand(ssStore.getTO(), secUrl, certs);
                 }
 
-                setupCmd.setNfsVersion(imageStoreDetailsUtil.getNfsVersion(ssStore.getId()));
+                Integer nfsVersion = imageStoreDetailsUtil.getNfsVersion(ssStore.getId());
+                setupCmd.setNfsVersion(nfsVersion);
 
                 //template/volume file upload key
                 String postUploadKey = _configDao.getValue(Config.SSVMPSK.key());
@@ -1135,7 +1136,7 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar
         if (dc.getDns2() != null) {
             buf.append(" dns2=").append(dc.getDns2());
         }
-        String nfsVersion = imageStoreDetailsUtil != null ? imageStoreDetailsUtil.getNfsVersion(secStore.getId()) : null;
+        Integer nfsVersion = imageStoreDetailsUtil != null ? imageStoreDetailsUtil.getNfsVersion(secStore.getId()) : null;
         buf.append(" nfsVersion=").append(nfsVersion);
 
         String bootArgs = buf.toString();

services/secondary-storage/server/src/org/apache/cloudstack/storage/resource/LocalNfsSecondaryStorageResource.java

@@ -53,7 +53,7 @@ public class LocalNfsSecondaryStorageResource extends NfsSecondaryStorageResourc
     }
 
     @Override
-    synchronized public String getRootDir(String secUrl, String nfsVersion) {
+    synchronized public String getRootDir(String secUrl, Integer nfsVersion) {
         try {
             URI uri = new URI(secUrl);
             String dir = mountUri(uri, nfsVersion);
@@ -66,7 +66,7 @@ public class LocalNfsSecondaryStorageResource extends NfsSecondaryStorageResourc
     }
 
     @Override
-    protected void mount(String localRootPath, String remoteDevice, URI uri, String nfsVersion) {
+    protected void mount(String localRootPath, String remoteDevice, URI uri, Integer nfsVersion) {
         ensureLocalRootPathExists(localRootPath, uri);
 
         if (mountExists(localRootPath, uri)) {

services/secondary-storage/server/src/org/apache/cloudstack/storage/resource/LocalSecondaryStorageResource.java

@@ -72,7 +72,7 @@ public class LocalSecondaryStorageResource extends ServerResourceBase implements
     }
 
     @Override
-    public String getRootDir(String url, String nfsVersion) {
+    public String getRootDir(String url, Integer nfsVersion) {
         return getRootDir();
 
     }

services/secondary-storage/server/src/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java

@@ -205,7 +205,7 @@ public class NfsSecondaryStorageResource extends ServerResourceBase implements S
     private String _storageIp;
     private String _storageNetmask;
     private String _storageGateway;
-    private String _nfsVersion;
+    private Integer _nfsVersion;
     private final List<String> nfsIps = new ArrayList<String>();
     protected String _parent = "/mnt/SecStorage";
     final private String _tmpltpp = "template.properties";
@@ -229,6 +229,26 @@ public class NfsSecondaryStorageResource extends ServerResourceBase implements S
         _inSystemVM = inSystemVM;
     }
 
+    /**
+     * Retrieve converted "nfsVersion" value from params
+     * @param params
+     * @return nfsVersion value if exists, null in other case
+     */
+    public static Integer retrieveNfsVersionFromParams(Map<String, Object> params){
+        Integer nfsVersion = null;
+        if (params.get("nfsVersion") != null){
+            String nfsVersionParam = (String)params.get("nfsVersion");
+            try {
+                nfsVersion = Integer.valueOf(nfsVersionParam);
+            }
+            catch (NumberFormatException e){
+                s_logger.error("Couldn't cast " + nfsVersionParam + " to integer");
+                return null;
+            }
+        }
+        return nfsVersion;
+    }
+
     @Override
     public Answer executeRequest(Command cmd) {
         if (cmd instanceof DownloadProgressCommand) {
@@ -559,7 +579,7 @@ public class NfsSecondaryStorageResource extends ServerResourceBase implements S
         return new CopyCmdAnswer("");
     }
 
-    protected File getFile(String path, String nfsPath, String nfsVersion) {
+    protected File getFile(String path, String nfsPath, Integer nfsVersion) {
         String filePath = getRootDir(nfsPath, nfsVersion) + File.separator + path;
         File f = new File(filePath);
         if (!f.exists()) {
@@ -688,7 +708,7 @@ public class NfsSecondaryStorageResource extends ServerResourceBase implements S
         return Long.parseLong(StringUtils.substringAfterLast(StringUtils.substringBeforeLast(key, S3Utils.SEPARATOR), S3Utils.SEPARATOR));
     }
 
-    private String determineStorageTemplatePath(final String storagePath, String dataPath, String nfsVersion) {
+    private String determineStorageTemplatePath(final String storagePath, String dataPath, Integer nfsVersion) {
         return join(asList(getRootDir(storagePath, nfsVersion), dataPath), File.separator);
     }
 
@@ -2046,7 +2066,7 @@ public class NfsSecondaryStorageResource extends ServerResourceBase implements S
     }
 
     @Override
-    synchronized public String getRootDir(String secUrl, String nfsVersion) {
+    synchronized public String getRootDir(String secUrl, Integer nfsVersion) {
         if (!_inSystemVM) {
             return _parent;
         }
@@ -2209,7 +2229,7 @@ public class NfsSecondaryStorageResource extends ServerResourceBase implements S
             startAdditionalServices();
             _params.put("install.numthreads", "50");
             _params.put("secondary.storage.vm", "true");
-            _nfsVersion = (String)params.get("nfsVersion");
+            _nfsVersion = retrieveNfsVersionFromParams(params);
         }
 
         try {
@@ -2385,10 +2405,11 @@ public class NfsSecondaryStorageResource extends ServerResourceBase implements S
      *            crresponding to the remote device. Will throw for unsupported
      *            scheme.
      * @param imgStoreId
+     * @param nfsVersion NFS version to use in mount command
      * @return name of folder in _parent that device was mounted.
      * @throws UnknownHostException
      */
-    protected String mountUri(URI uri, String nfsVersion) throws UnknownHostException {
+    protected String mountUri(URI uri, Integer nfsVersion) throws UnknownHostException {
         String uriHostIp = getUriHostIp(uri);
         String nfsPath = uriHostIp + ":" + uri.getPath();
 
@@ -2432,7 +2453,7 @@ public class NfsSecondaryStorageResource extends ServerResourceBase implements S
         s_logger.debug("Successfully umounted " + localRootPath);
     }
 
-    protected void mount(String localRootPath, String remoteDevice, URI uri, String nfsVersion) {
+    protected void mount(String localRootPath, String remoteDevice, URI uri, Integer nfsVersion) {
         s_logger.debug("mount " + uri.toString() + " on " + localRootPath + ((nfsVersion != null) ? " nfsVersion="+nfsVersion : ""));
         ensureLocalRootPathExists(localRootPath, uri);
 
@@ -2448,7 +2469,7 @@ public class NfsSecondaryStorageResource extends ServerResourceBase implements S
         checkForVolumesDir(localRootPath);
     }
 
-    protected void attemptMount(String localRootPath, String remoteDevice, URI uri, String nfsVersion) {
+    protected void attemptMount(String localRootPath, String remoteDevice, URI uri, Integer nfsVersion) {
         String result;
         s_logger.debug("Make cmdline call to mount " + remoteDevice + " at " + localRootPath + " based on uri " + uri
                 + ((nfsVersion != null) ? " nfsVersion=" + nfsVersion : ""));

services/secondary-storage/server/src/org/apache/cloudstack/storage/resource/SecondaryStorageResource.java

@@ -23,6 +23,6 @@ import com.cloud.resource.ServerResource;
  */
 public interface SecondaryStorageResource extends ServerResource {
 
-    String getRootDir(String cmd, String nfsVersion);
+    String getRootDir(String cmd, Integer nfsVersion);
 
 }

services/secondary-storage/server/src/org/apache/cloudstack/storage/template/DownloadManagerImpl.java

@@ -44,6 +44,7 @@ import org.apache.cloudstack.storage.command.DownloadCommand;
 import org.apache.cloudstack.storage.command.DownloadCommand.ResourceType;
 import org.apache.cloudstack.storage.command.DownloadProgressCommand;
 import org.apache.cloudstack.storage.command.DownloadProgressCommand.RequestType;
+import org.apache.cloudstack.storage.resource.NfsSecondaryStorageResource;
 import org.apache.cloudstack.storage.resource.SecondaryStorageResource;
 import org.apache.log4j.Logger;
 
@@ -89,7 +90,7 @@ public class DownloadManagerImpl extends ManagerBase implements DownloadManager
     StorageLayer _storage;
     public Map<String, Processor> _processors;
 
-    private String _nfsVersion;
+    private Integer _nfsVersion;
 
     public class Completion implements DownloadCompleteCallback {
         private final String jobId;
@@ -984,7 +985,7 @@ public class DownloadManagerImpl extends ManagerBase implements DownloadManager
         String inSystemVM = (String)params.get("secondary.storage.vm");
         if (inSystemVM != null && "true".equalsIgnoreCase(inSystemVM)) {
             s_logger.info("DownloadManager: starting additional services since we are inside system vm");
-            _nfsVersion = (String)params.get("nfsVersion");
+            _nfsVersion = NfsSecondaryStorageResource.retrieveNfsVersionFromParams(params);
             startAdditionalServices();
             blockOutgoingOnPrivate();
         }