)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":8543,"name":"Lucian Petrut","email":"lpetrut@cloudbasesolutions.com","username":"plucian"},"change_message_id":"f6e675f4bf75ca4534aabe90c4e93757c881ce10","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Force images to disk to avoid corruption"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Sometimes images downladed from glance that are then converted to raw"},{"line_number":10,"context_line":"format end up corrupt.  This is becasue qemu-img uses fiemap read blocks from the source image butis not using FIEMAP_FLAG_SYNC, so blocks and extents that are in the kernel page_cache but not on disk are skipped."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"This results in corrupt images."},{"line_number":13,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"da9df570_8a5ddf17","line":10,"updated":"2014-09-25 10:44:27.000000000","message":"Please fix up this line.","commit_id":"cfaa1555e485ae2c468fe91d1611853860c410dc"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"31c9c3d8b8dc80029f4f599c033140113b0318d4","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Sometimes images downladed from glance that are then converted to raw"},{"line_number":10,"context_line":"format end up corrupt.  This is becasue qemu-img uses fiemap read blocks"},{"line_number":11,"context_line":"from the source image butis not using FIEMAP_FLAG_SYNC, so blocks and"},{"line_number":12,"context_line":"extents that are in the kernel page_cache but not on disk are skipped."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"This results in corrupt images."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"da9df570_2c2078ab","line":11,"updated":"2014-09-26 03:20:18.000000000","message":"s/butis/but is/ :)","commit_id":"7fdf94c0fc9a124b68ccdb3810556f5474561230"},{"author":{"_account_id":12898,"name":"Tony Breeds","email":"tony@bakeyournoodle.com","username":"tonyb"},"change_message_id":"40d8a670975157005a26b03bc9900f57b9fd1847","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Sometimes images downladed from glance that are then converted to raw"},{"line_number":10,"context_line":"format end up corrupt.  This is becasue qemu-img uses fiemap read blocks"},{"line_number":11,"context_line":"from the source image butis not using FIEMAP_FLAG_SYNC, so blocks and"},{"line_number":12,"context_line":"extents that are in the kernel page_cache but not on disk are skipped."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"This results in corrupt images."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"da9df570_ac5b7ff4","line":11,"in_reply_to":"da9df570_2c2078ab","updated":"2014-09-26 11:27:58.000000000","message":"Done","commit_id":"7fdf94c0fc9a124b68ccdb3810556f5474561230"},{"author":{"_account_id":7069,"name":"Joshua Hesketh","email":"josh@nitrotech.org","username":"jhesketh"},"change_message_id":"98a06b7e926f9d34d87e16453cafcfb25580ae25","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Force images to disk to avoid corruption"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Sometimes images downladed from glance that are then converted to raw"},{"line_number":10,"context_line":"format end up corrupt.  This is becasue qemu-img uses fiemap read blocks"},{"line_number":11,"context_line":"from the source image but is not using FIEMAP_FLAG_SYNC, so blocks and"},{"line_number":12,"context_line":"extents that are in the kernel page_cache but not on disk are skipped."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"baa201ad_ec0c2f97","line":9,"updated":"2014-09-30 02:44:29.000000000","message":"s/downladed/downloaded/","commit_id":"d126ad55c752e653ee4b4c6cc0d9d64fb1c5d867"},{"author":{"_account_id":12898,"name":"Tony Breeds","email":"tony@bakeyournoodle.com","username":"tonyb"},"change_message_id":"951b64504e88a8ab08a20c0276bedc59eb6943e3","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Force images to disk to avoid corruption"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Sometimes images downladed from glance that are then converted to raw"},{"line_number":10,"context_line":"format end up corrupt.  This is becasue qemu-img uses fiemap read blocks"},{"line_number":11,"context_line":"from the source image but is not using FIEMAP_FLAG_SYNC, so blocks and"},{"line_number":12,"context_line":"extents that are in the kernel page_cache but not on disk are skipped."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"baa201ad_a5de660f","line":9,"in_reply_to":"baa201ad_ec0c2f97","updated":"2014-10-07 00:41:16.000000000","message":"Done","commit_id":"d126ad55c752e653ee4b4c6cc0d9d64fb1c5d867"},{"author":{"_account_id":2271,"name":"Michael Still","email":"mikal@stillhq.com","username":"mikalstill"},"change_message_id":"93e742bf13c8a3421fd7a2e03d37dc643b26e6b9","unresolved":false,"context_lines":[{"line_number":17,"context_line":"issue."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"A fix has been posted to qemu, but we\u0027ll need this fsync() until we\u0027re"},{"line_number":20,"context_line":"convinced the distros we care about have that fix."},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Change-Id: Ieb33489808a71c1fb40cd6835f0bb753dd69e293"},{"line_number":23,"context_line":"Closes-Bug: #1350766"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"baa201ad_c803be0d","line":20,"updated":"2014-09-30 21:39:22.000000000","message":"Can we get a link to that fix here so that we can work out when its landed etc?","commit_id":"d126ad55c752e653ee4b4c6cc0d9d64fb1c5d867"},{"author":{"_account_id":12898,"name":"Tony Breeds","email":"tony@bakeyournoodle.com","username":"tonyb"},"change_message_id":"951b64504e88a8ab08a20c0276bedc59eb6943e3","unresolved":false,"context_lines":[{"line_number":17,"context_line":"issue."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"A fix has been posted to qemu, but we\u0027ll need this fsync() until we\u0027re"},{"line_number":20,"context_line":"convinced the distros we care about have that fix."},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Change-Id: Ieb33489808a71c1fb40cd6835f0bb753dd69e293"},{"line_number":23,"context_line":"Closes-Bug: #1350766"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"baa201ad_05ec92c6","line":20,"in_reply_to":"baa201ad_c803be0d","updated":"2014-10-07 00:41:16.000000000","message":"Done","commit_id":"d126ad55c752e653ee4b4c6cc0d9d64fb1c5d867"},{"author":{"_account_id":2271,"name":"Michael Still","email":"mikal@stillhq.com","username":"mikalstill"},"change_message_id":"93e742bf13c8a3421fd7a2e03d37dc643b26e6b9","unresolved":false,"context_lines":[{"line_number":20,"context_line":"convinced the distros we care about have that fix."},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Change-Id: Ieb33489808a71c1fb40cd6835f0bb753dd69e293"},{"line_number":23,"context_line":"Closes-Bug: #1350766"},{"line_number":24,"context_line":"Co-Authored-By: Davanum Srinivas \u003cdims@linux.vnet.ibm.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"baa201ad_081ea670","line":23,"updated":"2014-09-30 21:39:22.000000000","message":"Can we change this to bug 1368815 please? The original bug has been marked as a duplicate of that one.","commit_id":"d126ad55c752e653ee4b4c6cc0d9d64fb1c5d867"},{"author":{"_account_id":12898,"name":"Tony Breeds","email":"tony@bakeyournoodle.com","username":"tonyb"},"change_message_id":"951b64504e88a8ab08a20c0276bedc59eb6943e3","unresolved":false,"context_lines":[{"line_number":20,"context_line":"convinced the distros we care about have that fix."},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Change-Id: Ieb33489808a71c1fb40cd6835f0bb753dd69e293"},{"line_number":23,"context_line":"Closes-Bug: #1350766"},{"line_number":24,"context_line":"Co-Authored-By: Davanum Srinivas \u003cdims@linux.vnet.ibm.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"baa201ad_85c0822b","line":23,"in_reply_to":"baa201ad_081ea670","updated":"2014-10-07 00:41:16.000000000","message":"Done","commit_id":"d126ad55c752e653ee4b4c6cc0d9d64fb1c5d867"}],"nova/image/glance.py":[{"author":{"_account_id":8543,"name":"Lucian Petrut","email":"lpetrut@cloudbasesolutions.com","username":"plucian"},"change_message_id":"f6e675f4bf75ca4534aabe90c4e93757c881ce10","unresolved":false,"context_lines":[{"line_number":18,"context_line":"from __future__ import absolute_import"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"import copy"},{"line_number":21,"context_line":"import fcntl"},{"line_number":22,"context_line":"import itertools"},{"line_number":23,"context_line":"import os"},{"line_number":24,"context_line":"import random"}],"source_content_type":"text/x-python","patch_set":1,"id":"da9df570_2a3cf3c7","line":21,"updated":"2014-09-25 10:44:27.000000000","message":"As you know, fcntl is not available on Windows so this would break the Hyper-V driver. I would suggest checking the os.name or sys.platform, skipping the import of fcntl on Windows.","commit_id":"cfaa1555e485ae2c468fe91d1611853860c410dc"},{"author":{"_account_id":8543,"name":"Lucian Petrut","email":"lpetrut@cloudbasesolutions.com","username":"plucian"},"change_message_id":"f6e675f4bf75ca4534aabe90c4e93757c881ce10","unresolved":false,"context_lines":[{"line_number":341,"context_line":""},{"line_number":342,"context_line":"        :param fd: file descriptor"},{"line_number":343,"context_line":"        \"\"\""},{"line_number":344,"context_line":"        if hasattr(fcntl, \u0027F_FULLSYNC\u0027):"},{"line_number":345,"context_line":"            try:"},{"line_number":346,"context_line":"                fcntl.fcntl(fd, fcntl.F_FULLSYNC)"},{"line_number":347,"context_line":"            except IOError as e:"}],"source_content_type":"text/x-python","patch_set":1,"id":"da9df570_ca7b678a","line":344,"updated":"2014-09-25 10:44:27.000000000","message":"Same here. I\u0027m not sure but I think os.fsync may be enough in all cases.","commit_id":"cfaa1555e485ae2c468fe91d1611853860c410dc"},{"author":{"_account_id":12898,"name":"Tony Breeds","email":"tony@bakeyournoodle.com","username":"tonyb"},"change_message_id":"7131c7c4d71e3b8ce996f185189c1b52757d2ff5","unresolved":false,"context_lines":[{"line_number":341,"context_line":""},{"line_number":342,"context_line":"        :param fd: file descriptor"},{"line_number":343,"context_line":"        \"\"\""},{"line_number":344,"context_line":"        if hasattr(fcntl, \u0027F_FULLSYNC\u0027):"},{"line_number":345,"context_line":"            try:"},{"line_number":346,"context_line":"                fcntl.fcntl(fd, fcntl.F_FULLSYNC)"},{"line_number":347,"context_line":"            except IOError as e:"}],"source_content_type":"text/x-python","patch_set":1,"id":"da9df570_5b5738ca","line":344,"in_reply_to":"da9df570_30d38605","updated":"2014-09-26 00:42:04.000000000","message":"If os.fsync() is adequate I\u0027ll use that.","commit_id":"cfaa1555e485ae2c468fe91d1611853860c410dc"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"03aca6508976046ac08db2f88856b1c9f371022f","unresolved":false,"context_lines":[{"line_number":341,"context_line":""},{"line_number":342,"context_line":"        :param fd: file descriptor"},{"line_number":343,"context_line":"        \"\"\""},{"line_number":344,"context_line":"        if hasattr(fcntl, \u0027F_FULLSYNC\u0027):"},{"line_number":345,"context_line":"            try:"},{"line_number":346,"context_line":"                fcntl.fcntl(fd, fcntl.F_FULLSYNC)"},{"line_number":347,"context_line":"            except IOError as e:"}],"source_content_type":"text/x-python","patch_set":1,"id":"da9df570_30d38605","line":344,"in_reply_to":"da9df570_ca7b678a","updated":"2014-09-25 12:21:07.000000000","message":"+1\n\nIt seems so... if it\u0027s not we might want to check the platform and do the appropriated call to each one (fcntl for linux and win32api for windows)\n\n(we should do the same when importing it, import win32api in case of windows or fcntl)","commit_id":"cfaa1555e485ae2c468fe91d1611853860c410dc"},{"author":{"_account_id":12898,"name":"Tony Breeds","email":"tony@bakeyournoodle.com","username":"tonyb"},"change_message_id":"7131c7c4d71e3b8ce996f185189c1b52757d2ff5","unresolved":false,"context_lines":[{"line_number":341,"context_line":""},{"line_number":342,"context_line":"        :param fd: file descriptor"},{"line_number":343,"context_line":"        \"\"\""},{"line_number":344,"context_line":"        if hasattr(fcntl, \u0027F_FULLSYNC\u0027):"},{"line_number":345,"context_line":"            try:"},{"line_number":346,"context_line":"                fcntl.fcntl(fd, fcntl.F_FULLSYNC)"},{"line_number":347,"context_line":"            except IOError as e:"}],"source_content_type":"text/x-python","patch_set":1,"id":"da9df570_fb50ecd5","line":344,"in_reply_to":"da9df570_ca7b678a","updated":"2014-09-26 00:42:04.000000000","message":"Done","commit_id":"cfaa1555e485ae2c468fe91d1611853860c410dc"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"03aca6508976046ac08db2f88856b1c9f371022f","unresolved":false,"context_lines":[{"line_number":386,"context_line":"                    data.write(chunk)"},{"line_number":387,"context_line":"            finally:"},{"line_number":388,"context_line":"                if close_file:"},{"line_number":389,"context_line":"                    # NOTE(tonyb): this fsync() is to work around an issue in"},{"line_number":390,"context_line":"                    #              qemu where fiemap is used to read blocks"},{"line_number":391,"context_line":"                    #              from the source image BUT is not using"},{"line_number":392,"context_line":"                    #              FIEMAP_FLAG_SYNC.  This results in corrupt"}],"source_content_type":"text/x-python","patch_set":1,"id":"da9df570_f033be5f","line":389,"updated":"2014-09-25 12:21:07.000000000","message":"it sounds like more a TODO telling that we should remove this piece of code later","commit_id":"cfaa1555e485ae2c468fe91d1611853860c410dc"},{"author":{"_account_id":12898,"name":"Tony Breeds","email":"tony@bakeyournoodle.com","username":"tonyb"},"change_message_id":"7131c7c4d71e3b8ce996f185189c1b52757d2ff5","unresolved":false,"context_lines":[{"line_number":386,"context_line":"                    data.write(chunk)"},{"line_number":387,"context_line":"            finally:"},{"line_number":388,"context_line":"                if close_file:"},{"line_number":389,"context_line":"                    # NOTE(tonyb): this fsync() is to work around an issue in"},{"line_number":390,"context_line":"                    #              qemu where fiemap is used to read blocks"},{"line_number":391,"context_line":"                    #              from the source image BUT is not using"},{"line_number":392,"context_line":"                    #              FIEMAP_FLAG_SYNC.  This results in corrupt"}],"source_content_type":"text/x-python","patch_set":1,"id":"da9df570_7b5cfced","line":389,"in_reply_to":"da9df570_f033be5f","updated":"2014-09-26 00:42:04.000000000","message":"Done","commit_id":"cfaa1555e485ae2c468fe91d1611853860c410dc"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"31c9c3d8b8dc80029f4f599c033140113b0318d4","unresolved":false,"context_lines":[{"line_number":340,"context_line":""},{"line_number":341,"context_line":"        :param fd: file descriptor"},{"line_number":342,"context_line":"        \"\"\""},{"line_number":343,"context_line":"        os.fsync(fd)"},{"line_number":344,"context_line":""},{"line_number":345,"context_line":"    def download(self, context, image_id, data\u003dNone, dst_path\u003dNone):"},{"line_number":346,"context_line":"        \"\"\"Calls out to Glance for data and writes data.\"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"da9df570_4c113c5c","line":343,"updated":"2014-09-26 03:20:18.000000000","message":"There is no reason to have this helper method. Just call os.fsync() directly. You can mock out os.fsync directly in the unit tests.","commit_id":"7fdf94c0fc9a124b68ccdb3810556f5474561230"},{"author":{"_account_id":12898,"name":"Tony Breeds","email":"tony@bakeyournoodle.com","username":"tonyb"},"change_message_id":"40d8a670975157005a26b03bc9900f57b9fd1847","unresolved":false,"context_lines":[{"line_number":340,"context_line":""},{"line_number":341,"context_line":"        :param fd: file descriptor"},{"line_number":342,"context_line":"        \"\"\""},{"line_number":343,"context_line":"        os.fsync(fd)"},{"line_number":344,"context_line":""},{"line_number":345,"context_line":"    def download(self, context, image_id, data\u003dNone, dst_path\u003dNone):"},{"line_number":346,"context_line":"        \"\"\"Calls out to Glance for data and writes data.\"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"da9df570_ec702774","line":343,"in_reply_to":"da9df570_4c113c5c","updated":"2014-09-26 11:27:58.000000000","message":"Done","commit_id":"7fdf94c0fc9a124b68ccdb3810556f5474561230"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"2c6f97265a795413dc608d1985ba6ad9404987b8","unresolved":false,"context_lines":[{"line_number":380,"context_line":"                    #              we\u0027ll need this fsync() until we\u0027re"},{"line_number":381,"context_line":"                    #              convinced the distros we care about have"},{"line_number":382,"context_line":"                    #              that fix."},{"line_number":383,"context_line":"                    os.fsync(data)"},{"line_number":384,"context_line":"                    data.close()"},{"line_number":385,"context_line":""},{"line_number":386,"context_line":"    def create(self, context, image_meta, data\u003dNone):"}],"source_content_type":"text/x-python","patch_set":3,"id":"baa201ad_7b770f03","line":383,"updated":"2014-09-30 08:48:38.000000000","message":"Should we call f.flush() before calling fsync?\n\nhttps://docs.python.org/2/library/os.html#os.fsync","commit_id":"d126ad55c752e653ee4b4c6cc0d9d64fb1c5d867"},{"author":{"_account_id":12898,"name":"Tony Breeds","email":"tony@bakeyournoodle.com","username":"tonyb"},"change_message_id":"951b64504e88a8ab08a20c0276bedc59eb6943e3","unresolved":false,"context_lines":[{"line_number":380,"context_line":"                    #              we\u0027ll need this fsync() until we\u0027re"},{"line_number":381,"context_line":"                    #              convinced the distros we care about have"},{"line_number":382,"context_line":"                    #              that fix."},{"line_number":383,"context_line":"                    os.fsync(data)"},{"line_number":384,"context_line":"                    data.close()"},{"line_number":385,"context_line":""},{"line_number":386,"context_line":"    def create(self, context, image_meta, data\u003dNone):"}],"source_content_type":"text/x-python","patch_set":3,"id":"baa201ad_a2653cb6","line":383,"in_reply_to":"baa201ad_7b770f03","updated":"2014-10-07 00:41:16.000000000","message":"Done. I spent a long time trying to determine if this would actually change anything.\n\nIn the end I decided if it\u0027s the documented procedure I\u0027d better just use it rather than rely on behavior that may (albeit unlikely) change in the future","commit_id":"d126ad55c752e653ee4b4c6cc0d9d64fb1c5d867"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"b87309880a30fc52cfa25499e5ba593c62ba7302","unresolved":false,"context_lines":[{"line_number":380,"context_line":"                    #              we\u0027ll need this fsync() until we\u0027re"},{"line_number":381,"context_line":"                    #              convinced the distros we care about have"},{"line_number":382,"context_line":"                    #              that fix."},{"line_number":383,"context_line":"                    os.fsync(data)"},{"line_number":384,"context_line":"                    data.close()"},{"line_number":385,"context_line":""},{"line_number":386,"context_line":"    def create(self, context, image_meta, data\u003dNone):"}],"source_content_type":"text/x-python","patch_set":3,"id":"baa201ad_de668d29","line":383,"in_reply_to":"baa201ad_a2653cb6","updated":"2014-10-07 13:44:35.000000000","message":"thank you for your time","commit_id":"d126ad55c752e653ee4b4c6cc0d9d64fb1c5d867"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"d73633eeb5d5a308640005c231ae89ee315906bc","unresolved":false,"context_lines":[{"line_number":372,"context_line":"                    #              images.  A fix has been posted to qemu, but"},{"line_number":373,"context_line":"                    #              we\u0027ll need this fsync() until we\u0027re"},{"line_number":374,"context_line":"                    #              convinced the distros we care about have"},{"line_number":375,"context_line":"                    #              that fix."},{"line_number":376,"context_line":"                    data.flush()"},{"line_number":377,"context_line":"                    os.fsync(data.fileno())"},{"line_number":378,"context_line":"                    data.close()"}],"source_content_type":"text/x-python","patch_set":7,"id":"5a890539_414bc686","line":375,"updated":"2014-11-24 09:11:48.000000000","message":"Well, I assume it\u0027s maybe nitpicking, but I would appreciate if we could keep track of the qemu upstream bug in the comment.\n\nAlso, I think it would be better to leave a FIXME comment instead of TODO.\nThis conversation sounds quite relevant to this http://lists.openstack.org/pipermail/openstack-dev/2013-March/006229.html\n\n\nHere, the problem is that we need to make sure we can safely remove that fsync() call once the bugfix is shipped correctly on what we consider \u0027distros\u0027","commit_id":"33c9720ec798123470d5cf67a7d2e1bb84f5e0e5"},{"author":{"_account_id":12898,"name":"Tony Breeds","email":"tony@bakeyournoodle.com","username":"tonyb"},"change_message_id":"5a8706daf700bfdf2c89f046b95f33e8cdbea738","unresolved":false,"context_lines":[{"line_number":372,"context_line":"                    #              images.  A fix has been posted to qemu, but"},{"line_number":373,"context_line":"                    #              we\u0027ll need this fsync() until we\u0027re"},{"line_number":374,"context_line":"                    #              convinced the distros we care about have"},{"line_number":375,"context_line":"                    #              that fix."},{"line_number":376,"context_line":"                    data.flush()"},{"line_number":377,"context_line":"                    os.fsync(data.fileno())"},{"line_number":378,"context_line":"                    data.close()"}],"source_content_type":"text/x-python","patch_set":7,"id":"5a890539_a40e388a","line":375,"in_reply_to":"5a890539_414bc686","updated":"2014-11-24 10:01:22.000000000","message":"Done","commit_id":"33c9720ec798123470d5cf67a7d2e1bb84f5e0e5"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1e6afa6235e99b75fab2d967fe81a4a6b828df93","unresolved":false,"context_lines":[{"line_number":73,"context_line":"# NOTE(tonyb): This fsync() is to work around an issue in qemu where fiemap"},{"line_number":74,"context_line":"#              is used to read blocks from the source image BUT is not using"},{"line_number":75,"context_line":"#              FIEMAP_FLAG_SYNC.  This results in corrupt images.  This is"},{"line_number":76,"context_line":"#              fixed upstream we need this until we\u0027re $distros are fixed."},{"line_number":77,"context_line":"#              See: https://wiki.openstack.org/wiki/Bug1368815 for tracking"},{"line_number":78,"context_line":"#              info and more details."},{"line_number":79,"context_line":"    cfg.BoolOpt(\u0027qemu_force_sync\u0027,"}],"source_content_type":"text/x-python","patch_set":10,"id":"5a890539_d5d9be85","line":76,"updated":"2014-12-05 15:13:15.000000000","message":"nit: this sentence is hard to read, I think it means:\n\n\"This is fixed in upstream qemu but we need this workaround until the various supporting distros are carrying the fix.\"","commit_id":"76900bd8a70b6b4c833ac0ce6643e025ee091024"},{"author":{"_account_id":12898,"name":"Tony Breeds","email":"tony@bakeyournoodle.com","username":"tonyb"},"change_message_id":"e1aa5d29fdc72ada421e0098bb3c456bb1dcdd25","unresolved":false,"context_lines":[{"line_number":73,"context_line":"# NOTE(tonyb): This fsync() is to work around an issue in qemu where fiemap"},{"line_number":74,"context_line":"#              is used to read blocks from the source image BUT is not using"},{"line_number":75,"context_line":"#              FIEMAP_FLAG_SYNC.  This results in corrupt images.  This is"},{"line_number":76,"context_line":"#              fixed upstream we need this until we\u0027re $distros are fixed."},{"line_number":77,"context_line":"#              See: https://wiki.openstack.org/wiki/Bug1368815 for tracking"},{"line_number":78,"context_line":"#              info and more details."},{"line_number":79,"context_line":"    cfg.BoolOpt(\u0027qemu_force_sync\u0027,"}],"source_content_type":"text/x-python","patch_set":10,"id":"5a890539_3492fa58","line":76,"in_reply_to":"5a890539_d5d9be85","updated":"2014-12-05 19:42:45.000000000","message":"okay, that happened after edit/re-edit and then reading what I thought was there rather than what was actually there.\n\nI\u0027ll clean it up.","commit_id":"76900bd8a70b6b4c833ac0ce6643e025ee091024"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"d81d504ca6b292c66f00e6e638c1551ba608f827","unresolved":false,"context_lines":[{"line_number":81,"context_line":"                help\u003d\u0027qemu-img convert -O raw is buggy in qemu \u003c2.2.0.  This \u0027"},{"line_number":82,"context_line":"                      \u0027workaround forces either a sync() or fsync() as \u0027"},{"line_number":83,"context_line":"                      \u0027required\u0027),"},{"line_number":84,"context_line":"    ]"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":87,"context_line":"CONF \u003d cfg.CONF"}],"source_content_type":"text/x-python","patch_set":10,"id":"5a890539_6baad91b","line":84,"updated":"2014-12-05 23:04:45.000000000","message":"I kinda think this belongs somewhere other than the glance code, since it\u0027s kinda buried. Maybe not a big deal, but assuming the next workaround we have is not in glance, the author of that fix will just have to move this, right? Not sure where I\u0027d recommend putting it, but having it here seems awkward.","commit_id":"76900bd8a70b6b4c833ac0ce6643e025ee091024"},{"author":{"_account_id":12898,"name":"Tony Breeds","email":"tony@bakeyournoodle.com","username":"tonyb"},"change_message_id":"e3503ca60c8376f7f89747ca6063ff6b87dbbd1b","unresolved":false,"context_lines":[{"line_number":81,"context_line":"                help\u003d\u0027qemu-img convert -O raw is buggy in qemu \u003c2.2.0.  This \u0027"},{"line_number":82,"context_line":"                      \u0027workaround forces either a sync() or fsync() as \u0027"},{"line_number":83,"context_line":"                      \u0027required\u0027),"},{"line_number":84,"context_line":"    ]"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":87,"context_line":"CONF \u003d cfg.CONF"}],"source_content_type":"text/x-python","patch_set":10,"id":"5a890539_b097cdea","line":84,"in_reply_to":"5a890539_6baad91b","updated":"2014-12-06 04:14:18.000000000","message":"I have to admit I just assumed that the next user of this specific work around would be able to just CONF.import_group(\u0027workarounds\u0027, ...) and test the flag.\n\nBefore I submit my next PS I\u0027ll test that works and if not workout a more obvious place to define the group.  I\u0027ll also check that defining additional workarounds can be done anywhere else in the code.\n\nI just assumed that both of these things would just work.","commit_id":"76900bd8a70b6b4c833ac0ce6643e025ee091024"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"d29fc38a58a01e5a859657a1750c4bd6f483ee93","unresolved":false,"context_lines":[{"line_number":382,"context_line":"            finally:"},{"line_number":383,"context_line":"                if CONF.workarounds.qemu_force_sync:"},{"line_number":384,"context_line":"                    data.flush()"},{"line_number":385,"context_line":"                    os.fsync(data.fileno())"},{"line_number":386,"context_line":"                if close_file:"},{"line_number":387,"context_line":"                    data.close()"},{"line_number":388,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"3a961159_2d8626ba","line":385,"updated":"2014-12-08 20:16:29.000000000","message":"Tony, after looking at this patch again, it dawned on me that it would be more appropriate to call os.fsync() on the file that was downloaded from Glance at the place where the raw conversion is happening, and not every time any image is download (as is the case in the code above).\n\nWhat about adding code that calls os.fsync() on the downloaded image file here instead?\n\nhttps://github.com/openstack/nova/blob/master/nova/virt/images.py#L114","commit_id":"adda254d2f0e3b2b52824513b07bbcce6dc5b63b"},{"author":{"_account_id":12898,"name":"Tony Breeds","email":"tony@bakeyournoodle.com","username":"tonyb"},"change_message_id":"bcfafda26584a79188b8699adeaaec4554719202","unresolved":false,"context_lines":[{"line_number":382,"context_line":"            finally:"},{"line_number":383,"context_line":"                if CONF.workarounds.qemu_force_sync:"},{"line_number":384,"context_line":"                    data.flush()"},{"line_number":385,"context_line":"                    os.fsync(data.fileno())"},{"line_number":386,"context_line":"                if close_file:"},{"line_number":387,"context_line":"                    data.close()"},{"line_number":388,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"3a961159_297d2c6f","line":385,"in_reply_to":"3a961159_28bc4dbb","updated":"2014-12-09 09:54:02.000000000","message":"Okay so after reading some fs code and talking to people that know a lot more then me it seems that the second syscall pattern will make the kernel behave differently but I over estimated the probability of that being a bad thing.","commit_id":"adda254d2f0e3b2b52824513b07bbcce6dc5b63b"},{"author":{"_account_id":12898,"name":"Tony Breeds","email":"tony@bakeyournoodle.com","username":"tonyb"},"change_message_id":"ad0e8452ad76146876c46f9ccdfeb82c0966fd48","unresolved":false,"context_lines":[{"line_number":382,"context_line":"            finally:"},{"line_number":383,"context_line":"                if CONF.workarounds.qemu_force_sync:"},{"line_number":384,"context_line":"                    data.flush()"},{"line_number":385,"context_line":"                    os.fsync(data.fileno())"},{"line_number":386,"context_line":"                if close_file:"},{"line_number":387,"context_line":"                    data.close()"},{"line_number":388,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"3a961159_28bc4dbb","line":385,"in_reply_to":"3a961159_2d8626ba","updated":"2014-12-09 00:27:29.000000000","message":"At the point you suggest we no longer have an open file descriptor for the data we just grabbed from glance.\nThat means we either need to sync() (which is a whole FS/ page_cache operation)\nOR do an open/flush/fsync/close before calling qemu-img\nIf we do the fsync() outside the glance code we\u0027d be changing the important system calls from:\n\n fd\u003dopen(path); \n write(fd,...) x lots;\n fsync(fd);\n close(fd)\nto:\n fd\u003dopen(path); \n write(fd,...) x lots;\n close(fd)\n fd\u003dopen(path); \n fsync(fd);\n close(fd)\n\nlogically that code flow should result in the data hitting the disk.  However I\u0027m worried that the second syscall flow could behave differently inside the kernel and create the performance penalty we\u0027re trying to avoid by localising the fsync().\n\nan API change to download() will do both (minimise the perfomance impact of the fsync() ) but API change :(\n\nThoughts?","commit_id":"adda254d2f0e3b2b52824513b07bbcce6dc5b63b"}],"nova/tests/image/test_glance.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"31c9c3d8b8dc80029f4f599c033140113b0318d4","unresolved":false,"context_lines":[{"line_number":460,"context_line":""},{"line_number":461,"context_line":"    @mock.patch(\u0027__builtin__.open\u0027)"},{"line_number":462,"context_line":"    @mock.patch(\u0027nova.image.glance.GlanceImageService.show\u0027)"},{"line_number":463,"context_line":"    @mock.patch(\u0027nova.image.glance.GlanceImageService._fsync\u0027)"},{"line_number":464,"context_line":"    def test_download_no_data_dest_path(self, fsync_mock, show_mock,"},{"line_number":465,"context_line":"                                        open_mock):"},{"line_number":466,"context_line":"        client \u003d mock.MagicMock()"}],"source_content_type":"text/x-python","patch_set":2,"id":"da9df570_6c160057","line":463,"updated":"2014-09-26 03:20:18.000000000","message":"Just do:\n\n @mock.patch(\u0027os.fsync\u0027)\n\nhere.","commit_id":"7fdf94c0fc9a124b68ccdb3810556f5474561230"},{"author":{"_account_id":12898,"name":"Tony Breeds","email":"tony@bakeyournoodle.com","username":"tonyb"},"change_message_id":"40d8a670975157005a26b03bc9900f57b9fd1847","unresolved":false,"context_lines":[{"line_number":460,"context_line":""},{"line_number":461,"context_line":"    @mock.patch(\u0027__builtin__.open\u0027)"},{"line_number":462,"context_line":"    @mock.patch(\u0027nova.image.glance.GlanceImageService.show\u0027)"},{"line_number":463,"context_line":"    @mock.patch(\u0027nova.image.glance.GlanceImageService._fsync\u0027)"},{"line_number":464,"context_line":"    def test_download_no_data_dest_path(self, fsync_mock, show_mock,"},{"line_number":465,"context_line":"                                        open_mock):"},{"line_number":466,"context_line":"        client \u003d mock.MagicMock()"}],"source_content_type":"text/x-python","patch_set":2,"id":"da9df570_cce8c3e4","line":463,"in_reply_to":"da9df570_6c160057","updated":"2014-09-26 11:27:58.000000000","message":"Done","commit_id":"7fdf94c0fc9a124b68ccdb3810556f5474561230"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"31c9c3d8b8dc80029f4f599c033140113b0318d4","unresolved":false,"context_lines":[{"line_number":549,"context_line":"    @mock.patch(\u0027__builtin__.open\u0027)"},{"line_number":550,"context_line":"    @mock.patch(\u0027nova.image.glance.GlanceImageService._get_transfer_module\u0027)"},{"line_number":551,"context_line":"    @mock.patch(\u0027nova.image.glance.GlanceImageService.show\u0027)"},{"line_number":552,"context_line":"    @mock.patch(\u0027nova.image.glance.GlanceImageService._fsync\u0027)"},{"line_number":553,"context_line":"    def test_download_direct_exception_fallback(self, fsync_mock, show_mock,"},{"line_number":554,"context_line":"                                                get_tran_mock,"},{"line_number":555,"context_line":"                                                open_mock):"}],"source_content_type":"text/x-python","patch_set":2,"id":"da9df570_8c0ec439","line":552,"updated":"2014-09-26 03:20:18.000000000","message":"ditto","commit_id":"7fdf94c0fc9a124b68ccdb3810556f5474561230"},{"author":{"_account_id":12898,"name":"Tony Breeds","email":"tony@bakeyournoodle.com","username":"tonyb"},"change_message_id":"40d8a670975157005a26b03bc9900f57b9fd1847","unresolved":false,"context_lines":[{"line_number":549,"context_line":"    @mock.patch(\u0027__builtin__.open\u0027)"},{"line_number":550,"context_line":"    @mock.patch(\u0027nova.image.glance.GlanceImageService._get_transfer_module\u0027)"},{"line_number":551,"context_line":"    @mock.patch(\u0027nova.image.glance.GlanceImageService.show\u0027)"},{"line_number":552,"context_line":"    @mock.patch(\u0027nova.image.glance.GlanceImageService._fsync\u0027)"},{"line_number":553,"context_line":"    def test_download_direct_exception_fallback(self, fsync_mock, show_mock,"},{"line_number":554,"context_line":"                                                get_tran_mock,"},{"line_number":555,"context_line":"                                                open_mock):"}],"source_content_type":"text/x-python","patch_set":2,"id":"da9df570_0cef2bea","line":552,"in_reply_to":"da9df570_8c0ec439","updated":"2014-09-26 11:27:58.000000000","message":"Done","commit_id":"7fdf94c0fc9a124b68ccdb3810556f5474561230"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"31c9c3d8b8dc80029f4f599c033140113b0318d4","unresolved":false,"context_lines":[{"line_number":604,"context_line":"    @mock.patch(\u0027__builtin__.open\u0027)"},{"line_number":605,"context_line":"    @mock.patch(\u0027nova.image.glance.GlanceImageService._get_transfer_module\u0027)"},{"line_number":606,"context_line":"    @mock.patch(\u0027nova.image.glance.GlanceImageService.show\u0027)"},{"line_number":607,"context_line":"    @mock.patch(\u0027nova.image.glance.GlanceImageService._fsync\u0027)"},{"line_number":608,"context_line":"    def test_download_direct_no_mod_fallback(self, fsync_mock, show_mock,"},{"line_number":609,"context_line":"                                              get_tran_mock,"},{"line_number":610,"context_line":"                                              open_mock):"}],"source_content_type":"text/x-python","patch_set":2,"id":"da9df570_ac0b8828","line":607,"updated":"2014-09-26 03:20:18.000000000","message":"ditto","commit_id":"7fdf94c0fc9a124b68ccdb3810556f5474561230"},{"author":{"_account_id":12898,"name":"Tony Breeds","email":"tony@bakeyournoodle.com","username":"tonyb"},"change_message_id":"40d8a670975157005a26b03bc9900f57b9fd1847","unresolved":false,"context_lines":[{"line_number":604,"context_line":"    @mock.patch(\u0027__builtin__.open\u0027)"},{"line_number":605,"context_line":"    @mock.patch(\u0027nova.image.glance.GlanceImageService._get_transfer_module\u0027)"},{"line_number":606,"context_line":"    @mock.patch(\u0027nova.image.glance.GlanceImageService.show\u0027)"},{"line_number":607,"context_line":"    @mock.patch(\u0027nova.image.glance.GlanceImageService._fsync\u0027)"},{"line_number":608,"context_line":"    def test_download_direct_no_mod_fallback(self, fsync_mock, show_mock,"},{"line_number":609,"context_line":"                                              get_tran_mock,"},{"line_number":610,"context_line":"                                              open_mock):"}],"source_content_type":"text/x-python","patch_set":2,"id":"da9df570_2ceceff2","line":607,"in_reply_to":"da9df570_ac0b8828","updated":"2014-09-26 11:27:58.000000000","message":"Done","commit_id":"7fdf94c0fc9a124b68ccdb3810556f5474561230"}],"nova/tests/unit/image/test_glance.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1e6afa6235e99b75fab2d967fe81a4a6b828df93","unresolved":false,"context_lines":[{"line_number":438,"context_line":""},{"line_number":439,"context_line":"    @mock.patch(\u0027__builtin__.open\u0027)"},{"line_number":440,"context_line":"    @mock.patch(\u0027os.fsync\u0027)"},{"line_number":441,"context_line":"    def test_download_sync(self, fsync_mock, open_mock):"},{"line_number":442,"context_line":"        self.flags(qemu_force_sync\u003dTrue, group\u003d\u0027workarounds\u0027)"},{"line_number":443,"context_line":"        # Setup a minimal environment to get to the fsync() call in:"},{"line_number":444,"context_line":"        # download()"}],"source_content_type":"text/x-python","patch_set":10,"id":"5a890539_d547fe1c","line":441,"updated":"2014-12-05 15:13:15.000000000","message":"This is exactly the same as the previous test except you\u0027re explicitly setting the flag value to True, which is the default, so I don\u0027t really see the point in the additional test.","commit_id":"76900bd8a70b6b4c833ac0ce6643e025ee091024"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"d81d504ca6b292c66f00e6e638c1551ba608f827","unresolved":false,"context_lines":[{"line_number":438,"context_line":""},{"line_number":439,"context_line":"    @mock.patch(\u0027__builtin__.open\u0027)"},{"line_number":440,"context_line":"    @mock.patch(\u0027os.fsync\u0027)"},{"line_number":441,"context_line":"    def test_download_sync(self, fsync_mock, open_mock):"},{"line_number":442,"context_line":"        self.flags(qemu_force_sync\u003dTrue, group\u003d\u0027workarounds\u0027)"},{"line_number":443,"context_line":"        # Setup a minimal environment to get to the fsync() call in:"},{"line_number":444,"context_line":"        # download()"}],"source_content_type":"text/x-python","patch_set":10,"id":"5a890539_4b743dd0","line":441,"in_reply_to":"5a890539_d4dff60a","updated":"2014-12-05 23:04:45.000000000","message":"I think you can test true and false and ignore the default.","commit_id":"76900bd8a70b6b4c833ac0ce6643e025ee091024"},{"author":{"_account_id":12898,"name":"Tony Breeds","email":"tony@bakeyournoodle.com","username":"tonyb"},"change_message_id":"e1aa5d29fdc72ada421e0098bb3c456bb1dcdd25","unresolved":false,"context_lines":[{"line_number":438,"context_line":""},{"line_number":439,"context_line":"    @mock.patch(\u0027__builtin__.open\u0027)"},{"line_number":440,"context_line":"    @mock.patch(\u0027os.fsync\u0027)"},{"line_number":441,"context_line":"    def test_download_sync(self, fsync_mock, open_mock):"},{"line_number":442,"context_line":"        self.flags(qemu_force_sync\u003dTrue, group\u003d\u0027workarounds\u0027)"},{"line_number":443,"context_line":"        # Setup a minimal environment to get to the fsync() call in:"},{"line_number":444,"context_line":"        # download()"}],"source_content_type":"text/x-python","patch_set":10,"id":"5a890539_d4dff60a","line":441,"in_reply_to":"5a890539_d547fe1c","updated":"2014-12-05 19:42:45.000000000","message":"okay.  I though it was a good idea to explicitly test each state True/False and \"default\". Happy to reduce the testcases","commit_id":"76900bd8a70b6b4c833ac0ce6643e025ee091024"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1e6afa6235e99b75fab2d967fe81a4a6b828df93","unresolved":false,"context_lines":[{"line_number":490,"context_line":"    @mock.patch(\u0027__builtin__.open\u0027)"},{"line_number":491,"context_line":"    @mock.patch(\u0027nova.image.glance.GlanceImageService.show\u0027)"},{"line_number":492,"context_line":"    def test_download_data_no_dest_path(self, show_mock, open_mock):"},{"line_number":493,"context_line":"        # Make sure we don\u0027t call os.fysnc() for no good reason"},{"line_number":494,"context_line":"        self.flags(qemu_force_sync\u003dFalse, group\u003d\u0027workarounds\u0027)"},{"line_number":495,"context_line":"        client \u003d mock.MagicMock()"},{"line_number":496,"context_line":"        client.call.return_value \u003d [1, 2, 3]"}],"source_content_type":"text/x-python","patch_set":10,"id":"5a890539_d5f53ecd","line":493,"updated":"2014-12-05 15:13:15.000000000","message":"Rather than do this for every test, can we do it in setUpClass and then when we want to test the True case, we set that in the test case to override the setUpClass?  I know it\u0027s a bit odd since True is the default in the code, but it would cleanup the tests and make the True test more explicit - which would then satisfy my issue with the first two tests being functionally identical.","commit_id":"76900bd8a70b6b4c833ac0ce6643e025ee091024"},{"author":{"_account_id":12898,"name":"Tony Breeds","email":"tony@bakeyournoodle.com","username":"tonyb"},"change_message_id":"e1aa5d29fdc72ada421e0098bb3c456bb1dcdd25","unresolved":false,"context_lines":[{"line_number":490,"context_line":"    @mock.patch(\u0027__builtin__.open\u0027)"},{"line_number":491,"context_line":"    @mock.patch(\u0027nova.image.glance.GlanceImageService.show\u0027)"},{"line_number":492,"context_line":"    def test_download_data_no_dest_path(self, show_mock, open_mock):"},{"line_number":493,"context_line":"        # Make sure we don\u0027t call os.fysnc() for no good reason"},{"line_number":494,"context_line":"        self.flags(qemu_force_sync\u003dFalse, group\u003d\u0027workarounds\u0027)"},{"line_number":495,"context_line":"        client \u003d mock.MagicMock()"},{"line_number":496,"context_line":"        client.call.return_value \u003d [1, 2, 3]"}],"source_content_type":"text/x-python","patch_set":10,"id":"5a890539_f432325d","line":493,"in_reply_to":"5a890539_d5f53ecd","updated":"2014-12-05 19:42:45.000000000","message":"I\u0027ll look at that for the next patchset.","commit_id":"76900bd8a70b6b4c833ac0ce6643e025ee091024"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1e6afa6235e99b75fab2d967fe81a4a6b828df93","unresolved":false,"context_lines":[{"line_number":575,"context_line":"    @mock.patch(\u0027nova.image.glance.GlanceImageService._get_transfer_module\u0027)"},{"line_number":576,"context_line":"    @mock.patch(\u0027nova.image.glance.GlanceImageService.show\u0027)"},{"line_number":577,"context_line":"    def test_download_direct_file_uri(self, show_mock, get_tran_mock):"},{"line_number":578,"context_line":"        # Make sure we don\u0027t call os.fysnc() for no good reason"},{"line_number":579,"context_line":"        self.flags(allowed_direct_url_schemes\u003d[\u0027file\u0027], group\u003d\u0027glance\u0027)"},{"line_number":580,"context_line":"        show_mock.return_value \u003d {"},{"line_number":581,"context_line":"            \u0027locations\u0027: ["}],"source_content_type":"text/x-python","patch_set":10,"id":"5a890539_75600aa5","line":578,"updated":"2014-12-05 15:13:15.000000000","message":"Did you mean to set a flag here?","commit_id":"76900bd8a70b6b4c833ac0ce6643e025ee091024"},{"author":{"_account_id":12898,"name":"Tony Breeds","email":"tony@bakeyournoodle.com","username":"tonyb"},"change_message_id":"e1aa5d29fdc72ada421e0098bb3c456bb1dcdd25","unresolved":false,"context_lines":[{"line_number":575,"context_line":"    @mock.patch(\u0027nova.image.glance.GlanceImageService._get_transfer_module\u0027)"},{"line_number":576,"context_line":"    @mock.patch(\u0027nova.image.glance.GlanceImageService.show\u0027)"},{"line_number":577,"context_line":"    def test_download_direct_file_uri(self, show_mock, get_tran_mock):"},{"line_number":578,"context_line":"        # Make sure we don\u0027t call os.fysnc() for no good reason"},{"line_number":579,"context_line":"        self.flags(allowed_direct_url_schemes\u003d[\u0027file\u0027], group\u003d\u0027glance\u0027)"},{"line_number":580,"context_line":"        show_mock.return_value \u003d {"},{"line_number":581,"context_line":"            \u0027locations\u0027: ["}],"source_content_type":"text/x-python","patch_set":10,"id":"5a890539_f4fb1279","line":578,"in_reply_to":"5a890539_75600aa5","updated":"2014-12-05 19:42:45.000000000","message":"No, overzealous commenting.","commit_id":"76900bd8a70b6b4c833ac0ce6643e025ee091024"}],"nova/virt/images.py":[{"author":{"_account_id":8412,"name":"Mike Durnosvistov","email":"glacierr.dev@gmail.com","username":"mdurnosvistov"},"change_message_id":"a4a5df815fc0ad26a3aadb067eb6a1d4a732e033","unresolved":false,"context_lines":[{"line_number":62,"context_line":""},{"line_number":63,"context_line":"def maybe_fsync(path):"},{"line_number":64,"context_line":"    if CONF.workarounds.qemu_force_sync:"},{"line_number":65,"context_line":"        data \u003d open(path, \u0027wb\u0027)"},{"line_number":66,"context_line":"        data.flush()"},{"line_number":67,"context_line":"        os.fsync(data.fileno())"},{"line_number":68,"context_line":"        data.close()"}],"source_content_type":"text/x-python","patch_set":12,"id":"3a961159_93ac7ea2","line":65,"updated":"2014-12-09 13:31:03.000000000","message":"I\u0027m just wondering how about `with` expression?","commit_id":"b9f06220d224e145f00f310c83b616a130591e83"},{"author":{"_account_id":8802,"name":"Vladik Romanovsky","email":"vromanso@redhat.com","username":"vladikr"},"change_message_id":"ef82ab84939fb27d804f918a4d0142837110486d","unresolved":false,"context_lines":[{"line_number":135,"context_line":"            raise exception.FlavorDiskTooSmall()"},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"        if fmt !\u003d \"raw\" and CONF.force_raw_images:"},{"line_number":138,"context_line":"            maybe_fsync(path_tmp)"},{"line_number":139,"context_line":"            staged \u003d \"%s.converted\" % path"},{"line_number":140,"context_line":"            LOG.debug(\"%s was %s, converting to raw\" % (image_href, fmt))"},{"line_number":141,"context_line":"            with fileutils.remove_path_on_error(staged):"}],"source_content_type":"text/x-python","patch_set":12,"id":"3a961159_cbf72ef4","line":138,"updated":"2014-12-16 16:22:32.000000000","message":"I think you need to return a meaningful exception, when maybe_fsync() fails","commit_id":"b9f06220d224e145f00f310c83b616a130591e83"},{"author":{"_account_id":8802,"name":"Vladik Romanovsky","email":"vromanso@redhat.com","username":"vladikr"},"change_message_id":"91adfe80a090b5315105007ef08b59ccb1de4b40","unresolved":false,"context_lines":[{"line_number":135,"context_line":"            raise exception.FlavorDiskTooSmall()"},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"        if fmt !\u003d \"raw\" and CONF.force_raw_images:"},{"line_number":138,"context_line":"            maybe_fsync(path_tmp)"},{"line_number":139,"context_line":"            staged \u003d \"%s.converted\" % path"},{"line_number":140,"context_line":"            LOG.debug(\"%s was %s, converting to raw\" % (image_href, fmt))"},{"line_number":141,"context_line":"            with fileutils.remove_path_on_error(staged):"}],"source_content_type":"text/x-python","patch_set":12,"id":"3a961159_e6aa7b84","line":138,"in_reply_to":"3a961159_4b3b5a38","updated":"2014-12-16 20:09:38.000000000","message":"I would throw something that would provide some context around the operation that is taking place, so it would be clear what\u0027s going on..\nIIRC, os.fsync fails with very uninformative exceptions, like \"invalid argument\" or \"bad descriptor\"..\nIf you disagree with that, I would at least log something meaningful here.","commit_id":"b9f06220d224e145f00f310c83b616a130591e83"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"052fc5c0e39029ced780fb6287b453b11720e2c0","unresolved":false,"context_lines":[{"line_number":135,"context_line":"            raise exception.FlavorDiskTooSmall()"},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"        if fmt !\u003d \"raw\" and CONF.force_raw_images:"},{"line_number":138,"context_line":"            maybe_fsync(path_tmp)"},{"line_number":139,"context_line":"            staged \u003d \"%s.converted\" % path"},{"line_number":140,"context_line":"            LOG.debug(\"%s was %s, converting to raw\" % (image_href, fmt))"},{"line_number":141,"context_line":"            with fileutils.remove_path_on_error(staged):"}],"source_content_type":"text/x-python","patch_set":12,"id":"3a961159_4b3b5a38","line":138,"in_reply_to":"3a961159_cbf72ef4","updated":"2014-12-16 19:35:55.000000000","message":"Such as what? We can already throw a variety of OSError exceptions in this function, and probably more.","commit_id":"b9f06220d224e145f00f310c83b616a130591e83"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"a96cd2c93d06557944e25503c7d0db93b8d27866","unresolved":false,"context_lines":[{"line_number":50,"context_line":"    cfg.BoolOpt(\u0027qemu_force_sync\u0027,"},{"line_number":51,"context_line":"                default\u003dTrue,"},{"line_number":52,"context_line":"                help\u003d\u0027qemu-img convert -O raw is buggy in qemu \u003c2.2.0.  This \u0027"},{"line_number":53,"context_line":"                      \u0027workaround forces either a sync() or fsync() as \u0027"},{"line_number":54,"context_line":"                      \u0027required\u0027),"},{"line_number":55,"context_line":"    ]"},{"line_number":56,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"3a961159_7a6f6992","line":53,"updated":"2014-12-22 15:21:06.000000000","message":"indentation here is dodgy","commit_id":"4067ba22f051e3994d1a32b87d803fff66dffd0b"},{"author":{"_account_id":12898,"name":"Tony Breeds","email":"tony@bakeyournoodle.com","username":"tonyb"},"change_message_id":"14807403f23e4c994ee154f6feeeceae93b6a16a","unresolved":false,"context_lines":[{"line_number":50,"context_line":"    cfg.BoolOpt(\u0027qemu_force_sync\u0027,"},{"line_number":51,"context_line":"                default\u003dTrue,"},{"line_number":52,"context_line":"                help\u003d\u0027qemu-img convert -O raw is buggy in qemu \u003c2.2.0.  This \u0027"},{"line_number":53,"context_line":"                      \u0027workaround forces either a sync() or fsync() as \u0027"},{"line_number":54,"context_line":"                      \u0027required\u0027),"},{"line_number":55,"context_line":"    ]"},{"line_number":56,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"3a961159_74e935e5","line":53,"in_reply_to":"3a961159_7a6f6992","updated":"2014-12-22 21:13:54.000000000","message":"*sigh*  If I do another spin I\u0027ll fix that.","commit_id":"4067ba22f051e3994d1a32b87d803fff66dffd0b"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"a96cd2c93d06557944e25503c7d0db93b8d27866","unresolved":false,"context_lines":[{"line_number":56,"context_line":""},{"line_number":57,"context_line":"CONF \u003d cfg.CONF"},{"line_number":58,"context_line":"CONF.register_opts(image_opts)"},{"line_number":59,"context_line":"CONF.register_opts(workarounds_opts, group\u003d\u0027workarounds\u0027)"},{"line_number":60,"context_line":"IMAGE_API \u003d image.API()"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"3a961159_7af689d8","line":59,"updated":"2014-12-22 15:21:06.000000000","message":"why is this not in the libvirt section. i think that a workarounds section is not the correct way to go","commit_id":"4067ba22f051e3994d1a32b87d803fff66dffd0b"},{"author":{"_account_id":12898,"name":"Tony Breeds","email":"tony@bakeyournoodle.com","username":"tonyb"},"change_message_id":"14807403f23e4c994ee154f6feeeceae93b6a16a","unresolved":false,"context_lines":[{"line_number":56,"context_line":""},{"line_number":57,"context_line":"CONF \u003d cfg.CONF"},{"line_number":58,"context_line":"CONF.register_opts(image_opts)"},{"line_number":59,"context_line":"CONF.register_opts(workarounds_opts, group\u003d\u0027workarounds\u0027)"},{"line_number":60,"context_line":"IMAGE_API \u003d image.API()"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"3a961159_14c0f14e","line":59,"in_reply_to":"3a961159_7af689d8","updated":"2014-12-22 21:13:54.000000000","message":"The intent is to create a generic workarounds group for other things as well.\nso putting it in the libvirt group would eventually create more work.\n\nThere isn\u0027t a great place to put the defn.  Here will do for now.  If needed it can be moved later.","commit_id":"4067ba22f051e3994d1a32b87d803fff66dffd0b"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"a96cd2c93d06557944e25503c7d0db93b8d27866","unresolved":false,"context_lines":[{"line_number":60,"context_line":"IMAGE_API \u003d image.API()"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"def maybe_fsync(path):"},{"line_number":64,"context_line":"    if CONF.workarounds.qemu_force_sync:"},{"line_number":65,"context_line":"        with open(path, \u0027ab\u0027) as data:"},{"line_number":66,"context_line":"            data.flush()"}],"source_content_type":"text/x-python","patch_set":14,"id":"3a961159_9a18951f","line":63,"updated":"2014-12-22 15:21:06.000000000","message":"consider changing this to force_fsycnc","commit_id":"4067ba22f051e3994d1a32b87d803fff66dffd0b"},{"author":{"_account_id":12898,"name":"Tony Breeds","email":"tony@bakeyournoodle.com","username":"tonyb"},"change_message_id":"14807403f23e4c994ee154f6feeeceae93b6a16a","unresolved":false,"context_lines":[{"line_number":60,"context_line":"IMAGE_API \u003d image.API()"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"def maybe_fsync(path):"},{"line_number":64,"context_line":"    if CONF.workarounds.qemu_force_sync:"},{"line_number":65,"context_line":"        with open(path, \u0027ab\u0027) as data:"},{"line_number":66,"context_line":"            data.flush()"}],"source_content_type":"text/x-python","patch_set":14,"id":"3a961159_94922155","line":63,"in_reply_to":"3a961159_9a18951f","updated":"2014-12-22 21:13:54.000000000","message":"I prefer maybe_fsync as it may fsync() or not depending on the value of the config variable.  force_fsync() gives a false impression.","commit_id":"4067ba22f051e3994d1a32b87d803fff66dffd0b"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"a7fafcf141ee144534a18a9f7d2862997ea49dda","unresolved":false,"context_lines":[{"line_number":64,"context_line":"    if CONF.workarounds.qemu_force_sync:"},{"line_number":65,"context_line":"        with open(path, \u0027ab\u0027) as data:"},{"line_number":66,"context_line":"            data.flush()"},{"line_number":67,"context_line":"            os.fsync(data.fileno())"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"def qemu_img_info(path):"}],"source_content_type":"text/x-python","patch_set":14,"id":"3a961159_7b3f84df","line":67,"updated":"2015-01-15 13:00:56.000000000","message":"If we are using the fixed QEMU, the  os.fsync(fd) will essentially be a no-op in terms of overhead.  So this doesn\u0027t create any harm if running fixed QEMU unless I\u0027m missing something. IOW I\u0027m not convinced we need to add a config option for this.\n\nLets just *unconditionally* run this fsync regarldless. In 2-3 years when every distro we care about has upgraded to QEMU \u003e\u003d 2.2.0 we can just drop it. No config option required.","commit_id":"4067ba22f051e3994d1a32b87d803fff66dffd0b"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"ec3b5c997c9c3b7cc2ee430909ae320e4a464589","unresolved":false,"context_lines":[{"line_number":64,"context_line":"    if CONF.workarounds.qemu_force_sync:"},{"line_number":65,"context_line":"        with open(path, \u0027ab\u0027) as data:"},{"line_number":66,"context_line":"            data.flush()"},{"line_number":67,"context_line":"            os.fsync(data.fileno())"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"def qemu_img_info(path):"}],"source_content_type":"text/x-python","patch_set":14,"id":"3a961159_72b6c226","line":67,"in_reply_to":"3a961159_2cedc48e","updated":"2015-01-15 14:19:12.000000000","message":"Ok, I concur with that. Just abandon this patch \u0026 let distros deal with making sure they have a fixed QEMU. The distros need to do that anyway to make sure other non-openstack software works correctly too.","commit_id":"4067ba22f051e3994d1a32b87d803fff66dffd0b"},{"author":{"_account_id":1812,"name":"p-draigbrady","email":"P@draigBrady.com","username":"p-draigbrady"},"change_message_id":"e669829e6297539d710ecd21811fd1421948536c","unresolved":false,"context_lines":[{"line_number":64,"context_line":"    if CONF.workarounds.qemu_force_sync:"},{"line_number":65,"context_line":"        with open(path, \u0027ab\u0027) as data:"},{"line_number":66,"context_line":"            data.flush()"},{"line_number":67,"context_line":"            os.fsync(data.fileno())"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"def qemu_img_info(path):"}],"source_content_type":"text/x-python","patch_set":14,"id":"3a961159_2cedc48e","line":67,"in_reply_to":"3a961159_7b3f84df","updated":"2015-01-15 14:10:37.000000000","message":"Well the fsync can have other global side effects depending on file system and it\u0027s not done unconditionally with fixed QEMU, only when needed.  More importantly a sync in nova here avoids one aspect of the issue, but the complete fix needs to be in qemu as nova doesn\u0027t have visibility into all unsynced files that qemu may read.\n\nAlso all distros are already updated as per https://wiki.openstack.org/wiki/Bug1368815 Well I\u0027ve no visibility into the suse bugs but I presume they\u0027re updated at this stage\n\nIn summary, while I think there are benefits to this patch,\nit\u0027s a net loss to apply it, and we should just assume qemu\ndoes the right thing now.","commit_id":"4067ba22f051e3994d1a32b87d803fff66dffd0b"}]}