Change in vdsm[master]: Fix race in fileUtils.createdir().

zhshzhou at linux.vnet.ibm.com zhshzhou at linux.vnet.ibm.com
Wed Oct 24 06:56:04 UTC 2012


Zhou Zheng Sheng has posted comments on this change.

Change subject: Fix race in fileUtils.createdir().
......................................................................


Patch Set 2: (2 inline comments)

....................................................
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
Python 2.7.3 (default, Jul 24 2012, 10:05:38) 
[GCC 4.7.0 20120507 (Red Hat 4.7.0-5)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> print 040000
16384

I don't see any difference between the actual value of 040000 and stat.S_IFDIR. You can consider stat.S_IFDIR a symbol constant.

I can not agree to os.makedirs(path, 755) and os.makedirs(path, 040755) are equivalent. Do you mean os.makedirs(path, 0755) and os.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 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:
How can this function distinguish octal and decimal values? In computer memory, octal and decimal values share the same presentation. 755 or 0755 are just different presentations to human, when running, 0755 equals to decimal value 493, why we have to normalize them? So when the function sees decimal value 755, how can it convert 755 to 493 to set the permission?

Am I missing something?
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 <ewarszaw at redhat.com>
Gerrit-Reviewer: Adam Litke <agl at us.ibm.com>
Gerrit-Reviewer: Ayal Baron <abaron at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw at redhat.com>
Gerrit-Reviewer: Shu Ming <shuming at linux.vnet.ibm.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan at redhat.com>
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou at linux.vnet.ibm.com>


More information about the vdsm-patches mailing list