Change in vdsm[master]: virt: Ensure uncaught exceptions on disconnect events are ca...

[Vinzenz Feenstra]

Vinzenz Feenstra has uploaded a new change for review.

Change subject: virt: Ensure uncaught exceptions on disconnect events are caught
......................................................................

virt: Ensure uncaught exceptions on disconnect events are caught

Previously we did not catch unhandled exceptions and they
were not logged either which made it pretty difficult to
actually detect any existing issue in the first place.

This patch ensures that future unhandled exceptions are
at least logged so we'll be aware of them.

Change-Id: Ie5b91acad0e5d9c5a973b626f10fc0d543e6bcc6
Backport-To: 4.0
Bug-Url: https://bugzilla.redhat.com/1349383
Signed-off-by: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
---
M vdsm/virt/vm.py
1 file changed, 1 insertion(+), 0 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/68/59868/1

diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index 9447f81..879f64b 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -861,6 +861,7 @@
                 self.conf['clientIp'] = clientIp
             self._clientPort = clientPort
 
+    @utils.traceback()
     def _timedDesktopLock(self):
         # This is not a definite fix, we're aware that there is still the
         # possibility of a race condition, however this covers more cases


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie5b91acad0e5d9c5a973b626f10fc0d543e6bcc6
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;

gerrit-hooks has posted comments on this change.

Change subject: virt: Ensure uncaught exceptions on disconnect events are caught
......................................................................


Patch Set 1:

* #1349383::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1349383::OK, public bug
* Check Product::#1349383::OK, Correct classification oVirt
* Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6',
'ovirt-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie5b91acad0e5d9c5a973b626f10fc0d543e6bcc6
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

Francesco Romani has posted comments on this change.

Change subject: virt: Ensure uncaught exceptions on disconnect events are caught
......................................................................


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie5b91acad0e5d9c5a973b626f10fc0d543e6bcc6
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

[Vinzenz Feenstra]

Vinzenz Feenstra has posted comments on this change.

Change subject: virt: Ensure uncaught exceptions on disconnect events are caught
......................................................................


Patch Set 1: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie5b91acad0e5d9c5a973b626f10fc0d543e6bcc6
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

[Vinzenz Feenstra]

Vinzenz Feenstra has posted comments on this change.

Change subject: virt: Ensure uncaught exceptions on disconnect events are caught
......................................................................


Patch Set 1:

Tested together with https://gerrit.ovirt.org/#/c/59866/ - No additional tracebacks found

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie5b91acad0e5d9c5a973b626f10fc0d543e6bcc6
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: virt: Ensure uncaught exceptions on disconnect events are caught
......................................................................


Patch Set 1:

(1 comment)

https://gerrit.ovirt.org/#/c/59868/1/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 860:             with self._confLock:
Line 861:                 self.conf['clientIp'] = clientIp
Line 862:             self._clientPort = clientPort
Line 863: 
Line 864:     @utils.traceback()
Can you explain why we need this only on this method?
Line 865:     def _timedDesktopLock(self):
Line 866:         # This is not a definite fix, we're aware that there is still the
Line 867:         # possibility of a race condition, however this covers more cases
Line 868:         # than before and a quick gain


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie5b91acad0e5d9c5a973b626f10fc0d543e6bcc6
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: Yes

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: virt: Ensure uncaught exceptions on disconnect events are caught
......................................................................


Patch Set 1:

(1 comment)

https://gerrit.ovirt.org/#/c/59868/1/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 860:             with self._confLock:
Line 861:                 self.conf['clientIp'] = clientIp
Line 862:             self._clientPort = clientPort
Line 863: 
Line 864:     @utils.traceback()
>  Can you explain why we need this only on this method? 
$ git
grep threading.Timer|wc -l
1
Line 865:     def _timedDesktopLock(self):
Line 866:         # This is not a definite fix, we're aware that there is still the
Line 867:         # possibility of a race condition, however this covers more cases
Line 868:         # than before and a quick gain


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie5b91acad0e5d9c5a973b626f10fc0d543e6bcc6
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: Yes

[Dan Kenigsberg]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: virt: Ensure uncaught exceptions on disconnect events are caught
......................................................................


virt: Ensure uncaught exceptions on disconnect events are caught

Previously we did not catch unhandled exceptions and they
were not logged either which made it pretty difficult to
actually detect any existing issue in the first place.

This patch ensures that future unhandled exceptions are
at least logged so we'll be aware of them.

Change-Id: Ie5b91acad0e5d9c5a973b626f10fc0d543e6bcc6
Backport-To: 4.0
Bug-Url: https://bugzilla.redhat.com/1349383
Signed-off-by: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Reviewed-on: https://gerrit.ovirt.org/59868
Continuous-Integration: Jenkins CI
Reviewed-by: Francesco Romani <fromani(a)redhat.com&gt;
---
M vdsm/virt/vm.py
1 file changed, 1 insertion(+), 0 deletions(-)

Approvals:
  Jenkins CI: Passed CI tests
  Vinzenz Feenstra: Verified
  Francesco Romani: Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ie5b91acad0e5d9c5a973b626f10fc0d543e6bcc6
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;

gerrit-hooks has posted comments on this change.

Change subject: virt: Ensure uncaught exceptions on disconnect events are caught
......................................................................


Patch Set 2:

* #1349383::Update tracker: OK
* Set MODIFIED::bug 1349383::::#1349383::::OK

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie5b91acad0e5d9c5a973b626f10fc0d543e6bcc6
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No