You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2021/02/03 10:38:56 UTC

[GitHub] [cloudstack] ravening opened a new pull request #4646: Add global settings for VM migration flags

ravening opened a new pull request #4646:
URL: https://github.com/apache/cloudstack/pull/4646


   ### Description
   Provide global settings or vm migration flags for kvm
   so that admins can configure the values
   
   
   
   <!--- Describe your changes in DETAIL - And how has behaviour functionally changed. -->
   
   <!-- For new features, provide link to FS, dev ML discussion etc. -->
   <!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. -->
   
   <!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged -->
   <!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" -->
   <!-- Fixes: # -->
   
   <!--- ********************************************************************************* -->
   <!--- NOTE: AUTOMATATION USES THE DESCRIPTIONS TO SET LABELS AND PRODUCE DOCUMENTATION. -->
   <!--- PLEASE PUT AN 'X' in only **ONE** box -->
   <!--- ********************************************************************************* -->
   
   ### 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)
   
   ### Feature/Enhancement Scale or Bug Severity
   
   #### Feature/Enhancement Scale
   
   - [ ] Major
   - [X] Minor
   
   #### Bug Severity
   
   - [ ] BLOCKER
   - [ ] Critical
   - [ ] Major
   - [X] Minor
   - [ ] Trivial
   
   
   ### Screenshots (if appropriate):
   
   ![Screenshot 2021-02-03 at 11 38 23](https://user-images.githubusercontent.com/10645273/106735279-54062500-6614-11eb-89c3-b3d4f140a81d.png)
   
   ### How Has This Been Tested?
   <!-- Please describe in detail how you tested your changes. -->
   <!-- Include details of your testing environment, and the tests you ran to -->
   <!-- see how your change affects other areas of the code, etc. -->
   
   
   <!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/master/CONTRIBUTING.md) document -->
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] nvazquez commented on pull request #4646: Add global settings for VM migration flags

Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #4646:
URL: https://github.com/apache/cloudstack/pull/4646#issuecomment-930083090


   @blueorangutan test


-- 
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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] sureshanaparti commented on pull request #4646: Add global settings for VM migration flags

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4646:
URL: https://github.com/apache/cloudstack/pull/4646#issuecomment-928968092


   Hi @ravening, Can you address the outstanding comments. 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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #4646: Add global settings for VM migration flags

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #4646:
URL: https://github.com/apache/cloudstack/pull/4646#discussion_r570051942



##########
File path: server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##########
@@ -422,6 +426,13 @@
                         "Indicates whether the host in down state can be put into maintenance state so thats its not enabled after it comes back.",
                         true, ConfigKey.Scope.Zone, null);
 
+    protected final ConfigKey<Integer> KvmVmMigrateSpeed = new ConfigKey<>("Advanced", Integer.class, KVM_VM_MIGRATE_SPEED, "-1",
+            "set the vm migrate speed (in MiB/s) on KVM. By default, it will try to guess the speed of the guest network (in MBps).", true);
+    protected final ConfigKey<Integer> KvmVmMigrateDowntime = new ConfigKey<>("Advanced", Integer.class, KVM_VM_MIGRATE_DOWNTIME, "-1",
+            "Sets maximum tolerable time in milliseconds for which the domain is allowed to be paused at the end of live migration on KVM.", true);
+    protected final ConfigKey<Integer> KvmVmMigratePauseAfter = new ConfigKey<>("Advanced", Integer.class, KVM_VM_MIGRATE_PAUSE_AFTER, "-1",
+            "Set an upper limit in milliseconds for how long live migration on KVM should wait, at which point VM is paused and migration will finish quickly.  Less than 1 means disabled.", true);
+

Review comment:
       these belong in the hypervisor plugin for KVM, not 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] nvazquez commented on a change in pull request #4646: Add global settings for VM migration flags

Posted by GitBox <gi...@apache.org>.
nvazquez commented on a change in pull request #4646:
URL: https://github.com/apache/cloudstack/pull/4646#discussion_r714446603



##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
##########
@@ -654,6 +658,54 @@ public String interpret(final BufferedReader reader) throws IOException {
         }
     }
 
