Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
blueorangutan commented on PR #8947: URL: https://github.com/apache/cloudstack/pull/8947#issuecomment-2176516474 Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 10002 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
blueorangutan commented on PR #8947: URL: https://github.com/apache/cloudstack/pull/8947#issuecomment-2176259563 Packaging result [SF]: ✖️ el7 ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 10001 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
blueorangutan commented on PR #8947: URL: https://github.com/apache/cloudstack/pull/8947#issuecomment-2176189811 @abh1sar a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
abh1sar commented on PR #8947: URL: https://github.com/apache/cloudstack/pull/8947#issuecomment-2176187128 @blueorangutan package -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
blueorangutan commented on PR #8947: URL: https://github.com/apache/cloudstack/pull/8947#issuecomment-2176008490 Packaging result [SF]: ✖️ el7 ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 9998 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
blueorangutan commented on PR #8947: URL: https://github.com/apache/cloudstack/pull/8947#issuecomment-2175936931 @abh1sar a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
abh1sar commented on PR #8947: URL: https://github.com/apache/cloudstack/pull/8947#issuecomment-2175933803 @blueorangutan package -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
abh1sar commented on PR #8947: URL: https://github.com/apache/cloudstack/pull/8947#issuecomment-2173562755 @borisstoyanov Merged with latest 4.19 and built packages -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
blueorangutan commented on PR #8947: URL: https://github.com/apache/cloudstack/pull/8947#issuecomment-2173483344 Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 9983 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
blueorangutan commented on PR #8947: URL: https://github.com/apache/cloudstack/pull/8947#issuecomment-2173338522 @abh1sar a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
abh1sar commented on PR #8947: URL: https://github.com/apache/cloudstack/pull/8947#issuecomment-2173337196 @blueorangutan package -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
borisstoyanov commented on PR #8947: URL: https://github.com/apache/cloudstack/pull/8947#issuecomment-2172917888 @abh1sar can you please resolve the conflicts. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
blueorangutan commented on PR #8947: URL: https://github.com/apache/cloudstack/pull/8947#issuecomment-2126281136 Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 9686 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
blueorangutan commented on PR #8947: URL: https://github.com/apache/cloudstack/pull/8947#issuecomment-2126216911 @abh1sar a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
abh1sar commented on PR #8947: URL: https://github.com/apache/cloudstack/pull/8947#issuecomment-2126215265 @blueorangutan package -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
blueorangutan commented on PR #8947: URL: https://github.com/apache/cloudstack/pull/8947#issuecomment-2102290709 Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 9586 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
blueorangutan commented on PR #8947: URL: https://github.com/apache/cloudstack/pull/8947#issuecomment-2102182946 @abh1sar a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
abh1sar commented on PR #8947: URL: https://github.com/apache/cloudstack/pull/8947#issuecomment-2102180268 @blueorangutan package -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
blueorangutan commented on PR #8947: URL: https://github.com/apache/cloudstack/pull/8947#issuecomment-2101599761 [SF] Trillian test result (tid-10190) Environment: kvm-rocky8 (x2), Advanced Networking with Mgmt server r8 Total time taken: 47086 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8947-t10190-kvm-rocky8.zip Smoke tests completed. 130 look OK, 1 have errors, 0 did not run Only failed and skipped tests results shown below: Test | Result | Time (s) | Test File --- | --- | --- | --- test_01_events_resource | `Error` | 424.89 | test_events_resource.py -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
blueorangutan commented on PR #8947: URL: https://github.com/apache/cloudstack/pull/8947#issuecomment-2100065325 @abh1sar a [SL] Trillian-Jenkins test job (rocky8 mgmt + kvm-rocky8) has been kicked to run smoke tests -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
abh1sar commented on PR #8947: URL: https://github.com/apache/cloudstack/pull/8947#issuecomment-2100062454 @blueorangutan test rocky8 kvm-rocky8 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
blueorangutan commented on PR #8947: URL: https://github.com/apache/cloudstack/pull/8947#issuecomment-2099969336 Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 9574 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
blueorangutan commented on PR #8947: URL: https://github.com/apache/cloudstack/pull/8947#issuecomment-2099857767 @abh1sar a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
abh1sar commented on PR #8947: URL: https://github.com/apache/cloudstack/pull/8947#issuecomment-2099857258 @blueorangutan package -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
blueorangutan commented on PR #8947: URL: https://github.com/apache/cloudstack/pull/8947#issuecomment-2096915786 [SF] Trillian test result (tid-10167) Environment: kvm-rocky8 (x2), Advanced Networking with Mgmt server r8 Total time taken: 46535 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8947-t10167-kvm-rocky8.zip Smoke tests completed. 128 look OK, 3 have errors, 0 did not run Only failed and skipped tests results shown below: Test | Result | Time (s) | Test File --- | --- | --- | --- test_01_events_resource | `Error` | 423.09 | test_events_resource.py test_01_restore_vm | `Error` | 0.22 | test_restore_vm.py test_02_restore_vm_allocated_root | `Error` | 0.15 | test_restore_vm.py ContextSuite context=TestRestoreVM>:teardown | `Error` | 1.24 | test_restore_vm.py test_02_list_cpvm_vm | `Failure` | 0.05 | test_ssvm.py test_04_cpvm_internals | `Failure` | 0.05 | test_ssvm.py test_12_destroy_cpvm | `Error` | 3.25 | test_ssvm.py -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
blueorangutan commented on PR #8947: URL: https://github.com/apache/cloudstack/pull/8947#issuecomment-2095361973 @abh1sar a [SL] Trillian-Jenkins test job (rocky8 mgmt + kvm-rocky8) has been kicked to run smoke tests -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
abh1sar commented on PR #8947: URL: https://github.com/apache/cloudstack/pull/8947#issuecomment-2095359026 @blueorangutan test rocky8 kvm-rocky8 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
blueorangutan commented on PR #8947: URL: https://github.com/apache/cloudstack/pull/8947#issuecomment-2095350737 Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✖️ debian ✔️ suse15. SL-JID 9543 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
blueorangutan commented on PR #8947: URL: https://github.com/apache/cloudstack/pull/8947#issuecomment-2095281409 @sureshanaparti a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
sureshanaparti commented on PR #8947: URL: https://github.com/apache/cloudstack/pull/8947#issuecomment-2095279450 @blueorangutan package -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
abh1sar commented on code in PR #8947:
URL: https://github.com/apache/cloudstack/pull/8947#discussion_r1588767855
##
server/src/main/java/com/cloud/storage/StorageManagerImpl.java:
##
@@ -1232,6 +1281,30 @@ public void
doInTransactionWithoutResult(TransactionStatus status) {
return deleteDataStoreInternal(sPool, forced);
}
+@Override
+public Pair, Boolean>
getStoragePoolNFSMountOpts(StoragePool pool, Map details) {
+boolean details_added = false;
+if (details == null) {
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
sureshanaparti commented on code in PR #8947:
URL: https://github.com/apache/cloudstack/pull/8947#discussion_r1587430998
##
server/src/main/java/com/cloud/storage/StorageManagerImpl.java:
##
@@ -1232,6 +1281,30 @@ public void
doInTransactionWithoutResult(TransactionStatus status) {
return deleteDataStoreInternal(sPool, forced);
}
+@Override
+public Pair, Boolean>
getStoragePoolNFSMountOpts(StoragePool pool, Map details) {
+boolean details_added = false;
+if (details == null) {
Review Comment:
can move this if check, to below if block
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
abh1sar commented on code in PR #8947:
URL: https://github.com/apache/cloudstack/pull/8947#discussion_r1587124084
##
server/src/main/java/com/cloud/storage/StorageManagerImpl.java:
##
@@ -1232,6 +1279,19 @@ public void
doInTransactionWithoutResult(TransactionStatus status) {
return deleteDataStoreInternal(sPool, forced);
}
+@Override
+public boolean addStoragePoolNFSMountOptsToDetailsMap(StoragePool pool,
Map details) {
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
abh1sar commented on code in PR #8947:
URL: https://github.com/apache/cloudstack/pull/8947#discussion_r1587123887
##
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java:
##
@@ -363,6 +365,41 @@ private StoragePool createCLVMStoragePool(Connect conn,
String uuid, String host
}
+private List getNFSMountOptsFromDetails(StoragePoolType type,
Map details) {
+List nfsMountOpts = null;
+if (type.equals(StoragePoolType.NetworkFilesystem)) {
+if (details != null &&
details.containsKey(ApiConstants.NFS_MOUNT_OPTIONS)) {
+nfsMountOpts =
Arrays.asList(details.get(ApiConstants.NFS_MOUNT_OPTIONS).replaceAll("\\s",
"").split(","));
+}
+}
+return nfsMountOpts;
+}
+
+private boolean destroyStoragePoolForNFSMountOptions(StoragePool sp,
Connect conn, List nfsMountOpts) {
+try {
+LibvirtStoragePoolDef poolDef = getStoragePoolDef(conn, sp);
+Set poolNfsMountOptsMap = poolDef.getNfsMountOpts();
Review Comment:
Done
##
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java:
##
@@ -363,6 +365,41 @@ private StoragePool createCLVMStoragePool(Connect conn,
String uuid, String host
}
+private List getNFSMountOptsFromDetails(StoragePoolType type,
Map details) {
+List nfsMountOpts = null;
+if (type.equals(StoragePoolType.NetworkFilesystem)) {
+if (details != null &&
details.containsKey(ApiConstants.NFS_MOUNT_OPTIONS)) {
+nfsMountOpts =
Arrays.asList(details.get(ApiConstants.NFS_MOUNT_OPTIONS).replaceAll("\\s",
"").split(","));
+}
+}
+return nfsMountOpts;
+}
+
+private boolean destroyStoragePoolForNFSMountOptions(StoragePool sp,
Connect conn, List nfsMountOpts) {
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
sureshanaparti commented on code in PR #8947:
URL: https://github.com/apache/cloudstack/pull/8947#discussion_r1584441199
##
server/src/main/java/com/cloud/storage/StorageManagerImpl.java:
##
@@ -1232,6 +1279,19 @@ public void
doInTransactionWithoutResult(TransactionStatus status) {
return deleteDataStoreInternal(sPool, forced);
}
+@Override
+public boolean addStoragePoolNFSMountOptsToDetailsMap(StoragePool pool,
Map details) {
Review Comment:
```suggestion
public Pair, Boolean>
getStoragePoolNFSMountOpts(StoragePool pool, Map details) {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
sureshanaparti commented on code in PR #8947:
URL: https://github.com/apache/cloudstack/pull/8947#discussion_r1584441199
##
server/src/main/java/com/cloud/storage/StorageManagerImpl.java:
##
@@ -1232,6 +1279,19 @@ public void
doInTransactionWithoutResult(TransactionStatus status) {
return deleteDataStoreInternal(sPool, forced);
}
+@Override
+public boolean addStoragePoolNFSMountOptsToDetailsMap(StoragePool pool,
Map details) {
Review Comment:
```suggestion
public Pair getStoragePoolNFSMountOpts(StoragePool pool,
Map details) {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
sureshanaparti commented on code in PR #8947:
URL: https://github.com/apache/cloudstack/pull/8947#discussion_r1584415469
##
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java:
##
@@ -363,6 +365,41 @@ private StoragePool createCLVMStoragePool(Connect conn,
String uuid, String host
}
+private List getNFSMountOptsFromDetails(StoragePoolType type,
Map details) {
+List nfsMountOpts = null;
+if (type.equals(StoragePoolType.NetworkFilesystem)) {
+if (details != null &&
details.containsKey(ApiConstants.NFS_MOUNT_OPTIONS)) {
+nfsMountOpts =
Arrays.asList(details.get(ApiConstants.NFS_MOUNT_OPTIONS).replaceAll("\\s",
"").split(","));
+}
+}
+return nfsMountOpts;
+}
+
+private boolean destroyStoragePoolForNFSMountOptions(StoragePool sp,
Connect conn, List nfsMountOpts) {
Review Comment:
```suggestion
private boolean destroyStoragePoolOnNFSMountOptionsChange(StoragePool
sp, Connect conn, List nfsMountOpts) {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
sureshanaparti commented on code in PR #8947:
URL: https://github.com/apache/cloudstack/pull/8947#discussion_r1584398685
##
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java:
##
@@ -363,6 +365,41 @@ private StoragePool createCLVMStoragePool(Connect conn,
String uuid, String host
}
+private List getNFSMountOptsFromDetails(StoragePoolType type,
Map details) {
+List nfsMountOpts = null;
+if (type.equals(StoragePoolType.NetworkFilesystem)) {
+if (details != null &&
details.containsKey(ApiConstants.NFS_MOUNT_OPTIONS)) {
+nfsMountOpts =
Arrays.asList(details.get(ApiConstants.NFS_MOUNT_OPTIONS).replaceAll("\\s",
"").split(","));
+}
+}
+return nfsMountOpts;
+}
+
+private boolean destroyStoragePoolForNFSMountOptions(StoragePool sp,
Connect conn, List nfsMountOpts) {
+try {
+LibvirtStoragePoolDef poolDef = getStoragePoolDef(conn, sp);
+Set poolNfsMountOptsMap = poolDef.getNfsMountOpts();
Review Comment:
```suggestion
Set poolNfsMountOpts = poolDef.getNfsMountOpts();
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
codecov-commenter commented on PR #8947: URL: https://github.com/apache/cloudstack/pull/8947#issuecomment-2084741654 ## [Codecov](https://app.codecov.io/gh/apache/cloudstack/pull/8947?dropdown=coverage=pr=h1_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache) Report All modified and coverable lines are covered by tests :white_check_mark: > Project coverage is 4.31%. Comparing base [(`0271494`)](https://app.codecov.io/gh/apache/cloudstack/commit/027149487b509be5cccef1b8ea73de7707bc35d7?dropdown=coverage=desc_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache) to head [(`c5de8ec`)](https://app.codecov.io/gh/apache/cloudstack/pull/8947?dropdown=coverage=pr=desc_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache). Additional details and impacted files ```diff @@ Coverage Diff @@ ## 4.19 #8947 +/- ## - Coverage 14.96% 4.31% -10.66% Files 5373 364 -5009 Lines468970 29214 -439756 Branches 607705094-55676 - Hits 701831260-68923 + Misses 391018 27813 -363205 + Partials 7769 141 -7628 ``` | [Flag](https://app.codecov.io/gh/apache/cloudstack/pull/8947/flags?src=pr=flags_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache) | Coverage Δ | | |---|---|---| | [uitests](https://app.codecov.io/gh/apache/cloudstack/pull/8947/flags?src=pr=flag_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache) | `4.31% <ø> (-0.01%)` | :arrow_down: | | [unittests](https://app.codecov.io/gh/apache/cloudstack/pull/8947/flags?src=pr=flag_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache) | `?` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more. [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/cloudstack/pull/8947?dropdown=coverage=pr=continue_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache). :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
sureshanaparti commented on code in PR #8947:
URL: https://github.com/apache/cloudstack/pull/8947#discussion_r1584369973
##
server/src/main/java/com/cloud/storage/StoragePoolAutomationImpl.java:
##
@@ -318,9 +323,12 @@ public boolean cancelMaintain(DataStore store) {
if (hosts == null || hosts.size() == 0) {
return true;
}
+
+Map details = new HashMap<>();
Review Comment:
if details passed is null, create it and add detail. otherwise, append to
it. (method would return updated details map)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
blueorangutan commented on PR #8947: URL: https://github.com/apache/cloudstack/pull/8947#issuecomment-2084415449 Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 9470 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
blueorangutan commented on PR #8947: URL: https://github.com/apache/cloudstack/pull/8947#issuecomment-2084355476 @abh1sar a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
abh1sar commented on PR #8947: URL: https://github.com/apache/cloudstack/pull/8947#issuecomment-2084353571 @blueorangutan package -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
abh1sar commented on code in PR #8947:
URL: https://github.com/apache/cloudstack/pull/8947#discussion_r1584111489
##
server/src/main/java/com/cloud/storage/StorageManagerImpl.java:
##
@@ -839,6 +839,21 @@ protected String createLocalStoragePoolName(Host host,
StoragePoolInfo storagePo
return String.format("%s-%s-%s", StringUtils.trim(host.getName()),
"local", storagePoolInformation.getUuid().split("-")[0]);
}
+protected void checkNfsOptions(String nfsopts) throws
InvalidParameterValueException {
+String[] options = nfsopts.replaceAll("\\s", "").split(",");
+Map optionsMap = new HashMap<>();
+for (String option : options) {
+String[] keyValue = option.split("=");
+if (keyValue.length > 2) {
+throw new InvalidParameterValueException("Invalid value for
NFS option " + keyValue[0]);
+}
+if (optionsMap.containsKey(keyValue[0])) {
+throw new InvalidParameterValueException("Duplicate NFS option
values found for option " + keyValue[0]);
+}
+optionsMap.put(keyValue[0], null);
Review Comment:
I can add a few lines in the documentation PR on what happens if incorrect
mount options are given if you are ok with the current behaviour.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
abh1sar commented on code in PR #8947:
URL: https://github.com/apache/cloudstack/pull/8947#discussion_r1584106723
##
server/src/main/java/com/cloud/storage/StorageManagerImpl.java:
##
@@ -839,6 +839,21 @@ protected String createLocalStoragePoolName(Host host,
StoragePoolInfo storagePo
return String.format("%s-%s-%s", StringUtils.trim(host.getName()),
"local", storagePoolInformation.getUuid().split("-")[0]);
}
+protected void checkNfsOptions(String nfsopts) throws
InvalidParameterValueException {
+String[] options = nfsopts.replaceAll("\\s", "").split(",");
+Map optionsMap = new HashMap<>();
+for (String option : options) {
+String[] keyValue = option.split("=");
+if (keyValue.length > 2) {
+throw new InvalidParameterValueException("Invalid value for
NFS option " + keyValue[0]);
+}
+if (optionsMap.containsKey(keyValue[0])) {
+throw new InvalidParameterValueException("Duplicate NFS option
values found for option " + keyValue[0]);
+}
+optionsMap.put(keyValue[0], null);
Review Comment:
I have updated the Testing section in PR description with this testcase.
Had to make some changes in `cancelMaintain()` to note and log the exception
passed from libvirt when mount fails.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
abh1sar commented on code in PR #8947:
URL: https://github.com/apache/cloudstack/pull/8947#discussion_r1583432991
##
server/src/main/java/com/cloud/storage/StoragePoolAutomationImpl.java:
##
@@ -318,9 +323,12 @@ public boolean cancelMaintain(DataStore store) {
if (hosts == null || hosts.size() == 0) {
return true;
}
+
+Map details = new HashMap<>();
Review Comment:
I kept it outside thinking that other kind of details can be added to
`details` map later
For ex. we can have something like this in future
```
Map details = new HashMap<>();
storageManager.addStoragePoolNFSMountOptsToDetailsMap(pool, details);
storageManager.addStoragePoolFutureFieldToDetailsMap(pool, details);
```
I can move it to the method if you think this is overkill.
##
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolDef.java:
##
@@ -75,6 +82,15 @@ public LibvirtStoragePoolDef(PoolType type, String poolName,
String uuid, String
_targetPath = targetPath;
}
+public LibvirtStoragePoolDef(PoolType type, String poolName, String uuid,
String host, String dir, String targetPath, List nfsMountOpts) {
+this(type, poolName, uuid, host, dir, targetPath);
+if (CollectionUtils.isNotEmpty(nfsMountOpts)) {
+for (String nfsMountOpt : nfsMountOpts) {
+this._nfsMountOpts.put(nfsMountOpt, null);
Review Comment:
Set would be perfect. Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
sureshanaparti commented on code in PR #8947:
URL: https://github.com/apache/cloudstack/pull/8947#discussion_r1580920735
##
server/src/main/java/com/cloud/storage/StoragePoolAutomationImpl.java:
##
@@ -318,9 +323,12 @@ public boolean cancelMaintain(DataStore store) {
if (hosts == null || hosts.size() == 0) {
return true;
}
+
+Map details = new HashMap<>();
Review Comment:
can create this map in the method itself and return (noticed map is created
before _addStoragePoolNFSMountOptsToDetailsMap()_ is called )
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
abh1sar commented on code in PR #8947:
URL: https://github.com/apache/cloudstack/pull/8947#discussion_r1580503575
##
server/src/main/java/com/cloud/storage/StorageManagerImpl.java:
##
@@ -839,6 +839,21 @@ protected String createLocalStoragePoolName(Host host,
StoragePoolInfo storagePo
return String.format("%s-%s-%s", StringUtils.trim(host.getName()),
"local", storagePoolInformation.getUuid().split("-")[0]);
}
+protected void checkNfsOptions(String nfsopts) throws
InvalidParameterValueException {
+String[] options = nfsopts.replaceAll("\\s", "").split(",");
+Map optionsMap = new HashMap<>();
+for (String option : options) {
+String[] keyValue = option.split("=");
+if (keyValue.length > 2) {
+throw new InvalidParameterValueException("Invalid value for
NFS option " + keyValue[0]);
+}
+if (optionsMap.containsKey(keyValue[0])) {
+throw new InvalidParameterValueException("Duplicate NFS option
values found for option " + keyValue[0]);
+}
+optionsMap.put(keyValue[0], null);
Review Comment:
That's a good point. Let me test this and get back.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
blueorangutan commented on PR #8947: URL: https://github.com/apache/cloudstack/pull/8947#issuecomment-2077255654 Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 9426 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
blueorangutan commented on PR #8947: URL: https://github.com/apache/cloudstack/pull/8947#issuecomment-2077083287 @sureshanaparti a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
sureshanaparti commented on PR #8947: URL: https://github.com/apache/cloudstack/pull/8947#issuecomment-2077081874 @blueorangutan package -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
sureshanaparti commented on code in PR #8947:
URL: https://github.com/apache/cloudstack/pull/8947#discussion_r1579396700
##
server/src/main/java/com/cloud/storage/StorageManagerImpl.java:
##
@@ -839,6 +839,21 @@ protected String createLocalStoragePoolName(Host host,
StoragePoolInfo storagePo
return String.format("%s-%s-%s", StringUtils.trim(host.getName()),
"local", storagePoolInformation.getUuid().split("-")[0]);
}
+protected void checkNfsOptions(String nfsopts) throws
InvalidParameterValueException {
+String[] options = nfsopts.replaceAll("\\s", "").split(",");
+Map optionsMap = new HashMap<>();
+for (String option : options) {
+String[] keyValue = option.split("=");
+if (keyValue.length > 2) {
+throw new InvalidParameterValueException("Invalid value for
NFS option " + keyValue[0]);
+}
+if (optionsMap.containsKey(keyValue[0])) {
+throw new InvalidParameterValueException("Duplicate NFS option
values found for option " + keyValue[0]);
+}
+optionsMap.put(keyValue[0], null);
Review Comment:
(re-)mount may fail with incorrect options, hope the relevant message is
thrown to the operator.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
sureshanaparti commented on code in PR #8947:
URL: https://github.com/apache/cloudstack/pull/8947#discussion_r1579394028
##
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java:
##
@@ -654,20 +654,54 @@ public KVMStoragePool createStoragePool(String name,
String host, int port, Stri
} catch (LibvirtException e) {
s_logger.error("Failure in attempting to see if an existing
storage pool might be using the path of the pool to be created:" + e);
}
+}
+
+List nfsopts = null;
+if (type == StoragePoolType.NetworkFilesystem) {
+if (details != null && details.containsKey("nfsopts")) {
+nfsopts =
Arrays.asList(details.get("nfsopts").replaceAll("\\s", "").split(","));
+}
+
+if (sp != null && nfsopts != null) {
+try {
+LibvirtStoragePoolDef poolDef = getStoragePoolDef(conn,
sp);
+Map poolNfsOptsMap = poolDef.getNfsOpts();
+boolean optionsDiffer = false;
+if (poolNfsOptsMap.size() != nfsopts.size()) {
+optionsDiffer = true;
+} else {
+for (String nfsopt : nfsopts) {
+if (!poolNfsOptsMap.containsKey(nfsopt)) {
Review Comment:
Ok, I'm assuming` key=vers` & `value=4.1`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
DaanHoogland commented on code in PR #8947:
URL: https://github.com/apache/cloudstack/pull/8947#discussion_r1577512619
##
test/integration/smoke/test_primary_storage_nfsopts_kvm.py:
##
@@ -0,0 +1,162 @@
+# 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.
+
+import marvin
+from marvin.cloudstackTestCase import cloudstackTestCase
+from marvin.cloudstackAPI import *
+from marvin.lib.utils import *
+from marvin.lib.base import *
+from marvin.lib.common import (list_clusters, list_hosts, list_storage_pools)
+import xml.etree.ElementTree as ET
+from lxml import etree
+from nose.plugins.attrib import attr
+
+class TestNFSOptsKVM(cloudstackTestCase):
+""" Test cases for host HA using KVM host(s)
+"""
+
+def setUp(self):
+self.testClient = super(TestNFSOptsKVM, self).getClsTestClient()
+self.apiclient = self.testClient.getApiClient()
+self.dbclient = self.testClient.getDbConnection()
+self.services = self.testClient.getParsedTestDataConfig()
+self.logger = logging.getLogger('TestHAKVM')
+
+self.cluster = list_clusters(self.apiclient)[0]
+self.hypervisor = self.testClient.getHypervisorInfo()
+self.host = self.getHost()
+self.storagePool = self.getPrimaryStorage(self.cluster.id)
+self.hostConfig =
self.config.__dict__["zones"][0].__dict__["pods"][0].__dict__["clusters"][0].__dict__["hosts"][0].__dict__
+self.cluster_id = self.host.clusterid
+self.hostConfig["password"]="P@ssword123"
+self.sshClient = SshClient(
+host=self.host.ipaddress,
+port=22,
+user=self.hostConfig['username'],
+passwd=self.hostConfig['password'])
+
+
+def getHost(self):
+response = list_hosts(
+self.apiclient,
+type='Routing',
+hypervisor='kvm',
+state='Up',
+id=None
+)
+
+if response and len(response) > 0:
+self.host = response[0]
+return self.host
+raise self.skipTest("Not enough KVM hosts found, skipping NFS options
test")
+
+def getPrimaryStorage(self, clusterId):
+response = list_storage_pools(
+self.apiclient,
+clusterid=clusterId,
+type='NetworkFilesystem',
+state='Up',
+id=None,
+)
+if response and len(response) > 0:
+self.storage_pool = response[0]
+return self.storage_pool
+raise self.skipTest("Not enough KVM hosts found, skipping NFS options
test")
+
+def getNFSOptionsFromVirsh(self, poolId):
+virsh_cmd = "virsh pool-dumpxml %s" % poolId
+xml_res = self.sshClient.execute(virsh_cmd)
+xml_as_str = ''.join(xml_res)
+self.debug(xml_as_str)
+parser = etree.XMLParser(remove_blank_text=True)
+root = ET.fromstring(xml_as_str, parser=parser)
+mount_opts =
root.findall("{http://libvirt.org/schemas/storagepool/fs/1.0}mount_opts;)[0]
+
+options_map = {}
+for child in mount_opts:
+option = child.get('name').split("=")
+options_map[option[0]] = option[1]
+return options_map
+
+def getUnusedNFSVersions(self, filter):
+nfsstat_cmd = "nfsstat -m | sed -n '/%s/{ n; p }'" % filter
+nfsstats = self.sshClient.execute(nfsstat_cmd)
+versions = {"4.1": 0, "4.2": 0, "3": 0}
+for stat in nfsstats:
+vers = stat[stat.find("vers"):].split("=")[1].split(",")[0]
+versions[vers] += 1
+
+for key in versions:
+if versions[key] == 0:
+return key
+return None
+
+def getNFSOptionForPool(self, option, poolId):
+nfsstat_cmd = "nfsstat -m | sed -n '/%s/{ n; p }'" % poolId
+nfsstat = self.sshClient.execute(nfsstat_cmd)
+if (nfsstat == None):
+return None
+stat = nfsstat[0]
+vers = stat[stat.find(option):].split("=")[1].split(",")[0]
+return vers
+
+@attr(tags=["devcloud", "advanced", "advancedns", "smoke", "basic", "sg"],
required_hardware="true")
+def test_primary_storage_nfs_options_kvm(self):
+"""
+Tests that NFS
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
abh1sar commented on code in PR #8947:
URL: https://github.com/apache/cloudstack/pull/8947#discussion_r1577401536
##
test/integration/smoke/test_primary_storage_nfsopts_kvm.py:
##
@@ -0,0 +1,162 @@
+# 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.
+
+import marvin
+from marvin.cloudstackTestCase import cloudstackTestCase
+from marvin.cloudstackAPI import *
+from marvin.lib.utils import *
+from marvin.lib.base import *
+from marvin.lib.common import (list_clusters, list_hosts, list_storage_pools)
+import xml.etree.ElementTree as ET
+from lxml import etree
+from nose.plugins.attrib import attr
+
+class TestNFSOptsKVM(cloudstackTestCase):
+""" Test cases for host HA using KVM host(s)
+"""
+
+def setUp(self):
+self.testClient = super(TestNFSOptsKVM, self).getClsTestClient()
+self.apiclient = self.testClient.getApiClient()
+self.dbclient = self.testClient.getDbConnection()
+self.services = self.testClient.getParsedTestDataConfig()
+self.logger = logging.getLogger('TestHAKVM')
+
+self.cluster = list_clusters(self.apiclient)[0]
+self.hypervisor = self.testClient.getHypervisorInfo()
+self.host = self.getHost()
+self.storagePool = self.getPrimaryStorage(self.cluster.id)
+self.hostConfig =
self.config.__dict__["zones"][0].__dict__["pods"][0].__dict__["clusters"][0].__dict__["hosts"][0].__dict__
+self.cluster_id = self.host.clusterid
+self.hostConfig["password"]="P@ssword123"
+self.sshClient = SshClient(
+host=self.host.ipaddress,
+port=22,
+user=self.hostConfig['username'],
+passwd=self.hostConfig['password'])
+
+
+def getHost(self):
+response = list_hosts(
+self.apiclient,
+type='Routing',
+hypervisor='kvm',
+state='Up',
+id=None
+)
+
+if response and len(response) > 0:
+self.host = response[0]
+return self.host
+raise self.skipTest("Not enough KVM hosts found, skipping NFS options
test")
+
+def getPrimaryStorage(self, clusterId):
+response = list_storage_pools(
+self.apiclient,
+clusterid=clusterId,
+type='NetworkFilesystem',
+state='Up',
+id=None,
+)
+if response and len(response) > 0:
+self.storage_pool = response[0]
+return self.storage_pool
+raise self.skipTest("Not enough KVM hosts found, skipping NFS options
test")
+
+def getNFSOptionsFromVirsh(self, poolId):
+virsh_cmd = "virsh pool-dumpxml %s" % poolId
+xml_res = self.sshClient.execute(virsh_cmd)
+xml_as_str = ''.join(xml_res)
+self.debug(xml_as_str)
+parser = etree.XMLParser(remove_blank_text=True)
+root = ET.fromstring(xml_as_str, parser=parser)
+mount_opts =
root.findall("{http://libvirt.org/schemas/storagepool/fs/1.0}mount_opts;)[0]
+
+options_map = {}
+for child in mount_opts:
+option = child.get('name').split("=")
+options_map[option[0]] = option[1]
+return options_map
+
+def getUnusedNFSVersions(self, filter):
+nfsstat_cmd = "nfsstat -m | sed -n '/%s/{ n; p }'" % filter
+nfsstats = self.sshClient.execute(nfsstat_cmd)
+versions = {"4.1": 0, "4.2": 0, "3": 0}
+for stat in nfsstats:
+vers = stat[stat.find("vers"):].split("=")[1].split(",")[0]
+versions[vers] += 1
+
+for key in versions:
+if versions[key] == 0:
+return key
+return None
+
+def getNFSOptionForPool(self, option, poolId):
+nfsstat_cmd = "nfsstat -m | sed -n '/%s/{ n; p }'" % poolId
+nfsstat = self.sshClient.execute(nfsstat_cmd)
+if (nfsstat == None):
+return None
+stat = nfsstat[0]
+vers = stat[stat.find(option):].split("=")[1].split(",")[0]
+return vers
+
+@attr(tags=["devcloud", "advanced", "advancedns", "smoke", "basic", "sg"],
required_hardware="true")
+def test_primary_storage_nfs_options_kvm(self):
+"""
+Tests that NFS mount
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
abh1sar commented on PR #8947: URL: https://github.com/apache/cloudstack/pull/8947#issuecomment-2074164098 @sureshanaparti @harikrishna-patnala @shwstppr have addressed all review comments. Will raise a documentation PR shortly. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
abh1sar commented on code in PR #8947:
URL: https://github.com/apache/cloudstack/pull/8947#discussion_r1577328091
##
ui/src/config/section/infra/primaryStorages.js:
##
@@ -34,7 +34,7 @@ export default {
fields.push('zonename')
return fields
},
- details: ['name', 'id', 'ipaddress', 'type', 'scope', 'tags', 'path',
'provider', 'hypervisor', 'overprovisionfactor', 'disksizetotal',
'disksizeallocated', 'disksizeused', 'clustername', 'podname', 'zonename',
'created'],
+ details: ['name', 'id', 'ipaddress', 'type', 'scope', 'tags', 'path',
'provider', 'hypervisor', 'overprovisionfactor', 'disksizetotal',
'disksizeallocated', 'disksizeused', 'clustername', 'podname', 'zonename',
'nfsopts', 'created'],
Review Comment:
Moved to under `type`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
abh1sar commented on code in PR #8947:
URL: https://github.com/apache/cloudstack/pull/8947#discussion_r1577323243
##
ui/src/config/section/infra/primaryStorages.js:
##
@@ -109,6 +109,16 @@ export default {
defaultArgs: { enabled: true },
show: (record) => { return record.state === 'Disabled' }
},
+{
+ api: 'updateStoragePool',
+ icon: 'control-outlined',
+ label: 'label.action.edit.nfs.options',
+ message: 'message.action.edit.nfs.options',
+ dataView: true,
+ popup: true,
+ show: (record) => { return (record.type === 'NetworkFilesystem' &&
record.hypervisor === 'KVM' && record.state === 'Maintenance') },
+ component: shallowRef(defineAsyncComponent(() =>
import('@/views/infra/NFSOptsPrimaryStorage.vue')))
+},
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
abh1sar commented on code in PR #8947: URL: https://github.com/apache/cloudstack/pull/8947#discussion_r1577322814 ## ui/public/locales/en.json: ## @@ -3184,6 +3188,7 @@ "message.success.disable.saml.auth": "Successfully disabled SAML authorization", "message.success.disable.vpn": "Successfully disabled VPN", "message.success.edit.acl": "Successfully edited ACL rule", +"message.success.edit.nfsopts": "Successfully updated NFS options", Review Comment: Have moved Edit NFS mount options to Edit Primary Storage. So this message was removed. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
abh1sar commented on code in PR #8947: URL: https://github.com/apache/cloudstack/pull/8947#discussion_r1577322261 ## ui/public/locales/en.json: ## @@ -2440,6 +2442,7 @@ "message.action.disable.zone": "Please confirm that you want to disable this zone.", "message.action.download.iso": "Please confirm that you want to download this ISO.", "message.action.download.template": "Please confirm that you want to download this Template.", +"message.action.edit.nfs.options": "Please confirm that you want to change NFS mount options.This will cause the storage pool to be remounted on all KVM hosts.", Review Comment: Mentioned now. Please check -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
abh1sar commented on code in PR #8947:
URL: https://github.com/apache/cloudstack/pull/8947#discussion_r1577321806
##
server/src/main/java/com/cloud/storage/StorageManagerImpl.java:
##
@@ -1085,6 +1114,24 @@ public PrimaryDataStoreInfo
updateStoragePool(UpdateStoragePoolCmd cmd) throws I
throw new IllegalArgumentException("Unable to find storage pool
with ID: " + id);
}
+Map inputDetails =
extractApiParamAsMap(cmd.getDetails());
+if (inputDetails.containsKey("nfsopts")) {
+Long accountId = cmd.getEntityOwnerId();
+if (!_accountMgr.isRootAdmin(accountId)) {
+throw new PermissionDeniedException("Only root admin can
modify nfs options");
+}
+if (!pool.getHypervisor().equals(HypervisorType.KVM) &&
!pool.getHypervisor().equals((HypervisorType.Simulator))) {
+throw new InvalidParameterValueException("NFS options can only
be set for the hypervisor type " + HypervisorType.KVM);
+}
+if (!pool.getPoolType().equals(StoragePoolType.NetworkFilesystem))
{
+throw new InvalidParameterValueException("NFS options can only
be set on pool type " + StoragePoolType.NetworkFilesystem);
+}
+if (!pool.isInMaintenance()) {
+throw new InvalidParameterValueException("The storage pool
should be in maintenance mode to edit nfs options");
+}
+checkNfsOptions(inputDetails.get("nfsopts"));
+}
Review Comment:
Done
##
server/src/main/java/com/cloud/storage/StoragePoolAutomationImpl.java:
##
@@ -318,9 +324,19 @@ public boolean cancelMaintain(DataStore store) {
if (hosts == null || hosts.size() == 0) {
return true;
}
+
+Map details = null;
+if
(pool.getPoolType().equals(Storage.StoragePoolType.NetworkFilesystem)) {
+details = new HashMap<>();
+StoragePoolDetailVO nfsopts =
storagePoolDetailsDao.findDetail(poolVO.getId(), "nfsopts");
+if (nfsopts != null) {
+details.put("nfsopts", nfsopts.getValue());
+}
+}
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
abh1sar commented on code in PR #8947:
URL: https://github.com/apache/cloudstack/pull/8947#discussion_r1577321600
##
server/src/main/java/com/cloud/storage/StorageManagerImpl.java:
##
@@ -903,6 +918,20 @@ public PrimaryDataStoreInfo
createPool(CreateStoragePoolCmd cmd) throws Resource
}
Map details = extractApiParamAsMap(cmd.getDetails());
+if (details.containsKey("nfsopts")) {
+Long accountId = cmd.getEntityOwnerId();
+if (!_accountMgr.isRootAdmin(accountId)) {
Review Comment:
yes, it is not required here. Removed
##
server/src/main/java/com/cloud/storage/StorageManagerImpl.java:
##
@@ -903,6 +918,20 @@ public PrimaryDataStoreInfo
createPool(CreateStoragePoolCmd cmd) throws Resource
}
Map details = extractApiParamAsMap(cmd.getDetails());
+if (details.containsKey("nfsopts")) {
+Long accountId = cmd.getEntityOwnerId();
+if (!_accountMgr.isRootAdmin(accountId)) {
+throw new PermissionDeniedException("Only root admin can set
nfs options");
+}
+if (!hypervisorType.equals(HypervisorType.KVM) &&
!hypervisorType.equals(HypervisorType.Simulator)) {
+throw new InvalidParameterValueException("NFS options can not
be set for the hypervisor type " + hypervisorType);
+}
+if (!"nfs".equals(uriParams.get("scheme"))) {
+throw new InvalidParameterValueException("NFS options can only
be set on pool type " + StoragePoolType.NetworkFilesystem);
+}
+checkNfsOptions(details.get("nfsopts"));
+}
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
abh1sar commented on code in PR #8947: URL: https://github.com/apache/cloudstack/pull/8947#discussion_r1577321283 ## server/src/main/java/com/cloud/api/query/QueryManagerImpl.java: ## @@ -2994,6 +2994,11 @@ private ListResponse createStoragesPoolResponse(Pair createStoragesPoolResponse(Pair
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
abh1sar commented on code in PR #8947:
URL: https://github.com/apache/cloudstack/pull/8947#discussion_r1577321008
##
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java:
##
@@ -654,20 +654,54 @@ public KVMStoragePool createStoragePool(String name,
String host, int port, Stri
} catch (LibvirtException e) {
s_logger.error("Failure in attempting to see if an existing
storage pool might be using the path of the pool to be created:" + e);
}
+}
+
+List nfsopts = null;
+if (type == StoragePoolType.NetworkFilesystem) {
+if (details != null && details.containsKey("nfsopts")) {
+nfsopts =
Arrays.asList(details.get("nfsopts").replaceAll("\\s", "").split(","));
+}
+
+if (sp != null && nfsopts != null) {
+try {
+LibvirtStoragePoolDef poolDef = getStoragePoolDef(conn,
sp);
+Map poolNfsOptsMap = poolDef.getNfsOpts();
+boolean optionsDiffer = false;
+if (poolNfsOptsMap.size() != nfsopts.size()) {
+optionsDiffer = true;
+} else {
+for (String nfsopt : nfsopts) {
+if (!poolNfsOptsMap.containsKey(nfsopt)) {
+optionsDiffer = true;
+break;
+}
+}
+}
+if (optionsDiffer == true) {
Review Comment:
Done
##
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java:
##
@@ -654,20 +654,54 @@ public KVMStoragePool createStoragePool(String name,
String host, int port, Stri
} catch (LibvirtException e) {
s_logger.error("Failure in attempting to see if an existing
storage pool might be using the path of the pool to be created:" + e);
}
+}
+
+List nfsopts = null;
+if (type == StoragePoolType.NetworkFilesystem) {
+if (details != null && details.containsKey("nfsopts")) {
+nfsopts =
Arrays.asList(details.get("nfsopts").replaceAll("\\s", "").split(","));
+}
+
+if (sp != null && nfsopts != null) {
+try {
+LibvirtStoragePoolDef poolDef = getStoragePoolDef(conn,
sp);
+Map poolNfsOptsMap = poolDef.getNfsOpts();
+boolean optionsDiffer = false;
+if (poolNfsOptsMap.size() != nfsopts.size()) {
+optionsDiffer = true;
+} else {
+for (String nfsopt : nfsopts) {
+if (!poolNfsOptsMap.containsKey(nfsopt)) {
+optionsDiffer = true;
+break;
+}
+}
+}
+if (optionsDiffer == true) {
+sp.destroy();
+sp = null;
+}
+} catch (LibvirtException e) {
+s_logger.error("Failure in destroying the pre-existing
storage pool for changing the NFS options" + e);
+}
+}
+}
Review Comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
abh1sar commented on code in PR #8947:
URL: https://github.com/apache/cloudstack/pull/8947#discussion_r1577320634
##
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolDef.java:
##
@@ -124,10 +138,17 @@ public AuthenticationType getAuthType() {
return _authType;
}
+public Map getNfsOpts() {
+return _nfsopts;
+}
+
@Override
public String toString() {
StringBuilder storagePoolBuilder = new StringBuilder();
-if (_poolType == PoolType.GLUSTERFS) {
+if (_poolType == PoolType.NETFS && _nfsopts != null) {
Review Comment:
Switch could be used when only _poolType is being checked. I have tried to
refactor the method using switch. Please check.
##
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolXMLParser.java:
##
@@ -92,6 +92,22 @@ public LibvirtStoragePoolDef parseStoragePoolXML(String
poolXML) {
return new
LibvirtStoragePoolDef(LibvirtStoragePoolDef.PoolType.valueOf(format.toUpperCase()),
poolName, uuid, host, port, path, targetPath);
+} else if (type.equalsIgnoreCase("netfs")) {
+List nfsopts = new ArrayList<>();
+Element mountOpts = (Element)
rootElement.getElementsByTagName("fs:mount_opts").item(0);
+if (mountOpts != null) {
+NodeList options =
mountOpts.getElementsByTagName("fs:option");
+for (int i = 0; i < options.getLength(); i++) {
+Element option = (Element) options.item(i);
+nfsopts.add(option.getAttribute("name"));
+}
+}
+
+String path = getAttrValue("dir", "path", source);
+Element target =
(Element)rootElement.getElementsByTagName("target").item(0);
+String targetPath = getTagValue("path", target);
+
+return new
LibvirtStoragePoolDef(LibvirtStoragePoolDef.PoolType.valueOf(type.toUpperCase()),
poolName, uuid, host, path, targetPath, nfsopts);
Review Comment:
Done
##
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java:
##
@@ -654,20 +654,54 @@ public KVMStoragePool createStoragePool(String name,
String host, int port, Stri
} catch (LibvirtException e) {
s_logger.error("Failure in attempting to see if an existing
storage pool might be using the path of the pool to be created:" + e);
}
+}
+
+List nfsopts = null;
+if (type == StoragePoolType.NetworkFilesystem) {
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
abh1sar commented on code in PR #8947:
URL: https://github.com/apache/cloudstack/pull/8947#discussion_r1577319326
##
api/src/main/java/org/apache/cloudstack/api/response/StoragePoolResponse.java:
##
@@ -101,6 +101,10 @@ public class StoragePoolResponse extends
BaseResponseWithAnnotations {
@Param(description = "the tags for the storage pool")
private String tags;
+@SerializedName("nfsopts")
+@Param(description = "the nfs options for the storage pool")
Review Comment:
Made the change
##
api/src/main/java/org/apache/cloudstack/api/response/StoragePoolResponse.java:
##
@@ -101,6 +101,10 @@ public class StoragePoolResponse extends
BaseResponseWithAnnotations {
@Param(description = "the tags for the storage pool")
private String tags;
+@SerializedName("nfsopts")
+@Param(description = "the nfs options for the storage pool")
+private String nfsopts;
Review Comment:
Made the change
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
abh1sar commented on code in PR #8947:
URL: https://github.com/apache/cloudstack/pull/8947#discussion_r1577319088
##
api/src/main/java/org/apache/cloudstack/api/response/StoragePoolResponse.java:
##
@@ -101,6 +101,10 @@ public class StoragePoolResponse extends
BaseResponseWithAnnotations {
@Param(description = "the tags for the storage pool")
private String tags;
+@SerializedName("nfsopts")
Review Comment:
Done
##
api/src/main/java/org/apache/cloudstack/api/response/StoragePoolResponse.java:
##
@@ -101,6 +101,10 @@ public class StoragePoolResponse extends
BaseResponseWithAnnotations {
@Param(description = "the tags for the storage pool")
private String tags;
+@SerializedName("nfsopts")
+@Param(description = "the nfs options for the storage pool")
Review Comment:
Added
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
abh1sar commented on code in PR #8947:
URL: https://github.com/apache/cloudstack/pull/8947#discussion_r1576908083
##
server/src/main/java/com/cloud/storage/StorageManagerImpl.java:
##
@@ -839,6 +839,21 @@ protected String createLocalStoragePoolName(Host host,
StoragePoolInfo storagePo
return String.format("%s-%s-%s", StringUtils.trim(host.getName()),
"local", storagePoolInformation.getUuid().split("-")[0]);
}
+protected void checkNfsOptions(String nfsopts) throws
InvalidParameterValueException {
+String[] options = nfsopts.replaceAll("\\s", "").split(",");
+Map optionsMap = new HashMap<>();
+for (String option : options) {
+String[] keyValue = option.split("=");
+if (keyValue.length > 2) {
+throw new InvalidParameterValueException("Invalid value for
NFS option " + keyValue[0]);
+}
+if (optionsMap.containsKey(keyValue[0])) {
+throw new InvalidParameterValueException("Duplicate NFS option
values found for option " + keyValue[0]);
+}
+optionsMap.put(keyValue[0], null);
Review Comment:
Don't want to perform validations on any mount options as idea is to leave
it to the admin if they are using the correct options are not. Cloudstack is
only providing the functionality to add options.
We can give an indication that this feature is tested with two mount options
(`vers` and `nconnect`). But I am thinking that admin can give even more mount
options if they desire to do so. That way we don't have to make any changes for
supporting more options.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
abh1sar commented on code in PR #8947:
URL: https://github.com/apache/cloudstack/pull/8947#discussion_r1576891323
##
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java:
##
@@ -654,20 +654,54 @@ public KVMStoragePool createStoragePool(String name,
String host, int port, Stri
} catch (LibvirtException e) {
s_logger.error("Failure in attempting to see if an existing
storage pool might be using the path of the pool to be created:" + e);
}
+}
+
+List nfsopts = null;
+if (type == StoragePoolType.NetworkFilesystem) {
+if (details != null && details.containsKey("nfsopts")) {
+nfsopts =
Arrays.asList(details.get("nfsopts").replaceAll("\\s", "").split(","));
+}
+
+if (sp != null && nfsopts != null) {
+try {
+LibvirtStoragePoolDef poolDef = getStoragePoolDef(conn,
sp);
+Map poolNfsOptsMap = poolDef.getNfsOpts();
+boolean optionsDiffer = false;
+if (poolNfsOptsMap.size() != nfsopts.size()) {
+optionsDiffer = true;
+} else {
+for (String nfsopt : nfsopts) {
+if (!poolNfsOptsMap.containsKey(nfsopt)) {
Review Comment:
The key here contains both the option and the value. For example the key
would be "vers=4.1", so if the version is changing optionsDiffer would be true.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
abh1sar commented on code in PR #8947:
URL: https://github.com/apache/cloudstack/pull/8947#discussion_r1576888091
##
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolDef.java:
##
@@ -187,6 +208,13 @@ public String toString() {
storagePoolBuilder.append("" + _targetPath + "\n");
storagePoolBuilder.append("\n");
}
+if (_poolType == PoolType.NETFS && _nfsopts != null) {
+storagePoolBuilder.append("\n");
+for (Map.Entry options : _nfsopts.entrySet()) {
+storagePoolBuilder.append("\n");
+}
+storagePoolBuilder.append("\n");
Review Comment:
fs:mount_opts have to be added after the target tag on line 209
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
JoaoJandre commented on code in PR #8947:
URL: https://github.com/apache/cloudstack/pull/8947#discussion_r1572658067
##
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolDef.java:
##
@@ -187,6 +208,13 @@ public String toString() {
storagePoolBuilder.append("" + _targetPath + "\n");
storagePoolBuilder.append("\n");
}
+if (_poolType == PoolType.NETFS && _nfsopts != null) {
+storagePoolBuilder.append("\n");
+for (Map.Entry options : _nfsopts.entrySet()) {
+storagePoolBuilder.append("\n");
+}
+storagePoolBuilder.append("\n");
Review Comment:
why not put this inside the if block at line 148?
##
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java:
##
@@ -654,20 +654,54 @@ public KVMStoragePool createStoragePool(String name,
String host, int port, Stri
} catch (LibvirtException e) {
s_logger.error("Failure in attempting to see if an existing
storage pool might be using the path of the pool to be created:" + e);
}
+}
+
+List nfsopts = null;
+if (type == StoragePoolType.NetworkFilesystem) {
+if (details != null && details.containsKey("nfsopts")) {
+nfsopts =
Arrays.asList(details.get("nfsopts").replaceAll("\\s", "").split(","));
+}
+
+if (sp != null && nfsopts != null) {
+try {
+LibvirtStoragePoolDef poolDef = getStoragePoolDef(conn,
sp);
+Map poolNfsOptsMap = poolDef.getNfsOpts();
+boolean optionsDiffer = false;
+if (poolNfsOptsMap.size() != nfsopts.size()) {
+optionsDiffer = true;
+} else {
+for (String nfsopt : nfsopts) {
+if (!poolNfsOptsMap.containsKey(nfsopt)) {
+optionsDiffer = true;
+break;
+}
+}
+}
+if (optionsDiffer == true) {
Review Comment:
```suggestion
if (optionsDiffer) {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
shwstppr commented on code in PR #8947:
URL: https://github.com/apache/cloudstack/pull/8947#discussion_r1572165142
##
ui/src/config/section/infra/primaryStorages.js:
##
@@ -109,6 +109,16 @@ export default {
defaultArgs: { enabled: true },
show: (record) => { return record.state === 'Disabled' }
},
+{
+ api: 'updateStoragePool',
+ icon: 'control-outlined',
+ label: 'label.action.edit.nfs.options',
+ message: 'message.action.edit.nfs.options',
+ dataView: true,
+ popup: true,
+ show: (record) => { return (record.type === 'NetworkFilesystem' &&
record.hypervisor === 'KVM' && record.state === 'Maintenance') },
+ component: shallowRef(defineAsyncComponent(() =>
import('@/views/infra/NFSOptsPrimaryStorage.vue')))
+},
Review Comment:
Probably as discussed privately, move the option add NFS mount options in
the edit pool form. Show the field only for supported hypervisors/parameters.
And show the remount warning when the value is changed. From what I remember,
update network form kind of does the same thing for offering change
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
sureshanaparti commented on code in PR #8947:
URL: https://github.com/apache/cloudstack/pull/8947#discussion_r1572158692
##
ui/src/config/section/infra/primaryStorages.js:
##
@@ -34,7 +34,7 @@ export default {
fields.push('zonename')
return fields
},
- details: ['name', 'id', 'ipaddress', 'type', 'scope', 'tags', 'path',
'provider', 'hypervisor', 'overprovisionfactor', 'disksizetotal',
'disksizeallocated', 'disksizeused', 'clustername', 'podname', 'zonename',
'created'],
+ details: ['name', 'id', 'ipaddress', 'type', 'scope', 'tags', 'path',
'provider', 'hypervisor', 'overprovisionfactor', 'disksizetotal',
'disksizeallocated', 'disksizeused', 'clustername', 'podname', 'zonename',
'nfsopts', 'created'],
Review Comment:
better keep mount options under the storage type or path
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
sureshanaparti commented on code in PR #8947:
URL: https://github.com/apache/cloudstack/pull/8947#discussion_r1572153229
##
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java:
##
@@ -654,20 +654,54 @@ public KVMStoragePool createStoragePool(String name,
String host, int port, Stri
} catch (LibvirtException e) {
s_logger.error("Failure in attempting to see if an existing
storage pool might be using the path of the pool to be created:" + e);
}
+}
+
+List nfsopts = null;
+if (type == StoragePoolType.NetworkFilesystem) {
+if (details != null && details.containsKey("nfsopts")) {
+nfsopts =
Arrays.asList(details.get("nfsopts").replaceAll("\\s", "").split(","));
+}
+
+if (sp != null && nfsopts != null) {
+try {
+LibvirtStoragePoolDef poolDef = getStoragePoolDef(conn,
sp);
+Map poolNfsOptsMap = poolDef.getNfsOpts();
+boolean optionsDiffer = false;
+if (poolNfsOptsMap.size() != nfsopts.size()) {
+optionsDiffer = true;
+} else {
+for (String nfsopt : nfsopts) {
+if (!poolNfsOptsMap.containsKey(nfsopt)) {
Review Comment:
what if value changes for a key (say, nfs version updated)?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
sureshanaparti commented on code in PR #8947:
URL: https://github.com/apache/cloudstack/pull/8947#discussion_r1572156609
##
server/src/main/java/com/cloud/storage/StorageManagerImpl.java:
##
@@ -839,6 +839,21 @@ protected String createLocalStoragePoolName(Host host,
StoragePoolInfo storagePo
return String.format("%s-%s-%s", StringUtils.trim(host.getName()),
"local", storagePoolInformation.getUuid().split("-")[0]);
}
+protected void checkNfsOptions(String nfsopts) throws
InvalidParameterValueException {
+String[] options = nfsopts.replaceAll("\\s", "").split(",");
+Map optionsMap = new HashMap<>();
+for (String option : options) {
+String[] keyValue = option.split("=");
+if (keyValue.length > 2) {
+throw new InvalidParameterValueException("Invalid value for
NFS option " + keyValue[0]);
+}
+if (optionsMap.containsKey(keyValue[0])) {
+throw new InvalidParameterValueException("Duplicate NFS option
values found for option " + keyValue[0]);
+}
+optionsMap.put(keyValue[0], null);
Review Comment:
validation of nfs version and nconnect values, supported nfs versions check
needed here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
sureshanaparti commented on code in PR #8947:
URL: https://github.com/apache/cloudstack/pull/8947#discussion_r1572148503
##
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolDef.java:
##
@@ -55,6 +59,7 @@ public String toString() {
private String _authUsername;
private AuthenticationType _authType;
private String _secretUuid;
+private Map _nfsopts = new HashMap<>();
Review Comment:
```suggestion
private Map _nfsMountOpts = new HashMap<>();
```
update wherever applicable
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
sureshanaparti commented on code in PR #8947:
URL: https://github.com/apache/cloudstack/pull/8947#discussion_r1572145833
##
api/src/main/java/org/apache/cloudstack/api/response/StoragePoolResponse.java:
##
@@ -101,6 +101,10 @@ public class StoragePoolResponse extends
BaseResponseWithAnnotations {
@Param(description = "the tags for the storage pool")
private String tags;
+@SerializedName("nfsopts")
+@Param(description = "the nfs options for the storage pool")
+private String nfsopts;
Review Comment:
```suggestion
private String nfsMountOpts;
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
sureshanaparti commented on code in PR #8947:
URL: https://github.com/apache/cloudstack/pull/8947#discussion_r1572145353
##
api/src/main/java/org/apache/cloudstack/api/response/StoragePoolResponse.java:
##
@@ -101,6 +101,10 @@ public class StoragePoolResponse extends
BaseResponseWithAnnotations {
@Param(description = "the tags for the storage pool")
private String tags;
+@SerializedName("nfsopts")
+@Param(description = "the nfs options for the storage pool")
Review Comment:
```suggestion
@Param(description = "the nfs mount options for the storage pool")
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
sureshanaparti commented on code in PR #8947:
URL: https://github.com/apache/cloudstack/pull/8947#discussion_r1572144602
##
api/src/main/java/org/apache/cloudstack/api/response/StoragePoolResponse.java:
##
@@ -101,6 +101,10 @@ public class StoragePoolResponse extends
BaseResponseWithAnnotations {
@Param(description = "the tags for the storage pool")
private String tags;
+@SerializedName("nfsopts")
+@Param(description = "the nfs options for the storage pool")
Review Comment:
add _since_ attribute
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
harikrishna-patnala commented on code in PR #8947:
URL: https://github.com/apache/cloudstack/pull/8947#discussion_r1572113888
##
server/src/main/java/com/cloud/storage/StorageManagerImpl.java:
##
@@ -903,6 +918,20 @@ public PrimaryDataStoreInfo
createPool(CreateStoragePoolCmd cmd) throws Resource
}
Map details = extractApiParamAsMap(cmd.getDetails());
+if (details.containsKey("nfsopts")) {
+Long accountId = cmd.getEntityOwnerId();
+if (!_accountMgr.isRootAdmin(accountId)) {
Review Comment:
Do we need this specific check, createStoragePoolCmd is anyways a root admin
command only ?
##
ui/public/locales/en.json:
##
@@ -2440,6 +2442,7 @@
"message.action.disable.zone": "Please confirm that you want to disable this
zone.",
"message.action.download.iso": "Please confirm that you want to download this
ISO.",
"message.action.download.template": "Please confirm that you want to download
this Template.",
+"message.action.edit.nfs.options": "Please confirm that you want to change NFS
mount options.This will cause the storage pool to be remounted on all KVM
hosts.",
Review Comment:
Do we also need to mention, changes will be in effect after cancelling the
maintenance mode ?
##
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java:
##
@@ -654,20 +654,54 @@ public KVMStoragePool createStoragePool(String name,
String host, int port, Stri
} catch (LibvirtException e) {
s_logger.error("Failure in attempting to see if an existing
storage pool might be using the path of the pool to be created:" + e);
}
+}
+
+List nfsopts = null;
+if (type == StoragePoolType.NetworkFilesystem) {
Review Comment:
use equals()
##
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolDef.java:
##
@@ -75,6 +80,15 @@ public LibvirtStoragePoolDef(PoolType type, String poolName,
String uuid, String
_targetPath = targetPath;
}
+public LibvirtStoragePoolDef(PoolType type, String poolName, String uuid,
String host, String dir, String targetPath, List nfsopts) {
+this(type, poolName, uuid, host, dir, targetPath);
+if (nfsopts != null) {
Review Comment:
Please use CollectionUtils.isNotEmpty() and in other places as well
##
ui/public/locales/en.json:
##
@@ -3184,6 +3188,7 @@
"message.success.disable.saml.auth": "Successfully disabled SAML
authorization",
"message.success.disable.vpn": "Successfully disabled VPN",
"message.success.edit.acl": "Successfully edited ACL rule",
+"message.success.edit.nfsopts": "Successfully updated NFS options",
Review Comment:
May be here ! mentioning about the effect of changes after cancelling the
maintenance mode ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
blueorangutan commented on PR #8947: URL: https://github.com/apache/cloudstack/pull/8947#issuecomment-2066167370 Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 9324 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
DaanHoogland commented on code in PR #8947:
URL: https://github.com/apache/cloudstack/pull/8947#discussion_r1571997393
##
api/src/main/java/org/apache/cloudstack/api/response/StoragePoolResponse.java:
##
@@ -101,6 +101,10 @@ public class StoragePoolResponse extends
BaseResponseWithAnnotations {
@Param(description = "the tags for the storage pool")
private String tags;
+@SerializedName("nfsopts")
Review Comment:
move to ApiConstant
##
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolDef.java:
##
@@ -124,10 +138,17 @@ public AuthenticationType getAuthType() {
return _authType;
}
+public Map getNfsOpts() {
+return _nfsopts;
+}
+
@Override
public String toString() {
StringBuilder storagePoolBuilder = new StringBuilder();
-if (_poolType == PoolType.GLUSTERFS) {
+if (_poolType == PoolType.NETFS && _nfsopts != null) {
Review Comment:
switch statement?
##
server/src/main/java/com/cloud/storage/StorageManagerImpl.java:
##
@@ -903,6 +918,20 @@ public PrimaryDataStoreInfo
createPool(CreateStoragePoolCmd cmd) throws Resource
}
Map details = extractApiParamAsMap(cmd.getDetails());
+if (details.containsKey("nfsopts")) {
+Long accountId = cmd.getEntityOwnerId();
+if (!_accountMgr.isRootAdmin(accountId)) {
+throw new PermissionDeniedException("Only root admin can set
nfs options");
+}
+if (!hypervisorType.equals(HypervisorType.KVM) &&
!hypervisorType.equals(HypervisorType.Simulator)) {
+throw new InvalidParameterValueException("NFS options can not
be set for the hypervisor type " + hypervisorType);
+}
+if (!"nfs".equals(uriParams.get("scheme"))) {
+throw new InvalidParameterValueException("NFS options can only
be set on pool type " + StoragePoolType.NetworkFilesystem);
+}
+checkNfsOptions(details.get("nfsopts"));
+}
Review Comment:
new method
##
test/integration/smoke/test_primary_storage_nfsopts_kvm.py:
##
@@ -0,0 +1,162 @@
+# 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.
+
+import marvin
+from marvin.cloudstackTestCase import cloudstackTestCase
+from marvin.cloudstackAPI import *
+from marvin.lib.utils import *
+from marvin.lib.base import *
+from marvin.lib.common import (list_clusters, list_hosts, list_storage_pools)
+import xml.etree.ElementTree as ET
+from lxml import etree
+from nose.plugins.attrib import attr
+
+class TestNFSOptsKVM(cloudstackTestCase):
+""" Test cases for host HA using KVM host(s)
+"""
+
+def setUp(self):
+self.testClient = super(TestNFSOptsKVM, self).getClsTestClient()
+self.apiclient = self.testClient.getApiClient()
+self.dbclient = self.testClient.getDbConnection()
+self.services = self.testClient.getParsedTestDataConfig()
+self.logger = logging.getLogger('TestHAKVM')
+
+self.cluster = list_clusters(self.apiclient)[0]
+self.hypervisor = self.testClient.getHypervisorInfo()
+self.host = self.getHost()
+self.storagePool = self.getPrimaryStorage(self.cluster.id)
+self.hostConfig =
self.config.__dict__["zones"][0].__dict__["pods"][0].__dict__["clusters"][0].__dict__["hosts"][0].__dict__
+self.cluster_id = self.host.clusterid
+self.hostConfig["password"]="P@ssword123"
+self.sshClient = SshClient(
+host=self.host.ipaddress,
+port=22,
+user=self.hostConfig['username'],
+passwd=self.hostConfig['password'])
+
+
+def getHost(self):
+response = list_hosts(
+self.apiclient,
+type='Routing',
+hypervisor='kvm',
+state='Up',
+id=None
+)
+
+if response and len(response) > 0:
+self.host = response[0]
+return self.host
+raise self.skipTest("Not enough KVM hosts found, skipping NFS options
test")
+
+def getPrimaryStorage(self, clusterId):
+response = list_storage_pools(
+
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
rohityadavcloud commented on PR #8947: URL: https://github.com/apache/cloudstack/pull/8947#issuecomment-2066050706 @blueorangutan package -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
blueorangutan commented on PR #8947: URL: https://github.com/apache/cloudstack/pull/8947#issuecomment-2066049537 @harikrishna-patnala a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
harikrishna-patnala commented on PR #8947: URL: https://github.com/apache/cloudstack/pull/8947#issuecomment-2066044406 @blueorangutan package -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
rohityadavcloud commented on PR #8947: URL: https://github.com/apache/cloudstack/pull/8947#issuecomment-2066041548 @abh1sar given this is a bugfix/improvement related to https://github.com/apache/cloudstack/issues/4482 pl change the base branch to 4.19 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage [cloudstack]
abh1sar opened a new pull request, #8947:
URL: https://github.com/apache/cloudstack/pull/8947
### Description
This PR adds the UI and CLI support through which NFS mount options can be
specified for a primary storage on KVM hosts.
The mount options tested for this PR are **NFS version** and **nconnect**.
Mount options can be given while adding a Primary storage using the add Zone
wizard. It can also be given from the Add Primary storage form. A new field
`NFS mount options` is provided in the respective forms.
Mount options can be later modified also from the Primary Storage deatail
view via a new action button.
The storage pool needs to be remounted on all the hosts for the new mount
option to take affect. So, for a pre-existing storage pool, it has to be in
maintenance mode for this functionality to be visible.
NFS mount options are saved in storage_pool_details table and the
createStoragePool and updateStoragePool APIs takes the input as a details
parameter.
**A note on the nconnect option.**
(https://documentation.suse.com/sles/15-SP5/html/SLES-all/cha-nfs.html)
This option defines the count of TCP conncetions that the client makes to
the NFS server. All the mounts from the same server on the client will have the
same nconnect value and all mounts will share the same number of TCP
connections.
The nconnect setting is applied only during the first mount process to the
particular NFS server. If the same client executes the mount command to the
same NFS server, all already established connections will be shared—no new
connection will be established. To change the nconnect setting, you have to
unmount all clients connections to the particular NFS server. Then you can
define a new value of the nconnect option.
So, from CloudStack’s perspective also, the first storage pool created from
a NFS server will set the nconnect setting on the hypervisor host corresponding
to the server. Specifying a different nconnect mount option while creating a
new storage pool from the same server will not change the nconnect setting on
the host.
Similarly if there is only one pre-existing storage pool from a give NFS
server mounted on the host, modifying the nconnect mount option via CloudStack
will change the nconnect setting on that host.
If there are more than one storage pools from the same server mounted on a
host. Changing the nconnect mount option on one of the storage pools via
CloudStack will not do anything. To change the nconnect setting on the host,
after modifying nconnect mount option on all storage pools, the host needs to
be restarted.
### Types of changes
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
- [ ] build/CI
### Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
- [ ] Major
- [x] Minor
Bug Severity
- [ ] BLOCKER
- [ ] Critical
- [ ] Major
- [ ] Minor
- [ ] Trivial
### Screenshots (if appropriate):
### How Has This Been Tested?
How did you try to break this feature and the system with this change?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
