Merge pull request #1615 from nvazquez/nfsConfigKey

CLOUDSTACK-9438: Fix for CLOUDSTACK-9252 - Make NFS version changeable in UIJIRA TICKET: https://issues.apache.org/jira/browse/CLOUDSTACK-9438

### Introduction

From #1361 it was possible to configure NFS version for secondary storage mount.
However, changing NFS version requires inserting an new detail on `image_store_details` table, with `name = 'nfs.version'` and `value = X` where X is desired NFS version, and then restarting management server for changes to take effect.

Our improvement aims to make NFS version changeable from UI, instead of previously described workflow.

### Proposed solution
Basically, NFS version is defined as an image store ConfigKey, this implied:
* Adding a new Config scope: **ImageStore**
* Make `ImageStoreDetailsDao` class to extend `ResourceDetailsDaoBase` and `ImageStoreDetailVO` implement `ResourceDetail`
* Insert `'display'` column on `image_store_details` table
* Extending `ListCfgsCmd` and `UpdateCfgCmd` to support **ImageStore** scope, which implied:
** Injecting `ImageStoreDetailsDao` and `ImageStoreDao` on `ConfigurationManagerImpl` class, on `cloud-server` module.

### Important
It is important to mention that `ImageStoreDaoImpl` and `ImageStoreDetailsDaoImpl` classes were moved from `cloud-engine-storage` to `cloud-engine-schema` module in order to Spring find those beans to inject on `ConfigurationManagerImpl` in `cloud-server` module.

We had this maven dependencies between modules:
* `cloud-server --> cloud-engine-schema`
* `cloud-engine-storage --> cloud-secondary-storage --> cloud-server`

As `ImageStoreDaoImpl` and `ImageStoreDetailsDao` were defined in `cloud-engine-storage`, and they needed in `cloud-server` module, to be injected on `ConfigurationManagerImpl`, if we added dependency from `cloud-server` to `cloud-engine-storage` we would introduce a dependency cycle. To avoid this cycle, we moved those classes to `cloud-engine-schema` module

* pr/1615:
  CLOUDSTACK-9438: Fix for CLOUDSTACK-9252 - Make NFS version changeable in UI

Signed-off-by: Rajani Karuturi <rajani.karuturi@accelerite.com>

api/src/org/apache/cloudstack/api/command/admin/config/ListCfgsByCmd.java

@@ -28,6 +28,7 @@ import org.apache.cloudstack.api.Parameter;
 import org.apache.cloudstack.api.response.AccountResponse;
 import org.apache.cloudstack.api.response.ClusterResponse;
 import org.apache.cloudstack.api.response.ConfigurationResponse;
+import org.apache.cloudstack.api.response.ImageStoreResponse;
 import org.apache.cloudstack.api.response.ListResponse;
 import org.apache.cloudstack.api.response.StoragePoolResponse;
 import org.apache.cloudstack.api.response.ZoneResponse;
@@ -76,6 +77,12 @@ public class ListCfgsByCmd extends BaseListCmd {
                description = "the ID of the Account to update the parameter value for corresponding account")
     private Long accountId;
 
+    @Parameter(name = ApiConstants.IMAGE_STORE_UUID,
+            type = CommandType.UUID,
+            entityType = ImageStoreResponse.class,
+            description = "the ID of the Image Store to update the parameter value for corresponding image store")
+    private Long imageStoreId;
+
     // ///////////////////////////////////////////////////
     // ///////////////// Accessors ///////////////////////
     // ///////////////////////////////////////////////////
@@ -104,6 +111,10 @@ public class ListCfgsByCmd extends BaseListCmd {
         return accountId;
     }
 
