Hello Dan Kenigsberg, Michal Skrivanek, Martin Polednik, I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/21536 to review the following change. Change subject: Ensure cancellation of migrations gets handled correctly ...................................................................... Ensure cancellation of migrations gets handled correctly If the migration is cancelled before the source calls libvirt.migrateToURI2 we have to raise an libvirtError with the VIR_ERROR_OPERATION_ABORTED error code to ensure that _finishSuccessfully() does not get called and we're correctly reporting that the migration has been cancelled and that we're destroying the VM on the destination in the process, since we're bailing out. Additionally we'll do the check if the migration has been cancelled right after the semaphore has been accquired, to avoid doing things like calling the before_vm_migration_source hook and calling the destination vdsm to create the VM instance. Change-Id: Id271a41466bc355ca505f764206457af419f0edd Bug-Url: https://bugzilla.redhat.com/1023131 Signed-off-by: Martin Polednik <mpole...@redhat.com> Signed-off-by: Vinzenz Feenstra <vfeen...@redhat.com> Reviewed-on: http://gerrit.ovirt.org/21445 Reviewed-by: Michal Skrivanek <michal.skriva...@redhat.com> Reviewed-by: Dan Kenigsberg <dan...@redhat.com> --- M vdsm/vm.py 1 file changed, 19 insertions(+), 2 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/36/21536/1 diff --git a/vdsm/vm.py b/vdsm/vm.py index 0921fb2..e64767b 100644 --- a/vdsm/vm.py +++ b/vdsm/vm.py @@ -145,7 +145,7 @@ 'message': 'Migration in progress'}, 'progress': 0} threading.Thread.__init__(self) - self._preparingMigrationEvt = False + self._preparingMigrationEvt = True self._migrationCanceledEvt = False self._monitorThread = None @@ -288,6 +288,19 @@ # vdsm < 4.13 expect this to exist self._machineParams['afterMigrationStatus'] = '' + @staticmethod + def _raiseAbortError(): + e = libvirt.libvirtError(defmsg='') + # we have to override the value to get what we want + # err might be None + e.err = (libvirt.VIR_ERR_OPERATION_ABORTED, # error code + libvirt.VIR_FROM_QEMU, # error domain + 'operation aborted', # error message + libvirt.VIR_ERR_WARNING, # error level + '', '', '', # str1, str2, str3, + -1, -1) # int1, int2 + raise e + def run(self): try: mstate = '' @@ -296,6 +309,8 @@ self._prepareGuest() MigrationSourceThread._ongoingMigrations.acquire() try: + if self._migrationCanceledEvt: + self._raiseAbortError() self.log.debug("migration semaphore acquired") if not mstate: self._vm.conf['_migrationParams'] = { @@ -321,7 +336,6 @@ self.log.error("Failed to migrate", exc_info=True) def _startUnderlyingMigration(self): - self._preparingMigrationEvt = True if self._mode == 'file': hooks.before_vm_hibernate(self._vm._dom.XMLDesc(0), self._vm.conf) try: @@ -383,6 +397,9 @@ (libvirt.VIR_MIGRATE_TUNNELLED if self._tunneled else 0), None, maxBandwidth) + else: + self._raiseAbortError() + finally: t.cancel() if MigrationMonitorThread._MIGRATION_MONITOR_INTERVAL: -- To view, visit http://gerrit.ovirt.org/21536 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Id271a41466bc355ca505f764206457af419f0edd Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.3 Gerrit-Owner: Vinzenz Feenstra <vfeen...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Martin Polednik <mpole...@redhat.com> Gerrit-Reviewer: Michal Skrivanek <michal.skriva...@redhat.com> _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches