From Dan Kenigsberg <dan...@redhat.com>: Dan Kenigsberg has posted comments on this change.
Change subject: storage: Added fast preallocation with fallocate() ...................................................................... Patch Set 2: (4 comments) https://gerrit.ovirt.org/#/c/69848/2/lib/vdsm/storage/misc.py File lib/vdsm/storage/misc.py: PS2, Line 200: Exception catching any exception here (even a SyntaxError) is not a good practice. Try to be more specific. If it is a must, you should log it which exception was swallowed for the benefit of whomever debugs it. PS2, Line 205: fallocate please include a unit test for the newly-define function. PS2, Line 216: open please use io.open() to be python3-proof. PS2, Line 216: fd strictly speaking, this is not a file descriptor, but a file object. so calling it plainly "f" might be better. -- To view, visit https://gerrit.ovirt.org/69848 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I690187a4c826f8a22d7181a336cd36bfbf347450 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Denis Chaplygin <dchap...@redhat.com> Gerrit-Reviewer: Adam Litke <ali...@redhat.com> Gerrit-Reviewer: Ala Hino <ah...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Denis Chaplygin <dchap...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Yaniv Bronhaim <ybron...@redhat.com> Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
