[GitHub] cloudstack pull request: CLOUDSTACK-9252: Support configurable NFS...

2016-03-21 Thread bvbharatk
Github user bvbharatk commented on the pull request: https://github.com/apache/cloudstack/pull/1361#issuecomment-199529926 ### ACS CI BVT Run **Sumarry:** Build Number 116 Hypervisor xenserver NetworkType Advanced Passed=105 Failed=13 Skipped=4

[GitHub] cloudstack pull request: CLOUDSTACK-9252: Support configurable NFS...

2016-02-24 Thread rafaelweingartner
Github user rafaelweingartner commented on the pull request: https://github.com/apache/cloudstack/pull/1361#issuecomment-188440358 merged based on testes and reviews --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If

[GitHub] cloudstack pull request: CLOUDSTACK-9252: Support configurable NFS...

2016-02-24 Thread asfgit
Github user asfgit closed the pull request at: https://github.com/apache/cloudstack/pull/1361 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature

[GitHub] cloudstack pull request: CLOUDSTACK-9252: Support configurable NFS...

2016-02-23 Thread GabrielBrascher
Github user GabrielBrascher commented on the pull request: https://github.com/apache/cloudstack/pull/1361#issuecomment-188040784 @nvazquez LGTM based on integration tests and code. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub

[GitHub] cloudstack pull request: CLOUDSTACK-9252: Support configurable NFS...

2016-02-22 Thread nvazquez
Github user nvazquez commented on the pull request: https://github.com/apache/cloudstack/pull/1361#issuecomment-187396165 Hi @rafaelweingartner @kishankavala I've run test_ssvm.py, test_secondary_storage.py and test_vm_life_cycle.py integration tests under real hardware. This are my

[GitHub] cloudstack pull request: CLOUDSTACK-9252: Support configurable NFS...

2016-02-17 Thread serg38
Github user serg38 commented on the pull request: https://github.com/apache/cloudstack/pull/1361#issuecomment-185256350 If requirements to use particular NFS version is known beforehand it can be specified during the image store creation using details argument

[GitHub] cloudstack pull request: CLOUDSTACK-9252: Support configurable NFS...

2016-02-17 Thread nvazquez
Github user nvazquez commented on the pull request: https://github.com/apache/cloudstack/pull/1361#issuecomment-185194859 Sure @kishankavala, to set NFS version for a store with id STORE_ID you'll need to insert a record in image_store_details_table which has: * store_id =

[GitHub] cloudstack pull request: CLOUDSTACK-9252: Support configurable NFS...

2016-02-17 Thread kishankavala
Github user kishankavala commented on the pull request: https://github.com/apache/cloudstack/pull/1361#issuecomment-185176731 @nvazquez Can you please repond to my earlier question also. >> How is the nfs.version set in image_store_details table? --- If your project is set up

[GitHub] cloudstack pull request: CLOUDSTACK-9252: Support configurable NFS...

2016-02-16 Thread rafaelweingartner
Github user rafaelweingartner commented on the pull request: https://github.com/apache/cloudstack/pull/1361#issuecomment-184915639 @serg38 , @nvazquez, we still need some other LGTM here. You could ask for some help reviewing this PR at Slack or @dev. Even with other LGTM, I think it

[GitHub] cloudstack pull request: CLOUDSTACK-9252: Support configurable NFS...

2016-02-16 Thread serg38
Github user serg38 commented on the pull request: https://github.com/apache/cloudstack/pull/1361#issuecomment-184799968 Hi committers, Can someone perform a second review and merge it at the earliest? We would like this to be part of 4.9? --- If your project is set up for

[GitHub] cloudstack pull request: CLOUDSTACK-9252: Support configurable NFS...

2016-02-09 Thread rafaelweingartner
Github user rafaelweingartner commented on the pull request: https://github.com/apache/cloudstack/pull/1361#issuecomment-181819761 @nvazquez, luckily the Jenkins build finished with success now. @kishankavala, for now, I believe the simplest solution is the better. When more

[GitHub] cloudstack pull request: CLOUDSTACK-9252: Support configurable NFS...

