Change in vdsm[master]: migration: on abort, override reason on dest VM

Wednesday, 24 June 2015

Francesco Romani has uploaded a new change for review.

Change subject: migration: on abort, override reason on dest VM
......................................................................

migration: on abort, override reason on dest VM

When a migration is aborted, the Engine looks
for the migration failure reason on Destination VM.
Unfortunately, it is (almost always) the source VM
that decided to abort the migration, so it is actually
the source VM that knows the failure reason.

We can change VDSM to share this information, or we
can fix Engine to ask source VM.

The first option is probably safer and simpler, so
this patch implements that by extending the destroy()
verb with an optional vmexitreason field, so destroying
agent (maybe source VDSM, maybe Engine) can propagate
the right reason.

Change-Id: I021774437e969930880ba1602893bbd5ed2c1c1a
Signed-off-by: Francesco Romani <fromani(a)redhat.com&gt;
---
M vdsm/API.py
M vdsm/rpc/bindingxmlrpc.py
M vdsm/rpc/vdsmapi-schema.json
M vdsm/virt/migration.py
M vdsm/virt/vm.py
M vdsm/virt/vmexitreason.py
6 files changed, 57 insertions(+), 12 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/01/42801/1

diff --git a/vdsm/API.py b/vdsm/API.py
index bc74e67..1b517e1 100644
--- a/vdsm/API.py
+++ b/vdsm/API.py
@@ -43,6 +43,7 @@
 import storage.volume
 import storage.sd
 import storage.image
+from virt import vmexitreason
 from virt import vmstatus
 from virt.vmdevices import graphics
 from virt.vmdevices import hwclass
@@ -327,7 +328,7 @@
         else:
             return errCode['nonresp']
 
-    def destroy(self):
+    def destroy(self, exitReason=vmexitreason.SUCCESS):
         """
         Destroy the specified VM.
         """
@@ -337,7 +338,7 @@
             v = self._cif.vmContainer.get(self._UUID)
             if not v:
                 return errCode['noVM']
-            res = v.destroy()
+            res = v.destroy(exitReason)
             status = utils.picklecopy(res)
             if status['status']['code'] == 0:
                 status['status']['message'] = "Machine
destroyed"
diff --git a/vdsm/rpc/bindingxmlrpc.py b/vdsm/rpc/bindingxmlrpc.py
index 21a545d..2a344c4 100644
--- a/vdsm/rpc/bindingxmlrpc.py
+++ b/vdsm/rpc/bindingxmlrpc.py
@@ -36,6 +36,7 @@
 from vdsm.netinfo import getDeviceByIP
 import API
 from vdsm.exception import VdsmException
+from virt import vmexitreason
 
 try:
     from gluster.api import getGlusterMethods
@@ -350,9 +351,9 @@
     #
     # Callable methods:
     #
-    def vmDestroy(self, vmId):
+    def vmDestroy(self, vmId, reason=vmexitreason.SUCCESS):
         vm = API.VM(vmId)
-        return vm.destroy()
+        return vm.destroy(reason)
 
     def vmCreate(self, vmParams):
         vm = API.VM(vmParams['vmId'])
diff --git a/vdsm/rpc/vdsmapi-schema.json b/vdsm/rpc/vdsmapi-schema.json
index b16657d..81ec00c 100644
--- a/vdsm/rpc/vdsmapi-schema.json
+++ b/vdsm/rpc/vdsmapi-schema.json
@@ -6575,12 +6575,16 @@
 #
 # Forcibly stop a running VM.
 #
-# @vmID:  The UUID of the VM
+# @vmID:       The UUID of the VM
+#
+# @exitReason: #optional Override VM exit reason.
+#              Ignored if SUCCESS.
+#              Internal usage only. (new in version 4.17.0)
 #
 # Since: 4.10.0
 ##
 {'command': {'class': 'VM', 'name': 'destroy'},
- 'data': {'vmID': 'UUID'}}
+ 'data': {'vmID': 'UUID', '*exitReason':
'VmExitReason'}}
 
 ##
 # @VM.getInfo:
diff --git a/vdsm/virt/migration.py b/vdsm/virt/migration.py
index 0e6a024..9ceecb8 100644
--- a/vdsm/virt/migration.py
+++ b/vdsm/virt/migration.py
@@ -67,6 +67,13 @@
     STALLED = 3
 
 
