[GitHub] cloudstack pull request: CLOUDSTACK-8841: Storage XenMotion from X...

[priyankparihar]

Github user priyankparihar commented on the pull request:

https://github.com/apache/cloudstack/pull/815#issuecomment-219637824
  
@rhtyd  
>please rebase against latest master and push -f.
Done.


---
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 is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8841: Storage XenMotion from X...

[rhtyd]

Github user rhtyd commented on the pull request:

https://github.com/apache/cloudstack/pull/815#issuecomment-216189486
  
@priyankparihar please rebase against latest master and push -f, update on 
status of your PR



---
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 is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8841: Storage XenMotion from X...

[koushik-das]

Github user koushik-das commented on the pull request:

https://github.com/apache/cloudstack/pull/815#issuecomment-214208033
  
@priyankparihar Are you saying that you have tested the fix in all 
scenarios with different HVs (XS, KVM, Vmware)? Also if HV doesn't allow 
certain operations it is best to prevent them with appropriate checks rather 
than some failure in the HV. 


---
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 is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8841: Storage XenMotion from X...

[priyankparihar]

Github user priyankparihar commented on the pull request:

https://github.com/apache/cloudstack/pull/815#issuecomment-214146326
  
@koushik-das 
Currently UI does not allow migration between different versions, and 
changing UI for this change is not elegant. Purpose of doing this change is if 
some user **intentionally** want to do migration from lower to higher then he 
can do it via API(from UI he can not do this). Other way around Hypervisor will 
not allow him.

@agneya2001 @sateesh-chodapuneedi @koushik-das 
Your point looks good, but  How ordering could be defined ?



---
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 is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8841: Storage XenMotion from X...

[priyankparihar]

Github user priyankparihar commented on the pull request:

https://github.com/apache/cloudstack/pull/815#issuecomment-212846127
  
@sateesh-chodapuneedi @koushik-das  I will get back to you soon.


---
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 is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8841: Storage XenMotion from X...

[sateesh-chodapuneedi]

Github user sateesh-chodapuneedi commented on the pull request:

https://github.com/apache/cloudstack/pull/815#issuecomment-212845624
  
>Purpose of doing this change is if some user intentionally want to do 
migration from lower to higher then he can do it via API(from UI he can not do 
this). Other way around hypervisor will not allow him.
@priyankparihar Even if user intentionally tries to perform migration via 
API, ACS should fail the attempt after checking for version check condition, 
rather than leaving this to hypervisor. Construct the condition per hypervisor 
documentation and simply fail it with information/documentation references, if 
required. 

Also leaving it to hypervisor may be costlier, this being, storage 
migration task, is an async job which increase async job count to be 
tracked/managed by management server and also may involve multiple hypervisor 
API calls, even for failure cases. It's good to reduce the jobs and API calls 
if the outcome is known definitively in advance. 


---
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 is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8841: Storage XenMotion from X...

[sateesh-chodapuneedi]

Github user sateesh-chodapuneedi commented on the pull request:

https://github.com/apache/cloudstack/pull/815#issuecomment-212842765
  
>>Currently UI does not allow migration between different versions, and 
changing UI for this change is not elegant. 
@priyankparihar 
Why allow this only from API? If hypervisor allows migration from lower 
version to higher version, then let the API that is supposed to find target 
hosts for migration pick all such hosts. Per hypervisor documentation (both 
XenServer & VMware) migration is allowed from lower to higher versions. In such 
case we should let the API `[findHostsForMigration 
API](https://cloudstack.apache.org/api/apidocs-4.2/root_admin/findHostsForMigration.html)`
 list hosts with higher versions compared to source host.


---
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 is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8841: Storage XenMotion from X...

[koushik-das]

Github user koushik-das commented on the pull request:

https://github.com/apache/cloudstack/pull/815#issuecomment-212832800
  
@priyankparihar This version check removal is at a generic place and impact 
all HVs. Since you have only verified for XS 6.2 -> 6.5, there are concerns as 
to what happens for other HVs/versions. 


---
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 is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8841: Storage XenMotion from X...

[rajesh-battala]

Github user rajesh-battala commented on the pull request:

https://github.com/apache/cloudstack/pull/815#issuecomment-212777172
  
Looks good to me. 


---
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 is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8841: Storage XenMotion from X...

[priyankparihar]

Github user priyankparihar commented on the pull request:

https://github.com/apache/cloudstack/pull/815#issuecomment-163542539
  
@DaanHoogland I did this change because UI does not allow  migration 
between different versions of hyper-visors of  but sometimes user wants to do 
migration from  Lower to Higher Version. So now he can do it via API.


---
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 is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8841: Storage XenMotion from X...

[DaanHoogland]

Github user DaanHoogland commented on the pull request:

https://github.com/apache/cloudstack/pull/815#issuecomment-162455918
  
Does what it says but probably more. Do we not mind any other version? 
@priyankparihar can you show how ACS doesn't allow invalid migrations, "Storage 
XenMotion( because inconsistent(version) Hypervisor Hosts will not be listed 
for migration in UI)."?


---
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 is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8841: Storage XenMotion from X...

[remibergsma]

Github user remibergsma commented on the pull request:

https://github.com/apache/cloudstack/pull/815#issuecomment-162346266
  
Pinging @DaanHoogland to review. Will run some tests.


---
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 is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8841: Storage XenMotion from X...

[bhaisaab]

Github user bhaisaab commented on the pull request:

https://github.com/apache/cloudstack/pull/815#issuecomment-140650617
  
LGTM cc @abhinandanprateek 


---
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 is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8841: Storage XenMotion from X...

[priyankparihar]

GitHub user priyankparihar opened a pull request:

https://github.com/apache/cloudstack/pull/815

CLOUDSTACK-8841: Storage XenMotion from XS 6.2 to XS 6.5 fails.

Removed Host version check in API. Because 
Case 1:(Lower to Higher  Version)
Storage XenMotion from lower version to higher version is valid.
Case 2:(Higher to Lower Version)
In this case system(Host) will not allow.

So no need to check version in API. 

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/priyankparihar/cloudstack CS-43756

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cloudstack/pull/815.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #815


commit 2fc9e973b11fc644f1027e93ba52ced5dda4f408
Author: Priyank Parihar 
Date:   2015-09-14T05:42:28Z

CLOUDSTACK-8841: Storage XenMotion from XS 6.2 to XS 6.5 fails.




---
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 is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---