+    public Long getImageStoreId() {
+        return imageStoreId;
+    }
+
     @Override
     public Long getPageSizeVal() {
         Long defaultPageSize = 500L;
@@ -147,6 +158,9 @@ public class ListCfgsByCmd extends BaseListCmd {
             if (getAccountId() != null) {
                 cfgResponse.setScope("account");
             }
+            if (getImageStoreId() != null){
+                cfgResponse.setScope("imagestore");
+            }
             configResponses.add(cfgResponse);
         }
 

api/src/org/apache/cloudstack/api/command/admin/config/UpdateCfgCmd.java

@@ -19,8 +19,8 @@ package org.apache.cloudstack.api.command.admin.config;
 import com.google.common.base.Strings;
 import org.apache.cloudstack.acl.RoleService;
 import org.apache.log4j.Logger;
-
 import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiArgValidator;
 import org.apache.cloudstack.api.ApiConstants;
 import org.apache.cloudstack.api.ApiErrorCode;
 import org.apache.cloudstack.api.BaseCmd;
@@ -29,6 +29,7 @@ import org.apache.cloudstack.api.ServerApiException;
 import org.apache.cloudstack.api.response.AccountResponse;
 import org.apache.cloudstack.api.response.ClusterResponse;
 import org.apache.cloudstack.api.response.ConfigurationResponse;
+import org.apache.cloudstack.api.response.ImageStoreResponse;
 import org.apache.cloudstack.api.response.StoragePoolResponse;
 import org.apache.cloudstack.api.response.ZoneResponse;
 import org.apache.cloudstack.config.Configuration;
@@ -75,6 +76,13 @@ public class UpdateCfgCmd extends BaseCmd {
                description = "the ID of the Account to update the parameter value for corresponding account")
     private Long accountId;
 
+    @Parameter(name = ApiConstants.IMAGE_STORE_UUID,
+            type = CommandType.UUID,
+            entityType = ImageStoreResponse.class,
+            description = "the ID of the Image Store to update the parameter value for corresponding image store",
+            validations = ApiArgValidator.PositiveNumber)
+    private Long imageStoreId;
+
     /////////////////////////////////////////////////////
     /////////////////// Accessors ///////////////////////
     /////////////////////////////////////////////////////
@@ -107,6 +115,10 @@ public class UpdateCfgCmd extends BaseCmd {
         return accountId;
     }
 
+    public Long getImageStoreId() {
+        return imageStoreId;
+    }
+
     /////////////////////////////////////////////////////
     /////////////// API Implementation///////////////////
     /////////////////////////////////////////////////////

engine/components-api/src/com/cloud/capacity/CapacityManager.java

@@ -73,6 +73,16 @@ public interface CapacityManager {
                     "If set to true, creates VMs as full clones on ESX hypervisor",
                     true,
                     ConfigKey.Scope.StoragePool);
+    static final ConfigKey<Integer> ImageStoreNFSVersion =
+            new ConfigKey<Integer>(
+                    Integer.class,
+                    "secstorage.nfs.version",
+                    "Advanced",
+                    null,
+                    "Enforces specific NFS version when mounting Secondary Storage. If NULL default selection is performed",
+                    true,
+                    ConfigKey.Scope.ImageStore,
+                    null);
 
     public boolean releaseVmCapacity(VirtualMachine vm, boolean moveFromReserved, boolean moveToReservered, Long hostId);
 

engine/schema/resources/META-INF/cloudstack/core/spring-engine-schema-core-daos-context.xml

@@ -176,8 +176,8 @@
   <bean id="hostTagsDaoImpl" class="com.cloud.host.dao.HostTagsDaoImpl" />
   <bean id="hostTransferMapDaoImpl" class="com.cloud.cluster.agentlb.dao.HostTransferMapDaoImpl" />
   <bean id="iPAddressDaoImpl" class="com.cloud.network.dao.IPAddressDaoImpl" />
-  <bean id="imageStoreDaoImpl" class="org.apache.cloudstack.storage.image.db.ImageStoreDaoImpl" />
+  <bean id="imageStoreDaoImpl" class="org.apache.cloudstack.storage.datastore.db.ImageStoreDaoImpl" />
-  <bean id="imageStoreDetailsDaoImpl" class="org.apache.cloudstack.storage.image.db.ImageStoreDetailsDaoImpl" />
+  <bean id="imageStoreDetailsDaoImpl" class="org.apache.cloudstack.storage.datastore.db.ImageStoreDetailsDaoImpl" />
   <bean id="imageStoreJoinDaoImpl" class="com.cloud.api.query.dao.ImageStoreJoinDaoImpl" />
   <bean id="snapshotDataStoreDaoImpl" class="org.apache.cloudstack.storage.image.db.SnapshotDataStoreDaoImpl" />
   <bean id="templateDataStoreDaoImpl" class="org.apache.cloudstack.storage.image.db.TemplateDataStoreDaoImpl" />

engine/schema/src/org/apache/cloudstack/storage/datastore/db/ImageStoreDaoImpl.java

@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package org.apache.cloudstack.storage.image.db;
+package org.apache.cloudstack.storage.datastore.db;
 
 import java.util.List;
 import java.util.Map;
@@ -26,8 +26,6 @@ import javax.naming.ConfigurationException;
 import org.springframework.stereotype.Component;
 
 import org.apache.cloudstack.engine.subsystem.api.storage.ZoneScope;
-import org.apache.cloudstack.storage.datastore.db.ImageStoreDao;
-import org.apache.cloudstack.storage.datastore.db.ImageStoreVO;
 
 import com.cloud.storage.DataStoreRole;
 import com.cloud.storage.ScopeType;

engine/schema/src/org/apache/cloudstack/storage/datastore/db/ImageStoreDetailVO.java

@@ -23,18 +23,18 @@ import javax.persistence.GenerationType;
 import javax.persistence.Id;
 import javax.persistence.Table;
 
-import org.apache.cloudstack.api.InternalIdentity;
+import org.apache.cloudstack.api.ResourceDetail;
 
 @Entity
 @Table(name = "image_store_details")
-public class ImageStoreDetailVO implements InternalIdentity {
+public class ImageStoreDetailVO implements ResourceDetail {
     @Id
     @GeneratedValue(strategy = GenerationType.IDENTITY)
     @Column(name = "id")
     long id;
 
     @Column(name = "store_id")
-    long storeId;
+    long resourceId;
 
     @Column(name = "name")
     String name;
@@ -42,13 +42,17 @@ public class ImageStoreDetailVO implements InternalIdentity {
     @Column(name = "value")
     String value;
 
+    @Column(name = "display")
+    private boolean display = true;
+
     public ImageStoreDetailVO() {
     }
 
-    public ImageStoreDetailVO(long storeId, String name, String value) {
+    public ImageStoreDetailVO(long storeId, String name, String value, boolean display) {
-        this.storeId = storeId;
+        this.resourceId = storeId;
         this.name = name;
         this.value = value;
+        this.display = display;
     }
 
     @Override
@@ -56,28 +60,24 @@ public class ImageStoreDetailVO implements InternalIdentity {
         return id;
     }
 
-    public long getStoreId() {
+    @Override
-        return storeId;
+    public long getResourceId() {
-    }
+        return resourceId;
-
-    public void setStoreId(long storeId) {
-        this.storeId = storeId;
     }
 
+    @Override
     public String getName() {
         return name;
     }
 
-    public void setName(String name) {
+    @Override
-        this.name = name;
-    }
-
     public String getValue() {
         return value;
     }
 
-    public void setValue(String value) {
+    @Override
-        this.value = value;
+    public boolean isDisplay() {
+        return display;
     }
 
 }

engine/schema/src/org/apache/cloudstack/storage/datastore/db/ImageStoreDetailsDao.java

@@ -18,9 +18,11 @@ package org.apache.cloudstack.storage.datastore.db;
 
 import java.util.Map;
 
+import org.apache.cloudstack.resourcedetail.ResourceDetailsDao;
+
 import com.cloud.utils.db.GenericDao;
 
-public interface ImageStoreDetailsDao extends GenericDao<ImageStoreDetailVO, Long> {
+public interface ImageStoreDetailsDao extends GenericDao<ImageStoreDetailVO, Long>, ResourceDetailsDao<ImageStoreDetailVO> {
 
     void update(long storeId, Map<String, String> details);
 

engine/schema/src/org/apache/cloudstack/storage/datastore/db/ImageStoreDetailsDaoImpl.java

@@ -14,34 +14,36 @@
 // 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.db;
+package org.apache.cloudstack.storage.datastore.db;
 
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
-
 import org.springframework.stereotype.Component;
 
-import org.apache.cloudstack.api.ApiConstants;
-import org.apache.cloudstack.storage.datastore.db.ImageStoreDetailVO;
-import org.apache.cloudstack.storage.datastore.db.ImageStoreDetailsDao;
-
 import com.cloud.utils.crypt.DBEncryptionUtil;
-import com.cloud.utils.db.GenericDaoBase;
+import com.cloud.utils.db.QueryBuilder;
 import com.cloud.utils.db.SearchBuilder;
 import com.cloud.utils.db.SearchCriteria;
+import com.cloud.utils.db.SearchCriteria.Op;
 import com.cloud.utils.db.TransactionLegacy;
 
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.framework.config.ConfigKey;
+import org.apache.cloudstack.framework.config.ConfigKey.Scope;
+import org.apache.cloudstack.framework.config.ScopedConfigStorage;
+import org.apache.cloudstack.resourcedetail.ResourceDetailsDaoBase;
+
 @Component
-public class ImageStoreDetailsDaoImpl extends GenericDaoBase<ImageStoreDetailVO, Long> implements ImageStoreDetailsDao {
+public class ImageStoreDetailsDaoImpl extends ResourceDetailsDaoBase<ImageStoreDetailVO> implements ImageStoreDetailsDao, ScopedConfigStorage {
 
     protected final SearchBuilder<ImageStoreDetailVO> storeSearch;
 
-    protected ImageStoreDetailsDaoImpl() {
+    public ImageStoreDetailsDaoImpl() {
         super();
         storeSearch = createSearchBuilder();
-        storeSearch.and("store", storeSearch.entity().getStoreId(), SearchCriteria.Op.EQ);
+        storeSearch.and("store", storeSearch.entity().getResourceId(), SearchCriteria.Op.EQ);
         storeSearch.done();
     }
 
@@ -54,7 +56,7 @@ public class ImageStoreDetailsDaoImpl extends GenericDaoBase<ImageStoreDetailVO,
         txn.start();
         expunge(sc);
         for (Map.Entry<String, String> entry : details.entrySet()) {
-            ImageStoreDetailVO detail = new ImageStoreDetailVO(storeId, entry.getKey(), entry.getValue());
+            ImageStoreDetailVO detail = new ImageStoreDetailVO(storeId, entry.getKey(), entry.getValue(), true);
             persist(detail);
         }
         txn.commit();
@@ -88,7 +90,30 @@ public class ImageStoreDetailsDaoImpl extends GenericDaoBase<ImageStoreDetailVO,
         for (ImageStoreDetailVO result : results) {
             remove(result.getId());
         }
+    }
 
+    @Override
+    public Scope getScope() {
+        return ConfigKey.Scope.ImageStore;
+    }
+
+    @Override
+    public ImageStoreDetailVO findDetail(long storeId, String name) {
+        QueryBuilder<ImageStoreDetailVO> sc = QueryBuilder.create(ImageStoreDetailVO.class);
+        sc.and(sc.entity().getResourceId(), Op.EQ, storeId);
+        sc.and(sc.entity().getName(), Op.EQ, name);
+        return sc.find();
+    }
+
+    @Override
+    public String getConfigValue(long id, ConfigKey<?> key) {
+        ImageStoreDetailVO vo = findDetail(id, key.key());
+        return vo == null ? null : vo.getValue();
+    }
+
+    @Override
+    public void addDetail(long resourceId, String key, String value, boolean display) {
+        super.addDetail(new ImageStoreDetailVO(resourceId, key, value, display));
     }
 
 }

engine/storage/integration-test/test/resource/fakeDriverTestContext.xml

@@ -35,8 +35,8 @@
         </property>
     </bean>
 
-    <bean id="imageStoreDaoImpl" class="org.apache.cloudstack.storage.image.db.ImageStoreDaoImpl" />
+    <bean id="imageStoreDaoImpl" class="org.apache.cloudstack.storage.datastore.db.ImageStoreDaoImpl" />
-    <bean id="imageStoreDetailsDaoImpl" class="org.apache.cloudstack.storage.image.db.ImageStoreDetailsDaoImpl" />
+    <bean id="imageStoreDetailsDaoImpl" class="org.apache.cloudstack.storage.datastore.db.ImageStoreDetailsDaoImpl" />
     <bean id="snapshotDataStoreDaoImpl" class="org.apache.cloudstack.storage.image.db.SnapshotDataStoreDaoImpl" />
     <bean id="templateDataStoreDaoImpl" class="org.apache.cloudstack.storage.image.db.TemplateDataStoreDaoImpl" />
     <bean id="volumeDataStoreDaoImpl" class="org.apache.cloudstack.storage.image.db.VolumeDataStoreDaoImpl" />

engine/storage/integration-test/test/resources/storageContext.xml

@@ -35,8 +35,8 @@
     </property>
   </bean>
 
-<bean id="imageStoreDaoImpl" class="org.apache.cloudstack.storage.image.db.ImageStoreDaoImpl" />
+<bean id="imageStoreDaoImpl" class="org.apache.cloudstack.storage.datastore.db.ImageStoreDaoImpl" />
-  <bean id="imageStoreDetailsDaoImpl" class="org.apache.cloudstack.storage.image.db.ImageStoreDetailsDaoImpl" /> 
+  <bean id="imageStoreDetailsDaoImpl" class="org.apache.cloudstack.storage.datastore.db.ImageStoreDetailsDaoImpl" /> 
   <bean id="snapshotDataStoreDaoImpl" class="org.apache.cloudstack.storage.image.db.SnapshotDataStoreDaoImpl" /> 
   <bean id="templateDataStoreDaoImpl" class="org.apache.cloudstack.storage.image.db.TemplateDataStoreDaoImpl" /> 
   <bean id="volumeDataStoreDaoImpl" class="org.apache.cloudstack.storage.image.db.VolumeDataStoreDaoImpl" />   

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

@@ -24,22 +24,23 @@ import javax.inject.Inject;
 
 import org.apache.cloudstack.storage.datastore.db.ImageStoreDetailsDao;
 
+import com.cloud.capacity.CapacityManager;
+
 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
+     * @return "secstorage.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 nfsVersionKey = CapacityManager.ImageStoreNFSVersion.key();
-            String nfsVersionParam = imgStoreDetails.get(NFS_VERSION_DETAILS_KEY);
+        if (imgStoreDetails != null && imgStoreDetails.containsKey(nfsVersionKey)){
+            String nfsVersionParam = imgStoreDetails.get(nfsVersionKey);
             return (nfsVersionParam != null ? Integer.valueOf(nfsVersionParam) : null);
         }
         return null;

engine/storage/src/org/apache/cloudstack/storage/image/datastore/ImageStoreHelper.java

@@ -129,15 +129,12 @@ public class ImageStoreHelper {
             Iterator<String> keyIter = details.keySet().iterator();
             while (keyIter.hasNext()) {
                 String key = keyIter.next().toString();
-                ImageStoreDetailVO detail = new ImageStoreDetailVO();
-                detail.setStoreId(store.getId());
-                detail.setName(key);
                 String value = details.get(key);
                 // encrypt swift key or s3 secret key
                 if (key.equals(ApiConstants.KEY) || key.equals(ApiConstants.S3_SECRET_KEY)) {
                     value = DBEncryptionUtil.encrypt(value);
                 }
-                detail.setValue(value);
+                ImageStoreDetailVO detail = new ImageStoreDetailVO(store.getId(), key, value, true);
                 imageStoreDetailsDao.persist(detail);
             }
         }

framework/config/src/org/apache/cloudstack/framework/config/ConfigKey.java

@@ -31,7 +31,7 @@ import com.cloud.utils.exception.CloudRuntimeException;
 public class ConfigKey<T> {
 
     public static enum Scope {
-        Global, Zone, Cluster, StoragePool, Account, ManagementServer
+        Global, Zone, Cluster, StoragePool, Account, ManagementServer, ImageStore
     }
 
     private final String _category;

framework/config/src/org/apache/cloudstack/framework/config/impl/ConfigDepotImpl.java

@@ -84,6 +84,7 @@ public class ConfigDepotImpl implements ConfigDepot, ConfigDepotAdmin {
         _scopeLevelConfigsMap.put(ConfigKey.Scope.Cluster, new HashSet<ConfigKey<?>>());
         _scopeLevelConfigsMap.put(ConfigKey.Scope.StoragePool, new HashSet<ConfigKey<?>>());
         _scopeLevelConfigsMap.put(ConfigKey.Scope.Account, new HashSet<ConfigKey<?>>());
+        _scopeLevelConfigsMap.put(ConfigKey.Scope.ImageStore, new HashSet<ConfigKey<?>>());
     }
 
     @Override

plugins/network-elements/juniper-contrail/test/org/apache/cloudstack/network/contrail/management/IntegrationTestConfiguration.java

@@ -80,8 +80,8 @@ import org.apache.cloudstack.region.dao.RegionDaoImpl;
 import org.apache.cloudstack.spring.lifecycle.registry.ExtensionRegistry;
 import org.apache.cloudstack.storage.datastore.PrimaryDataStoreProviderManager;
 import org.apache.cloudstack.storage.image.datastore.ImageStoreProviderManager;
+import org.apache.cloudstack.storage.datastore.db.ImageStoreDaoImpl;
 import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDaoImpl;
-import org.apache.cloudstack.storage.image.db.ImageStoreDaoImpl;
 import org.apache.cloudstack.storage.image.db.TemplateDataStoreDaoImpl;
 import org.apache.cloudstack.usage.UsageService;
 

server/src/com/cloud/capacity/CapacityManagerImpl.java

@@ -1101,6 +1101,6 @@ public class CapacityManagerImpl extends ManagerBase implements CapacityManager,
     @Override
     public ConfigKey<?>[] getConfigKeys() {
         return new ConfigKey<?>[] {CpuOverprovisioningFactor, MemOverprovisioningFactor, StorageCapacityDisableThreshold, StorageOverprovisioningFactor,
-            StorageAllocatedCapacityDisableThreshold, StorageOperationsExcludeCluster, VmwareCreateCloneFull};
+            StorageAllocatedCapacityDisableThreshold, StorageOperationsExcludeCluster, VmwareCreateCloneFull, ImageStoreNFSVersion};
     }
 }

server/src/com/cloud/configuration/ConfigurationManagerImpl.java

@@ -81,6 +81,9 @@ import org.apache.cloudstack.region.PortableIpVO;
 import org.apache.cloudstack.region.Region;
 import org.apache.cloudstack.region.RegionVO;
 import org.apache.cloudstack.region.dao.RegionDao;
+import org.apache.cloudstack.storage.datastore.db.ImageStoreDao;
+import org.apache.cloudstack.storage.datastore.db.ImageStoreDetailsDao;
+import org.apache.cloudstack.storage.datastore.db.ImageStoreVO;
 import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
 import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao;
 import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
@@ -216,6 +219,7 @@ import com.cloud.vm.dao.NicIpAliasDao;
 import com.cloud.vm.dao.NicIpAliasVO;
 import com.cloud.vm.dao.NicSecondaryIpDao;
 import com.cloud.vm.dao.VMInstanceDao;
+import com.google.common.base.Preconditions;
 
 public class ConfigurationManagerImpl extends ManagerBase implements ConfigurationManager, ConfigurationService, Configurable {
     public static final Logger s_logger = Logger.getLogger(ConfigurationManagerImpl.class);
@@ -335,6 +339,10 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
     AffinityGroupService _affinityGroupService;
     @Inject
     StorageManager _storageManager;
+    @Inject
+    ImageStoreDao _imageStoreDao;
+    @Inject
+    ImageStoreDetailsDao _imageStoreDetailsDao;
 
     // FIXME - why don't we have interface for DataCenterLinkLocalIpAddressDao?
     @Inject
@@ -520,6 +528,13 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
                     _accountDetailsDao.update(accountDetailVO.getId(), accountDetailVO);
                 }
                 break;
+
+            case ImageStore:
+                final ImageStoreVO imgStore = _imageStoreDao.findById(resourceId);
+                Preconditions.checkState(imgStore != null);
+                _imageStoreDetailsDao.addDetail(resourceId, name, value, true);
+                break;
+
             default:
                 throw new InvalidParameterValueException("Scope provided is invalid");
             }
@@ -626,6 +641,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
         final Long clusterId = cmd.getClusterId();
         final Long storagepoolId = cmd.getStoragepoolId();
         final Long accountId = cmd.getAccountId();
+        final Long imageStoreId = cmd.getImageStoreId();
         CallContext.current().setEventDetails(" Name: " + name + " New Value: " + (name.toLowerCase().contains("password") ? "*****" : value == null ? "" : value));
         // check if config value exists
         final ConfigurationVO config = _configDao.findByName(name);
@@ -676,6 +692,11 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
             id = storagepoolId;
             paramCountCheck++;
         }
+        if (imageStoreId != null) {
+            scope = ConfigKey.Scope.ImageStore.toString();
+            id = imageStoreId;
+            paramCountCheck++;
+        }
 
         if (paramCountCheck > 1) {
             throw new InvalidParameterValueException("cannot handle multiple IDs, provide only one ID corresponding to the scope");

server/src/com/cloud/server/ManagementServerImpl.java

@@ -1672,6 +1672,7 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
         final Long clusterId = cmd.getClusterId();
         final Long storagepoolId = cmd.getStoragepoolId();
         final Long accountId = cmd.getAccountId();
+        final Long imageStoreId = cmd.getImageStoreId();
         String scope = null;
         Long id = null;
         int paramCountCheck = 0;
@@ -1696,6 +1697,11 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
             id = storagepoolId;
             paramCountCheck++;
         }
+        if (imageStoreId != null) {
+            scope = ConfigKey.Scope.ImageStore.toString();
+            id = imageStoreId;
+            paramCountCheck++;
+        }
 
         if (paramCountCheck > 1) {
             throw new InvalidParameterValueException("cannot handle multiple IDs, provide only one ID corresponding to the scope");

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

@@ -20,48 +20,64 @@ import java.util.Map;
 
 import javax.inject.Inject;
 
+import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+import org.apache.cloudstack.framework.config.impl.ConfigurationVO;
 import org.apache.cloudstack.storage.datastore.db.ImageStoreDao;
 import org.apache.cloudstack.storage.datastore.db.ImageStoreDetailsDao;
 import org.apache.cloudstack.storage.datastore.db.ImageStoreVO;
 
+import com.cloud.capacity.CapacityManager;
+import com.google.common.base.Preconditions;
+
 public class ImageStoreDetailsUtil {
 
     @Inject
     protected ImageStoreDao imageStoreDao;
     @Inject
     protected ImageStoreDetailsDao imageStoreDetailsDao;
+    @Inject
+    protected ConfigurationDao configurationDao;
 
     /**
-     * Obtain NFS protocol version (if provided) for a store id.<br/>
+     * Retrieve global secondary storage NFS version default value
-     * It can be set by adding an entry in {@code image_store_details} table, providing {@code name=nfs.version} and {@code value=X} (e.g. 3)
+     * @return global default value
+     */
+    protected Integer getGlobalDefaultNfsVersion(){
+        ConfigurationVO globalNfsVersion = configurationDao.findByName(CapacityManager.ImageStoreNFSVersion.key());
+        Preconditions.checkState(globalNfsVersion != null, "Unable to find global NFS version for version key " + CapacityManager.ImageStoreNFSVersion.key());
+        String value = globalNfsVersion.getValue();
+        return (value != null ? Integer.valueOf(value) : null);
+    }
+    /**
+     * Obtain NFS protocol version (if provided) for a store id, if not use default config value<br/>
      * @param storeId image store id
-     * @return {@code null} if {@code nfs.version} is not found for storeId <br/>
+     * @return {@code null} if {@code secstorage.nfs.version} is not found for storeId <br/>
-     * {@code X} if {@code nfs.version} is found found for storeId
+     * {@code X} if {@code secstorage.nfs.version} is found found for storeId
      */
     public Integer getNfsVersion(long storeId) throws NumberFormatException {
-        String nfsVersion = null;
+
-        if (imageStoreDetailsDao.getDetails(storeId) != null){
+        final Map<String, String> storeDetails = imageStoreDetailsDao.getDetails(storeId);
-            Map<String, String> storeDetails = imageStoreDetailsDao.getDetails(storeId);
+        if (storeDetails != null && storeDetails.containsKey(CapacityManager.ImageStoreNFSVersion.key())) {
-            if (storeDetails != null && storeDetails.containsKey("nfs.version")){
+            final String version = storeDetails.get(CapacityManager.ImageStoreNFSVersion.key());
-                nfsVersion = storeDetails.get("nfs.version");
+            return (version != null ? Integer.valueOf(version) : null);
-            }
         }
-        return (nfsVersion != null ? Integer.valueOf(nfsVersion) : null);
+
+        return getGlobalDefaultNfsVersion();
+
     }
 
     /**
      * Obtain NFS protocol version (if provided) for a store uuid.<br/>
-     * It can be set by adding an entry in {@code image_store_details} table, providing {@code name=nfs.version} and {@code value=X} (e.g. 3)
+     * @param resourceId image store id
-     * @param storeId image store id
+     * @return {@code null} if {@code secstorage.nfs.version} is not found for storeUuid <br/>
-     * @return {@code null} if {@code nfs.version} is not found for storeUuid <br/>
+     * {@code X} if {@code secstorage.nfs.version} is found found for storeUuid
-     * {@code X} if {@code nfs.version} is found found for storeUuid
      */
     public Integer getNfsVersionByUuid(String storeUuid){
         ImageStoreVO imageStore = imageStoreDao.findByUuid(storeUuid);
         if (imageStore != null){
             return getNfsVersion(imageStore.getId());
         }
-        return null;
+        return getGlobalDefaultNfsVersion();
     }
 
 }

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

@@ -17,42 +17,54 @@
 package com.cloud.storage;
 
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNull;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
 import java.util.HashMap;
 import java.util.Map;
 
+import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+import org.apache.cloudstack.framework.config.impl.ConfigurationVO;
 import org.apache.cloudstack.storage.datastore.db.ImageStoreDao;
 import org.apache.cloudstack.storage.datastore.db.ImageStoreDetailsDao;
 import org.apache.cloudstack.storage.datastore.db.ImageStoreVO;
 import org.junit.Before;
 import org.junit.Test;
 
+import com.cloud.capacity.CapacityManager;
+
 public class ImageStoreDetailsUtilTest {
 
     private final static long STORE_ID = 1l;
     private final static String STORE_UUID = "aaaa-aaaa-aaaa-aaaa";
     private final static Integer NFS_VERSION = 3;
+    private final static Integer NFS_VERSION_DEFAULT = 2;
 
     ImageStoreDetailsUtil imageStoreDetailsUtil = new ImageStoreDetailsUtil();
 
     ImageStoreDao imgStoreDao = mock(ImageStoreDao.class);
     ImageStoreDetailsDao imgStoreDetailsDao = mock(ImageStoreDetailsDao.class);
+    ConfigurationDao configurationDao = mock(ConfigurationDao.class);
 
     @Before
     public void setup() throws Exception {
         Map<String, String> imgStoreDetails = new HashMap<String, String>();
-        imgStoreDetails.put("nfs.version", String.valueOf(NFS_VERSION));
+        String nfsVersionKey = CapacityManager.ImageStoreNFSVersion.key();
+        imgStoreDetails.put(nfsVersionKey, String.valueOf(NFS_VERSION));
         when(imgStoreDetailsDao.getDetails(STORE_ID)).thenReturn(imgStoreDetails);
 
         ImageStoreVO imgStoreVO = mock(ImageStoreVO.class);
         when(imgStoreVO.getId()).thenReturn(Long.valueOf(STORE_ID));
         when(imgStoreDao.findByUuid(STORE_UUID)).thenReturn(imgStoreVO);
 
+        ConfigurationVO confVO = mock(ConfigurationVO.class);
+        String defaultValue = (NFS_VERSION_DEFAULT == null ? null : String.valueOf(NFS_VERSION_DEFAULT));
+        when(confVO.getValue()).thenReturn(defaultValue);
+        when(configurationDao.findByName(nfsVersionKey)).thenReturn(confVO);
+
         imageStoreDetailsUtil.imageStoreDao = imgStoreDao;
         imageStoreDetailsUtil.imageStoreDetailsDao = imgStoreDetailsDao;
+        imageStoreDetailsUtil.configurationDao = configurationDao;
     }
 
     @Test
@@ -68,7 +80,7 @@ public class ImageStoreDetailsUtilTest {
         when(imgStoreDetailsDao.getDetails(STORE_ID)).thenReturn(imgStoreDetails);
 
         Integer nfsVersion = imageStoreDetailsUtil.getNfsVersion(STORE_ID);
-        assertNull(nfsVersion);
+        assertEquals(NFS_VERSION_DEFAULT, nfsVersion);
     }
 
     @Test
@@ -77,7 +89,7 @@ public class ImageStoreDetailsUtilTest {
         when(imgStoreDetailsDao.getDetails(STORE_ID)).thenReturn(imgStoreDetails);
 
         Integer nfsVersion = imageStoreDetailsUtil.getNfsVersion(STORE_ID);
-        assertNull(nfsVersion);
+        assertEquals(NFS_VERSION_DEFAULT, nfsVersion);
     }
 
     @Test
@@ -90,6 +102,12 @@ public class ImageStoreDetailsUtilTest {
     public void testGetNfsVersionByUuidNoImgStore(){
         when(imgStoreDao.findByUuid(STORE_UUID)).thenReturn(null);
         Integer nfsVersion = imageStoreDetailsUtil.getNfsVersionByUuid(STORE_UUID);
-        assertNull(nfsVersion);
+        assertEquals(NFS_VERSION_DEFAULT, nfsVersion);
     }
-}
+
+    @Test
+    public void testGetGlobalDefaultNfsVersion(){
+        Integer globalDefaultNfsVersion = imageStoreDetailsUtil.getGlobalDefaultNfsVersion();
+        assertEquals(NFS_VERSION_DEFAULT, globalDefaultNfsVersion);
+    }
+}

server/test/org/apache/cloudstack/networkoffering/ChildTestConfiguration.java

@@ -41,6 +41,8 @@ import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
 import org.apache.cloudstack.region.PortableIpDaoImpl;
 import org.apache.cloudstack.region.PortableIpRangeDaoImpl;
 import org.apache.cloudstack.region.dao.RegionDaoImpl;
+import org.apache.cloudstack.storage.datastore.db.ImageStoreDaoImpl;
+import org.apache.cloudstack.storage.datastore.db.ImageStoreDetailsDaoImpl;
 import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDaoImpl;
 import org.apache.cloudstack.test.utils.SpringUtils;
 
@@ -131,7 +133,7 @@ import com.cloud.vm.dao.VMInstanceDaoImpl;
     NetworkDomainDaoImpl.class, HostDetailsDaoImpl.class, HostTagsDaoImpl.class, ClusterDaoImpl.class, FirewallRulesDaoImpl.class,
     FirewallRulesCidrsDaoImpl.class, PhysicalNetworkDaoImpl.class, PhysicalNetworkTrafficTypeDaoImpl.class, PhysicalNetworkServiceProviderDaoImpl.class,
     LoadBalancerDaoImpl.class, NetworkServiceMapDaoImpl.class, PrimaryDataStoreDaoImpl.class, StoragePoolDetailsDaoImpl.class,
-    PortableIpRangeDaoImpl.class, RegionDaoImpl.class, PortableIpDaoImpl.class, AccountGuestVlanMapDaoImpl.class},
+    PortableIpRangeDaoImpl.class, RegionDaoImpl.class, PortableIpDaoImpl.class, AccountGuestVlanMapDaoImpl.class, ImageStoreDaoImpl.class, ImageStoreDetailsDaoImpl.class},
                includeFilters = {@Filter(value = ChildTestConfiguration.Library.class, type = FilterType.CUSTOM)},
                useDefaultFilters = false)
 public class

setup/db/db/schema-481to490.sql

@@ -545,3 +545,6 @@ INSERT IGNORE INTO `cloud`.`guest_os_hypervisor` (uuid,hypervisor_type, hypervis
 INSERT IGNORE INTO `cloud`.`guest_os_hypervisor` (uuid,hypervisor_type, hypervisor_version, guest_os_name, guest_os_id, created, is_user_defined) VALUES (UUID(), 'VMware', '5.0', 'centos64Guest', 228, now(), 0);
 INSERT IGNORE INTO `cloud`.`guest_os_hypervisor` (uuid,hypervisor_type, hypervisor_version, guest_os_name, guest_os_id, created, is_user_defined) VALUES (UUID(), 'VMware', '5.1', 'centos64Guest', 228, now(), 0);
 INSERT IGNORE INTO `cloud`.`guest_os_hypervisor` (uuid,hypervisor_type, hypervisor_version, guest_os_name, guest_os_id, created, is_user_defined) VALUES (UUID(), 'VMware', '5.5', 'centos64Guest', 228, now(), 0);
+
+ALTER TABLE `cloud`.`image_store_details` CHANGE COLUMN `value` `value` VARCHAR(255) NULL DEFAULT NULL COMMENT 'value of the detail', ADD COLUMN `display` tinyint(1) NOT 
+NULL DEFAULT '1' COMMENT 'True if the detail can be displayed to the end user' AFTER `value`;

test/integration/smoke/test_ssvm.py

@@ -20,12 +20,13 @@
 from marvin.cloudstackTestCase import cloudstackTestCase
 from marvin.cloudstackAPI import (stopSystemVm,
                                   rebootSystemVm,
-                                  destroySystemVm)
+                                  destroySystemVm, updateConfiguration)
 from marvin.lib.utils import (cleanup_resources,
                               get_process_status,
-                              get_host_credentials)
+                              get_host_credentials,
+                              wait_until)
 from marvin.lib.base import (PhysicalNetwork,
-                             NetScaler)
+                             NetScaler, ImageStore)
 from marvin.lib.common import (get_zone,
                                list_hosts,
                                list_ssvms,
@@ -33,6 +34,7 @@ from marvin.lib.common import (get_zone,
                                list_vlan_ipranges)
 from nose.plugins.attrib import attr
 import telnetlib
+import logging
 
 # Import System modules
 import time
@@ -42,12 +44,19 @@ _multiprocess_shared_ = True
 class TestSSVMs(cloudstackTestCase):
 
     def setUp(self):
+	test_case = super(TestSSVMs, self)
         self.apiclient = self.testClient.getApiClient()
         self.hypervisor = self.testClient.getHypervisorInfo()
         self.cleanup = []
+        self.config = test_case.getClsConfig()
         self.services = self.testClient.getParsedTestDataConfig()
         self.zone = get_zone(self.apiclient, self.testClient.getZoneForTests())
 
+        self.logger = logging.getLogger('TestSSVMs')
+        self.stream_handler = logging.StreamHandler()
+        self.logger.setLevel(logging.DEBUG)
+        self.logger.addHandler(self.stream_handler)
+
         # Default sleep is set to 90 seconds, which is too long if the SSVM takes up to 2min to start.
         # Second sleep in the loop will waste test time.
         self.services["sleep"] = 30
@@ -1197,3 +1206,162 @@ class TestSSVMs(cloudstackTestCase):
         # Call to verify cloud process is running
         self.test_04_cpvm_internals()
         return
+
+    @attr(
+        tags=[
+            "advanced",
+            "advancedns",
+            "smoke",
+            "basic",
+            "sg"],
+        required_hardware="true")
+    def test_11_ss_nfs_version_on_ssvm(self):
+        """Test NFS Version on Secondary Storage mounted properly on SSVM
+        """
+
+        # 1) List SSVM in zone
+        # 2) Get id and url from mounted nfs store
+        # 3) Update NFS version for previous image store
+        # 4) Stop SSVM
+        # 5) Check NFS version of mounted nfs store after SSVM starts 
+
+        nfs_version = self.config.nfsVersion
+        if nfs_version == None:
+            self.skipTest('No NFS version provided in test data')
+
+        #List SSVM for zone id
+        list_ssvm_response = list_ssvms(
+            self.apiclient,
+            systemvmtype='secondarystoragevm',
+            state='Running',
+            zoneid=self.zone.id
+        )
+        self.assertNotEqual(
+            list_ssvm_response,
+            None
+        )
+        self.assertEqual(
+            isinstance(list_ssvm_response, list),
+            True,
+            "Check list response returns a valid list"
+        )
+        self.assertEqual(
+            len(list_ssvm_response),
+            1,
+            "Check list System VMs response"
+        )
+
+        ssvm = list_ssvm_response[0]
+        image_stores_response = ImageStore.list(self.apiclient,zoneid=self.zone.id)
+
+        if self.hypervisor.lower() in ('vmware', 'hyperv'):
+            # SSH into SSVMs is done via management server for Vmware and Hyper-V
+            result = get_process_status(
+                self.apiclient.connection.mgtSvr,
+                22,
+                self.apiclient.connection.user,
+                self.apiclient.connection.passwd,
+                ssvm.privateip,
+                "mount | grep 'type nfs'",
+                hypervisor=self.hypervisor)
+
+        for res in result:
+            split_res = res.split("on")
+            mounted_img_store_url = split_res[0].strip()
+            for img_store in image_stores_response:
+                img_store_url = str(img_store.url)
+                if img_store_url.startswith("nfs://"):
+                    img_store_url = img_store_url[6:]
+                    #Add colon after ip address to match output from mount command
+                    first_slash = img_store_url.find('/')
+                    img_store_url = img_store_url[0:first_slash] + ':' + img_store_url[first_slash:]
+                    if img_store_url == mounted_img_store_url:
+                        img_store_id = img_store.id
+                        break
+
+        self.assertNotEqual(
+            img_store_id,
+            None,
+            "Check image store id mounted on SSVM"
+        )
+
+        #Update NFS version for image store mounted on SSVM
+        updateConfigurationCmd = updateConfiguration.updateConfigurationCmd()
+        updateConfigurationCmd.name = "secstorage.nfs.version"
+        updateConfigurationCmd.value = nfs_version
+        updateConfigurationCmd.imagestoreuuid = img_store_id
+
+        updateConfigurationResponse = self.apiclient.updateConfiguration(updateConfigurationCmd)
+        self.logger.debug("updated the parameter %s with value %s"%(updateConfigurationResponse.name, updateConfigurationResponse.value))
+
+        #Stop SSVM
+        self.debug("Stopping SSVM: %s" % ssvm.id)
+        cmd = stopSystemVm.stopSystemVmCmd()
+        cmd.id = ssvm.id
+        self.apiclient.stopSystemVm(cmd)
+
+        def checkForRunningSSVM():
+            new_list_ssvm_response = list_ssvms(
+                self.apiclient,
+                id=ssvm.id
+            )
+            if isinstance(new_list_ssvm_response, list):
+                return new_list_ssvm_response[0].state == 'Running', None                
+            
+        res, _ = wait_until(self.services["sleep"], self.services["timeout"], checkForRunningSSVM)
+        if not res:
+            self.fail("List SSVM call failed!")
+        
+        new_list_ssvm_response = list_ssvms(
+                self.apiclient,
+                id=ssvm.id
+        )
+
+        self.assertNotEqual(
+            new_list_ssvm_response,
+            None
+        )
+        self.assertEqual(
+            isinstance(new_list_ssvm_response, list),
+            True,
+            "Check list response returns a valid list"
+        )
+        ssvm = new_list_ssvm_response[0]
+        self.debug("SSVM state after debug: %s" % ssvm.state)
+        self.assertEqual(
+            ssvm.state,
+            'Running',
+            "Check whether SSVM is running or not"
+        )
+        # Wait for the agent to be up
+        self.waitForSystemVMAgent(ssvm.name)
+
+        #Check NFS version on mounted image store
+        result = get_process_status(
+                self.apiclient.connection.mgtSvr,
+                22,
+                self.apiclient.connection.user,
+                self.apiclient.connection.passwd,
+                ssvm.privateip,
+                "mount | grep '%s'"%mounted_img_store_url,
+                hypervisor=self.hypervisor)
+
+        self.assertNotEqual(
+            result,
+            None
+        )
+        self.assertEqual(
+            len(result),
+            1,
+            "Check result length"
+        )
+
+        res = result[0]
+        mounted_nfs_version = res.split("vers=")[1][0:1]
+        self.assertEqual(
+            int(mounted_nfs_version),
+            int(nfs_version),
+            "Check mounted NFS version to be the same as provided"
+        )
+
+        return

ui/scripts/system.js

@@ -19974,28 +19974,58 @@
                                                 }
                                             });
                                         }
