Change in vdsm[ovirt-3.6]: vm: always hold confLock when updating pauseCode

fromani at redhat.com fromani at redhat.com
Tue Sep 29 07:05:45 UTC 2015


Hello Dan Kenigsberg, Martin Polednik,

I'd like you to do a code review.  Please visit

    https://gerrit.ovirt.org/46771

to review the following change.

Change subject: vm: always hold confLock when updating pauseCode
......................................................................

vm: always hold confLock when updating pauseCode

An update of pauseCode can create a new field in Vm.conf.
If a client calls the getVMList() API, or for any other API
which needs to loop over all the VM conf, we can end up with

RuntimeError: dictionary changed size during iteration

This patch make sure that when we update Vm.conf with
pauseCode-related data, we always have Vm.confLock held.

Change-Id: Ie84e2442165fb524f2433fab7faa3b049b340038
Bug-Url: https://bugzilla.redhat.com/1261918
Backport-To: 3.6
Backport-To: 3.5
Signed-off-by: Francesco Romani <fromani at redhat.com>
Reviewed-on: https://gerrit.ovirt.org/46005
Continuous-Integration: Jenkins CI
Reviewed-by: Michal Skrivanek <michal.skrivanek at redhat.com>
Reviewed-by: Martin Polednik <mpolednik at redhat.com>
Reviewed-by: Dan Kenigsberg <danken at redhat.com>
---
M vdsm/virt/vm.py
1 file changed, 14 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/71/46771/1

diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index a82f90f..06a19b7 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -714,7 +714,8 @@
 
             self.lastStatus = vmstatus.UP
             if self._initTimePauseCode:
-                self.conf['pauseCode'] = self._initTimePauseCode
+                with self._confLock:
+                    self.conf['pauseCode'] = self._initTimePauseCode
                 if self._initTimePauseCode == 'ENOSPC':
                     self.cont()
             else:
@@ -1371,8 +1372,9 @@
             'clientIp': self.conf.get('clientIp', ''),
         }
 
-        if 'pauseCode' in self.conf:
-            stats['pauseCode'] = self.conf['pauseCode']
+        with self._confLock:
+            if 'pauseCode' in self.conf:
+                stats['pauseCode'] = self.conf['pauseCode']
         if self.isMigrating():
             stats['migrationProgress'] = self.migrateStatus()['progress']
 
@@ -1754,7 +1756,8 @@
                                    vmstatus.RESTORING_STATE):
             self._initTimePauseCode = self._readPauseCode()
         if not self.recovering and self._initTimePauseCode:
-            self.conf['pauseCode'] = self._initTimePauseCode
+            with self._confLock:
+                self.conf['pauseCode'] = self._initTimePauseCode
             if self._initTimePauseCode == 'ENOSPC':
                 self.cont()
 
@@ -1855,10 +1858,11 @@
                 self._timeoutExperienced)
         else:
             flags = libvirt.VIR_DOMAIN_NONE
-            if 'launchPaused' in self.conf:
-                flags |= libvirt.VIR_DOMAIN_START_PAUSED
-                self.conf['pauseCode'] = 'NOERR'
-                del self.conf['launchPaused']
+            with self._confLock:
+                if 'launchPaused' in self.conf:
+                    flags |= libvirt.VIR_DOMAIN_START_PAUSED
+                    self.conf['pauseCode'] = 'NOERR'
+                    del self.conf['launchPaused']
             self._dom = virdomain.Notifying(
                 self._connection.createXML(domxml, flags),
                 self._timeoutExperienced)
@@ -3627,7 +3631,8 @@
         if action == libvirt.VIR_DOMAIN_EVENT_IO_ERROR_PAUSE:
             self.log.info('abnormal vm stop device %s error %s',
                           blockDevAlias, err)
-            self.conf['pauseCode'] = reason
+            with self._confLock:
+                self.conf['pauseCode'] = reason
             self._setGuestCpuRunning(False)
             self._logGuestCpuStatus('onIOError')
             if reason == 'ENOSPC':


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie84e2442165fb524f2433fab7faa3b049b340038
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Martin Polednik <mpolednik at redhat.com>


More information about the vdsm-patches mailing list