2016-02-09 Thread kishankavala
Github user kishankavala commented on the pull request: https://github.com/apache/cloudstack/pull/1361#issuecomment-181771585 One more query: How is the nfs.version set in image_store_details table? --- If your project is set up for it, you can reply to this email and have your

[GitHub] cloudstack pull request: CLOUDSTACK-9252: Support configurable NFS...

2016-02-09 Thread nvazquez
Github user nvazquez commented on the pull request: https://github.com/apache/cloudstack/pull/1361#issuecomment-181909261 Thanks a lot @rafaelweingartner for your words and your help! --- If your project is set up for it, you can reply to this email and have your reply appear on

[GitHub] cloudstack pull request: CLOUDSTACK-9252: Support configurable NFS...

2016-02-08 Thread kishankavala
Github user kishankavala commented on the pull request: https://github.com/apache/cloudstack/pull/1361#issuecomment-181715514 @nvazquez Apologies for reviewing it late. 1. Since version is fetched from image_store_details, can we send a map with all details for the image_store

[GitHub] cloudstack pull request: CLOUDSTACK-9252: Support configurable NFS...

2016-02-08 Thread DaanHoogland
Github user DaanHoogland commented on the pull request: https://github.com/apache/cloudstack/pull/1361#issuecomment-181569850 @nvazquez I think the job doesn't clean the prior classes well. I cleared the workspace. Can you push again? --- If your project is set up for it, you can

[GitHub] cloudstack pull request: CLOUDSTACK-9252: Support configurable NFS...

2016-02-08 Thread nvazquez
Github user nvazquez commented on the pull request: https://github.com/apache/cloudstack/pull/1361#issuecomment-181571630 Thanks @DaanHoogland, I pushed again --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your

[GitHub] cloudstack pull request: CLOUDSTACK-9252: Support configurable NFS...

2016-02-08 Thread nvazquez
Github user nvazquez commented on the pull request: https://github.com/apache/cloudstack/pull/1361#issuecomment-181644342 This time it failed for timeout --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project

[GitHub] cloudstack pull request: CLOUDSTACK-9252: Support configurable NFS...

2016-02-08 Thread rafaelweingartner
Github user rafaelweingartner commented on the pull request: https://github.com/apache/cloudstack/pull/1361#issuecomment-181646167 Since it timed out, what about squashing those 3 last commits into one, and crossing the fingers hoping that jenkins succeeds. --- If your project is

[GitHub] cloudstack pull request: CLOUDSTACK-9252: Support configurable NFS...

2016-02-08 Thread nvazquez
Github user nvazquez commented on the pull request: https://github.com/apache/cloudstack/pull/1361#issuecomment-181646997 Sure, I'll do it --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have

[GitHub] cloudstack pull request: CLOUDSTACK-9252: Support configurable NFS...

2016-02-07 Thread nvazquez
Github user nvazquez commented on the pull request: https://github.com/apache/cloudstack/pull/1361#issuecomment-181041714 Hi @DaanHoogland can you help us with Jenkins build? A test file SecondaryStorageManagerTest.java is failing, but that file is not in the branch. I could compile

[GitHub] cloudstack pull request: CLOUDSTACK-9252: Support configurable NFS...

2016-02-07 Thread cristofolini
Github user cristofolini commented on the pull request: https://github.com/apache/cloudstack/pull/1361#issuecomment-181047258 @nvazquez That is very strange. As far as I can tell there isn't a file with that name neither on the current master nor on your branch. There is however a

[GitHub] cloudstack pull request: CLOUDSTACK-9252: Support configurable NFS...

2016-02-03 Thread rafaelweingartner
Github user rafaelweingartner commented on the pull request: https://github.com/apache/cloudstack/pull/1361#issuecomment-179310061 @nvazquez, if you have any doubt in some of the spring modules, just shoot me an email (If I have the knowledge I will not hesitate to help you). Now

[GitHub] cloudstack pull request: CLOUDSTACK-9252: Support configurable NFS...

2016-02-01 Thread nvazquez
Github user nvazquez commented on the pull request: https://github.com/apache/cloudstack/pull/1361#issuecomment-178190394 Hi @rafaelweingartner Thanks for your comments! I've been working following your indications, however I didn't create any test case. I wanted to ask

