Arik Hadas has posted comments on this change. Change subject: core: Refactor InternalMigrateVmCommand#canMigrateVm ......................................................................
Patch Set 1: Allon, two arguments why I think the current implementation should remain as is: 1. the error that is currently returned at MigrateVmCommand when migration support is set to MigrationSupport.IMPLICITLY_NON_MIGRATABLE and forcedMigrationForNonMigratableVM to false is VdcBllMessages.ACTION_TYPE_FAILED_VM_IS_NON_MIGRTABLE_AND_IS_NOT_FORCED_BY_USER_TO_MIGRATE which won't reflect the error properly if we'll use its canMigrateVm method from InternalMigrationVmCommand (with force = false). I added a new appropriate error to this case that is returned at InternalMigrationVmCommand#canMigrateVm. 2. I think the code is more self describable that way - it explicitly states that internal migration is not allowed unless the status is set to MIGRATABLE. what do you think? -- To view, visit http://gerrit.ovirt.org/10107 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ida10d53ccd5321d3ccc510bd2111fa013dcdcdbe Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Arik Hadas <aha...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Arik Hadas <aha...@redhat.com> Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com> Gerrit-Reviewer: Roy Golan <rgo...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
