Nir Soffer has posted comments on this change.

Change subject: oop: Use Storage.oop logger
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.ovirt.org/#/c/31908/1/vdsm/storage/outOfProcess.py
File vdsm/storage/outOfProcess.py:

Line 217:         '''
Line 218:         WARNING: Renaming a directory is not an atomic op.
Line 219:         Supported in the same manner as Python's os.rename.
Line 220:         '''
Line 221:         log.warning("renaming a directory is not an atomic operation")
At this point, this log message is just wrong - you don't have any clue that we 
are trying to rename a directory.
Line 222:         try:
Line 223:             return self._iop.rename(oldpath, newpath)
Line 224:         except OSError as e:
Line 225:             if e.errno != errno.ENOTEMPTY:


Line 223:             return self._iop.rename(oldpath, newpath)
Line 224:         except OSError as e:
Line 225:             if e.errno != errno.ENOTEMPTY:
Line 226:                 raise
Line 227: 
Now we know that we are trying to rename a non-empty directory. This is indeed 
not atomic, but it is the responsibility of the caller. This method should not 
log warnings about unsafe usage.

If you think this usage unsafe, forbid renaming directories, and force the 
caller to use renameDirectory(), which will be documented as unsafe.
Line 228:         _IOProcessFileUtils(self._iop).cleanupdir(newpath, False)
Line 229:         self.mkdir(newpath)
Line 230:         for fname in self.listdir(oldpath):
Line 231:             src = os.path.join(oldpath, fname)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id2d0474d1d69e227aa12b8b70022930f66e7387d
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Dima Kuznetsov <[email protected]>
Gerrit-Reviewer: Michal Skrivanek <[email protected]>
Gerrit-Reviewer: Nir Soffer <[email protected]>
Gerrit-Reviewer: Saggi Mizrahi <[email protected]>
Gerrit-Reviewer: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: Yeela Kaplan <[email protected]>
Gerrit-Reviewer: Yoav Kleinberger <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: mooli tayer <[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