[GitHub] cloudstack pull request: CLOUDSTACK-9252: Support configurable NFS...

2016-02-01 Thread rafaelweingartner
Github user rafaelweingartner commented on the pull request: https://github.com/apache/cloudstack/pull/1361#issuecomment-178267025 Hi @nvazquez, I see that you have created a spring bean as I said. However, I noticed some things that may not be needed or that seemed out of place

[GitHub] cloudstack pull request: CLOUDSTACK-9252: Support configurable NFS...

2016-02-01 Thread nvazquez
Github user nvazquez commented on the pull request: https://github.com/apache/cloudstack/pull/1361#issuecomment-178346694 @rafaelweingartner thanks for your reply! I'm familiar to Spring, I've worked with it, I know how it works but I wouldn't say I know it in depth. If you

[GitHub] cloudstack pull request: CLOUDSTACK-9252: Support configurable NFS...

2016-01-29 Thread nvazquez
Github user nvazquez commented on the pull request: https://github.com/apache/cloudstack/pull/1361#issuecomment-176752434 @rafaelweingartner I pushed some changes according to your comments. Is it better like this? --- If your project is set up for it, you can reply to this email

[GitHub] cloudstack pull request: CLOUDSTACK-9252: Support configurable NFS...

2016-01-29 Thread rafaelweingartner
Github user rafaelweingartner commented on the pull request: https://github.com/apache/cloudstack/pull/1361#issuecomment-176948581 It is better now. I only have to point two things out. 1 – About the class “com.cloud.storage.ImageStoreDetailsUtil” that you created. You

[GitHub] cloudstack pull request: CLOUDSTACK-9252: Support configurable NFS...

2016-01-26 Thread nvazquez
Github user nvazquez commented on the pull request: https://github.com/apache/cloudstack/pull/1361#issuecomment-175056442 Hi @rafaelweingartner, Thanks for your review! I agree with @serg38, it was thought just to extend the functionality by adding an entry on

[GitHub] cloudstack pull request: CLOUDSTACK-9252: Support configurable NFS...

2016-01-26 Thread rafaelweingartner
Github user rafaelweingartner commented on the pull request: https://github.com/apache/cloudstack/pull/1361#issuecomment-175093081 Ok, I understood that you want to maintain compatibility. What I do not understand is that: If there is a problem with some NFS (they do not support

[GitHub] cloudstack pull request: CLOUDSTACK-9252: Support configurable NFS...

2016-01-26 Thread nvazquez
Github user nvazquez commented on the pull request: https://github.com/apache/cloudstack/pull/1361#issuecomment-175103859 Ok, @rafaelweingartner I'll work on refactoring this. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as

[GitHub] cloudstack pull request: CLOUDSTACK-9252: Support configurable NFS...

2016-01-25 Thread serg38
Github user serg38 commented on the pull request: https://github.com/apache/cloudstack/pull/1361#issuecomment-174554271 Unfortunately NFS version negotiation doesn't work properly with all storage vendors. Some vendors e.g. Tintri require that vers=3 is supplied in mount command

[GitHub] cloudstack pull request: CLOUDSTACK-9252: Support configurable NFS...

2016-01-25 Thread rafaelweingartner
Github user rafaelweingartner commented on the pull request: https://github.com/apache/cloudstack/pull/1361#issuecomment-174734613 @serg38, I did not know that, giving your explanation I rest my arguments ;) @nvazquez, you changed the method “getMountPoint” to receive a

[GitHub] cloudstack pull request: CLOUDSTACK-9252: Support configurable NFS...

2016-01-25 Thread serg38
Github user serg38 commented on the pull request: https://github.com/apache/cloudstack/pull/1361#issuecomment-174746870 I believe the main reason is to provide the most of backward compatibility. image_store_details doesn't have NFS version in existing installations so after the

[GitHub] cloudstack pull request: CLOUDSTACK-9252: Support configurable NFS...

2016-01-23 Thread rafaelweingartner
Github user rafaelweingartner commented on the pull request: https://github.com/apache/cloudstack/pull/1361#issuecomment-174177981 Hi @ nvazquez, Are you sure that the problem is caused because we are not telling the NFS protocol version to the mount command? If you a