+_ABORT_REASON_TO_EXIT_REASON = {
+    MigrationAbortReason.CLIENT: vmexitreason.MIGRATION_ABORTED_CLIENT,
+    MigrationAbortReason.MAX_TIME: vmexitreason.MIGRATION_ABORTED_TIMEOUT,
+    MigrationAbortReason.STALLED: vmexitreason.MIGRATION_ABORTED_STALLED,
+}
+
+
 class SourceThread(threading.Thread):
     """
     A thread that takes care of migration on the source vdsm.
@@ -143,6 +150,14 @@
         if self._monitorThread is not None:
             # we need to report monotonic progress
             self._progress = self._monitorThread.progress
+
+    def _get_abort_reason(self):
+        # FIXME: racy.
+        if (self._monitorThread is not None and
+           self._monitorThread.abort_reason is not None):
+            return self._monitorThread.abort_reason
+        else:
+            return MigrationAbortReason.CLIENT
 
     def _addMigrationFields(self, res):
         """
@@ -240,7 +255,11 @@
         self.log.error(message)
         if not self.hibernating:
             try:
-                self._destServer.destroy(self._vm.id)
+                reason = _ABORT_REASON_TO_EXIT_REASON.get(
+                    self._get_abort_reason(),
+                    vmexitreason.SUCCESS
+                )
+                self._destServer.destroy(self._vm.id, reason)
             except Exception:
                 self.log.exception("Failed to destroy remote VM")
         # if the guest was stopped before migration, we need to cont it
@@ -509,6 +528,15 @@
         self._startTime = startTime
         self.daemon = True
         self.progress = 0
+        self._abort_reason = None
+
+    @property
+    def canceled(self):
+        return self._abort_reason is not None
+
+    @property
+    def abort_reason(self):
+        return self._abort_reason
 
     @property
     def enabled(self):
@@ -555,6 +583,7 @@
                                   'migration will be aborted.',
                                   now - self._startTime,
                                   migrationMaxTime)
+                self._abort_reason = MigrationAbortReason.MAX_TIME
                 abort = True
             elif (lowmark is None) or (lowmark > dataRemaining):
                 lowmark = dataRemaining
@@ -564,6 +593,7 @@
                 self._vm.log.warn(
                     'Migration is stuck: Hasn\'t progressed in %s seconds. '
                     'Aborting.' % (now - lastProgressTime))
+                self._abort_reason = MigrationAbortReason.STALLED
                 abort = True
 
             if abort:
diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index 4f7a271..bb8c062 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -3674,10 +3674,10 @@
         except Exception:
             self.log.exception("Failed to delete VM %s",
self.conf['vmId'])
 
-    def destroy(self):
+    def destroy(self, reason=vmexitreason.SUCCESS):
         self.log.debug('destroy Called')
 
-        result = self.doDestroy()
+        result = self.doDestroy(reason)
         if result['status']['code']:
             return result
         # Clean VM from the system
@@ -3685,14 +3685,17 @@
 
         return {'status': doneCode}
 
-    def doDestroy(self):
+    def doDestroy(self, reason):
         for dev in self._customDevices():
             hooks.before_device_destroy(dev._deviceXML, self.conf,
                                         dev.custom)
 
         hooks.before_vm_destroy(self._domain.xml, self.conf)
         with self._shutdownLock:
-            self._shutdownReason = vmexitreason.ADMIN_SHUTDOWN
+            if reason != vmexitreason.SUCCESS:
+                self._shutdownReason = reason
+            else:
+                self._shutdownReason = vmexitreason.ADMIN_SHUTDOWN
         self._destroyed = True
 
         return self.releaseVm()
diff --git a/vdsm/virt/vmexitreason.py b/vdsm/virt/vmexitreason.py
index c04e65e..944d893 100644
--- a/vdsm/virt/vmexitreason.py
+++ b/vdsm/virt/vmexitreason.py
@@ -28,6 +28,9 @@
 USER_SHUTDOWN = 7
 MIGRATION_FAILED = 8
 LIBVIRT_DOMAIN_MISSING = 9
+MIGRATION_ABORTED_CLIENT = 10
+MIGRATION_ABORTED_TIMEOUT = 11
+MIGRATION_ABORTED_STALLED = 12
 
 
 exitReasons = {
@@ -40,5 +43,8 @@
     ADMIN_SHUTDOWN: 'Admin shut down from the engine',
     USER_SHUTDOWN: 'User shut down from within the guest',
     MIGRATION_FAILED: 'VM failed to migrate',
-    LIBVIRT_DOMAIN_MISSING: 'Failed to find the libvirt domain'
+    LIBVIRT_DOMAIN_MISSING: 'Failed to find the libvirt domain',
+    MIGRATION_ABORTED_CLIENT: 'VM migration canceled by client',
+    MIGRATION_ABORTED_TIMEOUT: 'VM took too much time to migrate',
+    MIGRATION_ABORTED_STALLED: 'VM migration failed to progress'
 }


-- 
To view, visit https://gerrit.ovirt.org/42801
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I021774437e969930880ba1602893bbd5ed2c1c1a
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com&gt;

    

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011