Copilot commented on code in PR #13144:
URL: https://github.com/apache/cloudstack/pull/13144#discussion_r3273204068
##########
ui/src/views/compute/MigrateWizard.vue:
##########
@@ -338,11 +338,11 @@ export default {
submitForm () {
if (this.loading) return
this.loading = true
- const migrateApi = this.isUserVm
- ? this.requiresStorageMigration()
- ? 'migrateVirtualMachineWithVolume'
- : 'migrateVirtualMachine'
- : 'migrateSystemVm'
+ const migrateApi = !this.requiresStorageMigration()
+ ? this.isUserVm
+ ? 'migrateVirtualMachine'
+ : 'migrateSystemVm'
+ : 'migrateVirtualMachineWithVolume'
Review Comment:
`migrateApi` selection now routes system-VM migrations that require storage
motion to `migrateVirtualMachineWithVolume`, but `requiresStorageMigration()`
relies on `this.volumes` (local-storage check) and `fetchVolumes()` currently
calls `listVolumes` without `listsystemvms=true`. For system VMs this likely
returns no volumes, so `requiresStorageMigration()` can incorrectly return
false (e.g., auto-select with local volumes), causing `migrateSystemVm` to be
used and potentially skipping/failed volume migration. Consider requesting
system-VM volumes in `fetchVolumes()` (pass `listsystemvms=true` when migrating
non-UserVm) so storage-motion detection and mapping behave correctly for system
VMs.
##########
ui/src/components/view/InstanceVolumesStoragePoolSelectListView.vue:
##########
@@ -170,7 +170,8 @@ export default {
this.volumes = []
getAPI('listVolumes', {
listAll: true,
- virtualmachineid: this.resource.id
+ virtualmachineid: this.resource.id,
+ listsystemvms: true
}).then(response => {
Review Comment:
`listsystemvms` is an admin-only parameter on the `listVolumes` API
(ListVolumesCmd declares it as `authorized = {RoleType.Admin}`); passing
`listsystemvms: true` unconditionally here can cause the volumes fetch to fail
for non-root-admin users if this component is ever rendered outside
root-admin-only flows. Consider only adding `listsystemvms` when the current
user is Admin (or when explicitly needed for system-VM volume listing).
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