-                                    }
-
-                                    // Granular settings for storage pool for secondary storage is not required
-                                    /*  settings: {
-                                    title: 'label.menu.global.settings',
-                                    custom: cloudStack.uiCustom.granularSettings({
-                                    dataProvider: function(args) {
-                                    args.response.success({
-                                    data: [
-                                    { name: 'config.param.1', value: 1 },
-                                    { name: 'config.param.2', value: 2 }
-                                    ]
-                                    });
                                     },
-                                    actions: {
+
-                                    edit: function(args) {
+                                    // Granular settings for image store
-                                    // call updateStorageLevelParameters
+									settings: {
-                                    args.response.success();
+										title: 'label.settings',
-                                    }
+										custom: cloudStack.uiCustom.granularSettings({
-                                    }
+											dataProvider: function (args) {
-                                    })
+
-                                    } */
+												$.ajax({
+													url: createURL('listConfigurations&imagestoreuuid=' + args.context.secondaryStorage[0].id),
+													data: listViewDataProvider(args, {
+													},
+													{
+														searchBy: 'name'
+													}),
+													success: function (json) {
+														args.response.success({
+															data: json.listconfigurationsresponse.configuration
+														});
+													},
+
+													error: function (json) {
+														args.response.error(parseXMLHttpResponse(json));
+													}
+												});
+											},
+											actions: {
+												edit: function (args) {
+													// call updateStorageLevelParameters
+													var data = {
+														name: args.data.jsonObj.name,
+														value: args.data.value
+													};
+
+													$.ajax({
+														url: createURL('updateConfiguration&imagestoreuuid=' + args.context.secondaryStorage[0].id),
+														data: data,
+														success: function (json) {
+															var item = json.updateconfigurationresponse.configuration;
+															args.response.success({
+																data: item
+															});
+														},
+
+														error: function (json) {
+															args.response.error(parseXMLHttpResponse(json));
+														}
+													});
+												}
+											}
+										})
+									}
                                 }
                             }
                         }