[GitHub] rafaelweingartner commented on issue #2761: Add managed storage pool constraints to MigrateWithVolume API method
rafaelweingartner commented on issue #2761: Add managed storage pool constraints to MigrateWithVolume API method URL: https://github.com/apache/cloudstack/pull/2761#issuecomment-425532576 @rhtyd thanks. I have replied to the e-mail thread. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rafaelweingartner commented on issue #2761: Add managed storage pool constraints to MigrateWithVolume API method
rafaelweingartner commented on issue #2761: Add managed storage pool constraints to MigrateWithVolume API method URL: https://github.com/apache/cloudstack/pull/2761#issuecomment-417859867 @borisstoyanov after the rebase, the errors @DaanHoogland said were a problem disappeared, but new ones appeared. Is this some sort of environment problem? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rafaelweingartner commented on issue #2761: Add managed storage pool constraints to MigrateWithVolume API method
rafaelweingartner commented on issue #2761: Add managed storage pool constraints to MigrateWithVolume API method URL: https://github.com/apache/cloudstack/pull/2761#issuecomment-416901681 @borisstoyanov done. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rafaelweingartner commented on issue #2761: Add managed storage pool constraints to MigrateWithVolume API method
rafaelweingartner commented on issue #2761: Add managed storage pool constraints to MigrateWithVolume API method URL: https://github.com/apache/cloudstack/pull/2761#issuecomment-416790608 @GabrielBrascher do you have some feedback here? @DaanHoogland do you want to run tests again as those errors do not seem to be related to the changes here? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rafaelweingartner commented on issue #2761: Add managed storage pool constraints to MigrateWithVolume API method
rafaelweingartner commented on issue #2761: Add managed storage pool constraints to MigrateWithVolume API method URL: https://github.com/apache/cloudstack/pull/2761#issuecomment-412281205 @mike-tutkowski unit tests added. Do you approve the PR? @blueorangutan package @borisstoyanov can you please run the integration tests? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rafaelweingartner commented on issue #2761: Add managed storage pool constraints to MigrateWithVolume API method
rafaelweingartner commented on issue #2761: Add managed storage pool constraints to MigrateWithVolume API method URL: https://github.com/apache/cloudstack/pull/2761#issuecomment-412272289 I will try to create the unit-tests today. No need to kick any more tests for now. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rafaelweingartner commented on issue #2761: Add managed storage pool constraints to MigrateWithVolume API method
rafaelweingartner commented on issue #2761: Add managed storage pool constraints to MigrateWithVolume API method URL: https://github.com/apache/cloudstack/pull/2761#issuecomment-409890686 @DaanHoogland that is what I was going to say to Mike. We can squash at the merge "moment". Can we wait more one week or so before merging? I would like to cover this code with unit tests. It is a very complicated and delicate one code, and it does not have unit-tests for it. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rafaelweingartner commented on issue #2761: Add managed storage pool constraints to MigrateWithVolume API method
rafaelweingartner commented on issue #2761: Add managed storage pool constraints to MigrateWithVolume API method URL: https://github.com/apache/cloudstack/pull/2761#issuecomment-409769607 That is a good suggestion. I will implement it. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rafaelweingartner commented on issue #2761: Add managed storage pool constraints to MigrateWithVolume API method
rafaelweingartner commented on issue #2761: Add managed storage pool constraints to MigrateWithVolume API method URL: https://github.com/apache/cloudstack/pull/2761#issuecomment-409761288 Ah yeas, you are right. A terrible miss on my side. I am sorry. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rafaelweingartner commented on issue #2761: Add managed storage pool constraints to MigrateWithVolume API method
rafaelweingartner commented on issue #2761: Add managed storage pool constraints to MigrateWithVolume API method URL: https://github.com/apache/cloudstack/pull/2761#issuecomment-409760596 @mike-tutkowski why didn't it work? I checked ant the volume class implements the hashcode and equals method. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rafaelweingartner commented on issue #2761: Add managed storage pool constraints to MigrateWithVolume API method
rafaelweingartner commented on issue #2761: Add managed storage pool
constraints to MigrateWithVolume API method
URL: https://github.com/apache/cloudstack/pull/2761#issuecomment-409683757
Got it! I just changed a bit to avoid those nested IF/ELSE{IF}.
What do you think of the last commit now?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] rafaelweingartner commented on issue #2761: Add managed storage pool constraints to MigrateWithVolume API method
rafaelweingartner commented on issue #2761: Add managed storage pool constraints to MigrateWithVolume API method URL: https://github.com/apache/cloudstack/pull/2761#issuecomment-409556819 @Mike, I think I understood you and your suggestion https://github.com/mike-tutkowski/cloudstack/commit/07279b41a3277becdc3b6c79f69f46559958f442 now. Can you take a look at the latest code I pushed? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rafaelweingartner commented on issue #2761: Add managed storage pool constraints to MigrateWithVolume API method
rafaelweingartner commented on issue #2761: Add managed storage pool constraints to MigrateWithVolume API method URL: https://github.com/apache/cloudstack/pull/2761#issuecomment-408817913 So, now with the state of this PR. Is there something missing? I mean, do we need to apply some other changes? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rafaelweingartner commented on issue #2761: Add managed storage pool constraints to MigrateWithVolume API method
rafaelweingartner commented on issue #2761: Add managed storage pool constraints to MigrateWithVolume API method URL: https://github.com/apache/cloudstack/pull/2761#issuecomment-408537220 Yes, the second link has only one method. The problem with the second suggestion is that it causes a bug as I mentioned. I mean, it will create a situation where it will not allow the migration of VMs with volumes in cluster-wide managed storage, even though it should be supported. Imagine a VM that has two volumes one placed in local storage and other in a managed storage. The `migrateVirtualMachineWithVolume' is called to migrate the VM between hosts of the same cluster. The migration should be supported as long as the target host has a local storage to receive one of the volumes, and the other one does not need to be migrated as it is in a cluster-wide managed storage and the target host is in the same cluster as the source host. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rafaelweingartner commented on issue #2761: Add managed storage pool constraints to MigrateWithVolume API method
rafaelweingartner commented on issue #2761: Add managed storage pool constraints to MigrateWithVolume API method URL: https://github.com/apache/cloudstack/pull/2761#issuecomment-408278876 Ok, no problem, I got the gist. Those changes created a confusion for me though. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rafaelweingartner commented on issue #2761: Add managed storage pool constraints to MigrateWithVolume API method
rafaelweingartner commented on issue #2761: Add managed storage pool constraints to MigrateWithVolume API method URL: https://github.com/apache/cloudstack/pull/2761#issuecomment-408264657 Thanks mike. I applied your suggestions. I am only a little confused with the use of `executeManagedStorageChecksWhenTargetStoragePoolProvided` and `executeManagedStorageChecksWhenTargetStoragePoolNotProvided`. Can't I simply use `executeManagedStorageChecksWhenTargetStoragePoolProvided` in both places? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rafaelweingartner commented on issue #2761: Add managed storage pool constraints to MigrateWithVolume API method
rafaelweingartner commented on issue #2761: Add managed storage pool constraints to MigrateWithVolume API method URL: https://github.com/apache/cloudstack/pull/2761#issuecomment-408113305 @mike-tutkowski I just pushed a new commit with the changes you suggested. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rafaelweingartner commented on issue #2761: Add managed storage pool constraints to MigrateWithVolume API method
rafaelweingartner commented on issue #2761: Add managed storage pool constraints to MigrateWithVolume API method URL: https://github.com/apache/cloudstack/pull/2761#issuecomment-407902494 @mike-tutkowski you are right, this PR is fixing two problems. I just pushed a commit applying your requests. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
