Change in vdsm[master]: vm: replace try/finally with 'with'

fromani at redhat.com fromani at redhat.com
Tue Oct 28 08:08:25 UTC 2014


Francesco Romani has uploaded a new change for review.

Change subject: vm: replace try/finally with 'with'
......................................................................

vm: replace try/finally with 'with'

The creation of VM is throttled using a bounded semaphore.
This patch
* replaces the try/finally construct with 'with' to make
  code (a tiny bit) nicer ad smaller
* drops the log lines around the lock, because
  - we are now guaranteed the code is correct and always
    release the lock
  - if we care about performances and timings (e.g.time spent
    in locked region) we need more precise instrumentation
    anyway.

Change-Id: I1bf75d64ab4e838957cbd2b0890b8cbc6f9fc9b0
Signed-off-by: Francesco Romani <fromani at redhat.com>
---
M vdsm/virt/vm.py
1 file changed, 18 insertions(+), 21 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/29/34529/1

diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index 8b17a03..6e46119 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -1619,28 +1619,25 @@
         self.log.debug("Start")
         try:
             self.memCommit()
-            self._ongoingCreations.acquire()
-            self.log.debug("_ongoingCreations acquired")
-            self._vmCreationEvent.set()
-            try:
-                self._run()
-            except MissingLibvirtDomainError:
-                # we cannot continue without a libvirt domain object,
-                # not even on recovery, to avoid state desync or worse
-                # split-brain scenarios.
-                raise
-            except Exception as e:
-                # as above
-                if isinstance(e, libvirt.libvirtError) and \
-                   e.get_error_code() == libvirt.VIR_ERR_NO_DOMAIN:
-                    raise MissingLibvirtDomainError()
-                elif not self.recovering:
+            with self._ongoingCreations:
+                self._vmCreationEvent.set()
+                try:
+                    self._run()
+                except MissingLibvirtDomainError:
+                    # we cannot continue without a libvirt domain object,
+                    # not even on recovery, to avoid state desync or worse
+                    # split-brain scenarios.
                     raise
-                else:
-                    self.log.info("Skipping errors on recovery", exc_info=True)
-            finally:
-                self._ongoingCreations.release()
-                self.log.debug("_ongoingCreations released")
+                except Exception as e:
+                    # as above
+                    if isinstance(e, libvirt.libvirtError) and \
+                       e.get_error_code() == libvirt.VIR_ERR_NO_DOMAIN:
+                        raise MissingLibvirtDomainError()
+                    elif not self.recovering:
+                        raise
+                    else:
+                        self.log.info("Skipping errors on recovery",
+                                      exc_info=True)
 
             if ('migrationDest' in self.conf or 'restoreState' in self.conf) \
                     and self.lastStatus != vmstatus.DOWN:


-- 
To view, visit http://gerrit.ovirt.org/34529
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I1bf75d64ab4e838957cbd2b0890b8cbc6f9fc9b0
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani at redhat.com>


More information about the vdsm-patches mailing list