+    /**
+     * setLiveMigrateFlags
+     *
+     * Sets the user configured live migration flags
+     *
+     * @param params
+     */
+    public void setLiveMigrateFlags(final Map<String, String> params) {
+        if (params.get(KVM_VM_MIGRATE_SPEED) != null) {
+            _migrateSpeed = NumbersUtil.parseInt(params.get(KVM_VM_MIGRATE_SPEED), -1);

Review comment:
       What about `NumbersUtil.parseInt(params.get(KVM_VM_MIGRATE_SPEED), getDefaultMigrateSpeed());` instead? The next `if` block won't be needed

##########
File path: server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##########
@@ -453,6 +457,12 @@
             "Max length of vm userdata after base64 decoding. Default is 32768 and maximum is 1048576", true);
     public static final ConfigKey<Boolean> MIGRATE_VM_ACROSS_CLUSTERS = new ConfigKey<Boolean>(Boolean.class, "migrate.vm.across.clusters", "Advanced", "false",
             "Indicates whether the VM can be migrated to different cluster if no host is found in same cluster",true, ConfigKey.Scope.Zone, null);
+    protected final ConfigKey<Integer> KvmVmMigrateSpeed = new ConfigKey<>("Advanced", Integer.class, KVM_VM_MIGRATE_SPEED, "-1",

Review comment:
       Would it make sense to reduce the scope of these setting to Cluster level?




-- 
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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] ravening commented on a change in pull request #4646: Add global settings for VM migration flags

Posted by GitBox <gi...@apache.org>.
ravening commented on a change in pull request #4646:
URL: https://github.com/apache/cloudstack/pull/4646#discussion_r691010453



##########
File path: server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##########
@@ -422,6 +426,13 @@
                         "Indicates whether the host in down state can be put into maintenance state so thats its not enabled after it comes back.",
                         true, ConfigKey.Scope.Zone, null);
 
+    protected final ConfigKey<Integer> KvmVmMigrateSpeed = new ConfigKey<>("Advanced", Integer.class, KVM_VM_MIGRATE_SPEED, "-1",
+            "set the vm migrate speed (in MiB/s) on KVM. By default, it will try to guess the speed of the guest network (in MBps).", true);
+    protected final ConfigKey<Integer> KvmVmMigrateDowntime = new ConfigKey<>("Advanced", Integer.class, KVM_VM_MIGRATE_DOWNTIME, "-1",
+            "Sets maximum tolerable time in milliseconds for which the domain is allowed to be paused at the end of live migration on KVM.", true);
+    protected final ConfigKey<Integer> KvmVmMigratePauseAfter = new ConfigKey<>("Advanced", Integer.class, KVM_VM_MIGRATE_PAUSE_AFTER, "-1",
+            "Set an upper limit in milliseconds for how long live migration on KVM should wait, at which point VM is paused and migration will finish quickly.  Less than 1 means disabled.", true);
+

Review comment:
       @DaanHoogland do you know the right place where can I add it? I didnt find any file for kvm specific global settings.




-- 
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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] nvazquez commented on pull request #4646: Add global settings for VM migration flags

Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #4646:
URL: https://github.com/apache/cloudstack/pull/4646#issuecomment-930083090


   @blueorangutan test


-- 
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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] davidjumani commented on pull request #4646: Add global settings for VM migration flags

Posted by GitBox <gi...@apache.org>.
davidjumani commented on pull request #4646:
URL: https://github.com/apache/cloudstack/pull/4646#issuecomment-926458186


   @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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] blueorangutan commented on pull request #4646: Add global settings for VM migration flags

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4646:
URL: https://github.com/apache/cloudstack/pull/4646#issuecomment-931030779


   <b>Trillian Build Failed (tid-2257)<b/>


-- 
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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] ravening commented on a change in pull request #4646: Add global settings for VM migration flags

Posted by GitBox <gi...@apache.org>.
ravening commented on a change in pull request #4646:
URL: https://github.com/apache/cloudstack/pull/4646#discussion_r718243095



##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
##########
@@ -654,6 +658,54 @@ public String interpret(final BufferedReader reader) throws IOException {
         }
     }
 
+    /**
+     * setLiveMigrateFlags
+     *
+     * Sets the user configured live migration flags
+     *
+     * @param params
+     */
+    public void setLiveMigrateFlags(final Map<String, String> params) {
+        if (params.get(KVM_VM_MIGRATE_SPEED) != null) {
+            _migrateSpeed = NumbersUtil.parseInt(params.get(KVM_VM_MIGRATE_SPEED), -1);

Review comment:
       changed




-- 
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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] weizhouapache commented on a change in pull request #4646: Add global settings for VM migration flags

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #4646:
URL: https://github.com/apache/cloudstack/pull/4646#discussion_r692737718



##########
File path: server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##########
@@ -422,6 +426,13 @@
                         "Indicates whether the host in down state can be put into maintenance state so thats its not enabled after it comes back.",
                         true, ConfigKey.Scope.Zone, null);
 
