Alon Bar-Lev has posted comments on this change.

Change subject: vdsm_config: move download certificate
......................................................................


Patch Set 25:

(2 comments)

http://gerrit.ovirt.org/#/c/26718/25/vdsm_reg/vdsm-reg-setup.in
File vdsm_reg/vdsm-reg-setup.in:

Line 201:         Returns:
Line 202:         True -- Fingerprint matched
Line 203:         False -- Fingerprint didn't match
Line 204:         """
Line 205:         ret = True
always assume failure.... set success only if actually success.

it is much safer than this pattern.

also, you can always return success if fingerprint is None.... avoiding even 
more conditionals at caller.
Line 206:         engine_fprint = deployUtil.generateFingerPrint(ca_path)
Line 207:         if self.cfg_fprint.upper() != engine_fprint:
Line 208:             logging.error(
Line 209:                 "Fingerprint differs from Engine\n"


Line 211:                 "Engine fingerprint [%s]",
Line 212:                 self.cfg_fprint.upper(),
Line 213:                 engine_fprint
Line 214:             )
Line 215:             os.unlink(ca_path)
you must unpersist first in case of the 1st check.
Line 216:             ret = False
Line 217: 
Line 218:         return ret
Line 219: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia2c025eedd2be92b9418ddbe01efc02c913af2a7
Gerrit-PatchSet: 25
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Douglas Schilling Landgraf <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to