Change in vdsm[master]: vm: janitorial: add a restoringState attribute

Francesco Romani has uploaded a new change for review.

Change subject: vm: janitorial: add a restoringState attribute
......................................................................

vm: janitorial: add a restoringState attribute

A VM which needs to restore its state is identified by
checking for the presence of a 'restoreState' attribute in the
Vm.conf dict.

This attribute conveys two distinct informations: the need
to restore a state and the state to be restored.

This patch goes along the line of the
http://gerrit.ovirt.org/#/c/22783/
and aims to make the code cleaner and simpler introducing
an explicit attribute to identify VMs which needs to restore their
state.

Change-Id: Ia9fe3f1e38d55e7e661d41c312b5344a9e560e2a
Signed-off-by: Francesco Romani 
---
M vdsm/API.py
M vdsm/clientIF.py
M vdsm/vm.py
3 files changed, 14 insertions(+), 11 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/19/23919/1

diff --git a/vdsm/API.py b/vdsm/API.py
index 152b584..63a04a6 100644
--- a/vdsm/API.py
+++ b/vdsm/API.py
@@ -174,9 +174,11 @@
                 self.log.warning('vm %s already exists' % vmParams['vmId'])
                 return errCode['exist']

+            restoreState = False
             if 'hiberVolHandle' in vmParams:
                 vmParams['restoreState'], paramFilespec = \
                     self._getHibernationPaths(vmParams.pop('hiberVolHandle'))
+                restoreState = True
                 try:   # restore saved vm parameters
                 # NOTE: pickled params override command-line params. this
                 # might cause problems if an upgrade took place since the
@@ -260,7 +262,7 @@
                                                        'displayNetwork'))
             vmParams['displayPort'] = '-1'   # selected by libvirt
             vmParams['displaySecurePort'] = '-1'
-            return self._cif.createVm(vmParams)
+            return self._cif.createVm(vmParams, vmRestoreState=restoreState)

except OSError as e:
             self.log.debug("OS Error creating VM", exc_info=True)
diff --git a/vdsm/clientIF.py b/vdsm/clientIF.py
index fe51fa0..11e51fd 100644
--- a/vdsm/clientIF.py
+++ b/vdsm/clientIF.py
@@ -355,7 +355,7 @@

return {'status': doneCode, 'alignment': aligning}

-    def createVm(self, vmParams, vmRecover=False):
+    def createVm(self, vmParams, vmRecover=False, vmRestoreState=False):
         with self.vmContainerLock:
             self.log.info("vmContainerLock acquired by vm %s",
                           vmParams['vmId'])
@@ -365,7 +365,7 @@
                         self.log.warning('vm %s already exists' %
                                          vmParams['vmId'])
                         return errCode['exist']
-                vm = Vm(self, vmParams, vmRecover)
+                vm = Vm(self, vmParams, vmRecover, vmRestoreState)
                 self.vmContainer[vmParams['vmId']] = vm
             finally:
                 container_len = len(self.vmContainer)
diff --git a/vdsm/vm.py b/vdsm/vm.py
index 6e4feda..42b460f 100644
--- a/vdsm/vm.py
+++ b/vdsm/vm.py
@@ -1877,7 +1877,7 @@
                                    caps.Architecture.PPC64: 'scsi'}
         return DEFAULT_DISK_INTERFACES[self.arch]

-    def __init__(self, cif, params, recover=False):
+    def __init__(self, cif, params, recover=False, restoreState=False):
         """
         Initialize a new VM instance.

@@ -1890,6 +1890,7 @@
         """
         self._dom = None
         self.recovering = recover
+        self.restoringState = restoreState
         self.conf = {'pid': '0'}
         self.conf.update(params)
         self.cif = cif
@@ -1905,7 +1906,7 @@
         self._creationThread = threading.Thread(target=self._startUnderlyingVm)
         if 'migrationDest' in self.conf:
             self._lastStatus = 'Migration Destination'
-        elif 'restoreState' in self.conf:
+        elif self.restoringState:
             self._lastStatus = 'Restoring state'
         else:
             self._lastStatus = 'WaitForLaunch'
@@ -2249,7 +2250,7 @@
                 self._ongoingCreations.release()
                 self.log.debug("_ongoingCreations released")

-            if ('migrationDest' in self.conf or 'restoreState' in self.conf) \
+            if ('migrationDest' in self.conf or self.restoringState) \
                     and self.lastStatus != 'Down':
                 self._waitForIncomingMigrationFinish()

@@ -2275,7 +2276,7 @@
                 self.setDownStatus(ERROR, str(e))

def _incomingMigrationPending(self):
-        return 'migrationDest' in self.conf or 'restoreState' in self.conf
+        return 'migrationDest' in self.conf or self.restorignState

def stopDisksStatsCollection(self):
         self._volumesPrepared = False
@@ -2718,7 +2719,7 @@
             self.lastStatus = 'Down'
             with self._confLock:
                 self.conf['exitCode'] = code
-                if 'restoreState' in self.conf:
+                if self.restoringState:
                     self.conf['exitMessage'] = (
                         "Wake up from hibernation failed")
                 else:
@@ -3135,7 +3136,7 @@
                 self._timeoutExperienced)
             # Reinitialize the merge statuses
             self._checkMerge()
-        elif 'restoreState' in self.conf:
+        elif self.restoringState:
             fromSnapshot = self.conf.get('restoreFromSnapshot', False)
             srcDomXML = self.conf.pop('_srcDomXML')
             if fromSnapshot:
@@ -3666,9 +3667,9 @@
             self._monitorResponse = 0

def _waitForIncomingMigrationFinish(self):
-        if 'restoreState' in self.conf:
+        if self.restoringState:
             self.cont()
-            del self.conf['restoreState']
+            self.restoringState = False
             fromSnapshot = self.conf.pop('restoreFromSnapshot', False)
             hooks.after_vm_dehibernate(self._dom.XMLDesc(0), self.conf,
                                        {'FROM_SNAPSHOT': fromSnapshot})

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: vm: janitorial: add a restoringState attribute
......................................................................

Patch Set 1: Verified-1

Build Failed

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6128/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7021/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/6915/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_storage_functional_tests_localfs/86/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_storage_functional_tests_nfs/24/ : FAILURE

Francesco Romani has posted comments on this change.

Change subject: vm: janitorial: add a restoringState attribute
......................................................................

Patch Set 1:

fedora19-vm03 seems to have broken dependecies... Will report on infra.

Francesco Romani has abandoned this change.

Change subject: vm: janitorial: add a restoringState attribute
......................................................................

Abandoned

Abandoned for the same reasons of 23915