+    protected final ConfigKey<Integer> KvmVmMigrateSpeed = new ConfigKey<>("Advanced", Integer.class, KVM_VM_MIGRATE_SPEED, "-1",
+            "set the vm migrate speed (in MiB/s) on KVM. By default, it will try to guess the speed of the guest network (in MBps).", true);
+    protected final ConfigKey<Integer> KvmVmMigrateDowntime = new ConfigKey<>("Advanced", Integer.class, KVM_VM_MIGRATE_DOWNTIME, "-1",
+            "Sets maximum tolerable time in milliseconds for which the domain is allowed to be paused at the end of live migration on KVM.", true);
+    protected final ConfigKey<Integer> KvmVmMigratePauseAfter = new ConfigKey<>("Advanced", Integer.class, KVM_VM_MIGRATE_PAUSE_AFTER, "-1",
+            "Set an upper limit in milliseconds for how long live migration on KVM should wait, at which point VM is paused and migration will finish quickly.  Less than 1 means disabled.", true);
+

Review comment:
       > I'd say in the libvirt guru or - resource, I'd say. But to tell you the truth, I really don't know. As they are used in the AgentManager, maybe there. But for sure these shouldn't be initialised in `ConfigurationManagerImpl.initMessageBusListener()`.
   > For vmware there are such `ConfigKey`'s stored in the `VmwareGuru` and in a `VmwareManager`. Both seem resonable. In the KVM case the `KVMGuru` (which actually should be in the kvm hypervisor plugin, soit).
   
   agree with @DaanHoogland 
   `KVMGuru` looks like better place.




-- 
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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] ravening commented on pull request #4646: Add global settings for VM migration flags

Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4646:
URL: https://github.com/apache/cloudstack/pull/4646#issuecomment-930941738


   > @ravening Can you have a look at the test failures? Thanks
   
   @davidjumani one of the test is failing because of `Exception during migrate: org.libvirt.LibvirtException: numerical overflow: bandwidth must be less than 8796093022208` which is totally not related to my 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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] weizhouapache commented on pull request #4646: Add global settings for VM migration flags

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4646:
URL: https://github.com/apache/cloudstack/pull/4646#issuecomment-931118458


   @blueorangutan test
   
   


-- 
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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] davidjumani commented on pull request #4646: Add global settings for VM migration flags

Posted by GitBox <gi...@apache.org>.
davidjumani commented on pull request #4646:
URL: https://github.com/apache/cloudstack/pull/4646#issuecomment-930004150


   @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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] ravening commented on a change in pull request #4646: Add global settings for VM migration flags

Posted by GitBox <gi...@apache.org>.
ravening commented on a change in pull request #4646:
URL: https://github.com/apache/cloudstack/pull/4646#discussion_r718243553



##########
File path: server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##########
@@ -453,6 +457,12 @@
             "Max length of vm userdata after base64 decoding. Default is 32768 and maximum is 1048576", true);
     public static final ConfigKey<Boolean> MIGRATE_VM_ACROSS_CLUSTERS = new ConfigKey<Boolean>(Boolean.class, "migrate.vm.across.clusters", "Advanced", "false",
             "Indicates whether the VM can be migrated to different cluster if no host is found in same cluster",true, ConfigKey.Scope.Zone, null);
+    protected final ConfigKey<Integer> KvmVmMigrateSpeed = new ConfigKey<>("Advanced", Integer.class, KVM_VM_MIGRATE_SPEED, "-1",

Review comment:
       changed to cluster scope




-- 
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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] blueorangutan commented on pull request #4646: Add global settings for VM migration flags

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4646:
URL: https://github.com/apache/cloudstack/pull/4646#issuecomment-930084149


   @nvazquez a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) 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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] ravening commented on a change in pull request #4646: Add global settings for VM migration flags

Posted by GitBox <gi...@apache.org>.
ravening commented on a change in pull request #4646:
URL: https://github.com/apache/cloudstack/pull/4646#discussion_r712068595



##########
File path: server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##########
@@ -422,6 +426,13 @@
                         "Indicates whether the host in down state can be put into maintenance state so thats its not enabled after it comes back.",
                         true, ConfigKey.Scope.Zone, null);
 
