Change in vdsm[master]: BZ#XXXXXX Use only os.access to check permissions

[Federico Simoncelli]

Federico Simoncelli has posted comments on this change.

Change subject: BZ#XXXXXX Use only os.access to check permissions
......................................................................


Patch Set 1: (3 inline comments)

....................................................
File vdsm/storage/fileUtils.py
Line 185: def pathExists(filename, writable=False):
I already checked that nobody was relaying on this typo.

Line 189:         os.stat(filename)
This should be enough to circumvent the nfs stale handle. If anyone is concerned with the
fact that we are always running os.stat, the other solution would be:

 if os.access(filename, check):
     return True

 try:
     os.stat(filename)
 except OSError:
     return False
 return os.access(filename, check)

I'm not sure if this is where we want to optimize. Consider also that when the first
os.access return False we're going to do more operations.

Line 197
Re-implementing os.access is not easy. First of all we should check against the correct
constants: S_IRUSR, S_IWUSR, S_IRGRP, S_IWGRP, etc.
The value R_OK just happened to work in some rare cases.
Second, os.access does much more, it checks the process uid and gid against the file
owner/group, checks if the mount is read-only and a bunch of other things.

--
To view, visit http://gerrit.usersys.redhat.com/733
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic7b892886416e866178ac88c04cbfb68ed0c055b
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;