Antoni Segura Puimedon has uploaded a new change for review.
Change subject: More explicit octal values ......................................................................
More explicit octal values
Change 0XXX to 0oXXX so that that these values are more visibly octal and forwards compatible with Python 3.3+.
Change-Id: Icc81c0ac52cc779511e49133aa669defeb47a3f1 Signed-off-by: Antoni S. Puimedon asegurap@redhat.com --- M lib/vdsm/tool/transient.py M tests/fileUtilTests.py M tests/functional/storageTests.py M tests/functional/virtTests.py M tests/hooksTests.py M tests/miscTests.py M vds_bootstrap/setup M vds_bootstrap/vds_bootstrap.py M vdsm/netconf/ifcfg.py M vdsm/storage/blockVolume.py M vdsm/storage/fileSD.py M vdsm/storage/fileUtils.py M vdsm/storage/fileVolume.py M vdsm/storage/storageServer.py M vdsm/vdsm M vdsm/vm.py M vdsm_reg/createDaemon.py M vdsm_reg/vdsm-reg-setup.in 18 files changed, 38 insertions(+), 38 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/34/20534/1
diff --git a/lib/vdsm/tool/transient.py b/lib/vdsm/tool/transient.py index 7452f30..020eb4e 100644 --- a/lib/vdsm/tool/transient.py +++ b/lib/vdsm/tool/transient.py @@ -51,7 +51,7 @@ raise
os.chown(TRANSIENT_DISKS_REPO, vdsm_uid, vdsm_gid) - os.chmod(TRANSIENT_DISKS_REPO, 0750) + os.chmod(TRANSIENT_DISKS_REPO, 0o750) selinux.chcon(TRANSIENT_DISKS_REPO, SELINUX_VIRT_IMAGE_LABEL)
diff --git a/tests/fileUtilTests.py b/tests/fileUtilTests.py index 8cdad4c..6771727 100644 --- a/tests/fileUtilTests.py +++ b/tests/fileUtilTests.py @@ -180,11 +180,11 @@
class CopyUserModeToGroupTests(TestCaseBase): - MODE_MASK = 0777 + MODE_MASK = 0o777
# format: initialMode, expectedMode modesList = [ - (0770, 0770), (0700, 0770), (0750, 0770), (0650, 0660), + (0o770, 0o770), (0o700, 0o770), (0o750, 0o770), (0o650, 0o660), ]
def testCopyUserModeToGroup(self): diff --git a/tests/functional/storageTests.py b/tests/functional/storageTests.py index a185f58..e9b2c4f 100644 --- a/tests/functional/storageTests.py +++ b/tests/functional/storageTests.py @@ -278,7 +278,7 @@ undo = lambda: os.rmdir(rootDir) rollback.prependDefer(undo) os.chown(rootDir, uid, gid) - os.chmod(rootDir, 0755) + os.chmod(rootDir, 0o755)
connections = {} for uuid, subDir in backends.iteritems(): @@ -287,7 +287,7 @@ undo = lambda path=path: shutil.rmtree(path, ignore_errors=True) rollback.prependDefer(undo) os.chown(path, uid, gid) - os.chmod(path, 0775) + os.chmod(path, 0o775)
connections[uuid] = {'type': 'localfs', 'params': {'path': path}} @@ -499,7 +499,7 @@ undo = lambda: os.rmdir(rootDir) rollback.prependDefer(undo) os.chown(rootDir, uid, gid) - os.chmod(rootDir, 0755) + os.chmod(rootDir, 0o755)
connections = {} for uuid, subDir in backends.iteritems(): @@ -508,7 +508,7 @@ undo = lambda path=path: shutil.rmtree(path, ignore_errors=True) rollback.prependDefer(undo) os.chown(path, uid, gid) - os.chmod(path, 0775) + os.chmod(path, 0o775) self.asserts.assertEquals(0, exportNFS(path)) undo = lambda path=path: self.asserts.assertEquals( 0, unexportNFS(path)) diff --git a/tests/functional/virtTests.py b/tests/functional/virtTests.py index 2dbeeae..cdb695a 100644 --- a/tests/functional/virtTests.py +++ b/tests/functional/virtTests.py @@ -84,7 +84,7 @@ fd, path = tempfile.mkstemp() cmd = [_mkinitrd.cmd, "-f", path, _kernelVer] rc, out, err = execCmd(cmd, sudo=False) - os.chmod(path, 0644) + os.chmod(path, 0o644) return path
diff --git a/tests/hooksTests.py b/tests/hooksTests.py index 04a22d3..1018a4e 100644 --- a/tests/hooksTests.py +++ b/tests/hooksTests.py @@ -46,7 +46,7 @@ scripts.sort(key=lambda f: f.name) for n, script in enumerate(scripts): script.write(code % n) - os.chmod(os.path.join(dirName, script.name), 0775) + os.chmod(os.path.join(dirName, script.name), 0o775) script.close() return dirName, scripts
@@ -90,7 +90,7 @@ """ script.write(code) script.close() - os.chmod(script.name, 0775) + os.chmod(script.name, 0o775) return script.name, '683394fc34f6830dd1882418eefd9b66'
def test_getScriptInfo(self): @@ -103,7 +103,7 @@ dir = tempfile.mkdtemp() sName, md5 = self.createScript(dir) NEscript = tempfile.NamedTemporaryFile(dir=dir) - os.chmod(NEscript.name, 0000) + os.chmod(NEscript.name, 0o000) info = hooks._getHookInfo(dir) os.unlink(sName) expectedRes = dict([(os.path.basename(sName), {'md5': md5})]) @@ -122,7 +122,7 @@ domXMLFile.write(customProperty) """ script.write(code) - os.chmod(script.name, 0775) + os.chmod(script.name, 0o775) script.close() return dirName
diff --git a/tests/miscTests.py b/tests/miscTests.py index b22ef64..c836e55 100644 --- a/tests/miscTests.py +++ b/tests/miscTests.py @@ -425,11 +425,11 @@ f.write(data) f.flush() f.close() - os.chmod(srcPath, 0666) + os.chmod(srcPath, 0o666)
#Get a tempfilename dstFd, dstPath = tempfile.mkstemp() - os.chmod(dstPath, 0666) + os.chmod(dstPath, 0o666)
#Copy rc, out, err = misc.ddWatchCopy(srcPath, dstPath, None, len(data)) @@ -528,11 +528,11 @@ f.write(data) f.flush() f.close() - os.chmod(srcPath, 0666) + os.chmod(srcPath, 0o666)
#Get a tempfilename dstFd, dstPath = tempfile.mkstemp() - os.chmod(dstPath, 0666) + os.chmod(dstPath, 0o666)
#Copy rc, out, err = misc.ddWatchCopy(srcPath, dstPath, None, len(data)) diff --git a/vds_bootstrap/setup b/vds_bootstrap/setup index d372f57..778dc12 100755 --- a/vds_bootstrap/setup +++ b/vds_bootstrap/setup @@ -394,7 +394,7 @@
if __name__ == "__main__": # make sure nobody can access our files - os.umask(0077) + os.umask(0o077)
# working directory is script location os.chdir(os.path.dirname(sys.argv[0])) diff --git a/vds_bootstrap/vds_bootstrap.py b/vds_bootstrap/vds_bootstrap.py index a237489..a9dc901 100755 --- a/vds_bootstrap/vds_bootstrap.py +++ b/vds_bootstrap/vds_bootstrap.py @@ -201,7 +201,7 @@
# Adding VDSM_DIR to the current python path try: - os.mkdir(VDSM_DIR, 0755) + os.mkdir(VDSM_DIR, 0o755) except OSError: pass sys.path.append(VDSM_DIR) @@ -712,7 +712,7 @@ f = os.fdopen(fd, 'w') f.writelines(lines) f.close() - os.chmod(tmpName, 0644) + os.chmod(tmpName, 0o644) shutil.move(tmpName, VDSM_CONF) else: self.message = 'Basic configuration found, skipping this step' diff --git a/vdsm/netconf/ifcfg.py b/vdsm/netconf/ifcfg.py index c3caafe..cf198ef 100644 --- a/vdsm/netconf/ifcfg.py +++ b/vdsm/netconf/ifcfg.py @@ -511,7 +511,7 @@ configuration)) with open(fileName, 'w') as confFile: confFile.write(configuration) - os.chmod(fileName, 0664) + os.chmod(fileName, 0o664) try: selinux.restorecon(fileName) except: diff --git a/vdsm/storage/blockVolume.py b/vdsm/storage/blockVolume.py index 923f93a..2f80f37 100644 --- a/vdsm/storage/blockVolume.py +++ b/vdsm/storage/blockVolume.py @@ -416,7 +416,7 @@ self.imgUUID) if not os.path.isdir(imageDir): try: - os.mkdir(imageDir, 0755) + os.mkdir(imageDir, 0o755) except Exception: self.log.error("Unexpected error", exc_info=True) raise se.ImagePathError(imageDir) diff --git a/vdsm/storage/fileSD.py b/vdsm/storage/fileSD.py index dcd43fb..f9ff93b 100644 --- a/vdsm/storage/fileSD.py +++ b/vdsm/storage/fileSD.py @@ -182,7 +182,7 @@ for metaFile in (sd.LEASES, sd.IDS, sd.INBOX, sd.OUTBOX): try: fpath = os.path.join(self.getMDPath(), metaFile) - procPool.os.chmod(fpath, 0660) + procPool.os.chmod(fpath, 0o660) except Exception as e: raise se.StorageDomainMetadataCreationError( "Lease permission change file '%s' failed: %s" @@ -198,12 +198,12 @@ metadataDir = os.path.join(domPath, sd.DOMAIN_META_DATA)
procPool = oop.getProcessPool(sdUUID) - procPool.fileUtils.createdir(metadataDir, 0775) + procPool.fileUtils.createdir(metadataDir, 0o775)
for metaFile in (sd.LEASES, sd.IDS, sd.INBOX, sd.OUTBOX): try: procPool.truncateFile( - os.path.join(metadataDir, metaFile), 0, 0660) + os.path.join(metadataDir, metaFile), 0, 0o660) except Exception as e: raise se.StorageDomainMetadataCreationError( "create meta file '%s' failed: %s" % (metaFile, str(e))) @@ -546,7 +546,7 @@ """ masterdir = os.path.join(self.domaindir, sd.MASTER_FS_DIR) if not self.oop.fileUtils.pathExists(masterdir): - self.oop.os.mkdir(masterdir, 0755) + self.oop.os.mkdir(masterdir, 0o755)
def unmountMaster(self): """ diff --git a/vdsm/storage/fileUtils.py b/vdsm/storage/fileUtils.py index 6090a07..80762d8 100644 --- a/vdsm/storage/fileUtils.py +++ b/vdsm/storage/fileUtils.py @@ -393,11 +393,11 @@
def copyUserModeToGroup(path): mode = os.stat(path).st_mode - userMode = mode & 0700 # user mode mask + userMode = mode & 0o700 # user mode mask newGroupMode = userMode >> 3 - if (mode & 0070) != newGroupMode: # group mode mask + if (mode & 0o070) != newGroupMode: # group mode mask # setting the new group mode masking out the original one - os.chmod(path, (mode & 0707) | newGroupMode) + os.chmod(path, (mode & 0o707) | newGroupMode)
def padToBlockSize(path): diff --git a/vdsm/storage/fileVolume.py b/vdsm/storage/fileVolume.py index 1ae90de..026a135 100644 --- a/vdsm/storage/fileVolume.py +++ b/vdsm/storage/fileVolume.py @@ -69,11 +69,11 @@ @staticmethod def file_setrw(volPath, rw): sdUUID = getDomUuidFromVolumePath(volPath) - mode = 0440 + mode = 0o440 if rw: - mode |= 0220 + mode |= 0o220 if oop.getProcessPool(sdUUID).os.path.isdir(volPath): - mode |= 0110 + mode |= 0o110 oop.getProcessPool(sdUUID).os.chmod(volPath, mode)
@classmethod diff --git a/vdsm/storage/storageServer.py b/vdsm/storage/storageServer.py index 023f607..786fba4 100644 --- a/vdsm/storage/storageServer.py +++ b/vdsm/storage/storageServer.py @@ -486,7 +486,7 @@
lnPath = self._getLocalPath() os.symlink(self._path, lnPath) - os.chmod(lnPath, 0775) + os.chmod(lnPath, 0o775)
def isConnected(self): return os.path.exists(self._getLocalPath()) diff --git a/vdsm/vdsm b/vdsm/vdsm index a637097..67cf094 100755 --- a/vdsm/vdsm +++ b/vdsm/vdsm @@ -97,7 +97,7 @@ if pidfile: with open(pidfile, 'w') as f: f.write(pid + "\n") - os.chmod(pidfile, 0664) + os.chmod(pidfile, 0o664)
sysname, nodename, release, version, machine = os.uname() log.info('(PID: %s) I am the actual vdsm %s %s (%s)', diff --git a/vdsm/vm.py b/vdsm/vm.py index 0f9a859..0c12334 100644 --- a/vdsm/vm.py +++ b/vdsm/vm.py @@ -3307,7 +3307,7 @@ qemuImg.create(transientPath, format=qemuImg.FORMAT.QCOW2, backing=diskParams['path'], backingFormat=driveFormat) - os.fchmod(transientHandle, 0660) + os.fchmod(transientHandle, 0o660) except: os.unlink(transientPath) # Closing after deletion is correct self.log.error("Failed to create the transient disk for " diff --git a/vdsm_reg/createDaemon.py b/vdsm_reg/createDaemon.py index 46af070..1278596 100644 --- a/vdsm_reg/createDaemon.py +++ b/vdsm_reg/createDaemon.py @@ -36,7 +36,7 @@ """ # Default daemon parameters. # File mode creation mask of the daemon. - UMASK = 007 + UMASK = 0o007
# Default working directory for the daemon. WORKDIR = "/" diff --git a/vdsm_reg/vdsm-reg-setup.in b/vdsm_reg/vdsm-reg-setup.in index 7141278..6489d8a 100644 --- a/vdsm_reg/vdsm-reg-setup.in +++ b/vdsm_reg/vdsm-reg-setup.in @@ -102,7 +102,7 @@ #Fix file permissions to relevant mask if fReturn: try: - os.chmod("/config/etc/sysconfig/network-scripts/ifcfg-" + MGT_BRIDGE_NAME, 0644) + os.chmod("/config/etc/sysconfig/network-scripts/ifcfg-" + MGT_BRIDGE_NAME, 0o644) except: fReturn = False logging.error("renameBridge: failed to chmod bridge file") @@ -217,7 +217,7 @@ if not os.path.exists(upgradeDir): try: os.makedirs(upgradeDir) - os.chmod(upgradeDir, 0755) + os.chmod(upgradeDir, 0o755) except OSError as err: if err.errno != errno.EEXIST: log.error( @@ -255,7 +255,7 @@
log.info("After daemonize - My pid is %d", os.getpid()) file(pidfile, 'w').write(str(os.getpid()) + "\n") - os.chmod(pidfile, 0664) + os.chmod(pidfile, 0o664)
itr = 0 while daemonize and not registered:
oVirt Jenkins CI Server has posted comments on this change.
Change subject: More explicit octal values ......................................................................
Patch Set 1: Verified-1
Build Failed
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5066/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4262/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5140/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_network_functional_tests/738/ : FAILURE
Dan Kenigsberg has posted comments on this change.
Change subject: More explicit octal values ......................................................................
Patch Set 1:
Please refer to PEP-3127 in your commit message, and verify with ZhengSheng's pydiff. Thanks for this initiative!
Antoni Segura Puimedon has posted comments on this change.
Change subject: More explicit octal values (pep-3127) ......................................................................
Patch Set 2: Verified+1
➜ vdsm git:(9370bb0) gitpydiff checking lib/vdsm/tool/transient.py checking tests/fileUtilTests.py checking tests/functional/storageTests.py checking tests/functional/virtTests.py checking tests/hooksTests.py checking tests/miscTests.py checking vds_bootstrap/vds_bootstrap.py checking vdsm/netconf/ifcfg.py checking vdsm/storage/blockVolume.py checking vdsm/storage/fileSD.py checking vdsm/storage/fileUtils.py checking vdsm/storage/fileVolume.py checking vdsm/storage/storageServer.py checking vdsm/vm.py checking vdsm_reg/createDaemon.py ➜ vdsm git:(9370bb0)
oVirt Jenkins CI Server has posted comments on this change.
Change subject: More explicit octal values (pep-3127) ......................................................................
Patch Set 2: Verified-1
Build Failed
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5073/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4269/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5147/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_network_functional_tests/742/ : FAILURE
Dan Kenigsberg has posted comments on this change.
Change subject: More explicit octal values (pep-3127) ......................................................................
Patch Set 2: Code-Review+2
Dan Kenigsberg has submitted this change and it was merged.
Change subject: More explicit octal values (pep-3127) ......................................................................
More explicit octal values (pep-3127)
Change 0XXX to 0oXXX so that that these values are more visibly octal and forwards compatible with Python 3.3+.
This change follows the spirit of: http://www.python.org/dev/peps/pep-3127/
"The default octal representation of integers is silently confusing to people unfamiliar with C-like languages. It is extremely easy to inadvertently create an integer object with the wrong value, because '013' means 'decimal 11', not 'decimal 13', to the Python language itself, which is not the meaning that most humans would assign to this literal."
Change-Id: Icc81c0ac52cc779511e49133aa669defeb47a3f1 Signed-off-by: Antoni S. Puimedon asegurap@redhat.com Reviewed-on: http://gerrit.ovirt.org/20534 Reviewed-by: Dan Kenigsberg danken@redhat.com --- M lib/vdsm/tool/transient.py M tests/fileUtilTests.py M tests/functional/storageTests.py M tests/functional/virtTests.py M tests/hooksTests.py M tests/miscTests.py M vds_bootstrap/setup M vds_bootstrap/vds_bootstrap.py M vdsm/netconf/ifcfg.py M vdsm/storage/blockVolume.py M vdsm/storage/fileSD.py M vdsm/storage/fileUtils.py M vdsm/storage/fileVolume.py M vdsm/storage/storageServer.py M vdsm/vdsm M vdsm/vm.py M vdsm_reg/createDaemon.py M vdsm_reg/vdsm-reg-setup.in 18 files changed, 38 insertions(+), 38 deletions(-)
Approvals: Antoni Segura Puimedon: Verified Dan Kenigsberg: Looks good to me, approved
vdsm-patches@lists.fedorahosted.org