Yaniv Bronhaim has posted comments on this change.

Change subject: Added lib/vdsm/utils.py:touchFile
......................................................................


Patch Set 1:

(2 comments)

....................................................
File lib/vdsm/utils.py
Line 102:     path = os.path.abspath(path)
Line 103:     return stat.S_ISBLK(os.stat(path).st_mode)
Line 104: 
Line 105: 
Line 106: def touchFile(filePath, ignoreExceptions=False):
I would add setPermissions for that. and maybe do that as supervdsm verb.. 
otherwise you open the file as the caller user with all its limits.

as root it can be - touchFile(filePath, user, group, perm, raiseErrors=False)

your patch that uses vdsm-tool for that, will touch the file as root.

anyhow, to make things short, this is good infra and I think we don't need more 
than that for now. but maybe later this might be a bit limited. just raising 
the issue..
Line 107:     """
Line 108:     Subset of POSIX touch functionality
Line 109:     :param filePath: The file to touch
Line 110:     :param ignoreExceptions: Should file handling exceptions be 
ignored?


Line 110:     :param ignoreExceptions: Should file handling exceptions be 
ignored?
Line 111:     """
Line 112:     try:
Line 113:         with open(filePath, 'a'):
Line 114:             os.utime(filePath, None)
please add a comment why you update utime here.
Line 115:     except (IOError, OSError) as e:
Line 116:         if not ignoreExceptions:
Line 117:             raise e
Line 118: 


-- 
To view, visit http://gerrit.ovirt.org/21590
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifbbea57c425adc8b911bb9e1931c02edb71db820
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Assaf Muller <[email protected]>
Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]>
Gerrit-Reviewer: Assaf Muller <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Saggi Mizrahi <[email protected]>
Gerrit-Reviewer: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to