+    protected final ConfigKey<Integer> KvmVmMigrateSpeed = new ConfigKey<>("Advanced", Integer.class, KVM_VM_MIGRATE_SPEED, "-1",
+            "set the vm migrate speed (in MiB/s) on KVM. By default, it will try to guess the speed of the guest network (in MBps).", true);
+    protected final ConfigKey<Integer> KvmVmMigrateDowntime = new ConfigKey<>("Advanced", Integer.class, KVM_VM_MIGRATE_DOWNTIME, "-1",
+            "Sets maximum tolerable time in milliseconds for which the domain is allowed to be paused at the end of live migration on KVM.", true);
+    protected final ConfigKey<Integer> KvmVmMigratePauseAfter = new ConfigKey<>("Advanced", Integer.class, KVM_VM_MIGRATE_PAUSE_AFTER, "-1",
+            "Set an upper limit in milliseconds for how long live migration on KVM should wait, at which point VM is paused and migration will finish quickly.  Less than 1 means disabled.", true);
+

Review comment:
       @DaanHoogland @weizhouapache `KVmguru` doesnt have any code related to configKeys. shall i add that also?




-- 
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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] ravening commented on pull request #4646: Add global settings for VM migration flags

Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4646:
URL: https://github.com/apache/cloudstack/pull/4646#issuecomment-929926857


   > Hi @ravening, Can you address the outstanding comments. Thanks.
   
   @sureshanaparti 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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] sureshanaparti commented on pull request #4646: Add global settings for VM migration flags

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4646:
URL: https://github.com/apache/cloudstack/pull/4646#issuecomment-869191321


   @ravening any update on this PR?


-- 
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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] sureshanaparti commented on pull request #4646: Add global settings for VM migration flags

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4646:
URL: https://github.com/apache/cloudstack/pull/4646#issuecomment-928968092


   Hi @ravening, Can you address the outstanding comments. 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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] blueorangutan commented on pull request #4646: Add global settings for VM migration flags

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4646:
URL: https://github.com/apache/cloudstack/pull/4646#issuecomment-929960845


   @DaanHoogland a Jenkins job has been kicked to build packages. 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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #4646: Add global settings for VM migration flags

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #4646:
URL: https://github.com/apache/cloudstack/pull/4646#discussion_r712165570



##########
File path: server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##########
@@ -422,6 +426,13 @@
                         "Indicates whether the host in down state can be put into maintenance state so thats its not enabled after it comes back.",
                         true, ConfigKey.Scope.Zone, null);
 
+    protected final ConfigKey<Integer> KvmVmMigrateSpeed = new ConfigKey<>("Advanced", Integer.class, KVM_VM_MIGRATE_SPEED, "-1",
+            "set the vm migrate speed (in MiB/s) on KVM. By default, it will try to guess the speed of the guest network (in MBps).", true);
+    protected final ConfigKey<Integer> KvmVmMigrateDowntime = new ConfigKey<>("Advanced", Integer.class, KVM_VM_MIGRATE_DOWNTIME, "-1",
+            "Sets maximum tolerable time in milliseconds for which the domain is allowed to be paused at the end of live migration on KVM.", true);
+    protected final ConfigKey<Integer> KvmVmMigratePauseAfter = new ConfigKey<>("Advanced", Integer.class, KVM_VM_MIGRATE_PAUSE_AFTER, "-1",
+            "Set an upper limit in milliseconds for how long live migration on KVM should wait, at which point VM is paused and migration will finish quickly.  Less than 1 means disabled.", true);
+

Review comment:
       Yes, makes sense to me @ravening 




-- 
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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #4646: Add global settings for VM migration flags

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #4646:
URL: https://github.com/apache/cloudstack/pull/4646#discussion_r570051942



##########
File path: server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##########
@@ -422,6 +426,13 @@
                         "Indicates whether the host in down state can be put into maintenance state so thats its not enabled after it comes back.",
                         true, ConfigKey.Scope.Zone, null);
 
