Saggi Mizrahi has posted comments on this change.

Change subject: infra: Add logging in case PoolHandler crashes
......................................................................


Patch Set 3: (2 inline comments)

....................................................
File vdsm/storage/remoteFileHandler.py
Line 250: 
Line 251:         self.process.poll()
Line 252:         # Don't try to read if the process is in D state
Line 253:         if (self.process.returncode is not None and
Line 254:                 self.process.returncode != 0):
I don't trust non-blocking reads in python. They have very weird semantics.
Further more, if we use non blocking reads this code needs to loop until we get 
an empty read(). This is meaning less because this code is really only 
beneficial if the process crashed on boot because any other crash is 
"segmentation fault".
Line 255:             err = self.process.stderr.read()
Line 256:             out = self.process.stdout.read()
Line 257:             self.log.debug("Pool handler existed, OUT: '%s' ERR: 
'%s'",
Line 258:                            out, err)


Line 254:                 self.process.returncode != 0):
Line 255:             err = self.process.stderr.read()
Line 256:             out = self.process.stdout.read()
Line 257:             self.log.debug("Pool handler existed, OUT: '%s' ERR: 
'%s'",
Line 258:                            out, err)
Not, really
Line 259: 
Line 260:         try:
Line 261:             zombieReaper.autoReapPID(self.process.pid)
Line 262:         except AttributeError:


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie488886a608a6855019d4c36eb2b31c9a941f8c8
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizr...@redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizr...@redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybron...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to