Ryan Barry has posted comments on this change.

Change subject: Add utils.system.Bootloader
......................................................................


Patch Set 4:

(2 comments)

Intentional use of call() instead of check_call()

....................................................
File src/ovirt/node/utils/system.py
Line 95:     """
Line 96:     try:
Line 97:         process.call(["partprobe"] + [x for x in 
glob.glob("/dev/mapper/*")
Line 98:                                       if not re.match(r'.*\/control$', 
x)])
Line 99:         process.call(["udevadm", "settle"])
We don't actually want to fail if partprobe or udevadm fails. They're called to 
ensure that blkid has the best odds of working, but it's not essential to fail 
if they return a non-zero exit code, particularly given that partprobe is more 
likely than not to return a non-zero exit code if any multipathed devices are 
currently mounted and we don't want to bomb out. A non-zero return code from 
partprobe is non-critical.
Line 100:         process.check_call(["/sbin/blkid", "-c", "/dev/null", "-l", 
"-o",
Line 101:                             "device", "-t", 'LABEL="%s"' % label])
Line 102: 
Line 103:         # If we made it here, there are no non-zero exit codes, and 
we can


Line 100:         process.check_call(["/sbin/blkid", "-c", "/dev/null", "-l", 
"-o",
Line 101:                             "device", "-t", 'LABEL="%s"' % label])
Line 102: 
Line 103:         # If we made it here, there are no non-zero exit codes, and 
we can
Line 104:         # assume we found  a filesystem with this label
The purpose is to explain why we're not returning the actual return code or any 
output from the utilities we've called. Suggestions?
Line 105:         return True
Line 106: 
Line 107:     except process.CalledProcessError as e:
Line 108:         log.exception("Failed to resolve disks: %s" % e)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5a796a4c07bd99b927070ec62a7b1d990d4e1f9b
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-node
Gerrit-Branch: master
Gerrit-Owner: Ryan Barry <[email protected]>
Gerrit-Reviewer: Fabian Deutsch <[email protected]>
Gerrit-Reviewer: Nir Soffer <[email protected]>
Gerrit-Reviewer: Ryan Barry <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
node-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/node-patches

Reply via email to