+    protected final ConfigKey<Integer> KvmVmMigrateSpeed = new ConfigKey<>("Advanced", Integer.class, KVM_VM_MIGRATE_SPEED, "-1",
+            "set the vm migrate speed (in MiB/s) on KVM. By default, it will try to guess the speed of the guest network (in MBps).", true);
+    protected final ConfigKey<Integer> KvmVmMigrateDowntime = new ConfigKey<>("Advanced", Integer.class, KVM_VM_MIGRATE_DOWNTIME, "-1",
+            "Sets maximum tolerable time in milliseconds for which the domain is allowed to be paused at the end of live migration on KVM.", true);
+    protected final ConfigKey<Integer> KvmVmMigratePauseAfter = new ConfigKey<>("Advanced", Integer.class, KVM_VM_MIGRATE_PAUSE_AFTER, "-1",
+            "Set an upper limit in milliseconds for how long live migration on KVM should wait, at which point VM is paused and migration will finish quickly.  Less than 1 means disabled.", true);
+

Review comment:
       these belong in the hypervisor plugin for KVM, not 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] ravening commented on pull request #4646: Add global settings for VM migration flags

Posted by GitBox <gi...@apache.org>.
ravening commented on pull request #4646:
URL: https://github.com/apache/cloudstack/pull/4646#issuecomment-929926857


   > Hi @ravening, Can you address the outstanding comments. Thanks.
   
   @sureshanaparti 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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] DaanHoogland commented on pull request #4646: Add global settings for VM migration flags

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4646:
URL: https://github.com/apache/cloudstack/pull/4646#issuecomment-929960536


   @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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] blueorangutan commented on pull request #4646: Add global settings for VM migration flags

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4646:
URL: https://github.com/apache/cloudstack/pull/4646#issuecomment-930033956


   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 1443


-- 
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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] blueorangutan commented on pull request #4646: Add global settings for VM migration flags

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4646:
URL: https://github.com/apache/cloudstack/pull/4646#issuecomment-926458824


   @davidjumani a Jenkins job has been kicked to build packages. 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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] nvazquez commented on pull request #4646: Add global settings for VM migration flags

Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #4646:
URL: https://github.com/apache/cloudstack/pull/4646#issuecomment-1031039046


   Hi @ravening can you please fix 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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] blueorangutan commented on pull request #4646: Add global settings for VM migration flags

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4646:
URL: https://github.com/apache/cloudstack/pull/4646#issuecomment-926482631


   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 1388


-- 
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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] ravening commented on a change in pull request #4646: Add global settings for VM migration flags

Posted by GitBox <gi...@apache.org>.
ravening commented on a change in pull request #4646:
URL: https://github.com/apache/cloudstack/pull/4646#discussion_r718242770



##########
File path: server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##########
@@ -422,6 +426,13 @@
                         "Indicates whether the host in down state can be put into maintenance state so thats its not enabled after it comes back.",
                         true, ConfigKey.Scope.Zone, null);
 
+    protected final ConfigKey<Integer> KvmVmMigrateSpeed = new ConfigKey<>("Advanced", Integer.class, KVM_VM_MIGRATE_SPEED, "-1",
+            "set the vm migrate speed (in MiB/s) on KVM. By default, it will try to guess the speed of the guest network (in MBps).", true);
+    protected final ConfigKey<Integer> KvmVmMigrateDowntime = new ConfigKey<>("Advanced", Integer.class, KVM_VM_MIGRATE_DOWNTIME, "-1",
+            "Sets maximum tolerable time in milliseconds for which the domain is allowed to be paused at the end of live migration on KVM.", true);
+    protected final ConfigKey<Integer> KvmVmMigratePauseAfter = new ConfigKey<>("Advanced", Integer.class, KVM_VM_MIGRATE_PAUSE_AFTER, "-1",
+            "Set an upper limit in milliseconds for how long live migration on KVM should wait, at which point VM is paused and migration will finish quickly.  Less than 1 means disabled.", true);
+

Review comment:
       @DaanHoogland @weizhouapache moved the settings to `KVMguru`




-- 
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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] blueorangutan commented on pull request #4646: Add global settings for VM migration flags

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4646:
URL: https://github.com/apache/cloudstack/pull/4646#issuecomment-929960845






-- 
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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] weizhouapache commented on a change in pull request #4646: Add global settings for VM migration flags

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #4646:
URL: https://github.com/apache/cloudstack/pull/4646#discussion_r718463432



##########
File path: engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java
##########
@@ -1759,8 +1764,11 @@ public void processConnect(final Host host, final StartupCommand cmd, final bool
             if (cmd instanceof StartupRoutingCommand) {
                 if (((StartupRoutingCommand)cmd).getHypervisorType() == HypervisorType.KVM || ((StartupRoutingCommand)cmd).getHypervisorType() == HypervisorType.LXC) {
                     Map<String, String> params = new HashMap<String, String>();
-                    params.put(Config.RouterAggregationCommandEachTimeout.toString(), _configDao.getValue(Config.RouterAggregationCommandEachTimeout.toString()));
                     params.put(Config.MigrateWait.toString(), _configDao.getValue(Config.MigrateWait.toString()));
+                    params.put(ROUTER_AGGREGATION_COMMAND_EACH_TIMEOUT, _configDao.getValue(ROUTER_AGGREGATION_COMMAND_EACH_TIMEOUT));
+                    params.put(KVM_VM_MIGRATE_SPEED_STRING, _configDao.getValue(KVM_VM_MIGRATE_SPEED_STRING));

Review comment:
       KVM_VM_MIGRATE_SPEED is a cluster-level setting, but global config is uses 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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] sureshanaparti commented on pull request #4646: Add global settings for VM migration flags

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4646:
URL: https://github.com/apache/cloudstack/pull/4646#issuecomment-922642328


   Hi @ravening can you fix the conflicts, 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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] rhtyd commented on pull request #4646: Add global settings for VM migration flags

Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4646:
URL: https://github.com/apache/cloudstack/pull/4646#issuecomment-919712963


   Ping @ravening can you address 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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] davidjumani commented on pull request #4646: Add global settings for VM migration flags

Posted by GitBox <gi...@apache.org>.
davidjumani commented on pull request #4646:
URL: https://github.com/apache/cloudstack/pull/4646#issuecomment-930004150


   @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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] blueorangutan commented on pull request #4646: Add global settings for VM migration flags

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4646:
URL: https://github.com/apache/cloudstack/pull/4646#issuecomment-929994901


   Packaging result: :heavy_multiplication_x: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_multiplication_x: suse15. SL-JID 1439


-- 
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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] blueorangutan commented on pull request #4646: Add global settings for VM migration flags

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4646:
URL: https://github.com/apache/cloudstack/pull/4646#issuecomment-931027657


   @davidjumani a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) 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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] DaanHoogland commented on pull request #4646: Add global settings for VM migration flags

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4646:
URL: https://github.com/apache/cloudstack/pull/4646#issuecomment-929960536


   @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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] blueorangutan commented on pull request #4646: Add global settings for VM migration flags

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4646:
URL: https://github.com/apache/cloudstack/pull/4646#issuecomment-930004429


   @davidjumani a Jenkins job has been kicked to build packages. 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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] davidjumani commented on pull request #4646: Add global settings for VM migration flags

Posted by GitBox <gi...@apache.org>.
davidjumani commented on pull request #4646:
URL: https://github.com/apache/cloudstack/pull/4646#issuecomment-931026593


   @blueorangutan test


-- 
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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] blueorangutan commented on pull request #4646: Add global settings for VM migration flags

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4646:
URL: https://github.com/apache/cloudstack/pull/4646#issuecomment-926912668


   <b>Trillian test result (tid-2203)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 35558 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4646-t2203-kvm-centos7.zip
   Smoke tests completed. 89 look OK, 0 have errors
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   


-- 
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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] sureshanaparti edited a comment on pull request #4646: Add global settings for VM migration flags

Posted by GitBox <gi...@apache.org>.
sureshanaparti edited a comment on pull request #4646:
URL: https://github.com/apache/cloudstack/pull/4646#issuecomment-869191321


   Hi @ravening can you address the open comments in the PR?


-- 
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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] ravening commented on a change in pull request #4646: Add global settings for VM migration flags

Posted by GitBox <gi...@apache.org>.
ravening commented on a change in pull request #4646:
URL: https://github.com/apache/cloudstack/pull/4646#discussion_r691010453



##########
File path: server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##########
@@ -422,6 +426,13 @@
                         "Indicates whether the host in down state can be put into maintenance state so thats its not enabled after it comes back.",
                         true, ConfigKey.Scope.Zone, null);
 
