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

Reply via email to