Ayal Baron has posted comments on this change.
Change subject: Fix race in fileUtils.createdir().
......................................................................
Patch Set 2: Looks good to me, but someone else must approve
(3 inline comments)
I don't
....................................................
File vdsm/storage/fileUtils.py
Line 172: Recursively create directory if doesn't exist
Line 173:
Line 174: If already exists check that permissions are as requested.
Line 175: """
Line 176: DIR_BIT = 040000
S_IFDIR is 16384, to normalize you need to oct it but then you'd get a string.
The point here is that this function accepts mode which is either int or octal
(but passed as int) like makedirs supports.
e.g. makedirs(path, 755) and makedirs(path, 040755) are equivalent
Line 177: params = (dirPath, mode) if mode is not None else (dirPath,)
Line 178: try:
Line 179: os.makedirs(*params)
Line 180: except OSError as e:
Line 182: raise
Line 183: else:
Line 184: log.warning("Dir %s already exists", dirPath)
Line 185: if mode is not None:
Line 186: statinfo = os.stat(dirPath)
why? this would require splitting the test into 2, first to check that it's a
directory and another check for the permissions, instead of the below which
checks both in 1 shot.
Line 187: if statinfo[stat.ST_MODE] % DIR_BIT - mode % DIR_BIT
!= 0:
Line 188: raise OSError(errno.EPERM,
Line 189: "Existing %s dir permissions %s are not as
requested %s" %
Line 190: (dirPath, oct(statinfo[stat.ST_MODE]), oct(mode)))
Line 183: else:
Line 184: log.warning("Dir %s already exists", dirPath)
Line 185: if mode is not None:
Line 186: statinfo = os.stat(dirPath)
Line 187: if statinfo[stat.ST_MODE] % DIR_BIT - mode % DIR_BIT
!= 0:
stat.S_IMODE won't help because the whole idea here is that mode can either be
octal or decimal.
Need to normalize 'mode'
Alternative to above is:
stat.ST_MODE - (mode | DIR_BIT) != 0
logical or of mode with 040000 normalizes it
Line 188: raise OSError(errno.EPERM,
Line 189: "Existing %s dir permissions %s are not as
requested %s" %
Line 190: (dirPath, oct(statinfo[stat.ST_MODE]), oct(mode)))
Line 191:
--
To view, visit http://gerrit.ovirt.org/8732
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iac856c219a08fa0303e5da1d04f4f6b8e17e07a3
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Eduardo <[email protected]>
Gerrit-Reviewer: Shu Ming <[email protected]>
Gerrit-Reviewer: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: Yeela Kaplan <[email protected]>
Gerrit-Reviewer: Zhou Zheng Sheng <[email protected]>
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