+    protected final ConfigKey<Integer> KvmVmMigrateSpeed = new ConfigKey<>("Advanced", Integer.class, KVM_VM_MIGRATE_SPEED, "-1",
+            "set the vm migrate speed (in MiB/s) on KVM. By default, it will try to guess the speed of the guest network (in MBps).", true);
+    protected final ConfigKey<Integer> KvmVmMigrateDowntime = new ConfigKey<>("Advanced", Integer.class, KVM_VM_MIGRATE_DOWNTIME, "-1",
+            "Sets maximum tolerable time in milliseconds for which the domain is allowed to be paused at the end of live migration on KVM.", true);
+    protected final ConfigKey<Integer> KvmVmMigratePauseAfter = new ConfigKey<>("Advanced", Integer.class, KVM_VM_MIGRATE_PAUSE_AFTER, "-1",
+            "Set an upper limit in milliseconds for how long live migration on KVM should wait, at which point VM is paused and migration will finish quickly.  Less than 1 means disabled.", true);
+

Review comment:
       @DaanHoogland do you know the right place where can I add it? I didnt find any file for kvm specific global settings.




-- 
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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] weizhouapache commented on a change in pull request #4646: Add global settings for VM migration flags

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #4646:
URL: https://github.com/apache/cloudstack/pull/4646#discussion_r718463432



##########
File path: engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java
##########
@@ -1759,8 +1764,11 @@ public void processConnect(final Host host, final StartupCommand cmd, final bool
             if (cmd instanceof StartupRoutingCommand) {
                 if (((StartupRoutingCommand)cmd).getHypervisorType() == HypervisorType.KVM || ((StartupRoutingCommand)cmd).getHypervisorType() == HypervisorType.LXC) {
                     Map<String, String> params = new HashMap<String, String>();
-                    params.put(Config.RouterAggregationCommandEachTimeout.toString(), _configDao.getValue(Config.RouterAggregationCommandEachTimeout.toString()));
                     params.put(Config.MigrateWait.toString(), _configDao.getValue(Config.MigrateWait.toString()));
+                    params.put(ROUTER_AGGREGATION_COMMAND_EACH_TIMEOUT, _configDao.getValue(ROUTER_AGGREGATION_COMMAND_EACH_TIMEOUT));
+                    params.put(KVM_VM_MIGRATE_SPEED_STRING, _configDao.getValue(KVM_VM_MIGRATE_SPEED_STRING));

Review comment:
       KVM_VM_MIGRATE_SPEED is a cluster-level setting, but global config is uses 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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] davidjumani commented on pull request #4646: Add global settings for VM migration flags

Posted by GitBox <gi...@apache.org>.
davidjumani commented on pull request #4646:
URL: https://github.com/apache/cloudstack/pull/4646#issuecomment-930761935


   @ravening Can you have a look at the test failures? 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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] blueorangutan commented on pull request #4646: Add global settings for VM migration flags

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4646:
URL: https://github.com/apache/cloudstack/pull/4646#issuecomment-926524781


   @davidjumani a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) 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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] blueorangutan commented on pull request #4646: Add global settings for VM migration flags

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4646:
URL: https://github.com/apache/cloudstack/pull/4646#issuecomment-931119104


   @weizhouapache a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) 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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #4646: Add global settings for VM migration flags

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #4646:
URL: https://github.com/apache/cloudstack/pull/4646#discussion_r692728307



##########
File path: server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##########
@@ -422,6 +426,13 @@
                         "Indicates whether the host in down state can be put into maintenance state so thats its not enabled after it comes back.",
                         true, ConfigKey.Scope.Zone, null);
 
+    protected final ConfigKey<Integer> KvmVmMigrateSpeed = new ConfigKey<>("Advanced", Integer.class, KVM_VM_MIGRATE_SPEED, "-1",
+            "set the vm migrate speed (in MiB/s) on KVM. By default, it will try to guess the speed of the guest network (in MBps).", true);
+    protected final ConfigKey<Integer> KvmVmMigrateDowntime = new ConfigKey<>("Advanced", Integer.class, KVM_VM_MIGRATE_DOWNTIME, "-1",
+            "Sets maximum tolerable time in milliseconds for which the domain is allowed to be paused at the end of live migration on KVM.", true);
+    protected final ConfigKey<Integer> KvmVmMigratePauseAfter = new ConfigKey<>("Advanced", Integer.class, KVM_VM_MIGRATE_PAUSE_AFTER, "-1",
+            "Set an upper limit in milliseconds for how long live migration on KVM should wait, at which point VM is paused and migration will finish quickly.  Less than 1 means disabled.", true);
+

