Nir Soffer has posted comments on this change.
Change subject: Add utils.system.Bootloader
......................................................................
Patch Set 4: Code-Review-1
(2 comments)
Wrong 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"])
ovirt.node.utils.process call() and check_call() are just thin wrappers around
standard library subprocess call() and check_call(). call() will not raise
CalledProcessError when process fails, but return an error code. See
http://docs.python.org/2.7/library/subprocess.html#subprocess.call
So if you want to fail if partprob or udevadm fails, use check_call() instead
of call().
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
This comment is not very useful.
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
