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 2022/08/05 10:02:04 UTC
[GitHub] [cloudstack] BryanMLima opened a new pull request, #6491: Add live migration of system VMs
BryanMLima opened a new pull request, #6491:
URL: https://github.com/apache/cloudstack/pull/6491
### Description
Currently, there is no way to migrate running system VMs with volumes; this means, we cannot move their volumes to a new storage pool in runtime. This PR aims to add this feature by altering the behavior of the `migrateVirtualMachineWithVolume` API to support system VMs. Furthermore, the `listVolumes` API was changed to also return system VMs root volumes, to provide easy access to the system VMs volume's UUID to the root admins. Moreover, it was added unit test for `MigrateVirtualMachineWithVolumeCmd` class.
### 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
- [ ] Minor
- [ ] Trivial
### Screenshots (if appropriate):
### How Has This Been Tested?
This was tested on a local lab through CloudMonkey, using the command `migrate virtualmachinewithvolume` with the UUID of a system VM and migrating its volume to another storage pool. The command ran as expected. Furthermore, it was added unit test to verify the API behavior.
--
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 #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1203997994
@DaanHoogland a Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] acs-robot commented on pull request #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
acs-robot commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1258548216
Found UI changes, kicking a new UI QA build
@blueorangutan ui
--
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] GutoVeronezi commented on pull request #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
GutoVeronezi commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1241973871
@BryanMLima, can you check Travis errors?
--
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 #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1167732271
UI build: :heavy_check_mark:
Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/6491 (SL-JID-1856)
--
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] acs-robot commented on pull request #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
acs-robot commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1258634983
Found UI changes, kicking a new UI QA build
@blueorangutan ui
--
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] BryanMLima commented on a diff in pull request #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
BryanMLima commented on code in PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#discussion_r914124623
##########
api/src/main/java/org/apache/cloudstack/api/command/admin/vm/MigrateVirtualMachineWithVolumeCmd.java:
##########
@@ -152,20 +153,17 @@ public ApiCommandResourceType getApiResourceType() {
@Override
public void execute() {
if (hostId == null && MapUtils.isEmpty(migrateVolumeTo)) {
- throw new InvalidParameterValueException(String.format("Either %s or %s must be passed for migrating the VM", ApiConstants.HOST_ID, ApiConstants.MIGRATE_TO));
- }
-
- UserVm userVm = _userVmService.getUserVm(getVirtualMachineId());
- if (userVm == null) {
- throw new InvalidParameterValueException("Unable to find the VM by id=" + getVirtualMachineId());
+ throw new InvalidParameterValueException(String.format("Either %s or %s must be passed for migrating the VM.", ApiConstants.HOST_ID, ApiConstants.MIGRATE_TO));
}
+ VirtualMachine userVm = _userVmService.getVm(getVirtualMachineId());
Review Comment:
As I was adjusting the error messages, I could not verify this one. Furthermore, I verified that the method `ParamProcessWorker#translateUuidToInternalId` already makes this verification. If the user puts an nonexistent UUID, or and UUID from another entity, the `translateUuidToInternalId` method already throws an exception, for these reasons I removed this `null` check.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1274964293
UI build: :heavy_check_mark:
Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/6491 (SL-JID-2484)
--
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 #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1234802022
@acs-robot a Jenkins job has been kicked to build UI QA env. 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] blueorangutan commented on pull request #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1258635704
@acs-robot a Jenkins job has been kicked to build UI QA env. 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 pull request #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1260848928
@Pearl1594 I dare not merge this without your permission ;)
--
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] acs-robot commented on pull request #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
acs-robot commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1170350742
Found UI changes, kicking a new UI QA build
@blueorangutan ui
--
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 #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1205654849
<b>Trillian test result (tid-4613)</b>
Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
Total time taken: 39371 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6491-t4613-xenserver-71.zip
Smoke tests completed. 101 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] DaanHoogland commented on a diff in pull request #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on code in PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#discussion_r912839904
##########
api/src/main/java/org/apache/cloudstack/api/command/admin/vm/MigrateVirtualMachineWithVolumeCmd.java:
##########
@@ -152,20 +153,17 @@ public ApiCommandResourceType getApiResourceType() {
@Override
public void execute() {
if (hostId == null && MapUtils.isEmpty(migrateVolumeTo)) {
- throw new InvalidParameterValueException(String.format("Either %s or %s must be passed for migrating the VM", ApiConstants.HOST_ID, ApiConstants.MIGRATE_TO));
- }
-
- UserVm userVm = _userVmService.getUserVm(getVirtualMachineId());
- if (userVm == null) {
- throw new InvalidParameterValueException("Unable to find the VM by id=" + getVirtualMachineId());
+ throw new InvalidParameterValueException(String.format("Either %s or %s must be passed for migrating the VM.", ApiConstants.HOST_ID, ApiConstants.MIGRATE_TO));
}
+ VirtualMachine userVm = _userVmService.getVm(getVirtualMachineId());
Review Comment:
how about the `null` check that used to be there? in `UserVmManagerImpl.getVM(long)` I see no sign of `null`-safety.
--
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] BryanMLima commented on pull request #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
BryanMLima commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1231860091
Hey @DaanHoogland, could you take a look on this one again? I think it already has the required number of approvals.
--
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 #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1164599795
@acs-robot a Jenkins job has been kicked to build UI QA env. 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] GabrielBrascher commented on a diff in pull request #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on code in PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#discussion_r908156758
##########
api/src/main/java/org/apache/cloudstack/api/command/admin/vm/MigrateVirtualMachineWithVolumeCmd.java:
##########
@@ -152,20 +153,17 @@ public ApiCommandResourceType getApiResourceType() {
@Override
public void execute() {
if (hostId == null && MapUtils.isEmpty(migrateVolumeTo)) {
- throw new InvalidParameterValueException(String.format("Either %s or %s must be passed for migrating the VM", ApiConstants.HOST_ID, ApiConstants.MIGRATE_TO));
- }
-
- UserVm userVm = _userVmService.getUserVm(getVirtualMachineId());
- if (userVm == null) {
- throw new InvalidParameterValueException("Unable to find the VM by id=" + getVirtualMachineId());
+ throw new InvalidParameterValueException(String.format("Either %s or %s must be passed for migrating the VM.", ApiConstants.HOST_ID, ApiConstants.MIGRATE_TO));
}
+ VirtualMachine userVm = _userVmService.getVm(getVirtualMachineId());
if (!VirtualMachine.State.Running.equals(userVm.getState()) && hostId != null) {
- throw new InvalidParameterValueException(String.format("VM ID: %s is not in Running state to migrate it to new host", userVm.getUuid()));
+ throw new InvalidParameterValueException(String.format("VM [%s] is not in the Running state to migrate it to the new host.", userVm));
Review Comment:
As far as I know, `VirtualMachine.toString()` will end up executing the [VMInstanceVO.toString()](https://github.com/apache/cloudstack/blob/ea9124e49c6849828519402b9053557d0ff056a9/engine/schema/src/main/java/com/cloud/vm/VMInstanceVO.java#L504). Which would have a redundant "VM" in the log message.
Can you please double-check and, if I am correct, change:
- from
`VM [VM instance {id: "123", name: "i-2-123-VM", uuid: "6c7d6c7d-6c7d-35fe-84d7-69b044600ffe", type="User"}] is not in Running ...`
- to
`VM instance {id: "123", name: "i-2-123-VM", uuid: "6c7d6c7d-6c7d-35fe-84d7-69b044600ffe", type="User"} is not in .Running ...`
--
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] sonarcloud[bot] commented on pull request #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1170399776
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=6491)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6491&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6491&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6491&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6491&metric=new_coverage&view=list) [76.7% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6491&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6491&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6491&metric=new_duplicated_lines_density&view=list)
--
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] BryanMLima commented on a diff in pull request #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
BryanMLima commented on code in PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#discussion_r910280121
##########
api/src/main/java/org/apache/cloudstack/api/command/admin/vm/MigrateVirtualMachineWithVolumeCmd.java:
##########
@@ -152,20 +153,17 @@ public ApiCommandResourceType getApiResourceType() {
@Override
public void execute() {
if (hostId == null && MapUtils.isEmpty(migrateVolumeTo)) {
- throw new InvalidParameterValueException(String.format("Either %s or %s must be passed for migrating the VM", ApiConstants.HOST_ID, ApiConstants.MIGRATE_TO));
- }
-
- UserVm userVm = _userVmService.getUserVm(getVirtualMachineId());
- if (userVm == null) {
- throw new InvalidParameterValueException("Unable to find the VM by id=" + getVirtualMachineId());
+ throw new InvalidParameterValueException(String.format("Either %s or %s must be passed for migrating the VM.", ApiConstants.HOST_ID, ApiConstants.MIGRATE_TO));
}
+ VirtualMachine userVm = _userVmService.getVm(getVirtualMachineId());
if (!VirtualMachine.State.Running.equals(userVm.getState()) && hostId != null) {
- throw new InvalidParameterValueException(String.format("VM ID: %s is not in Running state to migrate it to new host", userVm.getUuid()));
+ throw new InvalidParameterValueException(String.format("VM [%s] is not in the Running state to migrate it to the new host.", userVm));
Review Comment:
You are correct, I made the change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1170351155
@acs-robot a Jenkins job has been kicked to build UI QA env. 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] shwstppr commented on a diff in pull request #6491: Add live migration of system VMs (KVM)
Posted by GitBox <gi...@apache.org>.
shwstppr commented on code in PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#discussion_r1007914208
##########
api/src/main/java/com/cloud/vm/UserVmService.java:
##########
@@ -425,6 +425,8 @@ UserVm createVirtualMachine(DeployVMCmd cmd) throws InsufficientCapacityExceptio
UserVm getUserVm(long vmId);
+ VirtualMachine getVm(long vmId);
Review Comment:
okay @BryanMLima
--
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 #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1274164615
@BryanMLima can you respond to / apply @shwstppr's comments?
--
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 #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1205659265
<b>Trillian test result (tid-4615)</b>
Environment: vmware-65u2 (x2), Advanced Networking with Mgmt server 7
Total time taken: 39770 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6491-t4615-vmware-65u2.zip
Smoke tests completed. 101 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] shwstppr commented on a diff in pull request #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
shwstppr commented on code in PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#discussion_r989678532
##########
api/src/main/java/org/apache/cloudstack/api/response/VolumeResponse.java:
##########
@@ -96,6 +96,10 @@ public class VolumeResponse extends BaseResponseWithTagInformation implements Co
@Param(description = "state of the virtual machine")
private String virtualMachineState;
+ @SerializedName("vmtype")
Review Comment:
better to add this in ApiConstants
##########
api/src/main/java/com/cloud/vm/UserVmService.java:
##########
@@ -425,6 +425,8 @@ UserVm createVirtualMachine(DeployVMCmd cmd) throws InsufficientCapacityExceptio
UserVm getUserVm(long vmId);
+ VirtualMachine getVm(long vmId);
Review Comment:
Does it make more sense to define this in VirtualMachineManager class?
##########
ui/src/utils/path.js:
##########
@@ -0,0 +1,33 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+export function createPathBasedOnVmType (vmtype, virtualmachineid) {
+ let path = ''
+ switch (vmtype) {
+ case 'ConsoleProxy':
+ case 'SecondaryStorageVm':
+ path = '/systemvm/'
+ break
+ case 'DomainRouter':
+ path = '/router/'
+ break
+ default:
+ path = '/vm/'
+ }
+
+ return path + virtualmachineid
+}
Review Comment:
does it make more sense to add this code in `ui/src/utils/plugins.js`?
##########
api/src/main/java/org/apache/cloudstack/api/command/admin/vm/MigrateVirtualMachineWithVolumeCmd.java:
##########
@@ -152,20 +153,17 @@ public ApiCommandResourceType getApiResourceType() {
@Override
public void execute() {
if (hostId == null && MapUtils.isEmpty(migrateVolumeTo)) {
- throw new InvalidParameterValueException(String.format("Either %s or %s must be passed for migrating the VM", ApiConstants.HOST_ID, ApiConstants.MIGRATE_TO));
- }
-
- UserVm userVm = _userVmService.getUserVm(getVirtualMachineId());
- if (userVm == null) {
- throw new InvalidParameterValueException("Unable to find the VM by id=" + getVirtualMachineId());
+ throw new InvalidParameterValueException(String.format("Either %s or %s must be passed for migrating the VM.", ApiConstants.HOST_ID, ApiConstants.MIGRATE_TO));
}
+ VirtualMachine userVm = _userVmService.getVm(getVirtualMachineId());
Review Comment:
`userVm` variable name should be changed here
##########
api/src/main/java/org/apache/cloudstack/api/command/user/volume/ListVolumesCmd.java:
##########
@@ -88,6 +88,10 @@ public class ListVolumesCmd extends BaseListTaggedResourcesCmd implements UserCm
RoleType.Admin})
private Boolean display;
+ @Parameter(name = ApiConstants.LIST_SYSTEM_VMS, type = CommandType.BOOLEAN, description = "list system VMs; only ROOT admin is eligible to pass this parameter", since = "4.17",
Review Comment:
is support added for this in UI, in migrate VM form?
`since` should be 4.18 now
##########
api/src/main/java/org/apache/cloudstack/api/command/admin/vm/MigrateVirtualMachineWithVolumeCmd.java:
##########
@@ -195,4 +192,16 @@ public void execute() {
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, e.getMessage());
}
}
+
+ private void setResponseBasedOnVmType(VirtualMachine userVm, VirtualMachine migratedVm) {
+ if (VirtualMachine.Type.User.equals(userVm.getType())) {
+ UserVmResponse userVmResponse = _responseGenerator.createUserVmResponse(ResponseView.Full, "virtualmachine", (UserVm) migratedVm).get(0);
+ userVmResponse.setResponseName(getCommandName());
+ setResponseObject(userVmResponse);
+ return;
+ }
+ SystemVmResponse systemVmResponse = _responseGenerator.createSystemVmResponse(migratedVm);
+ systemVmResponse.setResponseName(getCommandName());
+ setResponseObject(systemVmResponse);
Review Comment:
I hope these different response types won't affect UI, worth checking
--
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] sonarcloud[bot] commented on pull request #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1164650893
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=6491)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6491&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6491&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6491&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6491&metric=new_coverage&view=list) [76.7% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6491&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6491&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6491&metric=new_duplicated_lines_density&view=list)
--
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 #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1204882540
@blueorangutan test matrix
--
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 #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1205651310
<b>Trillian test result (tid-4614)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 39017 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6491-t4614-kvm-centos7.zip
Smoke tests completed. 101 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] sonarcloud[bot] commented on pull request #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1167774194
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=6491)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6491&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6491&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6491&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6491&metric=new_coverage&view=list) [76.7% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6491&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6491&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6491&metric=new_duplicated_lines_density&view=list)
--
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 #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1234805851
UI build: :heavy_check_mark:
Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/6491 (SL-JID-2280)
--
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] sonarcloud[bot] commented on pull request #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1234848525
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=6491)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6491&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6491&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6491&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6491&metric=new_coverage&view=list) [71.9% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6491&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6491&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6491&metric=new_duplicated_lines_density&view=list)
--
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 #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1234800423
@acs-robot a Jenkins job has been kicked to build UI QA env. 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] blueorangutan commented on pull request #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1258554846
UI build: :heavy_check_mark:
Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/6491 (SL-JID-2421)
--
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] BryanMLima commented on pull request #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
BryanMLima commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1270734404
Hey @DaanHoogland, I made some changes in the core behavior of this PR. I added a new parameter for the `listVolumes` API to specify when to list system VMs volumes. Therefore, the default behavior is not changed, as it were with the past changes, and the integration tests passed smoothly. If you could review again, I appreciate.
--
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 #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1164607425
UI build: :heavy_check_mark:
Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/6491 (SL-JID-1828)
--
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 #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1204883319
@DaanHoogland a Trillian-Jenkins matrix job (centos7 mgmt + xs71, centos7 mgmt + vmware65, centos7 mgmt + kvmcentos7) 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] RodrigoDLopez commented on pull request #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
RodrigoDLopez commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1202475994
> @GabrielBrascher @RodrigoDLopez are your concerns all met?
Yes, I do agree with this proposal. Before I approved this one, I ran some manual tests in a local lab, and everything was good, I think we are good to go.
--
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] sonarcloud[bot] commented on pull request #6491: Add live migration of system VMs (KVM)
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1275020173
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=6491)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6491&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6491&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6491&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6491&metric=new_coverage&view=list) [38.0% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6491&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6491&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6491&metric=new_duplicated_lines_density&view=list)
--
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] sonarcloud[bot] commented on pull request #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1258694278
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=6491)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6491&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6491&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6491&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6491&metric=new_coverage&view=list) [38.3% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6491&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6491&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6491&metric=new_duplicated_lines_density&view=list)
--
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 #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1259689206
UI build: :heavy_check_mark:
Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/6491 (SL-JID-2426)
--
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 #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1258549150
@acs-robot a Jenkins job has been kicked to build UI QA env. 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] blueorangutan commented on pull request #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1259682225
@acs-robot a Jenkins job has been kicked to build UI QA env. 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 merged pull request #6491: Add live migration of system VMs (KVM)
Posted by GitBox <gi...@apache.org>.
DaanHoogland merged PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491
--
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 #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1202086670
@GabrielBrascher @RodrigoDLopez are your concerns all met?
--
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 #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1203600380
Packaging result: :heavy_multiplication_x: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 3889
--
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] acs-robot commented on pull request #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
acs-robot commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1164598699
Found UI changes, kicking a new UI QA build
@blueorangutan ui
--
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 #6491: Add live migration of system VMs (KVM)
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1294748519
> @DaanHoogland @GutoVeronezi can we proceed with the merge on this PR. The Travis CI passed after retrying the failed job.
yes @BryanMLima , as soon as @shwstppr confirms his comments are addressed.
--
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 #6491: Add live migration of system VMs (KVM)
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1294868782
thanks for your patience and persistence @BryanMLima
--
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] GutoVeronezi commented on a diff in pull request #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
GutoVeronezi commented on code in PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#discussion_r967069212
##########
ui/src/views/compute/InstanceTab.vue:
##########
@@ -33,31 +33,8 @@
<router-link :to="{ path: '/iso/' + vm.isoid }">{{ vm.isoname }}</router-link> <br/>
<barcode-outlined /> {{ vm.isoid }}
</a-tab-pane>
- <a-tab-pane :tab="$t('label.volumes')" key="volumes" v-if="'listVolumes' in $store.getters.apis">
- <a-table
- class="table"
- size="small"
- :columns="volumeColumns"
- :dataSource="volumes"
- :rowKey="item => item.id"
- :pagination="false"
- >
- <template #name="{ text, record }">
- <hdd-outlined />
- <router-link :to="{ path: '/volume/' + record.id }">
- {{ text }}
- </router-link>
- <a-tag v-if="record.provisioningtype">
- {{ record.provisioningtype }}
- </a-tag>
- </template>
- <template #state="{ text }">
- <status :text="text ? text : ''" />{{ text }}
- </template>
- <template #size="{ record }">
- {{ parseFloat(record.size / (1024.0 * 1024.0 * 1024.0)).toFixed(2) }} GB
- </template>
- </a-table>
+ <a-tab-pane :tab="$t('label.volumes')" key="volumes">
+ <VolumesTab :resource="vm" :items="volumes" :loading="loading" />
Review Comment:
```suggestion
<volumes-tab :resource="vm" :items="volumes" :loading="loading" />
```
##########
api/src/main/java/org/apache/cloudstack/api/command/admin/vm/MigrateVirtualMachineWithVolumeCmd.java:
##########
@@ -152,20 +153,17 @@ public ApiCommandResourceType getApiResourceType() {
@Override
public void execute() {
if (hostId == null && MapUtils.isEmpty(migrateVolumeTo)) {
- throw new InvalidParameterValueException(String.format("Either %s or %s must be passed for migrating the VM", ApiConstants.HOST_ID, ApiConstants.MIGRATE_TO));
- }
-
- UserVm userVm = _userVmService.getUserVm(getVirtualMachineId());
- if (userVm == null) {
- throw new InvalidParameterValueException("Unable to find the VM by id=" + getVirtualMachineId());
+ throw new InvalidParameterValueException(String.format("Either %s or %s must be passed for migrating the VM.", ApiConstants.HOST_ID, ApiConstants.MIGRATE_TO));
Review Comment:
```suggestion
throw new InvalidParameterValueException(String.format("Either %s or %s must be passed for migrating the VM.", ApiConstants.HOST_ID, ApiConstants.MIGRATE_TO));
```
--
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] codecov[bot] commented on pull request #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1234835282
# [Codecov](https://codecov.io/gh/apache/cloudstack/pull/6491?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#6491](https://codecov.io/gh/apache/cloudstack/pull/6491?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9d03495) into [main](https://codecov.io/gh/apache/cloudstack/commit/65c707042273a4067dc1749e10193fd3de165e0b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (65c7070) will **increase** coverage by `0.00%`.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## main #6491 +/- ##
=========================================
Coverage 5.88% 5.88%
Complexity 3942 3942
=========================================
Files 2454 2454
Lines 242723 242721 -2
Branches 37988 37990 +2
=========================================
Hits 14289 14289
+ Misses 226850 226848 -2
Partials 1584 1584
```
| [Impacted Files](https://codecov.io/gh/apache/cloudstack/pull/6491?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ain/java/com/cloud/api/query/QueryManagerImpl.java](https://codecov.io/gh/apache/cloudstack/pull/6491/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2VydmVyL3NyYy9tYWluL2phdmEvY29tL2Nsb3VkL2FwaS9xdWVyeS9RdWVyeU1hbmFnZXJJbXBsLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...ava/com/cloud/api/query/dao/VolumeJoinDaoImpl.java](https://codecov.io/gh/apache/cloudstack/pull/6491/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2VydmVyL3NyYy9tYWluL2phdmEvY29tL2Nsb3VkL2FwaS9xdWVyeS9kYW8vVm9sdW1lSm9pbkRhb0ltcGwuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [.../src/main/java/com/cloud/vm/UserVmManagerImpl.java](https://codecov.io/gh/apache/cloudstack/pull/6491/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2VydmVyL3NyYy9tYWluL2phdmEvY29tL2Nsb3VkL3ZtL1VzZXJWbU1hbmFnZXJJbXBsLmphdmE=) | `0.00% <0.00%> (ø)` | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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] acs-robot commented on pull request #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
acs-robot commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1274955827
Found UI changes, kicking a new UI QA build
@blueorangutan ui
--
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] BryanMLima commented on a diff in pull request #6491: Add live migration of system VMs (KVM)
Posted by GitBox <gi...@apache.org>.
BryanMLima commented on code in PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#discussion_r992566585
##########
api/src/main/java/org/apache/cloudstack/api/command/admin/vm/MigrateVirtualMachineWithVolumeCmd.java:
##########
@@ -195,4 +192,16 @@ public void execute() {
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, e.getMessage());
}
}
+
+ private void setResponseBasedOnVmType(VirtualMachine userVm, VirtualMachine migratedVm) {
+ if (VirtualMachine.Type.User.equals(userVm.getType())) {
+ UserVmResponse userVmResponse = _responseGenerator.createUserVmResponse(ResponseView.Full, "virtualmachine", (UserVm) migratedVm).get(0);
+ userVmResponse.setResponseName(getCommandName());
+ setResponseObject(userVmResponse);
+ return;
+ }
+ SystemVmResponse systemVmResponse = _responseGenerator.createSystemVmResponse(migratedVm);
+ systemVmResponse.setResponseName(getCommandName());
+ setResponseObject(systemVmResponse);
Review Comment:
While testing in my local lab, I did not find any problems, but more testing is always better.
--
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 #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1167722797
@acs-robot a Jenkins job has been kicked to build UI QA env. 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 pull request #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1203551970
> > @GabrielBrascher @RodrigoDLopez are your concerns all met?
>
> Yes, I do agree with this proposal. Before I approved this one, I ran some manual tests in a local lab, and everything was good, I think we are good to go.
ah, your approval status is still ¨left review comments¨. I understand you have tested this. Let's wait for @GabrielBrascher 's reply and then merge.
--
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 #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1203553067
amend: let's run smoke tests as well;
@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] BryanMLima commented on a diff in pull request #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
BryanMLima commented on code in PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#discussion_r914124623
##########
api/src/main/java/org/apache/cloudstack/api/command/admin/vm/MigrateVirtualMachineWithVolumeCmd.java:
##########
@@ -152,20 +153,17 @@ public ApiCommandResourceType getApiResourceType() {
@Override
public void execute() {
if (hostId == null && MapUtils.isEmpty(migrateVolumeTo)) {
- throw new InvalidParameterValueException(String.format("Either %s or %s must be passed for migrating the VM", ApiConstants.HOST_ID, ApiConstants.MIGRATE_TO));
- }
-
- UserVm userVm = _userVmService.getUserVm(getVirtualMachineId());
- if (userVm == null) {
- throw new InvalidParameterValueException("Unable to find the VM by id=" + getVirtualMachineId());
+ throw new InvalidParameterValueException(String.format("Either %s or %s must be passed for migrating the VM.", ApiConstants.HOST_ID, ApiConstants.MIGRATE_TO));
}
+ VirtualMachine userVm = _userVmService.getVm(getVirtualMachineId());
Review Comment:
As I was adjusting the error messages, I could not verify this one. Furthermore, I verified that the method `ParamProcessWorker#translateUuidToInternalId` already makes this verification. If the user puts an nonexistent UUID, or and UUID from another entity, the `translateUuidToInternalId` method already throws an exception, for these reason I removed this `null` check.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] BryanMLima commented on pull request #6491: Add live migration of system VMs (KVM)
Posted by GitBox <gi...@apache.org>.
BryanMLima commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1293914271
@DaanHoogland @GutoVeronezi can we proceed with the merge on this PR. The Travis CI passed after retrying the failed job.
--
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] acs-robot commented on pull request #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
acs-robot commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1234801572
Found UI changes, kicking a new UI QA build
@blueorangutan ui
--
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] acs-robot commented on pull request #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
acs-robot commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1234799509
Found UI changes, kicking a new UI QA build
@blueorangutan ui
--
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 #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1170357628
UI build: :heavy_check_mark:
Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/6491 (SL-JID-1873)
--
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] acs-robot commented on pull request #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
acs-robot commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1259681253
Found UI changes, kicking a new UI QA build
@blueorangutan ui
--
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] sonarcloud[bot] commented on pull request #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1259782549
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=6491)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6491&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6491&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6491&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6491&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6491&metric=new_coverage&view=list) [38.3% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6491&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6491&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6491&metric=new_duplicated_lines_density&view=list)
--
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] BryanMLima commented on pull request #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
BryanMLima commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1274964669
> Title of the PR is confusing for me as live migration for system VMs is already supported. May it should read - support migrate with volume for system VMs. Also, this may need testing both UI and API
I agree, I forgot to mentioned that these changes are to support live migration of system VMs for KVM. I will make some changes in this PR description to make it clear.
--
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] shwstppr commented on pull request #6491: Add live migration of system VMs (KVM)
Posted by GitBox <gi...@apache.org>.
shwstppr commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1294836649
@DaanHoogland cc @BryanMLima my comments have been addressed
--
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] acs-robot commented on pull request #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
acs-robot commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1167719056
Found UI changes, kicking a new UI QA build
@blueorangutan ui
--
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] RodrigoDLopez commented on a diff in pull request #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
RodrigoDLopez commented on code in PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#discussion_r907379016
##########
api/src/main/java/org/apache/cloudstack/api/command/admin/vm/MigrateVirtualMachineWithVolumeCmd.java:
##########
@@ -195,4 +192,16 @@ public void execute() {
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, e.getMessage());
}
}
+
+ void setResponseBasedOnVmType(VirtualMachine userVm, VirtualMachine migratedVm) {
Review Comment:
```suggestion
private void setResponseBasedOnVmType(VirtualMachine userVm, VirtualMachine migratedVm) {
```
--
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] github-actions[bot] commented on pull request #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1234798486
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.
--
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 #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1234804676
UI build: :heavy_check_mark:
Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/6491 (SL-JID-2279)
--
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 #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1204883396
@Pearl1594 @rohityadavcloud could you shine your lights on this, please?
--
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 closed pull request #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
DaanHoogland closed pull request #6491: Add live migration of system VMs
URL: https://github.com/apache/cloudstack/pull/6491
--
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 #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1258640767
UI build: :heavy_check_mark:
Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/6491 (SL-JID-2422)
--
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] BryanMLima commented on a diff in pull request #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
BryanMLima commented on code in PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#discussion_r992542240
##########
api/src/main/java/com/cloud/vm/UserVmService.java:
##########
@@ -425,6 +425,8 @@ UserVm createVirtualMachine(DeployVMCmd cmd) throws InsufficientCapacityExceptio
UserVm getUserVm(long vmId);
+ VirtualMachine getVm(long vmId);
Review Comment:
It does, however, to make this happen it caused a circular dependency problem, and to solve it, it would need to refactor a lot of classes to fix the issue. The refactor could be interesting in another PR, but is out of the scope of this one.
--
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 #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1274956542
@acs-robot a Jenkins job has been kicked to build UI QA env. 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] blueorangutan commented on pull request #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1203553594
@DaanHoogland a Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1203996638
@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 #6491: Add live migration of system VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6491:
URL: https://github.com/apache/cloudstack/pull/6491#issuecomment-1204051153
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 3895
--
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