)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"296a2911044e3c83a19d85c3467dc773e7bd517d","unresolved":false,"context_lines":[{"line_number":15,"context_line":"per compute host and would be virt-driver-agnostic. It is default to 0"},{"line_number":16,"context_line":"which means no limit."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"blueprint: io-semaphore-for-concurrent-disk-ops"},{"line_number":19,"context_line":"Change-Id: I897999e8a4601694213f068367eae9608cdc7bbb"},{"line_number":20,"context_line":"Signed-off-by: Jack Ding \u003cjack.ding@windriver.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"3f79a3b5_e1a2b8fb","line":18,"updated":"2018-10-31 16:50:28.000000000","message":"This is not yet approved. I believe we said at the PTG that this should be an OK change to make since it would be backward compatible by default configuration. But per our specless blueprint process [1] we should bring this up during the nova meeting.\n\n[1] https://docs.openstack.org/nova/latest/contributor/blueprints.html","commit_id":"f571146bbec7862c92ff450868c840db72af5147"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cee0cb7fc4b62ee433d4294bf4a4b3e2527fb315","unresolved":false,"context_lines":[{"line_number":15,"context_line":"per compute host and would be virt-driver-agnostic. It is default to 0"},{"line_number":16,"context_line":"which means no limit."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"blueprint: io-semaphore-for-concurrent-disk-ops"},{"line_number":19,"context_line":"Change-Id: I897999e8a4601694213f068367eae9608cdc7bbb"},{"line_number":20,"context_line":"Signed-off-by: Jack Ding \u003cjack.ding@windriver.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"3f79a3b5_95f74929","line":18,"in_reply_to":"3f79a3b5_e1a2b8fb","updated":"2018-11-19 14:26:58.000000000","message":"Done","commit_id":"f571146bbec7862c92ff450868c840db72af5147"}],"nova/compute/manager.py":[{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"9aa2eb802dd97130b18c609a64f80255ef3b8b10","unresolved":false,"context_lines":[{"line_number":1202,"context_line":""},{"line_number":1203,"context_line":"        self.init_virt_events()"},{"line_number":1204,"context_line":""},{"line_number":1205,"context_line":"        # one-time initialization"},{"line_number":1206,"context_line":"        if CONF.max_concurrent_disk_ops !\u003d 0:"},{"line_number":1207,"context_line":"            utils.disk_ops_semaphore \u003d eventlet.semaphore.BoundedSemaphore("},{"line_number":1208,"context_line":"                CONF.max_concurrent_disk_ops)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_e670a044","line":1205,"updated":"2018-10-17 16:57:13.000000000","message":"I\u0027d be tempted to move this before self.driver.init_host() just out of paranoia that someone might someday do something in there that tries to use the semaphore.","commit_id":"c2aaebd3a3595b8f685e63d414e7674bf9ae024b"},{"author":{"_account_id":28433,"name":"Jack Ding","email":"jackding@gmail.com","username":"jackding"},"change_message_id":"285ffe1209bff45cde99c9139aca467a05d0c12c","unresolved":false,"context_lines":[{"line_number":1202,"context_line":""},{"line_number":1203,"context_line":"        self.init_virt_events()"},{"line_number":1204,"context_line":""},{"line_number":1205,"context_line":"        # one-time initialization"},{"line_number":1206,"context_line":"        if CONF.max_concurrent_disk_ops !\u003d 0:"},{"line_number":1207,"context_line":"            utils.disk_ops_semaphore \u003d eventlet.semaphore.BoundedSemaphore("},{"line_number":1208,"context_line":"                CONF.max_concurrent_disk_ops)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_c95ed7e7","line":1205,"in_reply_to":"3f79a3b5_e670a044","updated":"2018-10-17 18:24:38.000000000","message":"Done","commit_id":"c2aaebd3a3595b8f685e63d414e7674bf9ae024b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cee0cb7fc4b62ee433d4294bf4a4b3e2527fb315","unresolved":false,"context_lines":[{"line_number":1194,"context_line":""},{"line_number":1195,"context_line":"        # one-time initialization"},{"line_number":1196,"context_line":"        if CONF.max_concurrent_disk_ops !\u003d 0:"},{"line_number":1197,"context_line":"            utils.disk_ops_semaphore \u003d eventlet.semaphore.BoundedSemaphore("},{"line_number":1198,"context_line":"                CONF.max_concurrent_disk_ops)"},{"line_number":1199,"context_line":"        else:"},{"line_number":1200,"context_line":"            utils.disk_ops_semaphore \u003d compute_utils.UnlimitedSemaphore()"},{"line_number":1201,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_153499df","line":1198,"range":{"start_line":1197,"start_character":12,"end_line":1198,"end_character":45},"updated":"2018-11-19 14:26:58.000000000","message":"Just a thought, but would it be useful to make this a mutable option and then you just have to SIGHUP the service to reset this? You\u0027d have to implement it in the reset() method, and potentially wait for anything in the semaphore to complete before changing it? Seems non-trivial though so it could be a follow up if we really wanted that behavior.","commit_id":"f571146bbec7862c92ff450868c840db72af5147"},{"author":{"_account_id":28433,"name":"Jack Ding","email":"jackding@gmail.com","username":"jackding"},"change_message_id":"53e33f0f44f4e5f78dce0b43afffcd94f7c9100a","unresolved":false,"context_lines":[{"line_number":1194,"context_line":""},{"line_number":1195,"context_line":"        # one-time initialization"},{"line_number":1196,"context_line":"        if CONF.max_concurrent_disk_ops !\u003d 0:"},{"line_number":1197,"context_line":"            utils.disk_ops_semaphore \u003d eventlet.semaphore.BoundedSemaphore("},{"line_number":1198,"context_line":"                CONF.max_concurrent_disk_ops)"},{"line_number":1199,"context_line":"        else:"},{"line_number":1200,"context_line":"            utils.disk_ops_semaphore \u003d compute_utils.UnlimitedSemaphore()"},{"line_number":1201,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_89488db4","line":1198,"range":{"start_line":1197,"start_character":12,"end_line":1198,"end_character":45},"in_reply_to":"3f79a3b5_153499df","updated":"2018-11-20 15:42:09.000000000","message":"It would be nice to be able to adjust the value at run time, but for a large cluster, it might be a challenge to achieve consistent performance across nodes.","commit_id":"f571146bbec7862c92ff450868c840db72af5147"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cee0cb7fc4b62ee433d4294bf4a4b3e2527fb315","unresolved":false,"context_lines":[{"line_number":1231,"context_line":"                # _sync_scheduler_instance_info periodic task will."},{"line_number":1232,"context_line":"                self._update_scheduler_instance_info(context, instances)"},{"line_number":1233,"context_line":""},{"line_number":1234,"context_line":"    def cleanup_host(self):"},{"line_number":1235,"context_line":"        self.driver.register_event_listener(None)"},{"line_number":1236,"context_line":"        self.instance_events.cancel_all_events()"},{"line_number":1237,"context_line":"        self.driver.cleanup_host(host\u003dself.host)"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_554f7168","line":1234,"range":{"start_line":1234,"start_character":8,"end_line":1234,"end_character":20},"updated":"2018-11-19 14:26:58.000000000","message":"Is there anything we need to do on shutdown of the service to free up resources in the new semaphore?","commit_id":"f571146bbec7862c92ff450868c840db72af5147"},{"author":{"_account_id":28433,"name":"Jack Ding","email":"jackding@gmail.com","username":"jackding"},"change_message_id":"53e33f0f44f4e5f78dce0b43afffcd94f7c9100a","unresolved":false,"context_lines":[{"line_number":1231,"context_line":"                # _sync_scheduler_instance_info periodic task will."},{"line_number":1232,"context_line":"                self._update_scheduler_instance_info(context, instances)"},{"line_number":1233,"context_line":""},{"line_number":1234,"context_line":"    def cleanup_host(self):"},{"line_number":1235,"context_line":"        self.driver.register_event_listener(None)"},{"line_number":1236,"context_line":"        self.instance_events.cancel_all_events()"},{"line_number":1237,"context_line":"        self.driver.cleanup_host(host\u003dself.host)"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_69bad170","line":1234,"range":{"start_line":1234,"start_character":8,"end_line":1234,"end_character":20},"in_reply_to":"3f79a3b5_554f7168","updated":"2018-11-20 15:42:09.000000000","message":"release() is automatically called when using \"with semaphore:\" so I don\u0027t think we need any additional release here.","commit_id":"f571146bbec7862c92ff450868c840db72af5147"}],"nova/conf/compute.py":[{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"043f44ae1ffcbc7bb213f6e9ee41afdd6e4c5476","unresolved":false,"context_lines":[{"line_number":307,"context_line":"Number of concurrent disk-IO-intensive operations (glance image downloads,"},{"line_number":308,"context_line":"image format conversions, etc.) that we will do in parallel.  If this is set"},{"line_number":309,"context_line":"too high then response time suffers."},{"line_number":310,"context_line":"It is default to 0 which means no limit."},{"line_number":311,"context_line":" \"\"\"),"},{"line_number":312,"context_line":"]"},{"line_number":313,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_586cc132","line":310,"range":{"start_line":310,"start_character":0,"end_line":310,"end_character":40},"updated":"2018-10-31 15:29:46.000000000","message":"would be more correct as \"The default value of 0 means no limit.\"","commit_id":"ceddd841832423b963cccbe1ff085a6d1ad75a00"},{"author":{"_account_id":28433,"name":"Jack Ding","email":"jackding@gmail.com","username":"jackding"},"change_message_id":"cb83c5cdb2f2d4a054bc0f7a733d1472b179ecdf","unresolved":false,"context_lines":[{"line_number":307,"context_line":"Number of concurrent disk-IO-intensive operations (glance image downloads,"},{"line_number":308,"context_line":"image format conversions, etc.) that we will do in parallel.  If this is set"},{"line_number":309,"context_line":"too high then response time suffers."},{"line_number":310,"context_line":"It is default to 0 which means no limit."},{"line_number":311,"context_line":" \"\"\"),"},{"line_number":312,"context_line":"]"},{"line_number":313,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_61122858","line":310,"range":{"start_line":310,"start_character":0,"end_line":310,"end_character":40},"in_reply_to":"3f79a3b5_586cc132","updated":"2018-10-31 16:46:39.000000000","message":"Done","commit_id":"ceddd841832423b963cccbe1ff085a6d1ad75a00"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cee0cb7fc4b62ee433d4294bf4a4b3e2527fb315","unresolved":false,"context_lines":[{"line_number":301,"context_line":"  agent disabled. When used with libvirt the instance mode should be"},{"line_number":302,"context_line":"  configured as HVM."},{"line_number":303,"context_line":" \"\"\"),"},{"line_number":304,"context_line":"    cfg.IntOpt(\u0027max_concurrent_disk_ops\u0027,"},{"line_number":305,"context_line":"        default\u003d0,"},{"line_number":306,"context_line":"        help\u003d\"\"\""},{"line_number":307,"context_line":"Number of concurrent disk-IO-intensive operations (glance image downloads,"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_b57125ad","line":304,"range":{"start_line":304,"start_character":8,"end_line":304,"end_character":14},"updated":"2018-11-19 14:26:58.000000000","message":"Should there be a min value of 0 on this? What happens if it were set to -1? Looks like it blows up:\n\n\u003e\u003e\u003e from eventlet import semaphore\n\u003e\u003e\u003e sem \u003d semaphore.BoundedSemaphore(value\u003d-1)\nTraceback (most recent call last):\n  File \"\u003cstdin\u003e\", line 1, in \u003cmodule\u003e\n  File \"/home/osboxes/vexxhost/local/lib/python2.7/site-packages/eventlet/semaphore.py\", line 179, in __init__\n    super(BoundedSemaphore, self).__init__(value)\n  File \"/home/osboxes/vexxhost/local/lib/python2.7/site-packages/eventlet/semaphore.py\", line 41, in __init__\n    raise ValueError(msg)\nValueError: Semaphore() expect value \u003e\u003d 0, actual: -1\n\u003e\u003e\u003e \n\n\nSo we should have min\u003d0 on this option.","commit_id":"f571146bbec7862c92ff450868c840db72af5147"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cee0cb7fc4b62ee433d4294bf4a4b3e2527fb315","unresolved":false,"context_lines":[{"line_number":301,"context_line":"  agent disabled. When used with libvirt the instance mode should be"},{"line_number":302,"context_line":"  configured as HVM."},{"line_number":303,"context_line":" \"\"\"),"},{"line_number":304,"context_line":"    cfg.IntOpt(\u0027max_concurrent_disk_ops\u0027,"},{"line_number":305,"context_line":"        default\u003d0,"},{"line_number":306,"context_line":"        help\u003d\"\"\""},{"line_number":307,"context_line":"Number of concurrent disk-IO-intensive operations (glance image downloads,"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_f53d5db9","line":304,"range":{"start_line":304,"start_character":16,"end_line":304,"end_character":39},"updated":"2018-11-19 14:26:58.000000000","message":"This should go in the [compute] group. I know that\u0027s not where it is for StarlingX, but as a new option in nova if this is only used by the compute service, it should be in the [compute] group rather than [DEFAULT].","commit_id":"f571146bbec7862c92ff450868c840db72af5147"},{"author":{"_account_id":28433,"name":"Jack Ding","email":"jackding@gmail.com","username":"jackding"},"change_message_id":"53e33f0f44f4e5f78dce0b43afffcd94f7c9100a","unresolved":false,"context_lines":[{"line_number":301,"context_line":"  agent disabled. When used with libvirt the instance mode should be"},{"line_number":302,"context_line":"  configured as HVM."},{"line_number":303,"context_line":" \"\"\"),"},{"line_number":304,"context_line":"    cfg.IntOpt(\u0027max_concurrent_disk_ops\u0027,"},{"line_number":305,"context_line":"        default\u003d0,"},{"line_number":306,"context_line":"        help\u003d\"\"\""},{"line_number":307,"context_line":"Number of concurrent disk-IO-intensive operations (glance image downloads,"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_80df0266","line":304,"range":{"start_line":304,"start_character":8,"end_line":304,"end_character":14},"in_reply_to":"3f79a3b5_b57125ad","updated":"2018-11-20 15:42:09.000000000","message":"Done","commit_id":"f571146bbec7862c92ff450868c840db72af5147"},{"author":{"_account_id":28433,"name":"Jack Ding","email":"jackding@gmail.com","username":"jackding"},"change_message_id":"53e33f0f44f4e5f78dce0b43afffcd94f7c9100a","unresolved":false,"context_lines":[{"line_number":301,"context_line":"  agent disabled. When used with libvirt the instance mode should be"},{"line_number":302,"context_line":"  configured as HVM."},{"line_number":303,"context_line":" \"\"\"),"},{"line_number":304,"context_line":"    cfg.IntOpt(\u0027max_concurrent_disk_ops\u0027,"},{"line_number":305,"context_line":"        default\u003d0,"},{"line_number":306,"context_line":"        help\u003d\"\"\""},{"line_number":307,"context_line":"Number of concurrent disk-IO-intensive operations (glance image downloads,"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_605c26ef","line":304,"range":{"start_line":304,"start_character":16,"end_line":304,"end_character":39},"in_reply_to":"3f79a3b5_f53d5db9","updated":"2018-11-20 15:42:09.000000000","message":"Done","commit_id":"f571146bbec7862c92ff450868c840db72af5147"}],"nova/tests/unit/virt/libvirt/test_driver.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cee0cb7fc4b62ee433d4294bf4a4b3e2527fb315","unresolved":false,"context_lines":[{"line_number":18586,"context_line":"        path \u003d \u0027/test/disk\u0027"},{"line_number":18587,"context_line":"        _path_qcow \u003d path + \u0027_qcow\u0027"},{"line_number":18588,"context_line":""},{"line_number":18589,"context_line":"        with mock.patch.object(utils, \u0027disk_ops_semaphore\u0027,"},{"line_number":18590,"context_line":"                               new_callable\u003dcompute_utils.UnlimitedSemaphore):"},{"line_number":18591,"context_line":"            self.drvr._disk_raw_to_qcow2(path)"},{"line_number":18592,"context_line":"        mock_execute.assert_has_calls(["},{"line_number":18593,"context_line":"            mock.call(\u0027qemu-img\u0027, \u0027convert\u0027, \u0027-f\u0027, \u0027raw\u0027,"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_15901997","line":18590,"range":{"start_line":18589,"start_character":8,"end_line":18590,"end_character":78},"updated":"2018-11-19 14:26:58.000000000","message":"Should probably assert this is called.","commit_id":"f571146bbec7862c92ff450868c840db72af5147"},{"author":{"_account_id":28433,"name":"Jack Ding","email":"jackding@gmail.com","username":"jackding"},"change_message_id":"53e33f0f44f4e5f78dce0b43afffcd94f7c9100a","unresolved":false,"context_lines":[{"line_number":18586,"context_line":"        path \u003d \u0027/test/disk\u0027"},{"line_number":18587,"context_line":"        _path_qcow \u003d path + \u0027_qcow\u0027"},{"line_number":18588,"context_line":""},{"line_number":18589,"context_line":"        with mock.patch.object(utils, \u0027disk_ops_semaphore\u0027,"},{"line_number":18590,"context_line":"                               new_callable\u003dcompute_utils.UnlimitedSemaphore):"},{"line_number":18591,"context_line":"            self.drvr._disk_raw_to_qcow2(path)"},{"line_number":18592,"context_line":"        mock_execute.assert_has_calls(["},{"line_number":18593,"context_line":"            mock.call(\u0027qemu-img\u0027, \u0027convert\u0027, \u0027-f\u0027, \u0027raw\u0027,"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_55df5866","line":18590,"range":{"start_line":18589,"start_character":8,"end_line":18590,"end_character":78},"in_reply_to":"3f79a3b5_15901997","updated":"2018-11-20 15:42:09.000000000","message":"\u003e Should probably assert this is called.\n\nHow to assert it is called? disk_ops_semaphore is not a function.","commit_id":"f571146bbec7862c92ff450868c840db72af5147"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6c25e83a575cb8f9299f01244f9fdd3d1bdc7fc3","unresolved":false,"context_lines":[{"line_number":18586,"context_line":"        path \u003d \u0027/test/disk\u0027"},{"line_number":18587,"context_line":"        _path_qcow \u003d path + \u0027_qcow\u0027"},{"line_number":18588,"context_line":""},{"line_number":18589,"context_line":"        with mock.patch.object(utils, \u0027disk_ops_semaphore\u0027,"},{"line_number":18590,"context_line":"                               new_callable\u003dcompute_utils.UnlimitedSemaphore):"},{"line_number":18591,"context_line":"            self.drvr._disk_raw_to_qcow2(path)"},{"line_number":18592,"context_line":"        mock_execute.assert_has_calls(["},{"line_number":18593,"context_line":"            mock.call(\u0027qemu-img\u0027, \u0027convert\u0027, \u0027-f\u0027, \u0027raw\u0027,"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_dc7f0c14","line":18590,"range":{"start_line":18589,"start_character":8,"end_line":18590,"end_character":78},"in_reply_to":"3f79a3b5_55df5866","updated":"2018-11-20 22:49:44.000000000","message":"Aren\u0027t you using a context manager? You can mock a context manager and assert it\u0027s called by checking the return_value.__exit__ I believe (or maybe it\u0027s __enter__ I always have to re-learn it).","commit_id":"f571146bbec7862c92ff450868c840db72af5147"},{"author":{"_account_id":28433,"name":"Jack Ding","email":"jackding@gmail.com","username":"jackding"},"change_message_id":"931cfdec793631a88f283e354cb345c7ad20ff1f","unresolved":false,"context_lines":[{"line_number":18586,"context_line":"        path \u003d \u0027/test/disk\u0027"},{"line_number":18587,"context_line":"        _path_qcow \u003d path + \u0027_qcow\u0027"},{"line_number":18588,"context_line":""},{"line_number":18589,"context_line":"        with mock.patch.object(utils, \u0027disk_ops_semaphore\u0027,"},{"line_number":18590,"context_line":"                               new_callable\u003dcompute_utils.UnlimitedSemaphore):"},{"line_number":18591,"context_line":"            self.drvr._disk_raw_to_qcow2(path)"},{"line_number":18592,"context_line":"        mock_execute.assert_has_calls(["},{"line_number":18593,"context_line":"            mock.call(\u0027qemu-img\u0027, \u0027convert\u0027, \u0027-f\u0027, \u0027raw\u0027,"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_c40963db","line":18590,"range":{"start_line":18589,"start_character":8,"end_line":18590,"end_character":78},"in_reply_to":"3f79a3b5_dc7f0c14","updated":"2018-11-21 20:15:00.000000000","message":"Good to know, thanks. Assert added.","commit_id":"f571146bbec7862c92ff450868c840db72af5147"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3dec8e80ed8ca92843e4b7b8993646ba3aba3163","unresolved":false,"context_lines":[{"line_number":18582,"context_line":"        path \u003d \u0027/test/disk\u0027"},{"line_number":18583,"context_line":"        _path_qcow \u003d path + \u0027_qcow\u0027"},{"line_number":18584,"context_line":""},{"line_number":18585,"context_line":"        with mock.patch.object(compute_utils, \u0027disk_ops_semaphore\u0027,"},{"line_number":18586,"context_line":"                               new_callable\u003dcompute_utils.UnlimitedSemaphore):"},{"line_number":18587,"context_line":"            self.drvr._disk_raw_to_qcow2(path)"},{"line_number":18588,"context_line":"        mock_execute.assert_has_calls(["},{"line_number":18589,"context_line":"            mock.call(\u0027qemu-img\u0027, \u0027convert\u0027, \u0027-f\u0027, \u0027raw\u0027,"}],"source_content_type":"text/x-python","patch_set":7,"id":"3f79a3b5_ff5988ff","line":18586,"range":{"start_line":18585,"start_character":8,"end_line":18586,"end_character":78},"updated":"2018-11-21 19:31:25.000000000","message":"Just a thought, but rather than do this all over the place in the compute / virt related tests, we could write a fixture that stubs this out so we can use that fixture in the test class setup and not worry about it, but spot-check that it\u0027s called in the various methods we care about its usage.","commit_id":"75240f61dcaf3bab9e2b9ab78770c21ecf11f0c3"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e1a388d5f099d43833f7fe4fb4cce1bf927e7d54","unresolved":false,"context_lines":[{"line_number":18582,"context_line":"        path \u003d \u0027/test/disk\u0027"},{"line_number":18583,"context_line":"        _path_qcow \u003d path + \u0027_qcow\u0027"},{"line_number":18584,"context_line":""},{"line_number":18585,"context_line":"        with mock.patch.object(compute_utils, \u0027disk_ops_semaphore\u0027,"},{"line_number":18586,"context_line":"                               new_callable\u003dcompute_utils.UnlimitedSemaphore):"},{"line_number":18587,"context_line":"            self.drvr._disk_raw_to_qcow2(path)"},{"line_number":18588,"context_line":"        mock_execute.assert_has_calls(["},{"line_number":18589,"context_line":"            mock.call(\u0027qemu-img\u0027, \u0027convert\u0027, \u0027-f\u0027, \u0027raw\u0027,"}],"source_content_type":"text/x-python","patch_set":7,"id":"3f79a3b5_1fefa486","line":18586,"range":{"start_line":18585,"start_character":8,"end_line":18586,"end_character":78},"in_reply_to":"3f79a3b5_ff5988ff","updated":"2018-11-21 19:59:45.000000000","message":"Here is how I got this to work with an assertion:\n\ndiff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py\nindex 31128d8..105ff21 100644\n--- a/nova/tests/unit/virt/libvirt/test_driver.py\n+++ b/nova/tests/unit/virt/libvirt/test_driver.py\n@@ -18582,9 +18582,9 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin):\n         path \u003d \u0027/test/disk\u0027\n         _path_qcow \u003d path + \u0027_qcow\u0027\n \n-        with mock.patch.object(compute_utils, \u0027disk_ops_semaphore\u0027,\n-                               new_callable\u003dcompute_utils.UnlimitedSemaphore):\n+        with mock.patch.object(compute_utils, \u0027disk_ops_semaphore\u0027) as sema:\n             self.drvr._disk_raw_to_qcow2(path)\n+        sema.__enter__.assert_called_once()\n         mock_execute.assert_has_calls([\n             mock.call(\u0027qemu-img\u0027, \u0027convert\u0027, \u0027-f\u0027, \u0027raw\u0027,\n                       \u0027-O\u0027, \u0027qcow2\u0027, path, _path_qcow)])\n\n\n\nThe other thing that could potentially be done is adding a fixture which records calls to the various qemu-img and glance API download methods and if they are called in a test run, but they aren\u0027t wrapped in the semaphore context manager, they fail - otherwise we could be adding qemu-img convert code, or refactoring code, and lose the semaphore but we wouldn\u0027t realize it.","commit_id":"75240f61dcaf3bab9e2b9ab78770c21ecf11f0c3"}],"nova/tests/unit/virt/libvirt/test_imagebackend.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cee0cb7fc4b62ee433d4294bf4a4b3e2527fb315","unresolved":false,"context_lines":[{"line_number":678,"context_line":"                       return_value\u003dTEMPLATE_SIZE)"},{"line_number":679,"context_line":"    @mock.patch(\u0027nova.privsep.qemu.convert_image\u0027)"},{"line_number":680,"context_line":"    def _create_image(self, sparse, mock_convert_image, mock_get, mock_create,"},{"line_number":681,"context_line":"                      mock_ignored, mock_disk_op_sema):"},{"line_number":682,"context_line":"        fn \u003d mock.MagicMock()"},{"line_number":683,"context_line":""},{"line_number":684,"context_line":"        image \u003d self.image_class(self.INSTANCE, self.NAME)"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_55ebf10f","line":681,"range":{"start_line":681,"start_character":36,"end_line":681,"end_character":53},"updated":"2018-11-19 14:26:58.000000000","message":"Assert this is called for create_image in at least one of the tests in here. Maybe in the _create_image test method below?","commit_id":"f571146bbec7862c92ff450868c840db72af5147"},{"author":{"_account_id":28433,"name":"Jack Ding","email":"jackding@gmail.com","username":"jackding"},"change_message_id":"931cfdec793631a88f283e354cb345c7ad20ff1f","unresolved":false,"context_lines":[{"line_number":678,"context_line":"                       return_value\u003dTEMPLATE_SIZE)"},{"line_number":679,"context_line":"    @mock.patch(\u0027nova.privsep.qemu.convert_image\u0027)"},{"line_number":680,"context_line":"    def _create_image(self, sparse, mock_convert_image, mock_get, mock_create,"},{"line_number":681,"context_line":"                      mock_ignored, mock_disk_op_sema):"},{"line_number":682,"context_line":"        fn \u003d mock.MagicMock()"},{"line_number":683,"context_line":""},{"line_number":684,"context_line":"        image \u003d self.image_class(self.INSTANCE, self.NAME)"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_84efebf8","line":681,"range":{"start_line":681,"start_character":36,"end_line":681,"end_character":53},"in_reply_to":"3f79a3b5_55ebf10f","updated":"2018-11-21 20:15:00.000000000","message":"Done","commit_id":"f571146bbec7862c92ff450868c840db72af5147"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c903952f00b9ae1b6accb386394cc8f1a1a1d6ed","unresolved":false,"context_lines":[{"line_number":1035,"context_line":"            image \u003d self.image_class(self.INSTANCE, self.NAME)"},{"line_number":1036,"context_line":"            image.create_image(fn, self.TEMPLATE_PATH, self.SIZE,"},{"line_number":1037,"context_line":"                context\u003dself.CONTEXT)"},{"line_number":1038,"context_line":"            compute_utils.disk_ops_semaphore.__enter__.assert_called_once()"},{"line_number":1039,"context_line":"            fn.assert_called_with(context\u003dself.CONTEXT,"},{"line_number":1040,"context_line":"                                  target\u003dself.TEMPLATE_PATH)"},{"line_number":1041,"context_line":"            self.disk.get_disk_size.assert_called_with(self.TEMPLATE_PATH)"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f79a3b5_35a2bf99","line":1038,"range":{"start_line":1038,"start_character":12,"end_line":1038,"end_character":75},"updated":"2018-11-21 22:05:51.000000000","message":"This seems a bit dangerous given it\u0027s a global...","commit_id":"728f20e8f4ac2e3d4b893b7169b81d20471d0be9"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"bd936ffc0fbaff6dc141ce9c7b8300e4082f436f","unresolved":false,"context_lines":[{"line_number":1035,"context_line":"            image \u003d self.image_class(self.INSTANCE, self.NAME)"},{"line_number":1036,"context_line":"            image.create_image(fn, self.TEMPLATE_PATH, self.SIZE,"},{"line_number":1037,"context_line":"                context\u003dself.CONTEXT)"},{"line_number":1038,"context_line":"            compute_utils.disk_ops_semaphore.__enter__.assert_called_once()"},{"line_number":1039,"context_line":"            fn.assert_called_with(context\u003dself.CONTEXT,"},{"line_number":1040,"context_line":"                                  target\u003dself.TEMPLATE_PATH)"},{"line_number":1041,"context_line":"            self.disk.get_disk_size.assert_called_with(self.TEMPLATE_PATH)"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f79a3b5_6ad23f61","line":1038,"range":{"start_line":1038,"start_character":12,"end_line":1038,"end_character":75},"in_reply_to":"3f79a3b5_35a2bf99","updated":"2018-11-27 22:31:28.000000000","message":"Agree this looks weird, but I *think* it\u0027s actually okay. L1032 is creating an instance of Mock specific to this method, and compute_utils.disk_ops_semaphore is accessing that instance. It would be clearer if L1032 said \u0027... as mock_sema\u0027 and L1038 used mock_sema.__enter__.assert_called_once(), but (again, I *think*) the effect is the same.","commit_id":"728f20e8f4ac2e3d4b893b7169b81d20471d0be9"}],"nova/tests/unit/virt/libvirt/test_utils.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cee0cb7fc4b62ee433d4294bf4a4b3e2527fb315","unresolved":false,"context_lines":[{"line_number":520,"context_line":"            os.unlink(dst_path)"},{"line_number":521,"context_line":""},{"line_number":522,"context_line":"    @mock.patch.object(utils, \u0027disk_ops_semaphore\u0027)"},{"line_number":523,"context_line":"    def _do_test_extract_snapshot(self, mock_execute, mock_disk_op_sema,"},{"line_number":524,"context_line":"                                  src_format\u003d\u0027qcow2\u0027,"},{"line_number":525,"context_line":"                                  dest_format\u003d\u0027raw\u0027, out_format\u003d\u0027raw\u0027):"},{"line_number":526,"context_line":"        libvirt_utils.extract_snapshot(\u0027/path/to/disk/image\u0027, src_format,"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_d5b70122","line":523,"range":{"start_line":523,"start_character":54,"end_line":523,"end_character":71},"updated":"2018-11-19 14:26:58.000000000","message":"Assert this is called.","commit_id":"f571146bbec7862c92ff450868c840db72af5147"},{"author":{"_account_id":28433,"name":"Jack Ding","email":"jackding@gmail.com","username":"jackding"},"change_message_id":"931cfdec793631a88f283e354cb345c7ad20ff1f","unresolved":false,"context_lines":[{"line_number":520,"context_line":"            os.unlink(dst_path)"},{"line_number":521,"context_line":""},{"line_number":522,"context_line":"    @mock.patch.object(utils, \u0027disk_ops_semaphore\u0027)"},{"line_number":523,"context_line":"    def _do_test_extract_snapshot(self, mock_execute, mock_disk_op_sema,"},{"line_number":524,"context_line":"                                  src_format\u003d\u0027qcow2\u0027,"},{"line_number":525,"context_line":"                                  dest_format\u003d\u0027raw\u0027, out_format\u003d\u0027raw\u0027):"},{"line_number":526,"context_line":"        libvirt_utils.extract_snapshot(\u0027/path/to/disk/image\u0027, src_format,"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_64c5ef3d","line":523,"range":{"start_line":523,"start_character":54,"end_line":523,"end_character":71},"in_reply_to":"3f79a3b5_d5b70122","updated":"2018-11-21 20:15:00.000000000","message":"Done","commit_id":"f571146bbec7862c92ff450868c840db72af5147"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cee0cb7fc4b62ee433d4294bf4a4b3e2527fb315","unresolved":false,"context_lines":[{"line_number":695,"context_line":"        expected_commands \u003d [(\u0027rm\u0027, \u0027t.qcow2.part\u0027),"},{"line_number":696,"context_line":"                             (\u0027mv\u0027, \u0027t.qcow2.converted\u0027, \u0027t.qcow2\u0027)]"},{"line_number":697,"context_line":"        with mock.patch.object(utils, \u0027disk_ops_semaphore\u0027,"},{"line_number":698,"context_line":"                               new_callable\u003dcompute_utils.UnlimitedSemaphore):"},{"line_number":699,"context_line":"            images.fetch_to_raw(context, image_id, target)"},{"line_number":700,"context_line":"        self.assertEqual(self.executes, expected_commands)"},{"line_number":701,"context_line":"        mock_convert_image.assert_called_with("}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_952f4956","line":698,"updated":"2018-11-19 14:26:58.000000000","message":"same, assert this is called","commit_id":"f571146bbec7862c92ff450868c840db72af5147"},{"author":{"_account_id":28433,"name":"Jack Ding","email":"jackding@gmail.com","username":"jackding"},"change_message_id":"931cfdec793631a88f283e354cb345c7ad20ff1f","unresolved":false,"context_lines":[{"line_number":695,"context_line":"        expected_commands \u003d [(\u0027rm\u0027, \u0027t.qcow2.part\u0027),"},{"line_number":696,"context_line":"                             (\u0027mv\u0027, \u0027t.qcow2.converted\u0027, \u0027t.qcow2\u0027)]"},{"line_number":697,"context_line":"        with mock.patch.object(utils, \u0027disk_ops_semaphore\u0027,"},{"line_number":698,"context_line":"                               new_callable\u003dcompute_utils.UnlimitedSemaphore):"},{"line_number":699,"context_line":"            images.fetch_to_raw(context, image_id, target)"},{"line_number":700,"context_line":"        self.assertEqual(self.executes, expected_commands)"},{"line_number":701,"context_line":"        mock_convert_image.assert_called_with("}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_8435ab45","line":698,"in_reply_to":"3f79a3b5_952f4956","updated":"2018-11-21 20:15:00.000000000","message":"Done","commit_id":"f571146bbec7862c92ff450868c840db72af5147"}],"nova/tests/unit/virt/test_images.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cee0cb7fc4b62ee433d4294bf4a4b3e2527fb315","unresolved":false,"context_lines":[{"line_number":105,"context_line":"                               images.fetch_to_raw,"},{"line_number":106,"context_line":"                               None, \u0027href123\u0027, \u0027/no/path\u0027)"},{"line_number":107,"context_line":""},{"line_number":108,"context_line":"    @mock.patch.object(utils, \u0027disk_ops_semaphore\u0027,"},{"line_number":109,"context_line":"                       new_callable\u003dcompute_utils.UnlimitedSemaphore)"},{"line_number":110,"context_line":"    @mock.patch(\u0027nova.privsep.utils.supports_direct_io\u0027, return_value\u003dTrue)"},{"line_number":111,"context_line":"    @mock.patch(\u0027oslo_concurrency.processutils.execute\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_353e7588","line":108,"updated":"2018-11-19 14:26:58.000000000","message":"At least one of the tests that call _convert_image should assert the mock is called.","commit_id":"f571146bbec7862c92ff450868c840db72af5147"},{"author":{"_account_id":28433,"name":"Jack Ding","email":"jackding@gmail.com","username":"jackding"},"change_message_id":"931cfdec793631a88f283e354cb345c7ad20ff1f","unresolved":false,"context_lines":[{"line_number":105,"context_line":"                               images.fetch_to_raw,"},{"line_number":106,"context_line":"                               None, \u0027href123\u0027, \u0027/no/path\u0027)"},{"line_number":107,"context_line":""},{"line_number":108,"context_line":"    @mock.patch.object(utils, \u0027disk_ops_semaphore\u0027,"},{"line_number":109,"context_line":"                       new_callable\u003dcompute_utils.UnlimitedSemaphore)"},{"line_number":110,"context_line":"    @mock.patch(\u0027nova.privsep.utils.supports_direct_io\u0027, return_value\u003dTrue)"},{"line_number":111,"context_line":"    @mock.patch(\u0027oslo_concurrency.processutils.execute\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_df9fecaf","line":108,"in_reply_to":"3f79a3b5_353e7588","updated":"2018-11-21 20:15:00.000000000","message":"Done","commit_id":"f571146bbec7862c92ff450868c840db72af5147"}],"nova/tests/unit/virt/xenapi/test_vm_utils.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cee0cb7fc4b62ee433d4294bf4a4b3e2527fb315","unresolved":false,"context_lines":[{"line_number":153,"context_line":"        with mock.patch.object(six.moves.builtins, \u0027open\u0027) as mock_open:"},{"line_number":154,"context_line":"            mock_open.return_value.__enter__.return_value \u003d \u0027open_fd\u0027"},{"line_number":155,"context_line":"            with mock.patch.object("},{"line_number":156,"context_line":"                    utils, \u0027disk_ops_semaphore\u0027,"},{"line_number":157,"context_line":"                    new_callable\u003dcompute_utils.UnlimitedSemaphore):"},{"line_number":158,"context_line":"                vm_utils.generate_configdrive(\u0027session\u0027, \u0027context\u0027, \u0027instance\u0027,"},{"line_number":159,"context_line":"                                              \u0027vm_ref\u0027, \u0027userdevice\u0027,"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_d565c1b1","line":156,"updated":"2018-11-19 14:26:58.000000000","message":"Assert the mock is called.","commit_id":"f571146bbec7862c92ff450868c840db72af5147"},{"author":{"_account_id":28433,"name":"Jack Ding","email":"jackding@gmail.com","username":"jackding"},"change_message_id":"931cfdec793631a88f283e354cb345c7ad20ff1f","unresolved":false,"context_lines":[{"line_number":153,"context_line":"        with mock.patch.object(six.moves.builtins, \u0027open\u0027) as mock_open:"},{"line_number":154,"context_line":"            mock_open.return_value.__enter__.return_value \u003d \u0027open_fd\u0027"},{"line_number":155,"context_line":"            with mock.patch.object("},{"line_number":156,"context_line":"                    utils, \u0027disk_ops_semaphore\u0027,"},{"line_number":157,"context_line":"                    new_callable\u003dcompute_utils.UnlimitedSemaphore):"},{"line_number":158,"context_line":"                vm_utils.generate_configdrive(\u0027session\u0027, \u0027context\u0027, \u0027instance\u0027,"},{"line_number":159,"context_line":"                                              \u0027vm_ref\u0027, \u0027userdevice\u0027,"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_df228c53","line":156,"in_reply_to":"3f79a3b5_d565c1b1","updated":"2018-11-21 20:15:00.000000000","message":"Done","commit_id":"f571146bbec7862c92ff450868c840db72af5147"}],"nova/utils.py":[{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"9aa2eb802dd97130b18c609a64f80255ef3b8b10","unresolved":false,"context_lines":[{"line_number":93,"context_line":"_SERVICE_TYPES \u003d service_types.ServiceTypes()"},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"# This semaphore is used to enforce a limit on disk-IO-intensive operations"},{"line_number":96,"context_line":"# (image downloads, image conversions) at any given time."},{"line_number":97,"context_line":"disk_ops_semaphore \u003d None"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_c65ec4e7","line":96,"updated":"2018-10-17 16:57:13.000000000","message":"maybe add a mention of where the value will be initialized?","commit_id":"c2aaebd3a3595b8f685e63d414e7674bf9ae024b"},{"author":{"_account_id":28433,"name":"Jack Ding","email":"jackding@gmail.com","username":"jackding"},"change_message_id":"285ffe1209bff45cde99c9139aca467a05d0c12c","unresolved":false,"context_lines":[{"line_number":93,"context_line":"_SERVICE_TYPES \u003d service_types.ServiceTypes()"},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"# This semaphore is used to enforce a limit on disk-IO-intensive operations"},{"line_number":96,"context_line":"# (image downloads, image conversions) at any given time."},{"line_number":97,"context_line":"disk_ops_semaphore \u003d None"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_69d6a329","line":96,"in_reply_to":"3f79a3b5_c65ec4e7","updated":"2018-10-17 18:24:38.000000000","message":"Done","commit_id":"c2aaebd3a3595b8f685e63d414e7674bf9ae024b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cee0cb7fc4b62ee433d4294bf4a4b3e2527fb315","unresolved":false,"context_lines":[{"line_number":95,"context_line":"# This semaphore is used to enforce a limit on disk-IO-intensive operations"},{"line_number":96,"context_line":"# (image downloads, image conversions) at any given time."},{"line_number":97,"context_line":"# It is initialized at ComputeManager.init_host()"},{"line_number":98,"context_line":"disk_ops_semaphore \u003d None"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"if hasattr(inspect, \u0027getfullargspec\u0027):"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_550771c9","line":98,"updated":"2018-11-19 14:26:58.000000000","message":"Why not put this in nova.compute.utils since it\u0027s compute-specific? Again, assume this doesn\u0027t already exist in StarlingX and is new for nova (which it is).","commit_id":"f571146bbec7862c92ff450868c840db72af5147"},{"author":{"_account_id":28433,"name":"Jack Ding","email":"jackding@gmail.com","username":"jackding"},"change_message_id":"53e33f0f44f4e5f78dce0b43afffcd94f7c9100a","unresolved":false,"context_lines":[{"line_number":95,"context_line":"# This semaphore is used to enforce a limit on disk-IO-intensive operations"},{"line_number":96,"context_line":"# (image downloads, image conversions) at any given time."},{"line_number":97,"context_line":"# It is initialized at ComputeManager.init_host()"},{"line_number":98,"context_line":"disk_ops_semaphore \u003d None"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"if hasattr(inspect, \u0027getfullargspec\u0027):"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_609d066a","line":98,"in_reply_to":"3f79a3b5_550771c9","updated":"2018-11-20 15:42:09.000000000","message":"Done","commit_id":"f571146bbec7862c92ff450868c840db72af5147"}],"nova/virt/images.py":[{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"9aa2eb802dd97130b18c609a64f80255ef3b8b10","unresolved":false,"context_lines":[{"line_number":119,"context_line":"def _convert_image(source, dest, in_format, out_format, run_as_root):"},{"line_number":120,"context_line":"    try:"},{"line_number":121,"context_line":"        if not run_as_root:"},{"line_number":122,"context_line":"            with utils.disk_ops_semaphore:"},{"line_number":123,"context_line":"                nova.privsep.qemu.unprivileged_convert_image("},{"line_number":124,"context_line":"                    source, dest, in_format, out_format, CONF.instances_path)"},{"line_number":125,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_86dbec4c","line":122,"updated":"2018-10-17 16:57:13.000000000","message":"if you move this a line earlier you\u0027d only need one call to \"with utils.disk_ops_semaphore:\"","commit_id":"c2aaebd3a3595b8f685e63d414e7674bf9ae024b"},{"author":{"_account_id":28433,"name":"Jack Ding","email":"jackding@gmail.com","username":"jackding"},"change_message_id":"285ffe1209bff45cde99c9139aca467a05d0c12c","unresolved":false,"context_lines":[{"line_number":119,"context_line":"def _convert_image(source, dest, in_format, out_format, run_as_root):"},{"line_number":120,"context_line":"    try:"},{"line_number":121,"context_line":"        if not run_as_root:"},{"line_number":122,"context_line":"            with utils.disk_ops_semaphore:"},{"line_number":123,"context_line":"                nova.privsep.qemu.unprivileged_convert_image("},{"line_number":124,"context_line":"                    source, dest, in_format, out_format, CONF.instances_path)"},{"line_number":125,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_acb7e988","line":122,"in_reply_to":"3f79a3b5_86dbec4c","updated":"2018-10-17 18:24:38.000000000","message":"Done","commit_id":"c2aaebd3a3595b8f685e63d414e7674bf9ae024b"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cee0cb7fc4b62ee433d4294bf4a4b3e2527fb315","unresolved":false,"context_lines":[{"line_number":1967,"context_line":"                update_task_state(task_state\u003dtask_states.IMAGE_UPLOADING,"},{"line_number":1968,"context_line":"                        expected_state\u003dtask_states.IMAGE_PENDING_UPLOAD)"},{"line_number":1969,"context_line":"                with libvirt_utils.file_open(out_path, \u0027rb\u0027) as image_file:"},{"line_number":1970,"context_line":"                    self._image_api.update(context,"},{"line_number":1971,"context_line":"                                           image_id,"},{"line_number":1972,"context_line":"                                           metadata,"},{"line_number":1973,"context_line":"                                           image_file)"},{"line_number":1974,"context_line":"        except Exception:"},{"line_number":1975,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":1976,"context_line":"                LOG.exception(_(\"Failed to snapshot image\"))"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_d5138101","line":1973,"range":{"start_line":1970,"start_character":20,"end_line":1973,"end_character":54},"updated":"2018-11-19 14:26:58.000000000","message":"Should uploading an image file to glance be in the semaphore?","commit_id":"f571146bbec7862c92ff450868c840db72af5147"},{"author":{"_account_id":28433,"name":"Jack Ding","email":"jackding@gmail.com","username":"jackding"},"change_message_id":"53e33f0f44f4e5f78dce0b43afffcd94f7c9100a","unresolved":false,"context_lines":[{"line_number":1967,"context_line":"                update_task_state(task_state\u003dtask_states.IMAGE_UPLOADING,"},{"line_number":1968,"context_line":"                        expected_state\u003dtask_states.IMAGE_PENDING_UPLOAD)"},{"line_number":1969,"context_line":"                with libvirt_utils.file_open(out_path, \u0027rb\u0027) as image_file:"},{"line_number":1970,"context_line":"                    self._image_api.update(context,"},{"line_number":1971,"context_line":"                                           image_id,"},{"line_number":1972,"context_line":"                                           metadata,"},{"line_number":1973,"context_line":"                                           image_file)"},{"line_number":1974,"context_line":"        except Exception:"},{"line_number":1975,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":1976,"context_line":"                LOG.exception(_(\"Failed to snapshot image\"))"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_801aa2c4","line":1973,"range":{"start_line":1970,"start_character":20,"end_line":1973,"end_character":54},"in_reply_to":"3f79a3b5_d5138101","updated":"2018-11-20 15:42:09.000000000","message":"Done","commit_id":"f571146bbec7862c92ff450868c840db72af5147"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cee0cb7fc4b62ee433d4294bf4a4b3e2527fb315","unresolved":false,"context_lines":[{"line_number":2425,"context_line":"            qemu_img_extra_arg \u003d [\u0027-F\u0027, b_file_fmt]"},{"line_number":2426,"context_line":""},{"line_number":2427,"context_line":"        qemu_img_extra_arg.append(active_disk_object.source_path)"},{"line_number":2428,"context_line":"        utils.execute(\"qemu-img\", \"rebase\", \"-b\", backing_file,"},{"line_number":2429,"context_line":"                      *qemu_img_extra_arg)"},{"line_number":2430,"context_line":""},{"line_number":2431,"context_line":"    def _volume_snapshot_delete(self, context, instance, volume_id,"},{"line_number":2432,"context_line":"                                snapshot_id, delete_info\u003dNone):"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_9526696a","line":2429,"range":{"start_line":2428,"start_character":8,"end_line":2429,"end_character":42},"updated":"2018-11-19 14:26:58.000000000","message":"What about putting this in the semaphore?","commit_id":"f571146bbec7862c92ff450868c840db72af5147"},{"author":{"_account_id":28433,"name":"Jack Ding","email":"jackding@gmail.com","username":"jackding"},"change_message_id":"53e33f0f44f4e5f78dce0b43afffcd94f7c9100a","unresolved":false,"context_lines":[{"line_number":2425,"context_line":"            qemu_img_extra_arg \u003d [\u0027-F\u0027, b_file_fmt]"},{"line_number":2426,"context_line":""},{"line_number":2427,"context_line":"        qemu_img_extra_arg.append(active_disk_object.source_path)"},{"line_number":2428,"context_line":"        utils.execute(\"qemu-img\", \"rebase\", \"-b\", backing_file,"},{"line_number":2429,"context_line":"                      *qemu_img_extra_arg)"},{"line_number":2430,"context_line":""},{"line_number":2431,"context_line":"    def _volume_snapshot_delete(self, context, instance, volume_id,"},{"line_number":2432,"context_line":"                                snapshot_id, delete_info\u003dNone):"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_207f0edf","line":2429,"range":{"start_line":2428,"start_character":8,"end_line":2429,"end_character":42},"in_reply_to":"3f79a3b5_9526696a","updated":"2018-11-20 15:42:09.000000000","message":"Done","commit_id":"f571146bbec7862c92ff450868c840db72af5147"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c903952f00b9ae1b6accb386394cc8f1a1a1d6ed","unresolved":false,"context_lines":[{"line_number":8367,"context_line":"        path_qcow \u003d path + \u0027_qcow\u0027"},{"line_number":8368,"context_line":"        # execute operation with disk concurrency semaphore"},{"line_number":8369,"context_line":"        with compute_utils.disk_ops_semaphore:"},{"line_number":8370,"context_line":"            utils.execute(\u0027qemu-img\u0027, \u0027convert\u0027, \u0027-f\u0027, \u0027raw\u0027,"},{"line_number":8371,"context_line":"                          \u0027-O\u0027, \u0027qcow2\u0027, path, path_qcow)"},{"line_number":8372,"context_line":"        os.rename(path_qcow, path)"},{"line_number":8373,"context_line":""},{"line_number":8374,"context_line":"    def finish_migration(self, context, migration, instance, disk_info,"}],"source_content_type":"text/x-python","patch_set":7,"id":"3f79a3b5_3f7620d8","line":8371,"range":{"start_line":8370,"start_character":12,"end_line":8371,"end_character":57},"updated":"2018-11-21 22:05:51.000000000","message":"Hmm, not really related to this change, but I wonder why this isn\u0027t going through nova.privsep.qemu.convert_image?","commit_id":"75240f61dcaf3bab9e2b9ab78770c21ecf11f0c3"},{"author":{"_account_id":28433,"name":"Jack Ding","email":"jackding@gmail.com","username":"jackding"},"change_message_id":"6643cd57af84fe2d5e6e67dfb1791c38e2bf397a","unresolved":false,"context_lines":[{"line_number":8367,"context_line":"        path_qcow \u003d path + \u0027_qcow\u0027"},{"line_number":8368,"context_line":"        # execute operation with disk concurrency semaphore"},{"line_number":8369,"context_line":"        with compute_utils.disk_ops_semaphore:"},{"line_number":8370,"context_line":"            utils.execute(\u0027qemu-img\u0027, \u0027convert\u0027, \u0027-f\u0027, \u0027raw\u0027,"},{"line_number":8371,"context_line":"                          \u0027-O\u0027, \u0027qcow2\u0027, path, path_qcow)"},{"line_number":8372,"context_line":"        os.rename(path_qcow, path)"},{"line_number":8373,"context_line":""},{"line_number":8374,"context_line":"    def finish_migration(self, context, migration, instance, disk_info,"}],"source_content_type":"text/x-python","patch_set":7,"id":"3f79a3b5_79222604","line":8371,"range":{"start_line":8370,"start_character":12,"end_line":8371,"end_character":57},"in_reply_to":"3f79a3b5_2c0fa08b","updated":"2018-11-27 15:42:02.000000000","message":"the race is on :)","commit_id":"75240f61dcaf3bab9e2b9ab78770c21ecf11f0c3"},{"author":{"_account_id":28433,"name":"Jack Ding","email":"jackding@gmail.com","username":"jackding"},"change_message_id":"99ef24fea8190795097569320054560e9cd0b636","unresolved":false,"context_lines":[{"line_number":8367,"context_line":"        path_qcow \u003d path + \u0027_qcow\u0027"},{"line_number":8368,"context_line":"        # execute operation with disk concurrency semaphore"},{"line_number":8369,"context_line":"        with compute_utils.disk_ops_semaphore:"},{"line_number":8370,"context_line":"            utils.execute(\u0027qemu-img\u0027, \u0027convert\u0027, \u0027-f\u0027, \u0027raw\u0027,"},{"line_number":8371,"context_line":"                          \u0027-O\u0027, \u0027qcow2\u0027, path, path_qcow)"},{"line_number":8372,"context_line":"        os.rename(path_qcow, path)"},{"line_number":8373,"context_line":""},{"line_number":8374,"context_line":"    def finish_migration(self, context, migration, instance, disk_info,"}],"source_content_type":"text/x-python","patch_set":7,"id":"3f79a3b5_d3efd9f7","line":8371,"range":{"start_line":8370,"start_character":12,"end_line":8371,"end_character":57},"in_reply_to":"3f79a3b5_3f7620d8","updated":"2018-11-22 14:48:49.000000000","message":"\u003e Hmm, not really related to this change, but I wonder why this isn\u0027t\n \u003e going through nova.privsep.qemu.convert_image?\n\nI agree it should. I have another review open to do exactly this: https://review.openstack.org/#/c/616692/6/nova/virt/libvirt/driver.py","commit_id":"75240f61dcaf3bab9e2b9ab78770c21ecf11f0c3"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"67e5a4f261e67bc699efce66a3f8bf3e2fbdff8d","unresolved":false,"context_lines":[{"line_number":8367,"context_line":"        path_qcow \u003d path + \u0027_qcow\u0027"},{"line_number":8368,"context_line":"        # execute operation with disk concurrency semaphore"},{"line_number":8369,"context_line":"        with compute_utils.disk_ops_semaphore:"},{"line_number":8370,"context_line":"            utils.execute(\u0027qemu-img\u0027, \u0027convert\u0027, \u0027-f\u0027, \u0027raw\u0027,"},{"line_number":8371,"context_line":"                          \u0027-O\u0027, \u0027qcow2\u0027, path, path_qcow)"},{"line_number":8372,"context_line":"        os.rename(path_qcow, path)"},{"line_number":8373,"context_line":""},{"line_number":8374,"context_line":"    def finish_migration(self, context, migration, instance, disk_info,"}],"source_content_type":"text/x-python","patch_set":7,"id":"3f79a3b5_2c0fa08b","line":8371,"range":{"start_line":8370,"start_character":12,"end_line":8371,"end_character":57},"in_reply_to":"3f79a3b5_d3efd9f7","updated":"2018-11-26 23:03:37.000000000","message":"What a coincidence: https://review.openstack.org/#/c/619705/","commit_id":"75240f61dcaf3bab9e2b9ab78770c21ecf11f0c3"}],"nova/virt/libvirt/utils.py":[{"author":{"_account_id":6062,"name":"jichenjc","email":"jichenjc@cn.ibm.com","username":"jichenjc"},"change_message_id":"83eea4341e2c0203e8ecc2f21e8d1dad2a3fb07b","unresolved":false,"context_lines":[{"line_number":93,"context_line":"                 M for Mebibytes, \u0027G\u0027 for Gibibytes, \u0027T\u0027 for Tebibytes)."},{"line_number":94,"context_line":"                 If no suffix is given, it will be interpreted as bytes."},{"line_number":95,"context_line":"    \"\"\""},{"line_number":96,"context_line":"    utils.execute(\u0027qemu-img\u0027, \u0027create\u0027, \u0027-f\u0027, disk_format, path, size)"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"def create_cow_image(backing_file, path, size\u003dNone):"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f79a3b5_990c2d3b","line":96,"range":{"start_line":96,"start_character":4,"end_line":96,"end_character":70},"updated":"2018-11-22 03:04:27.000000000","message":"this?","commit_id":"728f20e8f4ac2e3d4b893b7169b81d20471d0be9"},{"author":{"_account_id":28433,"name":"Jack Ding","email":"jackding@gmail.com","username":"jackding"},"change_message_id":"113516fac008b79e7e071ebc65007fc32821f149","unresolved":false,"context_lines":[{"line_number":93,"context_line":"                 M for Mebibytes, \u0027G\u0027 for Gibibytes, \u0027T\u0027 for Tebibytes)."},{"line_number":94,"context_line":"                 If no suffix is given, it will be interpreted as bytes."},{"line_number":95,"context_line":"    \"\"\""},{"line_number":96,"context_line":"    utils.execute(\u0027qemu-img\u0027, \u0027create\u0027, \u0027-f\u0027, disk_format, path, size)"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"def create_cow_image(backing_file, path, size\u003dNone):"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f79a3b5_f3a0d5a5","line":96,"range":{"start_line":96,"start_character":4,"end_line":96,"end_character":70},"in_reply_to":"3f79a3b5_990c2d3b","updated":"2018-11-22 15:00:06.000000000","message":"In our tests image creations did not cause disk contention, especially if it is copy-on-write.","commit_id":"728f20e8f4ac2e3d4b893b7169b81d20471d0be9"},{"author":{"_account_id":6062,"name":"jichenjc","email":"jichenjc@cn.ibm.com","username":"jichenjc"},"change_message_id":"83eea4341e2c0203e8ecc2f21e8d1dad2a3fb07b","unresolved":false,"context_lines":[{"line_number":125,"context_line":"        csv_opts \u003d \",\".join(cow_opts)"},{"line_number":126,"context_line":"        cow_opts \u003d [\u0027-o\u0027, csv_opts]"},{"line_number":127,"context_line":"    cmd \u003d base_cmd + cow_opts + [path]"},{"line_number":128,"context_line":"    utils.execute(*cmd)"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"def create_ploop_image(disk_format, path, size, fs_type):"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f79a3b5_f903e109","line":128,"range":{"start_line":128,"start_character":4,"end_line":128,"end_character":23},"updated":"2018-11-22 03:04:27.000000000","message":"how about this?","commit_id":"728f20e8f4ac2e3d4b893b7169b81d20471d0be9"}],"releasenotes/notes/bp-io-semaphore-for-concurrent-disk-ops-690890c9f01fa18c.yaml":[{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"043f44ae1ffcbc7bb213f6e9ee41afdd6e4c5476","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Introduce a new config option max_concurrent_disk_ops that can be set in"},{"line_number":5,"context_line":"    nova.conf per compute host and would be virt-driver-agnostic."},{"line_number":6,"context_line":"    It is default to 0 which means no limit."},{"line_number":7,"context_line":"    When configured with a number greater than 0, an I/O semaphore is used to"},{"line_number":8,"context_line":"    limit the number of concurrent disk-IO-intensive operations. This could"},{"line_number":9,"context_line":"    reduce disk contention from image operations like image download, image"},{"line_number":10,"context_line":"    format conversion, snapshot extraction, etc."}],"source_content_type":"text/x-yaml","patch_set":5,"id":"3f79a3b5_bb014f06","line":10,"range":{"start_line":4,"start_character":4,"end_line":10,"end_character":48},"updated":"2018-10-31 15:29:46.000000000","message":"I think it\u0027d be clearer like this:\n\nIntroduced a new config option \u0027max_concurrent_disk_ops\u0027 to reduce disk contention by specifying the maximum number of concurrent disk-IO-intensive operations per compute node.  This would include operations such as image download, image format conversion, snapshot extraction, etc.  The default value is 0, which means that there is no limit.","commit_id":"ceddd841832423b963cccbe1ff085a6d1ad75a00"},{"author":{"_account_id":28433,"name":"Jack Ding","email":"jackding@gmail.com","username":"jackding"},"change_message_id":"cb83c5cdb2f2d4a054bc0f7a733d1472b179ecdf","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Introduce a new config option max_concurrent_disk_ops that can be set in"},{"line_number":5,"context_line":"    nova.conf per compute host and would be virt-driver-agnostic."},{"line_number":6,"context_line":"    It is default to 0 which means no limit."},{"line_number":7,"context_line":"    When configured with a number greater than 0, an I/O semaphore is used to"},{"line_number":8,"context_line":"    limit the number of concurrent disk-IO-intensive operations. This could"},{"line_number":9,"context_line":"    reduce disk contention from image operations like image download, image"},{"line_number":10,"context_line":"    format conversion, snapshot extraction, etc."}],"source_content_type":"text/x-yaml","patch_set":5,"id":"3f79a3b5_e106381c","line":10,"range":{"start_line":4,"start_character":4,"end_line":10,"end_character":48},"in_reply_to":"3f79a3b5_bb014f06","updated":"2018-10-31 16:46:39.000000000","message":"Done","commit_id":"ceddd841832423b963cccbe1ff085a6d1ad75a00"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cee0cb7fc4b62ee433d4294bf4a4b3e2527fb315","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Introduced a new config option \u0027max_concurrent_disk_ops\u0027 to reduce disk"},{"line_number":5,"context_line":"    contention by specifying the maximum number of concurrent"},{"line_number":6,"context_line":"    disk-IO-intensive operations per compute node.  This would include"},{"line_number":7,"context_line":"    operations such as image download, image format conversion, snapshot"}],"source_content_type":"text/x-yaml","patch_set":6,"id":"3f79a3b5_b5b7e5f7","line":4,"range":{"start_line":4,"start_character":35,"end_line":4,"end_character":60},"updated":"2018-11-19 14:26:58.000000000","message":"Use double `` for formatting of the option, and include the group it\u0027s in, e.g. ``[compute]/max_concurrent_disk_ops``.","commit_id":"f571146bbec7862c92ff450868c840db72af5147"},{"author":{"_account_id":28433,"name":"Jack Ding","email":"jackding@gmail.com","username":"jackding"},"change_message_id":"53e33f0f44f4e5f78dce0b43afffcd94f7c9100a","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Introduced a new config option \u0027max_concurrent_disk_ops\u0027 to reduce disk"},{"line_number":5,"context_line":"    contention by specifying the maximum number of concurrent"},{"line_number":6,"context_line":"    disk-IO-intensive operations per compute node.  This would include"},{"line_number":7,"context_line":"    operations such as image download, image format conversion, snapshot"}],"source_content_type":"text/x-yaml","patch_set":6,"id":"3f79a3b5_e91b41c9","line":4,"range":{"start_line":4,"start_character":35,"end_line":4,"end_character":60},"in_reply_to":"3f79a3b5_b5b7e5f7","updated":"2018-11-20 15:42:09.000000000","message":"Done","commit_id":"f571146bbec7862c92ff450868c840db72af5147"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cee0cb7fc4b62ee433d4294bf4a4b3e2527fb315","unresolved":false,"context_lines":[{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Introduced a new config option \u0027max_concurrent_disk_ops\u0027 to reduce disk"},{"line_number":5,"context_line":"    contention by specifying the maximum number of concurrent"},{"line_number":6,"context_line":"    disk-IO-intensive operations per compute node.  This would include"},{"line_number":7,"context_line":"    operations such as image download, image format conversion, snapshot"},{"line_number":8,"context_line":"    extraction, etc."},{"line_number":9,"context_line":"    The default value is 0, which means that there is no limit."}],"source_content_type":"text/x-yaml","patch_set":6,"id":"3f79a3b5_55ae91a1","line":6,"range":{"start_line":6,"start_character":45,"end_line":6,"end_character":49},"updated":"2018-11-19 14:26:58.000000000","message":"service","commit_id":"f571146bbec7862c92ff450868c840db72af5147"},{"author":{"_account_id":28433,"name":"Jack Ding","email":"jackding@gmail.com","username":"jackding"},"change_message_id":"53e33f0f44f4e5f78dce0b43afffcd94f7c9100a","unresolved":false,"context_lines":[{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Introduced a new config option \u0027max_concurrent_disk_ops\u0027 to reduce disk"},{"line_number":5,"context_line":"    contention by specifying the maximum number of concurrent"},{"line_number":6,"context_line":"    disk-IO-intensive operations per compute node.  This would include"},{"line_number":7,"context_line":"    operations such as image download, image format conversion, snapshot"},{"line_number":8,"context_line":"    extraction, etc."},{"line_number":9,"context_line":"    The default value is 0, which means that there is no limit."}],"source_content_type":"text/x-yaml","patch_set":6,"id":"3f79a3b5_891acdc4","line":6,"range":{"start_line":6,"start_character":45,"end_line":6,"end_character":49},"in_reply_to":"3f79a3b5_55ae91a1","updated":"2018-11-20 15:42:09.000000000","message":"Done","commit_id":"f571146bbec7862c92ff450868c840db72af5147"}]}