Dan Kenigsberg has posted comments on this change.

Change subject: Added utility to ensure that files written to the file system 
happen atomically.
......................................................................


Patch Set 1: I would prefer that you didn't submit this

(3 inline comments)

the configNet use case is a bit complex due to ovirt-node's use of bind-mounted 
files; and we do backups already. So I'm not quick with taking this patch - 
let's be sure we've taken care of all scenarios.

....................................................
File vdsm/utils.py
Line 831:     def __unicode__(self):
Line 832:         return unicode(self.cmd)
Line 833: 
Line 834: @contextmanager
Line 835: def atomicWrite(file):
"file" is a built-in class. let's choose another name.
Line 836:     '''Context manager that makes the write happen to a temporary 
file and if
Line 837:     and only it is successful, overwrites the original file. It 
creates the
Line 838:     temporary file in the same directory of the destination file, as 
rename is
Line 839:     only atomic within the same filesystem.'''


Line 836:     '''Context manager that makes the write happen to a temporary 
file and if
Line 837:     and only it is successful, overwrites the original file. It 
creates the
Line 838:     temporary file in the same directory of the destination file, as 
rename is
Line 839:     only atomic within the same filesystem.'''
Line 840:     tempFile = file+'vdsm_temp'
this means that two threads cannot take "atomicWrite" concurrently. better add 
randomness to the name.
Line 841:     f = open(tempFile, 'w')
Line 842:     yield f
Line 843:     f.flush()
Line 844:     os.fsync(f.fileno())


Line 842:     yield f
Line 843:     f.flush()
Line 844:     os.fsync(f.fileno())
Line 845:     f.close()
Line 846:     os.rename(tempFile, file)
I think there were issues with renaming of bind-mounted files on ovirt-node.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibecd61d6746231a5a8cb17bad9a3302b01454f27
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Antoni Segura Puimedon <[email protected]>
Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Igor Lvovsky <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to