Review comment:
       I'd say in the libvirt guru or - resource, I'd say. But to tell you the truth, I really don't know. As they are used in the AgentManager, maybe there. But for sure these shouldn't be initialised in `ConfigurationManagerImpl.initMessageBusListener()`.
   For vmware there are such `ConfigKey`'s stored in the `VmwareGuru` and in a `VmwareManager`. Both seem resonable. In the KVM case the `KVMGuru` (which actually should be in the kvm hypervisor plugin, soit).




-- 
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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] davidjumani commented on pull request #4646: Add global settings for VM migration flags

Posted by GitBox <gi...@apache.org>.
davidjumani commented on pull request #4646:
URL: https://github.com/apache/cloudstack/pull/4646#issuecomment-926524491


   @blueorangutan test


-- 
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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] blueorangutan commented on pull request #4646: Add global settings for VM migration flags

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4646:
URL: https://github.com/apache/cloudstack/pull/4646#issuecomment-930583624


   <b>Trillian test result (tid-2238)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 37783 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4646-t2238-kvm-centos7.zip
   Smoke tests completed. 89 look OK, 1 have errors
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_01_secure_vm_migration | `Error` | 164.71 | test_vm_life_cycle.py
   test_02_unsecure_vm_migration | `Error` | 274.26 | test_vm_life_cycle.py
   test_08_migrate_vm | `Error` | 47.85 | test_vm_life_cycle.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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] blueorangutan commented on pull request #4646: Add global settings for VM migration flags

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4646:
URL: https://github.com/apache/cloudstack/pull/4646#issuecomment-931122441


   <b>Trillian Build Failed (tid-2258)<b/>


-- 
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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] ravening commented on a change in pull request #4646: Add global settings for VM migration flags

Posted by GitBox <gi...@apache.org>.
ravening commented on a change in pull request #4646:
URL: https://github.com/apache/cloudstack/pull/4646#discussion_r718242770



##########
File path: server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##########
@@ -422,6 +426,13 @@
                         "Indicates whether the host in down state can be put into maintenance state so thats its not enabled after it comes back.",
                         true, ConfigKey.Scope.Zone, null);
 
+    protected final ConfigKey<Integer> KvmVmMigrateSpeed = new ConfigKey<>("Advanced", Integer.class, KVM_VM_MIGRATE_SPEED, "-1",
+            "set the vm migrate speed (in MiB/s) on KVM. By default, it will try to guess the speed of the guest network (in MBps).", true);
+    protected final ConfigKey<Integer> KvmVmMigrateDowntime = new ConfigKey<>("Advanced", Integer.class, KVM_VM_MIGRATE_DOWNTIME, "-1",
+            "Sets maximum tolerable time in milliseconds for which the domain is allowed to be paused at the end of live migration on KVM.", true);
+    protected final ConfigKey<Integer> KvmVmMigratePauseAfter = new ConfigKey<>("Advanced", Integer.class, KVM_VM_MIGRATE_PAUSE_AFTER, "-1",
+            "Set an upper limit in milliseconds for how long live migration on KVM should wait, at which point VM is paused and migration will finish quickly.  Less than 1 means disabled.", true);
+

Review comment:
       @DaanHoogland @weizhouapache moved the settings to `KVMguru`

##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
##########
@@ -654,6 +658,54 @@ public String interpret(final BufferedReader reader) throws IOException {
         }
     }
 
+    /**
+     * setLiveMigrateFlags
+     *
+     * Sets the user configured live migration flags
+     *
+     * @param params
+     */
+    public void setLiveMigrateFlags(final Map<String, String> params) {
+        if (params.get(KVM_VM_MIGRATE_SPEED) != null) {
+            _migrateSpeed = NumbersUtil.parseInt(params.get(KVM_VM_MIGRATE_SPEED), -1);

Review comment:
       changed

##########
File path: server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##########
@@ -453,6 +457,12 @@
             "Max length of vm userdata after base64 decoding. Default is 32768 and maximum is 1048576", true);
     public static final ConfigKey<Boolean> MIGRATE_VM_ACROSS_CLUSTERS = new ConfigKey<Boolean>(Boolean.class, "migrate.vm.across.clusters", "Advanced", "false",
             "Indicates whether the VM can be migrated to different cluster if no host is found in same cluster",true, ConfigKey.Scope.Zone, null);
+    protected final ConfigKey<Integer> KvmVmMigrateSpeed = new ConfigKey<>("Advanced", Integer.class, KVM_VM_MIGRATE_SPEED, "-1",

Review comment:
       changed to cluster scope




-- 
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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org