[
https://issues.apache.org/jira/browse/CLOUDSTACK-9438?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15410163#comment-15410163
]
ASF GitHub Bot commented on CLOUDSTACK-9438:
--------------------------------------------
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1615#discussion_r73766528
--- Diff: server/src/com/cloud/storage/ImageStoreDetailsUtil.java ---
@@ -20,48 +20,68 @@
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;
+
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/>
- * 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)
+ * Retrieve global secondary storage NFS version default value
+ * @return global default value
+ */
+ protected Integer getGlobalDefaultNfsVersion(){
+ ConfigurationVO globalNfsVersion =
configurationDao.findByName(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/>
- * {@code X} if {@code nfs.version} is found found for storeId
+ * @return {@code null} if {@code secstorage.nfs.version} is not found
for storeId <br/>
+ * {@code X} if {@code secstorage.nfs.version} is found found for
storeId
*/
public Integer getNfsVersion(long storeId) throws
NumberFormatException {
- String nfsVersion = null;
+ Integer nfsVersion;
if (imageStoreDetailsDao.getDetails(storeId) != null){
Map<String, String> storeDetails =
imageStoreDetailsDao.getDetails(storeId);
--- End diff --
I recognize that this code predates this patch, but it is a double API
call. Consider the following refactoring:
```
public Integer getNfsVersion(long storeId) throws NumberFormatException {
final Map<String, String> storeDetails =
imageStoreDetailsDao.getDetails(storeId);
if (storeDetails != null &&
storeDetails.containsKey(CapacityManager.ImageStoreNFSVersion.key())) {
final String version =
storeDetails.get(CapacityManager.ImageStoreNFSVersion.key());
return (val != null ? Integer.valueOf(val) : null);
}
return getGlobalDefaultNfsVersion();
}
```
> Fix for CLOUDSTACK-9252 - Make NFS version changeable in UI
> -----------------------------------------------------------
>
> Key: CLOUDSTACK-9438
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9438
> Project: CloudStack
> Issue Type: Improvement
> Security Level: Public(Anyone can view this level - this is the
> default.)
> Reporter: Nicolas Vazquez
> Assignee: Nicolas Vazquez
>
> h3. Introduction
> From [9252|https://issues.apache.org/jira/browse/CLOUDSTACK-9252] 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.
> h3. 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.
> h4. 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 {{ImageStoreDetailsDaoImpl}} 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
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
