Change in vdsm[master]: kill and report asyncProc if fails on first write to stdin

nsoffer at redhat.com nsoffer at redhat.com
Thu Jan 14 18:34:39 UTC 2016


Nir Soffer has posted comments on this change.

Change subject: kill and report asyncProc if fails on first write to stdin
......................................................................


Patch Set 2: Code-Review+1

(2 comments)

Code looks fine, but commit message needs more love.

https://gerrit.ovirt.org/#/c/51763/2//COMMIT_MSG
Commit Message:

Line 3: AuthorDate: 2016-01-13 09:57:14 +0200
Line 4: Commit:     Yaniv Bronhaim <ybronhei at redhat.com>
Line 5: CommitDate: 2016-01-14 04:10:38 -0500
Line 6: 
Line 7: kill and report asyncProc if fails on first write to stdin
Can you explain why this is needed? We have the full picture (part of deathSignal removal) but others may not have a clue what are you trying to do here.
Line 8: 
Line 9: Change-Id: I57b68742ce1dcae1d9fdad020b7b4a41ca6ad028


https://gerrit.ovirt.org/#/c/51763/2/lib/vdsm/commands.py
File lib/vdsm/commands.py:

Line 75:         if data is not None:
Line 76:             try:
Line 77:                 p.stdin.write(data)
Line 78:                 p.stdin.flush()
Line 79:             except:
> Please use specific exception here.
If we use specific exception, and an unexpected exception will be raised, we will not perform the cleanup needed. In such case I think it is better to handle *any* error. The original error is re-raised, so we are not swallowing it.
Line 80:                 exc_info = sys.exc_info()
Line 81:                 try:
Line 82:                     p.kill()
Line 83:                 except Exception:


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I57b68742ce1dcae1d9fdad020b7b4a41ca6ad028
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list