)]}'
{"nova/tests/unit/virt/libvirt/test_imagecache.py":[{"author":{"_account_id":15524,"name":"dane-fichter","email":"dane.fichter@jhuapl.edu","username":"dane-fichter"},"change_message_id":"272b73a130a1031c80341925b8f42e5db6c4dc8d","unresolved":false,"context_lines":[{"line_number":1013,"context_line":"    def setUp(self):"},{"line_number":1014,"context_line":"        # Ensure that any previous ImageCacheLocalDir singleton is replaced"},{"line_number":1015,"context_line":"        # so config will be re-read."},{"line_number":1016,"context_line":"        reload(imagecache)"},{"line_number":1017,"context_line":""},{"line_number":1018,"context_line":"        super(ImageCacheLocalDirTestCase, self).setUp()"},{"line_number":1019,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"bab6814e_ccffaea8","line":1016,"updated":"2016-05-19 21:27:30.000000000","message":"This reload() call appears to be the offending statement in your unit test failures.","commit_id":"bfb929ee9dde14defee76cc9fa1d4d9fa7b82124"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"4324628871b17dc67c5446998dc1569f6bcc3d66","unresolved":false,"context_lines":[{"line_number":1013,"context_line":"    def setUp(self):"},{"line_number":1014,"context_line":"        # Ensure that any previous ImageCacheLocalDir singleton is replaced"},{"line_number":1015,"context_line":"        # so config will be re-read."},{"line_number":1016,"context_line":"        reload(imagecache)"},{"line_number":1017,"context_line":""},{"line_number":1018,"context_line":"        super(ImageCacheLocalDirTestCase, self).setUp()"},{"line_number":1019,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"bab6814e_ab77b739","line":1016,"in_reply_to":"bab6814e_ccffaea8","updated":"2016-05-25 16:19:03.000000000","message":"Yep, it was, thanks. Removed.","commit_id":"bfb929ee9dde14defee76cc9fa1d4d9fa7b82124"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"92e30a36642131bc9bf350aa9d11d93ae8684e07","unresolved":false,"context_lines":[{"line_number":1201,"context_line":"    def test_get_func_output_path_exists(self, mock_update_mtime):"},{"line_number":1202,"context_line":"        # Calling get_func_output_path when the file exists should update its"},{"line_number":1203,"context_line":"        # mtime and not raise any exceptions"},{"line_number":1204,"context_line":"        fallback_host \u003d uuidutils.generate_uuid()"},{"line_number":1205,"context_line":"        self.existing_paths.add(self.expected_path)"},{"line_number":1206,"context_line":""},{"line_number":1207,"context_line":"        mock_create \u003d mock.Mock()"}],"source_content_type":"text/x-python","patch_set":13,"id":"bab6814e_38535be3","line":1204,"range":{"start_line":1204,"start_character":24,"end_line":1204,"end_character":49},"updated":"2016-05-18 19:14:10.000000000","message":"why not mock.sentinel...?","commit_id":"bfb929ee9dde14defee76cc9fa1d4d9fa7b82124"},{"author":{"_account_id":15524,"name":"dane-fichter","email":"dane.fichter@jhuapl.edu","username":"dane-fichter"},"change_message_id":"d8f8a979128c184a0d8f5a3156b3a746e51c3dd6","unresolved":false,"context_lines":[{"line_number":775,"context_line":"        # Ensure we get a new singleton with each test run"},{"line_number":776,"context_line":"        imagecache.ImageCacheLocalDir._singleton \u003d None"},{"line_number":777,"context_line":""},{"line_number":778,"context_line":"        self.image_id \u003d \u0027e557355c-19e9-4749-837a-87a222610fea\u0027"},{"line_number":779,"context_line":"        self.image_hash \u003d \u002787ee9ef6f36612843bc8e61eb4ab6a1593380891\u0027"},{"line_number":780,"context_line":""},{"line_number":781,"context_line":"        self.instances_path \u003d \u0027/instances\u0027"}],"source_content_type":"text/x-python","patch_set":14,"id":"bab6814e_4baedd2e","line":778,"updated":"2016-05-26 13:00:03.000000000","message":"I think if you explicitly encode this string, you\u0027ll resolve your python3 errors. When you\u0027re getting the cache filename using this ID, you\u0027re doing a sha1 hash of this ID, and this is failing with a type error since hashlib doesn\u0027t know what encoding to use.","commit_id":"8cb61f6d14cf7d7db01f64ec6d6d04f63f38e12b"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"6ac84a6d6d8c877134d8b468289d8bf0433e8141","unresolved":false,"context_lines":[{"line_number":775,"context_line":"        # Ensure we get a new singleton with each test run"},{"line_number":776,"context_line":"        imagecache.ImageCacheLocalDir._singleton \u003d None"},{"line_number":777,"context_line":""},{"line_number":778,"context_line":"        self.image_id \u003d \u0027e557355c-19e9-4749-837a-87a222610fea\u0027"},{"line_number":779,"context_line":"        self.image_hash \u003d \u002787ee9ef6f36612843bc8e61eb4ab6a1593380891\u0027"},{"line_number":780,"context_line":""},{"line_number":781,"context_line":"        self.instances_path \u003d \u0027/instances\u0027"}],"source_content_type":"text/x-python","patch_set":14,"id":"bab6814e_d9a6200e","line":778,"in_reply_to":"bab6814e_4baedd2e","updated":"2016-05-26 14:34:24.000000000","message":"It\u0027s related. I\u0027ve actually just added a prior patch which fixes it by calling encode() before sha1(), which is the real issue. sha1() wants bytes in python3, so it doesn\u0027t take unicode either.\n\nAlthough it turns out this isn\u0027t necessary anyway. I misread the original code, and shouldn\u0027t be hashing the name in get_func_output_path(). It\u0027s hopefully fixed now, anyway.","commit_id":"8cb61f6d14cf7d7db01f64ec6d6d04f63f38e12b"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"504f4dfd92811fba6f86cfcbc3221f0cb5b38d0f","unresolved":false,"context_lines":[{"line_number":854,"context_line":"        return path in self.existing_paths"},{"line_number":855,"context_line":""},{"line_number":856,"context_line":"    @contextlib.contextmanager"},{"line_number":857,"context_line":"    def default_mocks(self):"},{"line_number":858,"context_line":"        with mock.patch.object(utils, \u0027synchronized\u0027, autospec\u003dTrue,"},{"line_number":859,"context_line":"                               return_value\u003dself.fake_synchronized) \\"},{"line_number":860,"context_line":"                as self.mock_synchronized, \\"},{"line_number":861,"context_line":"                mock.patch.object(os.path, \u0027exists\u0027, autospec\u003dTrue,"},{"line_number":862,"context_line":"                                  side_effect\u003dself.fake_exists) \\"},{"line_number":863,"context_line":"                        as self.mock_exists, \\"},{"line_number":864,"context_line":"                mock.patch.object(images, \u0027qemu_img_info\u0027, autospec\u003dTrue) \\"},{"line_number":865,"context_line":"                        as self.mock_qemu_img_info:"},{"line_number":866,"context_line":"            yield"},{"line_number":867,"context_line":""},{"line_number":868,"context_line":"    def assert_qemu_img_info(self, result):"},{"line_number":869,"context_line":"        # Check that result corresponds to the return of qemu_img_info"}],"source_content_type":"text/x-python","patch_set":19,"id":"7aa08908_635744ca","line":866,"range":{"start_line":857,"start_character":0,"end_line":866,"end_character":17},"updated":"2016-06-08 20:47:07.000000000","message":"I wonder if something like the following isn\u0027t easier to read:\n\n\n    @contextlib.contextmanager\n    def default_mocks(self):\n        with test.nested(\n            mock.patch.object(utils, \u0027synchronized\u0027, autospec\u003dTrue),\n            mock.patch.object(os.path, \u0027exists\u0027, autospec\u003dTrue),\n            mock.patch.object(images, \u0027qemu_img_info\u0027, autospec\u003dTrue),\n        ) as (self.mock_synchronized, self.mock_exists, self.mock_qemu_info):\n            self.mock_synchronized.return_value \u003d self.fake_synchronized\n            self.mock_exists.side_effect \u003d self.fake_exists\n            yield","commit_id":"773feef5e4bc5208b9eb2def54e1c6dd732aca41"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"279d64e0a0cf7b5f5026d0abe7deeae9ee13f648","unresolved":false,"context_lines":[{"line_number":854,"context_line":"        return path in self.existing_paths"},{"line_number":855,"context_line":""},{"line_number":856,"context_line":"    @contextlib.contextmanager"},{"line_number":857,"context_line":"    def default_mocks(self):"},{"line_number":858,"context_line":"        with mock.patch.object(utils, \u0027synchronized\u0027, autospec\u003dTrue,"},{"line_number":859,"context_line":"                               return_value\u003dself.fake_synchronized) \\"},{"line_number":860,"context_line":"                as self.mock_synchronized, \\"},{"line_number":861,"context_line":"                mock.patch.object(os.path, \u0027exists\u0027, autospec\u003dTrue,"},{"line_number":862,"context_line":"                                  side_effect\u003dself.fake_exists) \\"},{"line_number":863,"context_line":"                        as self.mock_exists, \\"},{"line_number":864,"context_line":"                mock.patch.object(images, \u0027qemu_img_info\u0027, autospec\u003dTrue) \\"},{"line_number":865,"context_line":"                        as self.mock_qemu_img_info:"},{"line_number":866,"context_line":"            yield"},{"line_number":867,"context_line":""},{"line_number":868,"context_line":"    def assert_qemu_img_info(self, result):"},{"line_number":869,"context_line":"        # Check that result corresponds to the return of qemu_img_info"}],"source_content_type":"text/x-python","patch_set":19,"id":"7aa08908_28a95ee0","line":866,"range":{"start_line":857,"start_character":0,"end_line":866,"end_character":17},"in_reply_to":"7aa08908_635744ca","updated":"2016-06-09 17:54:48.000000000","message":"Agreed.","commit_id":"773feef5e4bc5208b9eb2def54e1c6dd732aca41"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"504f4dfd92811fba6f86cfcbc3221f0cb5b38d0f","unresolved":false,"context_lines":[{"line_number":897,"context_line":"        mock_ensure_tree.assert_called_once_with(self.default_cache_dir)"},{"line_number":898,"context_line":""},{"line_number":899,"context_line":"    @mock.patch.object(libvirt_utils, \u0027update_mtime\u0027, autospec\u003dTrue)"},{"line_number":900,"context_line":"    def test_get_image_info_exists(self, mock_update_mtime):"},{"line_number":901,"context_line":"        # Calling get_image_info when the image exists should return the"},{"line_number":902,"context_line":"        # existing image without fetching it."},{"line_number":903,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"7aa08908_89e5310a","line":900,"range":{"start_line":900,"start_character":8,"end_line":900,"end_character":34},"updated":"2016-06-08 20:47:07.000000000","message":"Additionally, I would probably assert that fetch_image isn\u0027t called in this case.","commit_id":"773feef5e4bc5208b9eb2def54e1c6dd732aca41"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"279d64e0a0cf7b5f5026d0abe7deeae9ee13f648","unresolved":false,"context_lines":[{"line_number":897,"context_line":"        mock_ensure_tree.assert_called_once_with(self.default_cache_dir)"},{"line_number":898,"context_line":""},{"line_number":899,"context_line":"    @mock.patch.object(libvirt_utils, \u0027update_mtime\u0027, autospec\u003dTrue)"},{"line_number":900,"context_line":"    def test_get_image_info_exists(self, mock_update_mtime):"},{"line_number":901,"context_line":"        # Calling get_image_info when the image exists should return the"},{"line_number":902,"context_line":"        # existing image without fetching it."},{"line_number":903,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"7aa08908_28f79ef6","line":900,"range":{"start_line":900,"start_character":8,"end_line":900,"end_character":34},"in_reply_to":"7aa08908_89e5310a","updated":"2016-06-09 17:54:48.000000000","message":"Yeah, that would probably be clearer. It\u0027s currently implicit, because if it had been called it would have exploded :)","commit_id":"773feef5e4bc5208b9eb2def54e1c6dd732aca41"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"504f4dfd92811fba6f86cfcbc3221f0cb5b38d0f","unresolved":false,"context_lines":[{"line_number":924,"context_line":"        # Assert that we\u0027re locked when fetching the image"},{"line_number":925,"context_line":"        mock_fetch_image.side_effect \u003d self.assert_in_lock"},{"line_number":926,"context_line":""},{"line_number":927,"context_line":"        with self.default_mocks():"},{"line_number":928,"context_line":"            pool \u003d imagecache.ImageCacheLocalDir.get()"},{"line_number":929,"context_line":""},{"line_number":930,"context_line":"            result \u003d pool.get_image_info(mock.sentinel.context, self.image_id)"},{"line_number":931,"context_line":"        self.assert_qemu_img_info(result)"},{"line_number":932,"context_line":""},{"line_number":933,"context_line":"        self.mock_synchronized.assert_called_once_with("},{"line_number":934,"context_line":"            self.image_hash, external\u003dTrue,"},{"line_number":935,"context_line":"            lock_path\u003dself.default_lock_path)"},{"line_number":936,"context_line":""},{"line_number":937,"context_line":"        mock_fetch_image.assert_called_once_with("},{"line_number":938,"context_line":"            mock.sentinel.context, self.expected_image_path, self.image_id)"}],"source_content_type":"text/x-python","patch_set":19,"id":"7aa08908_23bd3cd8","line":935,"range":{"start_line":927,"start_character":0,"end_line":935,"end_character":45},"updated":"2016-06-08 20:47:07.000000000","message":"You could remove this code that\u0027s common to 3 tests with a custom assert like the following.\n\n\n    def _assert_get_image_info(self, fallback\u003dNone):\n        with self.default_mocks():\n            cache \u003d imagecache.ImageCacheLocalDir.get()\n            result \u003d cache.get_image_info(\n                mock.sentinel.context, self.image_id, fallback)\n            self.assert_qemu_img_info(result)\n        self.mock_synchronized.assert_called_once_with(\n            self.image_hash, external\u003dTrue, lock_path\u003dself.default_lock_path)","commit_id":"773feef5e4bc5208b9eb2def54e1c6dd732aca41"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"357894a1edb00dd1333a1cabe05c97ce14741bfa","unresolved":false,"context_lines":[{"line_number":924,"context_line":"        # Assert that we\u0027re locked when fetching the image"},{"line_number":925,"context_line":"        mock_fetch_image.side_effect \u003d self.assert_in_lock"},{"line_number":926,"context_line":""},{"line_number":927,"context_line":"        with self.default_mocks():"},{"line_number":928,"context_line":"            pool \u003d imagecache.ImageCacheLocalDir.get()"},{"line_number":929,"context_line":""},{"line_number":930,"context_line":"            result \u003d pool.get_image_info(mock.sentinel.context, self.image_id)"},{"line_number":931,"context_line":"        self.assert_qemu_img_info(result)"},{"line_number":932,"context_line":""},{"line_number":933,"context_line":"        self.mock_synchronized.assert_called_once_with("},{"line_number":934,"context_line":"            self.image_hash, external\u003dTrue,"},{"line_number":935,"context_line":"            lock_path\u003dself.default_lock_path)"},{"line_number":936,"context_line":""},{"line_number":937,"context_line":"        mock_fetch_image.assert_called_once_with("},{"line_number":938,"context_line":"            mock.sentinel.context, self.expected_image_path, self.image_id)"}],"source_content_type":"text/x-python","patch_set":19,"id":"7aa08908_b4d44f1f","line":935,"range":{"start_line":927,"start_character":0,"end_line":935,"end_character":45},"in_reply_to":"7aa08908_23bd3cd8","updated":"2016-06-16 17:01:21.000000000","message":"I already went half way with assert_qemu_img_info, don\u0027t know why I didn\u0027t finish the job.","commit_id":"773feef5e4bc5208b9eb2def54e1c6dd732aca41"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"279d64e0a0cf7b5f5026d0abe7deeae9ee13f648","unresolved":false,"context_lines":[{"line_number":924,"context_line":"        # Assert that we\u0027re locked when fetching the image"},{"line_number":925,"context_line":"        mock_fetch_image.side_effect \u003d self.assert_in_lock"},{"line_number":926,"context_line":""},{"line_number":927,"context_line":"        with self.default_mocks():"},{"line_number":928,"context_line":"            pool \u003d imagecache.ImageCacheLocalDir.get()"},{"line_number":929,"context_line":""},{"line_number":930,"context_line":"            result \u003d pool.get_image_info(mock.sentinel.context, self.image_id)"},{"line_number":931,"context_line":"        self.assert_qemu_img_info(result)"},{"line_number":932,"context_line":""},{"line_number":933,"context_line":"        self.mock_synchronized.assert_called_once_with("},{"line_number":934,"context_line":"            self.image_hash, external\u003dTrue,"},{"line_number":935,"context_line":"            lock_path\u003dself.default_lock_path)"},{"line_number":936,"context_line":""},{"line_number":937,"context_line":"        mock_fetch_image.assert_called_once_with("},{"line_number":938,"context_line":"            mock.sentinel.context, self.expected_image_path, self.image_id)"}],"source_content_type":"text/x-python","patch_set":19,"id":"7aa08908_c8bd3a04","line":935,"range":{"start_line":927,"start_character":0,"end_line":935,"end_character":45},"in_reply_to":"7aa08908_23bd3cd8","updated":"2016-06-09 17:54:48.000000000","message":"Yup.","commit_id":"773feef5e4bc5208b9eb2def54e1c6dd732aca41"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"504f4dfd92811fba6f86cfcbc3221f0cb5b38d0f","unresolved":false,"context_lines":[{"line_number":945,"context_line":"        mock_fetch_image.side_effect \u003d exception.ImageNotFound("},{"line_number":946,"context_line":"            image_id\u003dself.image_id)"},{"line_number":947,"context_line":""},{"line_number":948,"context_line":"        with self.default_mocks():"},{"line_number":949,"context_line":"            pool \u003d imagecache.ImageCacheLocalDir.get()"},{"line_number":950,"context_line":""},{"line_number":951,"context_line":"            self.assertRaises(exception.ImageNotFound, pool.get_image_info,"},{"line_number":952,"context_line":"                              mock.sentinel.context, self.image_id)"},{"line_number":953,"context_line":""},{"line_number":954,"context_line":"        self.mock_synchronized.assert_called_once_with("},{"line_number":955,"context_line":"            self.image_hash, external\u003dTrue,"},{"line_number":956,"context_line":"            lock_path\u003dself.default_lock_path)"},{"line_number":957,"context_line":""},{"line_number":958,"context_line":"        mock_fetch_image.assert_called_once_with("},{"line_number":959,"context_line":"            mock.sentinel.context, self.expected_image_path, self.image_id)"}],"source_content_type":"text/x-python","patch_set":19,"id":"7aa08908_43ade899","line":956,"range":{"start_line":948,"start_character":0,"end_line":956,"end_character":45},"updated":"2016-06-08 20:47:07.000000000","message":"Or handle all 4 cases with something like:\n\n\n    def _assert_get_image_info(self, should_raise\u003dFalse, fallback\u003dNone):\n        with self.default_mocks():\n            cache \u003d imagecache.ImageCacheLocalDir.get()\n            if should_raise:\n                self.assertRaises(\n                    exception.ImageNotFound, cache.get_image_info,\n                    mock.sentinel.context, self.image_id)\n            else:\n                result \u003d cache.get_image_info(\n                    mock.sentinel.context, self.image_id, fallback)\n                self.assert_qemu_img_info(result)\n        self.mock_synchronized.assert_called_once_with(\n            self.image_hash, external\u003dTrue, lock_path\u003dself.default_lock_path)","commit_id":"773feef5e4bc5208b9eb2def54e1c6dd732aca41"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"279d64e0a0cf7b5f5026d0abe7deeae9ee13f648","unresolved":false,"context_lines":[{"line_number":945,"context_line":"        mock_fetch_image.side_effect \u003d exception.ImageNotFound("},{"line_number":946,"context_line":"            image_id\u003dself.image_id)"},{"line_number":947,"context_line":""},{"line_number":948,"context_line":"        with self.default_mocks():"},{"line_number":949,"context_line":"            pool \u003d imagecache.ImageCacheLocalDir.get()"},{"line_number":950,"context_line":""},{"line_number":951,"context_line":"            self.assertRaises(exception.ImageNotFound, pool.get_image_info,"},{"line_number":952,"context_line":"                              mock.sentinel.context, self.image_id)"},{"line_number":953,"context_line":""},{"line_number":954,"context_line":"        self.mock_synchronized.assert_called_once_with("},{"line_number":955,"context_line":"            self.image_hash, external\u003dTrue,"},{"line_number":956,"context_line":"            lock_path\u003dself.default_lock_path)"},{"line_number":957,"context_line":""},{"line_number":958,"context_line":"        mock_fetch_image.assert_called_once_with("},{"line_number":959,"context_line":"            mock.sentinel.context, self.expected_image_path, self.image_id)"}],"source_content_type":"text/x-python","patch_set":19,"id":"7aa08908_e83ab697","line":956,"range":{"start_line":948,"start_character":0,"end_line":956,"end_character":45},"in_reply_to":"7aa08908_43ade899","updated":"2016-06-09 17:54:48.000000000","message":"Less sure, will consider.","commit_id":"773feef5e4bc5208b9eb2def54e1c6dd732aca41"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"357894a1edb00dd1333a1cabe05c97ce14741bfa","unresolved":false,"context_lines":[{"line_number":945,"context_line":"        mock_fetch_image.side_effect \u003d exception.ImageNotFound("},{"line_number":946,"context_line":"            image_id\u003dself.image_id)"},{"line_number":947,"context_line":""},{"line_number":948,"context_line":"        with self.default_mocks():"},{"line_number":949,"context_line":"            pool \u003d imagecache.ImageCacheLocalDir.get()"},{"line_number":950,"context_line":""},{"line_number":951,"context_line":"            self.assertRaises(exception.ImageNotFound, pool.get_image_info,"},{"line_number":952,"context_line":"                              mock.sentinel.context, self.image_id)"},{"line_number":953,"context_line":""},{"line_number":954,"context_line":"        self.mock_synchronized.assert_called_once_with("},{"line_number":955,"context_line":"            self.image_hash, external\u003dTrue,"},{"line_number":956,"context_line":"            lock_path\u003dself.default_lock_path)"},{"line_number":957,"context_line":""},{"line_number":958,"context_line":"        mock_fetch_image.assert_called_once_with("},{"line_number":959,"context_line":"            mock.sentinel.context, self.expected_image_path, self.image_id)"}],"source_content_type":"text/x-python","patch_set":19,"id":"7aa08908_94a95327","line":956,"range":{"start_line":948,"start_character":0,"end_line":956,"end_character":45},"in_reply_to":"7aa08908_e83ab697","updated":"2016-06-16 17:01:21.000000000","message":"Decided not to do this, as already have to handle the fallback case. Don\u0027t want the tests to get too complex.","commit_id":"773feef5e4bc5208b9eb2def54e1c6dd732aca41"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"504f4dfd92811fba6f86cfcbc3221f0cb5b38d0f","unresolved":false,"context_lines":[{"line_number":969,"context_line":"            image_id\u003dself.image_id)"},{"line_number":970,"context_line":""},{"line_number":971,"context_line":"        with self.default_mocks():"},{"line_number":972,"context_line":"            pool \u003d imagecache.ImageCacheLocalDir.get()"},{"line_number":973,"context_line":""},{"line_number":974,"context_line":"            result \u003d pool.get_image_info(mock.sentinel.context, self.image_id,"},{"line_number":975,"context_line":"                                         fallback\u003d\u0027host\u0027)"}],"source_content_type":"text/x-python","patch_set":19,"id":"7aa08908_a970ed0b","line":972,"range":{"start_line":972,"start_character":12,"end_line":972,"end_character":16},"updated":"2016-06-08 20:47:07.000000000","message":"I would go with \"cache\" instead now that this class has been renamed (to no longer include the word \"pool\").","commit_id":"773feef5e4bc5208b9eb2def54e1c6dd732aca41"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"279d64e0a0cf7b5f5026d0abe7deeae9ee13f648","unresolved":false,"context_lines":[{"line_number":969,"context_line":"            image_id\u003dself.image_id)"},{"line_number":970,"context_line":""},{"line_number":971,"context_line":"        with self.default_mocks():"},{"line_number":972,"context_line":"            pool \u003d imagecache.ImageCacheLocalDir.get()"},{"line_number":973,"context_line":""},{"line_number":974,"context_line":"            result \u003d pool.get_image_info(mock.sentinel.context, self.image_id,"},{"line_number":975,"context_line":"                                         fallback\u003d\u0027host\u0027)"}],"source_content_type":"text/x-python","patch_set":19,"id":"7aa08908_884a1226","line":972,"range":{"start_line":972,"start_character":12,"end_line":972,"end_character":16},"in_reply_to":"7aa08908_a970ed0b","updated":"2016-06-09 17:54:48.000000000","message":"Yup.","commit_id":"773feef5e4bc5208b9eb2def54e1c6dd732aca41"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"504f4dfd92811fba6f86cfcbc3221f0cb5b38d0f","unresolved":false,"context_lines":[{"line_number":990,"context_line":"    def test_get_func_output_path_exists(self, mock_update_mtime):"},{"line_number":991,"context_line":"        # Calling get_func_output_path when the file exists should update its"},{"line_number":992,"context_line":"        # mtime and not raise any exceptions"},{"line_number":993,"context_line":"        fallback_host \u003d uuidutils.generate_uuid()"},{"line_number":994,"context_line":"        self.existing_paths.add(self.expected_func_path)"},{"line_number":995,"context_line":""},{"line_number":996,"context_line":"        mock_create \u003d mock.Mock()"}],"source_content_type":"text/x-python","patch_set":19,"id":"7aa08908_ae6bd3b5","line":993,"range":{"start_line":993,"start_character":8,"end_line":993,"end_character":49},"updated":"2016-06-08 20:47:07.000000000","message":"A sentinel or None for the fallback host would be more consistent with the other tests in this class.","commit_id":"773feef5e4bc5208b9eb2def54e1c6dd732aca41"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"279d64e0a0cf7b5f5026d0abe7deeae9ee13f648","unresolved":false,"context_lines":[{"line_number":990,"context_line":"    def test_get_func_output_path_exists(self, mock_update_mtime):"},{"line_number":991,"context_line":"        # Calling get_func_output_path when the file exists should update its"},{"line_number":992,"context_line":"        # mtime and not raise any exceptions"},{"line_number":993,"context_line":"        fallback_host \u003d uuidutils.generate_uuid()"},{"line_number":994,"context_line":"        self.existing_paths.add(self.expected_func_path)"},{"line_number":995,"context_line":""},{"line_number":996,"context_line":"        mock_create \u003d mock.Mock()"}],"source_content_type":"text/x-python","patch_set":19,"id":"7aa08908_a851ceda","line":993,"range":{"start_line":993,"start_character":8,"end_line":993,"end_character":49},"in_reply_to":"7aa08908_ae6bd3b5","updated":"2016-06-09 17:54:48.000000000","message":"Yup.","commit_id":"773feef5e4bc5208b9eb2def54e1c6dd732aca41"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"504f4dfd92811fba6f86cfcbc3221f0cb5b38d0f","unresolved":false,"context_lines":[{"line_number":1013,"context_line":"        mock_copy_image.side_effect \u003d \\"},{"line_number":1014,"context_line":"            lambda *args, **kwargs: self.assertTrue(self.locked)"},{"line_number":1015,"context_line":""},{"line_number":1016,"context_line":"        mock_create \u003d mock.Mock()"},{"line_number":1017,"context_line":""},{"line_number":1018,"context_line":"        with self.default_mocks():"},{"line_number":1019,"context_line":"            pool \u003d imagecache.ImageCacheLocalDir.get()"},{"line_number":1020,"context_line":"            pool.get_func_output_path(mock_create, self.func_name,"},{"line_number":1021,"context_line":"                                      fallback\u003dmock.sentinel.fallback)"},{"line_number":1022,"context_line":""},{"line_number":1023,"context_line":"        self.mock_exists.assert_any_call(self.expected_func_path)"},{"line_number":1024,"context_line":"        mock_copy_image.assert_called_once_with(src\u003dself.expected_func_path,"}],"source_content_type":"text/x-python","patch_set":19,"id":"7aa08908_6ec9ebab","line":1021,"range":{"start_line":1016,"start_character":1,"end_line":1021,"end_character":70},"updated":"2016-06-08 20:47:07.000000000","message":"You could place this code that\u0027s common to all 3 tests in a custom assert like the following.\n\n\n    def _assert_get_func_output_path(self, fallback\u003dNone, should_create\u003dFalse):\n        mock_create \u003d mock.Mock()\n        with self.default_mocks():\n            cache \u003d imagecache.ImageCacheLocalDir.get()\n            cache.get_func_output_path(mock_create, self.func_name, fallback)\n        if should_create:\n            mock_create.assert_called_once_with(self.expected_func_path)\n        else:\n            self.assertFalse(mock_create.called)","commit_id":"773feef5e4bc5208b9eb2def54e1c6dd732aca41"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"279d64e0a0cf7b5f5026d0abe7deeae9ee13f648","unresolved":false,"context_lines":[{"line_number":1013,"context_line":"        mock_copy_image.side_effect \u003d \\"},{"line_number":1014,"context_line":"            lambda *args, **kwargs: self.assertTrue(self.locked)"},{"line_number":1015,"context_line":""},{"line_number":1016,"context_line":"        mock_create \u003d mock.Mock()"},{"line_number":1017,"context_line":""},{"line_number":1018,"context_line":"        with self.default_mocks():"},{"line_number":1019,"context_line":"            pool \u003d imagecache.ImageCacheLocalDir.get()"},{"line_number":1020,"context_line":"            pool.get_func_output_path(mock_create, self.func_name,"},{"line_number":1021,"context_line":"                                      fallback\u003dmock.sentinel.fallback)"},{"line_number":1022,"context_line":""},{"line_number":1023,"context_line":"        self.mock_exists.assert_any_call(self.expected_func_path)"},{"line_number":1024,"context_line":"        mock_copy_image.assert_called_once_with(src\u003dself.expected_func_path,"}],"source_content_type":"text/x-python","patch_set":19,"id":"7aa08908_c86b7a84","line":1021,"range":{"start_line":1016,"start_character":1,"end_line":1021,"end_character":70},"in_reply_to":"7aa08908_6ec9ebab","updated":"2016-06-09 17:54:48.000000000","message":"Yup.","commit_id":"773feef5e4bc5208b9eb2def54e1c6dd732aca41"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"504f4dfd92811fba6f86cfcbc3221f0cb5b38d0f","unresolved":false,"context_lines":[{"line_number":1030,"context_line":"    @mock.patch.object(libvirt_utils, \u0027copy_image\u0027, autospec\u003dTrue)"},{"line_number":1031,"context_line":"    def test_get_func_output_path_not_exists_no_fallback(self,"},{"line_number":1032,"context_line":"                                                         mock_copy_image):"},{"line_number":1033,"context_line":"        # Calling get_func_output_path when the file does not exist should"},{"line_number":1034,"context_line":"        # attempt to retrieve it from the fallback host"},{"line_number":1035,"context_line":""},{"line_number":1036,"context_line":"        mock_copy_image.side_effect \u003d processutils.ProcessExecutionError"},{"line_number":1037,"context_line":"        mock_create \u003d mock.Mock()"}],"source_content_type":"text/x-python","patch_set":19,"id":"7aa08908_23bffc61","line":1034,"range":{"start_line":1033,"start_character":0,"end_line":1034,"end_character":55},"updated":"2016-06-08 20:47:07.000000000","message":"This comment is exactly the same as the one above, but it\u0027s actually a different test case.","commit_id":"773feef5e4bc5208b9eb2def54e1c6dd732aca41"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"279d64e0a0cf7b5f5026d0abe7deeae9ee13f648","unresolved":false,"context_lines":[{"line_number":1030,"context_line":"    @mock.patch.object(libvirt_utils, \u0027copy_image\u0027, autospec\u003dTrue)"},{"line_number":1031,"context_line":"    def test_get_func_output_path_not_exists_no_fallback(self,"},{"line_number":1032,"context_line":"                                                         mock_copy_image):"},{"line_number":1033,"context_line":"        # Calling get_func_output_path when the file does not exist should"},{"line_number":1034,"context_line":"        # attempt to retrieve it from the fallback host"},{"line_number":1035,"context_line":""},{"line_number":1036,"context_line":"        mock_copy_image.side_effect \u003d processutils.ProcessExecutionError"},{"line_number":1037,"context_line":"        mock_create \u003d mock.Mock()"}],"source_content_type":"text/x-python","patch_set":19,"id":"7aa08908_080462c4","line":1034,"range":{"start_line":1033,"start_character":0,"end_line":1034,"end_character":55},"in_reply_to":"7aa08908_23bffc61","updated":"2016-06-09 17:54:48.000000000","message":"Oops.","commit_id":"773feef5e4bc5208b9eb2def54e1c6dd732aca41"}],"nova/virt/libvirt/imagebackend.py":[{"author":{"_account_id":7634,"name":"Takashi Natsume","email":"takanattie@gmail.com","username":"natsumet"},"change_message_id":"a44432077bca4c58886dae2ce4fcfe0b35445e7d","unresolved":false,"context_lines":[{"line_number":225,"context_line":"                                             receive\u003dTrue)"},{"line_number":226,"context_line":"                    created \u003d True"},{"line_number":227,"context_line":"                except processutils.ProcessExecutionError:"},{"line_number":228,"context_line":"                    LOG.exception(_LW(\u0027Failed to side-load %(path)s from \u0027"},{"line_number":229,"context_line":"                                      \u0027%(host)s\u0027),"},{"line_number":230,"context_line":"                                  {\u0027path\u0027: path, \u0027host\u0027: fallback_from_host})"},{"line_number":231,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"bae84128_5d6ed395","line":228,"range":{"start_line":228,"start_character":24,"end_line":228,"end_character":33},"updated":"2016-02-22 09:25:35.000000000","message":"LOG.exception creates an ERROR level log, so LOG.warning should be used.","commit_id":"1a90c60a99cb281451bb7ae35a06e510339dbc34"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"340fa8c1cc7e6da4e062d3160b2ca442b1877bb8","unresolved":false,"context_lines":[{"line_number":225,"context_line":"                                             receive\u003dTrue)"},{"line_number":226,"context_line":"                    created \u003d True"},{"line_number":227,"context_line":"                except processutils.ProcessExecutionError:"},{"line_number":228,"context_line":"                    LOG.exception(_LW(\u0027Failed to side-load %(path)s from \u0027"},{"line_number":229,"context_line":"                                      \u0027%(host)s\u0027),"},{"line_number":230,"context_line":"                                  {\u0027path\u0027: path, \u0027host\u0027: fallback_from_host})"},{"line_number":231,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"bae84128_2928aee1","line":228,"range":{"start_line":228,"start_character":24,"end_line":228,"end_character":33},"in_reply_to":"bae84128_5d6ed395","updated":"2016-02-23 14:09:56.000000000","message":"Hmm, ok, and for sure I intended warning here. It probably is an ERROR though, tbh. This should never happen, and if it does it means that you have a corrupt disk, because the output below will not be identical to the output generated by another host.\n\nMostly I wanted to ensure that we captured the output of the failed copy_image above in the logs.\n\nI\u0027m open to suggestions of how to do this better, but as it stands I\u0027d probably leave it alone.","commit_id":"1a90c60a99cb281451bb7ae35a06e510339dbc34"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"3815294c89426f1f653314037cde25ae040c3e3d","unresolved":false,"context_lines":[{"line_number":55,"context_line":""},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"CachedImageInfo \u003d collections.namedtuple("},{"line_number":58,"context_line":"    \u0027CachedImageInfo\u0027, (\u0027path\u0027, \u0027file_format\u0027, \u0027virtual_size\u0027, \u0027disk_size\u0027))"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"class ImageCacheLocalPool(object):"}],"source_content_type":"text/x-python","patch_set":12,"id":"dab17558_99e7b1e3","line":58,"updated":"2016-05-14 10:04:03.000000000","message":"Perhaps we want to have an attribute that the image based on a backing file? Just a question.","commit_id":"b8c4e29a1c02704b1e95380d53e7d5a83e59fe81"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"66b901e05a7b90c7d73d5232488ec122f685f162","unresolved":false,"context_lines":[{"line_number":55,"context_line":""},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"CachedImageInfo \u003d collections.namedtuple("},{"line_number":58,"context_line":"    \u0027CachedImageInfo\u0027, (\u0027path\u0027, \u0027file_format\u0027, \u0027virtual_size\u0027, \u0027disk_size\u0027))"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"class ImageCacheLocalPool(object):"}],"source_content_type":"text/x-python","patch_set":12,"id":"dab17558_02cddd84","line":58,"in_reply_to":"dab17558_99e7b1e3","updated":"2016-05-16 14:32:05.000000000","message":"It definitely doesn\u0027t have a backing file. We explicitly check that in images.fetch_to_raw, in fact, because it would be a security hole.","commit_id":"b8c4e29a1c02704b1e95380d53e7d5a83e59fe81"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"3815294c89426f1f653314037cde25ae040c3e3d","unresolved":false,"context_lines":[{"line_number":58,"context_line":"    \u0027CachedImageInfo\u0027, (\u0027path\u0027, \u0027file_format\u0027, \u0027virtual_size\u0027, \u0027disk_size\u0027))"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"class ImageCacheLocalPool(object):"},{"line_number":62,"context_line":"    _singleton \u003d None"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":12,"id":"dab17558_b63a8432","line":61,"updated":"2016-05-14 10:04:03.000000000","message":"Could we place this class outside of imagebackend module? libvirt.imagecache looks appropriate for that.\n\nCache operations do not look related directly to image backend, image abstraction, etc.","commit_id":"b8c4e29a1c02704b1e95380d53e7d5a83e59fe81"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"f1c573662790d72b86788f85384641e257453220","unresolved":false,"context_lines":[{"line_number":58,"context_line":"    \u0027CachedImageInfo\u0027, (\u0027path\u0027, \u0027file_format\u0027, \u0027virtual_size\u0027, \u0027disk_size\u0027))"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"class ImageCacheLocalPool(object):"},{"line_number":62,"context_line":"    _singleton \u003d None"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":12,"id":"bab6814e_52c9de77","line":61,"in_reply_to":"dab17558_766c38d0","updated":"2016-05-18 19:04:38.000000000","message":"thanks","commit_id":"b8c4e29a1c02704b1e95380d53e7d5a83e59fe81"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"66b901e05a7b90c7d73d5232488ec122f685f162","unresolved":false,"context_lines":[{"line_number":58,"context_line":"    \u0027CachedImageInfo\u0027, (\u0027path\u0027, \u0027file_format\u0027, \u0027virtual_size\u0027, \u0027disk_size\u0027))"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"class ImageCacheLocalPool(object):"},{"line_number":62,"context_line":"    _singleton \u003d None"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":12,"id":"dab17558_e232595d","line":61,"in_reply_to":"dab17558_b63a8432","updated":"2016-05-16 14:32:05.000000000","message":"Ultimately yes, but for the moment I want to leave it here. The reason is that this refactor is of code that was here anyway (or in driver.py, which is worse). This is code which writes to the image cache. The stuff in imagecache \u0027manages\u0027 it. Also, it needs to die in a fire, and I\u0027m not quite ready to light that fire, yet.","commit_id":"b8c4e29a1c02704b1e95380d53e7d5a83e59fe81"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"d57970c1f911d02ed239858b46f9b3e787059058","unresolved":false,"context_lines":[{"line_number":58,"context_line":"    \u0027CachedImageInfo\u0027, (\u0027path\u0027, \u0027file_format\u0027, \u0027virtual_size\u0027, \u0027disk_size\u0027))"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"class ImageCacheLocalPool(object):"},{"line_number":62,"context_line":"    _singleton \u003d None"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":12,"id":"dab17558_766c38d0","line":61,"in_reply_to":"dab17558_e232595d","updated":"2016-05-17 11:02:47.000000000","message":"I\u0027ve changed my mind on this, I\u0027m going to move it. I\u0027ll add in some comments around how the 2 classes relate directly to one another.","commit_id":"b8c4e29a1c02704b1e95380d53e7d5a83e59fe81"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"3815294c89426f1f653314037cde25ae040c3e3d","unresolved":false,"context_lines":[{"line_number":63,"context_line":""},{"line_number":64,"context_line":"    @classmethod"},{"line_number":65,"context_line":"    def reset(cls):"},{"line_number":66,"context_line":"        \"\"\"Throw away the initialised singleton. Useful for testing.\"\"\""},{"line_number":67,"context_line":"        cls._singleton \u003d None"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":12,"id":"dab17558_968300cc","line":66,"updated":"2016-05-14 10:04:03.000000000","message":"Could we use Python\u0027s reload func in tests instead?","commit_id":"b8c4e29a1c02704b1e95380d53e7d5a83e59fe81"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"590bc0576912d9787e7b6b47728a30355f001197","unresolved":false,"context_lines":[{"line_number":63,"context_line":""},{"line_number":64,"context_line":"    @classmethod"},{"line_number":65,"context_line":"    def reset(cls):"},{"line_number":66,"context_line":"        \"\"\"Throw away the initialised singleton. Useful for testing.\"\"\""},{"line_number":67,"context_line":"        cls._singleton \u003d None"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":12,"id":"dab17558_edc1774f","line":66,"in_reply_to":"dab17558_968300cc","updated":"2016-05-17 10:18:13.000000000","message":"I wasn\u0027t aware of it. I\u0027m going to give it a try. It would be nice not to have a testing wart in this class.","commit_id":"b8c4e29a1c02704b1e95380d53e7d5a83e59fe81"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"d57970c1f911d02ed239858b46f9b3e787059058","unresolved":false,"context_lines":[{"line_number":63,"context_line":""},{"line_number":64,"context_line":"    @classmethod"},{"line_number":65,"context_line":"    def reset(cls):"},{"line_number":66,"context_line":"        \"\"\"Throw away the initialised singleton. Useful for testing.\"\"\""},{"line_number":67,"context_line":"        cls._singleton \u003d None"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":12,"id":"dab17558_b6150041","line":66,"in_reply_to":"dab17558_edc1774f","updated":"2016-05-17 11:02:47.000000000","message":"This works. I\u0027ll do this instead, thanks.","commit_id":"b8c4e29a1c02704b1e95380d53e7d5a83e59fe81"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"3815294c89426f1f653314037cde25ae040c3e3d","unresolved":false,"context_lines":[{"line_number":92,"context_line":"        return os.path.dirname(path) \u003d\u003d self.base_dir"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"    @staticmethod"},{"line_number":95,"context_line":"    def check_exists_and_mark_in_use(path):"},{"line_number":96,"context_line":"        if os.path.exists(path):"},{"line_number":97,"context_line":"            # NOTE(mikal): Update the mtime of the base file so the image"},{"line_number":98,"context_line":"            # cache manager knows it is in use."}],"source_content_type":"text/x-python","patch_set":12,"id":"dab17558_79cdadb6","line":95,"updated":"2016-05-14 10:04:03.000000000","message":"As i understand, this method must be called inside locking. Should it be noted in the doc string?","commit_id":"b8c4e29a1c02704b1e95380d53e7d5a83e59fe81"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"d57970c1f911d02ed239858b46f9b3e787059058","unresolved":false,"context_lines":[{"line_number":92,"context_line":"        return os.path.dirname(path) \u003d\u003d self.base_dir"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"    @staticmethod"},{"line_number":95,"context_line":"    def check_exists_and_mark_in_use(path):"},{"line_number":96,"context_line":"        if os.path.exists(path):"},{"line_number":97,"context_line":"            # NOTE(mikal): Update the mtime of the base file so the image"},{"line_number":98,"context_line":"            # cache manager knows it is in use."}],"source_content_type":"text/x-python","patch_set":12,"id":"dab17558_d6b24448","line":95,"in_reply_to":"dab17558_6275c9f8","updated":"2016-05-17 11:02:47.000000000","message":"On looking again, this is actually a private method. I\u0027ll mark it up as such.","commit_id":"b8c4e29a1c02704b1e95380d53e7d5a83e59fe81"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"66b901e05a7b90c7d73d5232488ec122f685f162","unresolved":false,"context_lines":[{"line_number":92,"context_line":"        return os.path.dirname(path) \u003d\u003d self.base_dir"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"    @staticmethod"},{"line_number":95,"context_line":"    def check_exists_and_mark_in_use(path):"},{"line_number":96,"context_line":"        if os.path.exists(path):"},{"line_number":97,"context_line":"            # NOTE(mikal): Update the mtime of the base file so the image"},{"line_number":98,"context_line":"            # cache manager knows it is in use."}],"source_content_type":"text/x-python","patch_set":12,"id":"dab17558_6275c9f8","line":95,"in_reply_to":"dab17558_79cdadb6","updated":"2016-05-16 14:32:05.000000000","message":"Good point. Could probably do with consistent docstrings on this in general.","commit_id":"b8c4e29a1c02704b1e95380d53e7d5a83e59fe81"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"3815294c89426f1f653314037cde25ae040c3e3d","unresolved":false,"context_lines":[{"line_number":141,"context_line":""},{"line_number":142,"context_line":"        _sync()"},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"        # NOTE: We should persist this information somehow and return it"},{"line_number":145,"context_line":"        # securely, because image inspection here isn\u0027t ideal. The image was"},{"line_number":146,"context_line":"        # sanity checked in fetch_image when it was downloaded, so this"},{"line_number":147,"context_line":"        # isn\u0027t the worst, but we could do better. Until then, this is"}],"source_content_type":"text/x-python","patch_set":12,"id":"dab17558_b9c5553e","line":144,"updated":"2016-05-14 10:04:03.000000000","message":"IIANM, the spec claims that the info will be persisted. So that this should be TODO, should not it?","commit_id":"b8c4e29a1c02704b1e95380d53e7d5a83e59fe81"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"f1c573662790d72b86788f85384641e257453220","unresolved":false,"context_lines":[{"line_number":141,"context_line":""},{"line_number":142,"context_line":"        _sync()"},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"        # NOTE: We should persist this information somehow and return it"},{"line_number":145,"context_line":"        # securely, because image inspection here isn\u0027t ideal. The image was"},{"line_number":146,"context_line":"        # sanity checked in fetch_image when it was downloaded, so this"},{"line_number":147,"context_line":"        # isn\u0027t the worst, but we could do better. Until then, this is"}],"source_content_type":"text/x-python","patch_set":12,"id":"bab6814e_92ba26e0","line":144,"in_reply_to":"dab17558_a29691ed","updated":"2016-05-18 19:04:38.000000000","message":"got it","commit_id":"b8c4e29a1c02704b1e95380d53e7d5a83e59fe81"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"66b901e05a7b90c7d73d5232488ec122f685f162","unresolved":false,"context_lines":[{"line_number":141,"context_line":""},{"line_number":142,"context_line":"        _sync()"},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"        # NOTE: We should persist this information somehow and return it"},{"line_number":145,"context_line":"        # securely, because image inspection here isn\u0027t ideal. The image was"},{"line_number":146,"context_line":"        # sanity checked in fetch_image when it was downloaded, so this"},{"line_number":147,"context_line":"        # isn\u0027t the worst, but we could do better. Until then, this is"}],"source_content_type":"text/x-python","patch_set":12,"id":"dab17558_a29691ed","line":144,"in_reply_to":"dab17558_b9c5553e","updated":"2016-05-16 14:32:05.000000000","message":"Subtly different. This is metadata about cached images. The spec is about cached ephemeral disks. We definitely want to do this, though. I just don\u0027t want to get hung up on it right now, though, because it\u0027s at least no worse than before.","commit_id":"b8c4e29a1c02704b1e95380d53e7d5a83e59fe81"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"3815294c89426f1f653314037cde25ae040c3e3d","unresolved":false,"context_lines":[{"line_number":150,"context_line":"        return CachedImageInfo(path, info.file_format, info.virtual_size,"},{"line_number":151,"context_line":"                               info.disk_size)"},{"line_number":152,"context_line":""},{"line_number":153,"context_line":"    def get_cached_func(self, func, name, fallback_from_host\u003dNone):"},{"line_number":154,"context_line":"        \"\"\"Get the cached output of a function from the cache. If it"},{"line_number":155,"context_line":"        don\u0027t exist, try to side-load it from another compute host."},{"line_number":156,"context_line":"        Failing that, generate it again locally."}],"source_content_type":"text/x-python","patch_set":12,"id":"dab17558_b94f15bd","line":153,"updated":"2016-05-14 10:04:03.000000000","message":"The method name misleads, since the method does not return a function.\n\nSince the class name is ImageCacheLocalPool, we could omit \u0027image\u0027 and \u0027cache\u0027 parts from these names, but named them \u0027get_from_storage\u0027 and \u0027get_from_func\u0027 or something like that. Just the first names came to mind.","commit_id":"b8c4e29a1c02704b1e95380d53e7d5a83e59fe81"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"66b901e05a7b90c7d73d5232488ec122f685f162","unresolved":false,"context_lines":[{"line_number":150,"context_line":"        return CachedImageInfo(path, info.file_format, info.virtual_size,"},{"line_number":151,"context_line":"                               info.disk_size)"},{"line_number":152,"context_line":""},{"line_number":153,"context_line":"    def get_cached_func(self, func, name, fallback_from_host\u003dNone):"},{"line_number":154,"context_line":"        \"\"\"Get the cached output of a function from the cache. If it"},{"line_number":155,"context_line":"        don\u0027t exist, try to side-load it from another compute host."},{"line_number":156,"context_line":"        Failing that, generate it again locally."}],"source_content_type":"text/x-python","patch_set":12,"id":"dab17558_a219718a","line":153,"in_reply_to":"dab17558_b94f15bd","updated":"2016-05-16 14:32:05.000000000","message":"You\u0027re the second person to comment on this. I\u0027ll change it. I\u0027ll have to re-use \u0027image\u0027, though, because in this case it really is a glance image. The class is called ImageCacheXXX because it relates to a cache of glance images, but due to a legacy design wart also contains other stuff which it shouldn\u0027t (and I want to remove at some point).","commit_id":"b8c4e29a1c02704b1e95380d53e7d5a83e59fe81"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"3815294c89426f1f653314037cde25ae040c3e3d","unresolved":false,"context_lines":[{"line_number":187,"context_line":"                    LOG.exception(_LW(\u0027Failed to side-load %(path)s from \u0027"},{"line_number":188,"context_line":"                                      \u0027%(host)s\u0027),"},{"line_number":189,"context_line":"                                  {\u0027path\u0027: path, \u0027host\u0027: fallback_from_host})"},{"line_number":190,"context_line":""},{"line_number":191,"context_line":"            # Generate a new one as a last resort"},{"line_number":192,"context_line":"            if not created:"},{"line_number":193,"context_line":"                func(path)"}],"source_content_type":"text/x-python","patch_set":12,"id":"dab17558_79712da8","line":190,"updated":"2016-05-14 10:04:03.000000000","message":"What is the reason to suppress the exception? How an instance launch will flow further in this case?","commit_id":"b8c4e29a1c02704b1e95380d53e7d5a83e59fe81"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"66b901e05a7b90c7d73d5232488ec122f685f162","unresolved":false,"context_lines":[{"line_number":187,"context_line":"                    LOG.exception(_LW(\u0027Failed to side-load %(path)s from \u0027"},{"line_number":188,"context_line":"                                      \u0027%(host)s\u0027),"},{"line_number":189,"context_line":"                                  {\u0027path\u0027: path, \u0027host\u0027: fallback_from_host})"},{"line_number":190,"context_line":""},{"line_number":191,"context_line":"            # Generate a new one as a last resort"},{"line_number":192,"context_line":"            if not created:"},{"line_number":193,"context_line":"                func(path)"}],"source_content_type":"text/x-python","patch_set":12,"id":"dab17558_c3ee3621","line":190,"in_reply_to":"dab17558_79712da8","updated":"2016-05-16 14:32:05.000000000","message":"Couple of things. Firstly, if this fails then we proceed to re-execute the function below.\n\nSecondly, the original code didn\u0027t have fallback_from_host here, but I added it because caching the output of these things is a bug, and this makes it less bad in some circumstances. In an earlier version of this patch series I actually removed all caching of function output, but after some discussion I put it back because some people are still using ext3, which remains expensive to generate (as opposed to ext4, which is not). Ideally we would not cache these at all, but that change is out of scope for this refactor. Not failing here means we don\u0027t add an additional failure mode over the old code.\n\nFor example, look in driver._create_image where we generate disk.local. Note that the callback function passed to cache() is _create_ephemeral, which does not handle fallback_from_host. Either the cached output exists, or it is regenerated. This code additionally tries to side-load it first.","commit_id":"b8c4e29a1c02704b1e95380d53e7d5a83e59fe81"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"f1c573662790d72b86788f85384641e257453220","unresolved":false,"context_lines":[{"line_number":187,"context_line":"                    LOG.exception(_LW(\u0027Failed to side-load %(path)s from \u0027"},{"line_number":188,"context_line":"                                      \u0027%(host)s\u0027),"},{"line_number":189,"context_line":"                                  {\u0027path\u0027: path, \u0027host\u0027: fallback_from_host})"},{"line_number":190,"context_line":""},{"line_number":191,"context_line":"            # Generate a new one as a last resort"},{"line_number":192,"context_line":"            if not created:"},{"line_number":193,"context_line":"                func(path)"}],"source_content_type":"text/x-python","patch_set":12,"id":"bab6814e_b2e2ca01","line":190,"in_reply_to":"dab17558_c3ee3621","updated":"2016-05-18 19:04:38.000000000","message":"got it. thanks for the explanation","commit_id":"b8c4e29a1c02704b1e95380d53e7d5a83e59fe81"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"3815294c89426f1f653314037cde25ae040c3e3d","unresolved":false,"context_lines":[{"line_number":190,"context_line":""},{"line_number":191,"context_line":"            # Generate a new one as a last resort"},{"line_number":192,"context_line":"            if not created:"},{"line_number":193,"context_line":"                func(path)"},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"        _sync()"},{"line_number":196,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"dab17558_bc3c23c0","line":193,"updated":"2016-05-14 10:04:03.000000000","message":"At the moment we have a problem with lvm and ephemeral images. Lvm does not use a cached image for such images, but wants just to create disks in lvm backend and then format them. _create_ephemeral method of driver.py allows that. But Image.cache function does not call the callback function if images exist (it was added later and broke lvm logic).\n\nAs a result ephemeral disks are not mounted automatically under lvm.\n\nI understand, that fixing of this problem is not a goal of the blueprint you\u0027re working on. But it would be perfect if we can imagine at least how we could fix it after the refactoring. Do you have ideas?","commit_id":"b8c4e29a1c02704b1e95380d53e7d5a83e59fe81"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"8b20da0df63672d320675d2c5ccda284a2a851a8","unresolved":false,"context_lines":[{"line_number":190,"context_line":""},{"line_number":191,"context_line":"            # Generate a new one as a last resort"},{"line_number":192,"context_line":"            if not created:"},{"line_number":193,"context_line":"                func(path)"},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"        _sync()"},{"line_number":196,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"bab6814e_c5dcd514","line":193,"in_reply_to":"dab17558_2657505b","updated":"2016-05-19 07:17:03.000000000","message":"Probably you are right.\n\nOther option is to eliminate this lvm flow, and just create and copy template in this case always. Because we take care about ext3, and have decided to copy that images from fallback hosts instead of create it.","commit_id":"b8c4e29a1c02704b1e95380d53e7d5a83e59fe81"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"66b901e05a7b90c7d73d5232488ec122f685f162","unresolved":false,"context_lines":[{"line_number":190,"context_line":""},{"line_number":191,"context_line":"            # Generate a new one as a last resort"},{"line_number":192,"context_line":"            if not created:"},{"line_number":193,"context_line":"                func(path)"},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"        _sync()"},{"line_number":196,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"dab17558_2657505b","line":193,"in_reply_to":"dab17558_bc3c23c0","updated":"2016-05-16 14:32:05.000000000","message":"Interesting. You\u0027ve actually already reviewed change where I start using this interface, so you can see I copied the bug you describe here from the old code.\n\nI can think of a workaround, actually, which is to pass a flag to driver._create_image which indicates whether we\u0027re actually creating, or just updating. In the former case we wouldn\u0027t check if a disk exists, just overwrite it.","commit_id":"b8c4e29a1c02704b1e95380d53e7d5a83e59fe81"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"3815294c89426f1f653314037cde25ae040c3e3d","unresolved":false,"context_lines":[{"line_number":194,"context_line":""},{"line_number":195,"context_line":"        _sync()"},{"line_number":196,"context_line":""},{"line_number":197,"context_line":"        return path"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":""},{"line_number":200,"context_line":"@six.add_metaclass(abc.ABCMeta)"}],"source_content_type":"text/x-python","patch_set":12,"id":"dab17558_b990b54f","line":197,"updated":"2016-05-14 10:04:03.000000000","message":"Why not CachedImageInfo instance?","commit_id":"b8c4e29a1c02704b1e95380d53e7d5a83e59fe81"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"f1c573662790d72b86788f85384641e257453220","unresolved":false,"context_lines":[{"line_number":194,"context_line":""},{"line_number":195,"context_line":"        _sync()"},{"line_number":196,"context_line":""},{"line_number":197,"context_line":"        return path"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":""},{"line_number":200,"context_line":"@six.add_metaclass(abc.ABCMeta)"}],"source_content_type":"text/x-python","patch_set":12,"id":"bab6814e_32753af9","line":197,"in_reply_to":"dab17558_a3d8521f","updated":"2016-05-18 19:04:38.000000000","message":"I was confused by similar function names. If they was named similar, did similar things, they should return something similar.\n\nNow in the new revision it is ok for me","commit_id":"b8c4e29a1c02704b1e95380d53e7d5a83e59fe81"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"66b901e05a7b90c7d73d5232488ec122f685f162","unresolved":false,"context_lines":[{"line_number":194,"context_line":""},{"line_number":195,"context_line":"        _sync()"},{"line_number":196,"context_line":""},{"line_number":197,"context_line":"        return path"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":""},{"line_number":200,"context_line":"@six.add_metaclass(abc.ABCMeta)"}],"source_content_type":"text/x-python","patch_set":12,"id":"dab17558_a3d8521f","line":197,"in_reply_to":"dab17558_b990b54f","updated":"2016-05-16 14:32:05.000000000","message":"I think originally I did return CachedImageInfo, then I realised I didn\u0027t need any info from it other than path, so I was (expensively) generating a bunch of metadata for no purpose.\n\nThis is a wart, but I want to kill this whole method anyway, so I\u0027ll live with it.","commit_id":"b8c4e29a1c02704b1e95380d53e7d5a83e59fe81"}],"nova/virt/libvirt/imagecache.py":[{"author":{"_account_id":15524,"name":"dane-fichter","email":"dane.fichter@jhuapl.edu","username":"dane-fichter"},"change_message_id":"272b73a130a1031c80341925b8f42e5db6c4dc8d","unresolved":false,"context_lines":[{"line_number":636,"context_line":""},{"line_number":637,"context_line":""},{"line_number":638,"context_line":"CachedImageInfo \u003d collections.namedtuple("},{"line_number":639,"context_line":"    \u0027CachedImageInfo\u0027, (\u0027path\u0027, \u0027file_format\u0027, \u0027virtual_size\u0027, \u0027disk_size\u0027))"},{"line_number":640,"context_line":""},{"line_number":641,"context_line":""},{"line_number":642,"context_line":"class ImageCacheLocalDir(object):"}],"source_content_type":"text/x-python","patch_set":13,"id":"bab6814e_ecbf12f9","line":639,"updated":"2016-05-19 21:27:30.000000000","message":"Keeping track of the Glance image ID would also be helpful.","commit_id":"bfb929ee9dde14defee76cc9fa1d4d9fa7b82124"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"a2dba5030f7d379c9fb4eac0efd513ca4bc8d28b","unresolved":false,"context_lines":[{"line_number":636,"context_line":""},{"line_number":637,"context_line":""},{"line_number":638,"context_line":"CachedImageInfo \u003d collections.namedtuple("},{"line_number":639,"context_line":"    \u0027CachedImageInfo\u0027, (\u0027path\u0027, \u0027file_format\u0027, \u0027virtual_size\u0027, \u0027disk_size\u0027))"},{"line_number":640,"context_line":""},{"line_number":641,"context_line":""},{"line_number":642,"context_line":"class ImageCacheLocalDir(object):"}],"source_content_type":"text/x-python","patch_set":13,"id":"bab6814e_235d7d47","line":639,"in_reply_to":"bab6814e_ecbf12f9","updated":"2016-05-20 15:47:24.000000000","message":"Well you already know what the image id is, because you\u0027re explicitly requesting it from the image cache. This metadata describes the on-disk properties of the image.\n\nUnless you mean from the POV of imagecachemanager? In that case, I guess you mean not hashing the image id to create the filename? If so, that\u0027s out of scope for this change, because it would be incompatible with the existing implementation.\n\nThis is a cleanup of the existing implementation, not a new implementation.","commit_id":"bfb929ee9dde14defee76cc9fa1d4d9fa7b82124"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"92e30a36642131bc9bf350aa9d11d93ae8684e07","unresolved":false,"context_lines":[{"line_number":668,"context_line":"            return cls._singleton"},{"line_number":669,"context_line":""},{"line_number":670,"context_line":"    def __init__(self):"},{"line_number":671,"context_line":"        self.lock_path \u003d os.path.join(CONF.instances_path, \u0027locks\u0027)"},{"line_number":672,"context_line":"        self.base_dir \u003d os.path.join(CONF.instances_path,"},{"line_number":673,"context_line":"                                     CONF.image_cache_subdirectory_name)"},{"line_number":674,"context_line":"        if not os.path.exists(self.base_dir):"}],"source_content_type":"text/x-python","patch_set":13,"id":"bab6814e_3e918bf1","line":671,"range":{"start_line":671,"start_character":60,"end_line":671,"end_character":65},"updated":"2016-05-18 19:14:10.000000000","message":"Might be a module level constant, used in both ImageCacheLocalDir and ImageCacheManager.","commit_id":"bfb929ee9dde14defee76cc9fa1d4d9fa7b82124"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"e5bb70a0836aa90b3f27abae1cb25ab03a4bc82d","unresolved":false,"context_lines":[{"line_number":668,"context_line":"            return cls._singleton"},{"line_number":669,"context_line":""},{"line_number":670,"context_line":"    def __init__(self):"},{"line_number":671,"context_line":"        self.lock_path \u003d os.path.join(CONF.instances_path, \u0027locks\u0027)"},{"line_number":672,"context_line":"        self.base_dir \u003d os.path.join(CONF.instances_path,"},{"line_number":673,"context_line":"                                     CONF.image_cache_subdirectory_name)"},{"line_number":674,"context_line":"        if not os.path.exists(self.base_dir):"}],"source_content_type":"text/x-python","patch_set":13,"id":"bab6814e_e1b0cd45","line":671,"range":{"start_line":671,"start_character":60,"end_line":671,"end_character":65},"in_reply_to":"bab6814e_2e1f6460","updated":"2016-05-19 18:28:27.000000000","message":"I meant just \u0027locks\u0027 string. But it might be a global function which returns full path, used in both classes.","commit_id":"bfb929ee9dde14defee76cc9fa1d4d9fa7b82124"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"e6b2bb61f053038183c95f089f1cb55f0846eda4","unresolved":false,"context_lines":[{"line_number":668,"context_line":"            return cls._singleton"},{"line_number":669,"context_line":""},{"line_number":670,"context_line":"    def __init__(self):"},{"line_number":671,"context_line":"        self.lock_path \u003d os.path.join(CONF.instances_path, \u0027locks\u0027)"},{"line_number":672,"context_line":"        self.base_dir \u003d os.path.join(CONF.instances_path,"},{"line_number":673,"context_line":"                                     CONF.image_cache_subdirectory_name)"},{"line_number":674,"context_line":"        if not os.path.exists(self.base_dir):"}],"source_content_type":"text/x-python","patch_set":13,"id":"bab6814e_2e1f6460","line":671,"range":{"start_line":671,"start_character":60,"end_line":671,"end_character":65},"in_reply_to":"bab6814e_3e918bf1","updated":"2016-05-19 15:14:38.000000000","message":"I didn\u0027t think you could use CONF in module context, because the config won\u0027t have been loaded yet? Could do a lazy initialisation dance, I guess. Not sure it\u0027s worth the complexity, though. Perhaps a common lock... Will think.","commit_id":"bfb929ee9dde14defee76cc9fa1d4d9fa7b82124"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"c616b37c75e95f4c813a2c6ace9d752735a276b2","unresolved":false,"context_lines":[{"line_number":694,"context_line":"        Args:"},{"line_number":695,"context_line":"            context: The current RequestContext."},{"line_number":696,"context_line":"            image_id: The image_id of the image to be fetched."},{"line_number":697,"context_line":"            instance: An instance we are fetching the image for. Used only"},{"line_number":698,"context_line":"                    for user_id and project_id."},{"line_number":699,"context_line":"            fallback_from_host: A compute host to side-load the image from"},{"line_number":700,"context_line":"                    if it is no longer in glance."}],"source_content_type":"text/x-python","patch_set":13,"id":"bab6814e_933665e5","line":697,"range":{"start_line":697,"start_character":12,"end_line":697,"end_character":20},"updated":"2016-05-19 20:22:37.000000000","message":"instance is no longer an arg","commit_id":"bfb929ee9dde14defee76cc9fa1d4d9fa7b82124"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"ace5caf0b304a22e2b0ca9f7acb626c97f179993","unresolved":false,"context_lines":[{"line_number":694,"context_line":"        Args:"},{"line_number":695,"context_line":"            context: The current RequestContext."},{"line_number":696,"context_line":"            image_id: The image_id of the image to be fetched."},{"line_number":697,"context_line":"            instance: An instance we are fetching the image for. Used only"},{"line_number":698,"context_line":"                    for user_id and project_id."},{"line_number":699,"context_line":"            fallback_from_host: A compute host to side-load the image from"},{"line_number":700,"context_line":"                    if it is no longer in glance."}],"source_content_type":"text/x-python","patch_set":13,"id":"bab6814e_46988fc0","line":697,"range":{"start_line":697,"start_character":12,"end_line":697,"end_character":20},"in_reply_to":"bab6814e_933665e5","updated":"2016-05-23 10:37:38.000000000","message":"Done","commit_id":"bfb929ee9dde14defee76cc9fa1d4d9fa7b82124"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"92e30a36642131bc9bf350aa9d11d93ae8684e07","unresolved":false,"context_lines":[{"line_number":758,"context_line":"            created \u003d False"},{"line_number":759,"context_line":"            if fallback_from_host is not None:"},{"line_number":760,"context_line":"                try:"},{"line_number":761,"context_line":"                    # Ideally we\u0027ll get the original backing file from the"},{"line_number":762,"context_line":"                    # source host. The reason we prefer this is that if we"},{"line_number":763,"context_line":"                    # have to generate a new one, or if the output already"},{"line_number":764,"context_line":"                    # existed in our own cache from a previous run,"}],"source_content_type":"text/x-python","patch_set":13,"id":"bab6814e_0d69fbbe","line":761,"updated":"2016-05-18 19:14:10.000000000","message":"If we did debug logging in the same case above, perhaps we should did the same here.","commit_id":"bfb929ee9dde14defee76cc9fa1d4d9fa7b82124"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"4324628871b17dc67c5446998dc1569f6bcc3d66","unresolved":false,"context_lines":[{"line_number":758,"context_line":"            created \u003d False"},{"line_number":759,"context_line":"            if fallback_from_host is not None:"},{"line_number":760,"context_line":"                try:"},{"line_number":761,"context_line":"                    # Ideally we\u0027ll get the original backing file from the"},{"line_number":762,"context_line":"                    # source host. The reason we prefer this is that if we"},{"line_number":763,"context_line":"                    # have to generate a new one, or if the output already"},{"line_number":764,"context_line":"                    # existed in our own cache from a previous run,"}],"source_content_type":"text/x-python","patch_set":13,"id":"bab6814e_6b014f6a","line":761,"in_reply_to":"bab6814e_0d69fbbe","updated":"2016-05-25 16:19:03.000000000","message":"Grr, forgot this.","commit_id":"bfb929ee9dde14defee76cc9fa1d4d9fa7b82124"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"e6b2bb61f053038183c95f089f1cb55f0846eda4","unresolved":false,"context_lines":[{"line_number":758,"context_line":"            created \u003d False"},{"line_number":759,"context_line":"            if fallback_from_host is not None:"},{"line_number":760,"context_line":"                try:"},{"line_number":761,"context_line":"                    # Ideally we\u0027ll get the original backing file from the"},{"line_number":762,"context_line":"                    # source host. The reason we prefer this is that if we"},{"line_number":763,"context_line":"                    # have to generate a new one, or if the output already"},{"line_number":764,"context_line":"                    # existed in our own cache from a previous run,"}],"source_content_type":"text/x-python","patch_set":13,"id":"bab6814e_aba06644","line":761,"in_reply_to":"bab6814e_0d69fbbe","updated":"2016-05-19 15:14:38.000000000","message":"Ok.","commit_id":"bfb929ee9dde14defee76cc9fa1d4d9fa7b82124"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"a3acf3653b68bcbe7f8d19a61ff5571a2de6c8d6","unresolved":false,"context_lines":[{"line_number":765,"context_line":"                    # for that matter, the output is guaranteed to be"},{"line_number":766,"context_line":"                    # different. This is a bug which results in (hopefully"},{"line_number":767,"context_line":"                    # only) subtle data corruption."},{"line_number":768,"context_line":"                    libvirt_utils.copy_image(src\u003dpath, dest\u003dpath,"},{"line_number":769,"context_line":"                                             host\u003dfallback_from_host,"},{"line_number":770,"context_line":"                                             receive\u003dTrue)"},{"line_number":771,"context_line":"                    created \u003d True"}],"source_content_type":"text/x-python","patch_set":13,"id":"bab6814e_1e00ea00","line":768,"updated":"2016-05-19 08:06:28.000000000","message":"This is faster for ext3 than to create and format the image. But what is for ext4? Do not we reduce the performance in this case?","commit_id":"bfb929ee9dde14defee76cc9fa1d4d9fa7b82124"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"e6b2bb61f053038183c95f089f1cb55f0846eda4","unresolved":false,"context_lines":[{"line_number":765,"context_line":"                    # for that matter, the output is guaranteed to be"},{"line_number":766,"context_line":"                    # different. This is a bug which results in (hopefully"},{"line_number":767,"context_line":"                    # only) subtle data corruption."},{"line_number":768,"context_line":"                    libvirt_utils.copy_image(src\u003dpath, dest\u003dpath,"},{"line_number":769,"context_line":"                                             host\u003dfallback_from_host,"},{"line_number":770,"context_line":"                                             receive\u003dTrue)"},{"line_number":771,"context_line":"                    created \u003d True"}],"source_content_type":"text/x-python","patch_set":13,"id":"bab6814e_abe98682","line":768,"in_reply_to":"bab6814e_1e00ea00","updated":"2016-05-19 15:14:38.000000000","message":"The main problem is that it\u0027s *wrong*, regardless of filesystem, for at least 2 reasons.\n\nFirstly filesystems typically contain a UUID which uniquely identifies the filesystem. If we cache a pre-created filesystem, this isn\u0027t going to be unique. This is a relatively minor wart, but it\u0027s still a wart.\n\nMore seriously, if we\u0027re migrating an instance which uses qcow2 with a backing file, unless we copy the file from the source host the backing file will be different on the target than on the source. At a bare minimum the bare filesystem created on the target will have a different uuid, but tooling differences may result in much more significant differences. Hopefully either all relevant blocks will have been overwritten already, or the differences will be so minor that they don\u0027t cause any problems in practise. However, it\u0027s still a potential source of data corruption.\n\nIdeally we would never cache these. Ephemeral disks should be flat, and created fresh each time. For ext4 the overhead for this is essentially nothing. For ext3, as you mention, it\u0027s a bit more than nothing, which is why I haven\u0027t just done it.\n\nPerformance-wise, this shouldn\u0027t be awful with rsync. It will potentially double transfer time when using bare ssh, though.","commit_id":"bfb929ee9dde14defee76cc9fa1d4d9fa7b82124"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"a2dba5030f7d379c9fb4eac0efd513ca4bc8d28b","unresolved":false,"context_lines":[{"line_number":765,"context_line":"                    # for that matter, the output is guaranteed to be"},{"line_number":766,"context_line":"                    # different. This is a bug which results in (hopefully"},{"line_number":767,"context_line":"                    # only) subtle data corruption."},{"line_number":768,"context_line":"                    libvirt_utils.copy_image(src\u003dpath, dest\u003dpath,"},{"line_number":769,"context_line":"                                             host\u003dfallback_from_host,"},{"line_number":770,"context_line":"                                             receive\u003dTrue)"},{"line_number":771,"context_line":"                    created \u003d True"}],"source_content_type":"text/x-python","patch_set":13,"id":"bab6814e_e62263a2","line":768,"in_reply_to":"bab6814e_aa0fb52c","updated":"2016-05-20 15:47:24.000000000","message":"We retain it in the first instance, which is why this function exists. I will kill it eventually, though :)","commit_id":"bfb929ee9dde14defee76cc9fa1d4d9fa7b82124"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"e5bb70a0836aa90b3f27abae1cb25ab03a4bc82d","unresolved":false,"context_lines":[{"line_number":765,"context_line":"                    # for that matter, the output is guaranteed to be"},{"line_number":766,"context_line":"                    # different. This is a bug which results in (hopefully"},{"line_number":767,"context_line":"                    # only) subtle data corruption."},{"line_number":768,"context_line":"                    libvirt_utils.copy_image(src\u003dpath, dest\u003dpath,"},{"line_number":769,"context_line":"                                             host\u003dfallback_from_host,"},{"line_number":770,"context_line":"                                             receive\u003dTrue)"},{"line_number":771,"context_line":"                    created \u003d True"}],"source_content_type":"text/x-python","patch_set":13,"id":"bab6814e_aa0fb52c","line":768,"in_reply_to":"bab6814e_abe98682","updated":"2016-05-19 18:28:27.000000000","message":"Anyway, since you said \u0027ideally\u0027, should we have an ability to handle of cache usage in depends on an ephemeral format?","commit_id":"bfb929ee9dde14defee76cc9fa1d4d9fa7b82124"},{"author":{"_account_id":15524,"name":"dane-fichter","email":"dane.fichter@jhuapl.edu","username":"dane-fichter"},"change_message_id":"d8f8a979128c184a0d8f5a3156b3a746e51c3dd6","unresolved":false,"context_lines":[{"line_number":299,"context_line":"                    # corrupt downloaded image."},{"line_number":300,"context_line":"                    #"},{"line_number":301,"context_line":"                    # TODO(mdbooth): fix this bug"},{"line_number":302,"context_line":"                    delete_lock_file(lock_file)"},{"line_number":303,"context_line":"                except OSError as e:"},{"line_number":304,"context_line":"                    LOG.debug(\u0027Failed to remove %(lock_file)s, \u0027"},{"line_number":305,"context_line":"                              \u0027error was %(error)s\u0027,"}],"source_content_type":"text/x-python","patch_set":14,"id":"bab6814e_309ca40d","line":302,"updated":"2016-05-26 13:00:03.000000000","message":"Shouldn\u0027t this patch have workflow -1 if there\u0027s an unresolved synchronization bug?","commit_id":"8cb61f6d14cf7d7db01f64ec6d6d04f63f38e12b"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"6ac84a6d6d8c877134d8b468289d8bf0433e8141","unresolved":false,"context_lines":[{"line_number":299,"context_line":"                    # corrupt downloaded image."},{"line_number":300,"context_line":"                    #"},{"line_number":301,"context_line":"                    # TODO(mdbooth): fix this bug"},{"line_number":302,"context_line":"                    delete_lock_file(lock_file)"},{"line_number":303,"context_line":"                except OSError as e:"},{"line_number":304,"context_line":"                    LOG.debug(\u0027Failed to remove %(lock_file)s, \u0027"},{"line_number":305,"context_line":"                              \u0027error was %(error)s\u0027,"}],"source_content_type":"text/x-python","patch_set":14,"id":"bab6814e_8603970f","line":302,"in_reply_to":"bab6814e_309ca40d","updated":"2016-05-26 14:34:24.000000000","message":"No, because this patch doesn\u0027t create the bug: it improves it by documenting it!\n\nI\u0027m not going to fix this here because it probably involves redesigning locking in the image cache. That\u0027s way too much fun for this series.","commit_id":"8cb61f6d14cf7d7db01f64ec6d6d04f63f38e12b"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"48e2f9e2277b97018b12ce0c7fdffd40b4bb8a01","unresolved":false,"context_lines":[{"line_number":524,"context_line":"            return cls._singleton"},{"line_number":525,"context_line":""},{"line_number":526,"context_line":"    def __init__(self):"},{"line_number":527,"context_line":"        self.lock_path \u003d os.path.join(CONF.instances_path, \u0027locks\u0027)"},{"line_number":528,"context_line":"        self.base_dir \u003d os.path.join(CONF.instances_path,"},{"line_number":529,"context_line":"                                     CONF.image_cache_subdirectory_name)"},{"line_number":530,"context_line":"        if not os.path.exists(self.base_dir):"}],"source_content_type":"text/x-python","patch_set":16,"id":"9abb7d3a_53d702ed","line":527,"range":{"start_line":527,"start_character":8,"end_line":527,"end_character":22},"updated":"2016-05-30 12:08:08.000000000","message":"not used now","commit_id":"93f09db2a8c3353b1a2cf53760d78e370c2e1667"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"321202a7b381b4e09b6f6e85497d23867e3a1cc2","unresolved":false,"context_lines":[{"line_number":524,"context_line":"            return cls._singleton"},{"line_number":525,"context_line":""},{"line_number":526,"context_line":"    def __init__(self):"},{"line_number":527,"context_line":"        self.lock_path \u003d os.path.join(CONF.instances_path, \u0027locks\u0027)"},{"line_number":528,"context_line":"        self.base_dir \u003d os.path.join(CONF.instances_path,"},{"line_number":529,"context_line":"                                     CONF.image_cache_subdirectory_name)"},{"line_number":530,"context_line":"        if not os.path.exists(self.base_dir):"}],"source_content_type":"text/x-python","patch_set":16,"id":"9abb7d3a_0d641247","line":527,"range":{"start_line":527,"start_character":8,"end_line":527,"end_character":22},"in_reply_to":"9abb7d3a_53d702ed","updated":"2016-05-31 08:44:02.000000000","message":"D\u0027oh!","commit_id":"93f09db2a8c3353b1a2cf53760d78e370c2e1667"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"1e6a1bbacd135e023c81e503ddb5b1a4a24daad2","unresolved":false,"context_lines":[{"line_number":538,"context_line":""},{"line_number":539,"context_line":"    def get_image_info(self, context, image_id, fallback\u003dNone):"},{"line_number":540,"context_line":"        \"\"\"Fetch an image from the cache, side-loading it from"},{"line_number":541,"context_line":"        `fallback_from_host` if it isn\u0027t in glance."},{"line_number":542,"context_line":""},{"line_number":543,"context_line":"        Args:"},{"line_number":544,"context_line":"            context: The current RequestContext."}],"source_content_type":"text/x-python","patch_set":19,"id":"7aa08908_9f59ee90","line":541,"range":{"start_line":541,"start_character":9,"end_line":541,"end_character":27},"updated":"2016-06-08 19:08:00.000000000","message":"fallback_from_host ---\u003e fallback","commit_id":"773feef5e4bc5208b9eb2def54e1c6dd732aca41"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"279d64e0a0cf7b5f5026d0abe7deeae9ee13f648","unresolved":false,"context_lines":[{"line_number":538,"context_line":""},{"line_number":539,"context_line":"    def get_image_info(self, context, image_id, fallback\u003dNone):"},{"line_number":540,"context_line":"        \"\"\"Fetch an image from the cache, side-loading it from"},{"line_number":541,"context_line":"        `fallback_from_host` if it isn\u0027t in glance."},{"line_number":542,"context_line":""},{"line_number":543,"context_line":"        Args:"},{"line_number":544,"context_line":"            context: The current RequestContext."}],"source_content_type":"text/x-python","patch_set":19,"id":"7aa08908_683d26bb","line":541,"range":{"start_line":541,"start_character":9,"end_line":541,"end_character":27},"in_reply_to":"7aa08908_9f59ee90","updated":"2016-06-09 17:54:48.000000000","message":"Damn you, pycharm! I blame my tools :)","commit_id":"773feef5e4bc5208b9eb2def54e1c6dd732aca41"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5fe24c14bf19c9f53935db73d40e68038f6a4a5d","unresolved":false,"context_lines":[{"line_number":567,"context_line":"                          \"service. Attempting to copy it from %(host)s\","},{"line_number":568,"context_line":"                          {\u0027image_id\u0027: image_id, \u0027host\u0027: fallback})"},{"line_number":569,"context_line":""},{"line_number":570,"context_line":"                libvirt_utils.copy_image(src\u003dpath, dest\u003dpath,"},{"line_number":571,"context_line":"                                         host\u003dfallback,"},{"line_number":572,"context_line":"                                         receive\u003dTrue)"},{"line_number":573,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"7aa08908_dd1c1b6c","line":570,"updated":"2016-06-08 19:04:34.000000000","message":"Below, you catch ProcessExecutionError when we do this. Should we catch it here as well and raise ImageNotFound or something unified with the from-glance case above? It would make the behavior of this \"get me the image, dammit\" function less complicated for the callers.","commit_id":"773feef5e4bc5208b9eb2def54e1c6dd732aca41"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"357894a1edb00dd1333a1cabe05c97ce14741bfa","unresolved":false,"context_lines":[{"line_number":567,"context_line":"                          \"service. Attempting to copy it from %(host)s\","},{"line_number":568,"context_line":"                          {\u0027image_id\u0027: image_id, \u0027host\u0027: fallback})"},{"line_number":569,"context_line":""},{"line_number":570,"context_line":"                libvirt_utils.copy_image(src\u003dpath, dest\u003dpath,"},{"line_number":571,"context_line":"                                         host\u003dfallback,"},{"line_number":572,"context_line":"                                         receive\u003dTrue)"},{"line_number":573,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"7aa08908_fe5b5884","line":570,"in_reply_to":"7aa08908_ad73a014","updated":"2016-06-16 17:01:21.000000000","message":"Done","commit_id":"773feef5e4bc5208b9eb2def54e1c6dd732aca41"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"279d64e0a0cf7b5f5026d0abe7deeae9ee13f648","unresolved":false,"context_lines":[{"line_number":567,"context_line":"                          \"service. Attempting to copy it from %(host)s\","},{"line_number":568,"context_line":"                          {\u0027image_id\u0027: image_id, \u0027host\u0027: fallback})"},{"line_number":569,"context_line":""},{"line_number":570,"context_line":"                libvirt_utils.copy_image(src\u003dpath, dest\u003dpath,"},{"line_number":571,"context_line":"                                         host\u003dfallback,"},{"line_number":572,"context_line":"                                         receive\u003dTrue)"},{"line_number":573,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"7aa08908_ad73a014","line":570,"in_reply_to":"7aa08908_dd1c1b6c","updated":"2016-06-09 17:54:48.000000000","message":"Agreed.","commit_id":"773feef5e4bc5208b9eb2def54e1c6dd732aca41"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5fe24c14bf19c9f53935db73d40e68038f6a4a5d","unresolved":false,"context_lines":[{"line_number":587,"context_line":"        don\u0027t exist, try to side-load it from another compute host."},{"line_number":588,"context_line":"        Failing that, generate it again locally."},{"line_number":589,"context_line":"        Args:"},{"line_number":590,"context_line":"            func: A function, taking a path as an argument, which will write"},{"line_number":591,"context_line":"                data to path"},{"line_number":592,"context_line":"            name: The name of the function output, used to reference cached"},{"line_number":593,"context_line":"                output."},{"line_number":594,"context_line":"            fallback: A compute host to side-load the output from."}],"source_content_type":"text/x-python","patch_set":19,"id":"7aa08908_3a14454b","line":591,"range":{"start_line":590,"start_character":12,"end_line":591,"end_character":28},"updated":"2016-06-08 19:04:34.000000000","message":"Despite this description, I\u0027m still kinda flailing to understand what this is or what it\u0027s going to be used for. It looks very similar to the function above, but .. isn\u0027t.\n\nCan you explain? Or maybe just point me to where later in the stack that this is used?","commit_id":"773feef5e4bc5208b9eb2def54e1c6dd732aca41"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"357894a1edb00dd1333a1cabe05c97ce14741bfa","unresolved":false,"context_lines":[{"line_number":587,"context_line":"        don\u0027t exist, try to side-load it from another compute host."},{"line_number":588,"context_line":"        Failing that, generate it again locally."},{"line_number":589,"context_line":"        Args:"},{"line_number":590,"context_line":"            func: A function, taking a path as an argument, which will write"},{"line_number":591,"context_line":"                data to path"},{"line_number":592,"context_line":"            name: The name of the function output, used to reference cached"},{"line_number":593,"context_line":"                output."},{"line_number":594,"context_line":"            fallback: A compute host to side-load the output from."}],"source_content_type":"text/x-python","patch_set":19,"id":"7aa08908_9efb3447","line":591,"range":{"start_line":590,"start_character":12,"end_line":591,"end_character":28},"in_reply_to":"7aa08908_0d78f4f7","updated":"2016-06-16 17:01:21.000000000","message":"I\u0027m making the docstring much more verbose.","commit_id":"773feef5e4bc5208b9eb2def54e1c6dd732aca41"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"279d64e0a0cf7b5f5026d0abe7deeae9ee13f648","unresolved":false,"context_lines":[{"line_number":587,"context_line":"        don\u0027t exist, try to side-load it from another compute host."},{"line_number":588,"context_line":"        Failing that, generate it again locally."},{"line_number":589,"context_line":"        Args:"},{"line_number":590,"context_line":"            func: A function, taking a path as an argument, which will write"},{"line_number":591,"context_line":"                data to path"},{"line_number":592,"context_line":"            name: The name of the function output, used to reference cached"},{"line_number":593,"context_line":"                output."},{"line_number":594,"context_line":"            fallback: A compute host to side-load the output from."}],"source_content_type":"text/x-python","patch_set":19,"id":"7aa08908_0d78f4f7","line":591,"range":{"start_line":590,"start_character":12,"end_line":591,"end_character":28},"in_reply_to":"7aa08908_3a14454b","updated":"2016-06-09 17:54:48.000000000","message":"It\u0027s used to cache the output of _create_ephemeral() or _create_swap(). As it happens, I believe that caching the output of these functions is a bug, but I didn\u0027t want to mix up fixing that with this refactor.","commit_id":"773feef5e4bc5208b9eb2def54e1c6dd732aca41"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5fe24c14bf19c9f53935db73d40e68038f6a4a5d","unresolved":false,"context_lines":[{"line_number":623,"context_line":"                                             receive\u003dTrue)"},{"line_number":624,"context_line":"                    created \u003d True"},{"line_number":625,"context_line":"                except processutils.ProcessExecutionError:"},{"line_number":626,"context_line":"                    LOG.exception(_LW(\u0027Failed to side-load %(path)s from \u0027"},{"line_number":627,"context_line":"                                      \u0027%(host)s\u0027),"},{"line_number":628,"context_line":"                                  {\u0027path\u0027: path, \u0027host\u0027: fallback})"},{"line_number":629,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"7aa08908_5d566b06","line":626,"range":{"start_line":626,"start_character":24,"end_line":626,"end_character":33},"updated":"2016-06-08 19:04:34.000000000","message":"Doesn\u0027t this (usually) just happen if the host doesn\u0027t have it anymore? Logging the exception is for worst-of-the-worst \"programming errors\" ... does that fit here?","commit_id":"773feef5e4bc5208b9eb2def54e1c6dd732aca41"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"279d64e0a0cf7b5f5026d0abe7deeae9ee13f648","unresolved":false,"context_lines":[{"line_number":623,"context_line":"                                             receive\u003dTrue)"},{"line_number":624,"context_line":"                    created \u003d True"},{"line_number":625,"context_line":"                except processutils.ProcessExecutionError:"},{"line_number":626,"context_line":"                    LOG.exception(_LW(\u0027Failed to side-load %(path)s from \u0027"},{"line_number":627,"context_line":"                                      \u0027%(host)s\u0027),"},{"line_number":628,"context_line":"                                  {\u0027path\u0027: path, \u0027host\u0027: fallback})"},{"line_number":629,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"7aa08908_c8337af5","line":626,"range":{"start_line":626,"start_character":24,"end_line":626,"end_character":33},"in_reply_to":"7aa08908_5d566b06","updated":"2016-06-09 17:54:48.000000000","message":"The \u0027old\u0027 code doesn\u0027t actually do this fetch from source for the backing files of ephemeral and swap disks. I added it because I think it\u0027s more likely to result in an identical backing file, but I still think the whole concept of using cached backing files for these things is broken.\n\nYou\u0027ll also notice that we check for missing backing files in reboot. This is because we don\u0027t trust ImageCacheManager to not delete things we\u0027re still using. That\u0027s a separate issue, but it\u0027s relevant here.\n\nIn summary, the old code didn\u0027t do the side-load thing at all, so it wouldn\u0027t have failed in the case that ImageCacheManager went postal and deleted our backing file on the source host. The old code would have just re-created a new backing file, which is guaranteed to be different to the old backing file (and therefore definitely corrupt, but hopefully not so badly that we\u0027ll notice). Consequently, in order to be \u0027no worse\u0027 than the old code, we just log the exception and carry on as before.","commit_id":"773feef5e4bc5208b9eb2def54e1c6dd732aca41"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"357894a1edb00dd1333a1cabe05c97ce14741bfa","unresolved":false,"context_lines":[{"line_number":623,"context_line":"                                             receive\u003dTrue)"},{"line_number":624,"context_line":"                    created \u003d True"},{"line_number":625,"context_line":"                except processutils.ProcessExecutionError:"},{"line_number":626,"context_line":"                    LOG.exception(_LW(\u0027Failed to side-load %(path)s from \u0027"},{"line_number":627,"context_line":"                                      \u0027%(host)s\u0027),"},{"line_number":628,"context_line":"                                  {\u0027path\u0027: path, \u0027host\u0027: fallback})"},{"line_number":629,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"7aa08908_516c1d36","line":626,"range":{"start_line":626,"start_character":24,"end_line":626,"end_character":33},"in_reply_to":"7aa08908_c8337af5","updated":"2016-06-16 17:01:21.000000000","message":"I\u0027ve added a comment up top explaining why this whole function is a bug.","commit_id":"773feef5e4bc5208b9eb2def54e1c6dd732aca41"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b3c3e6178e400b692770683bae2ef275942dd52a","unresolved":false,"context_lines":[{"line_number":517,"context_line":"            return cls._singleton"},{"line_number":518,"context_line":""},{"line_number":519,"context_line":"    def __init__(self):"},{"line_number":520,"context_line":"        self.lock_path \u003d os.path.join(CONF.instances_path, \u0027locks\u0027)"},{"line_number":521,"context_line":"        self.base_dir \u003d os.path.join(CONF.instances_path,"},{"line_number":522,"context_line":"                                     CONF.image_cache_subdirectory_name)"},{"line_number":523,"context_line":"        if not os.path.exists(self.base_dir):"}],"source_content_type":"text/x-python","patch_set":20,"id":"7aa08908_48f699d7","line":520,"range":{"start_line":520,"start_character":25,"end_line":520,"end_character":67},"updated":"2016-06-14 21:13:08.000000000","message":"Reuse lock_path() here.","commit_id":"a395dddf30ee8414042f5333a4bf0eeb77f6a8a9"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"a4007c103f0a471e825bb36915bd2c21421f8f7d","unresolved":false,"context_lines":[{"line_number":517,"context_line":"            return cls._singleton"},{"line_number":518,"context_line":""},{"line_number":519,"context_line":"    def __init__(self):"},{"line_number":520,"context_line":"        self.lock_path \u003d os.path.join(CONF.instances_path, \u0027locks\u0027)"},{"line_number":521,"context_line":"        self.base_dir \u003d os.path.join(CONF.instances_path,"},{"line_number":522,"context_line":"                                     CONF.image_cache_subdirectory_name)"},{"line_number":523,"context_line":"        if not os.path.exists(self.base_dir):"}],"source_content_type":"text/x-python","patch_set":20,"id":"7aa08908_2395ca87","line":520,"range":{"start_line":520,"start_character":25,"end_line":520,"end_character":67},"in_reply_to":"7aa08908_48f699d7","updated":"2016-06-16 15:07:49.000000000","message":"Done","commit_id":"a395dddf30ee8414042f5333a4bf0eeb77f6a8a9"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b3c3e6178e400b692770683bae2ef275942dd52a","unresolved":false,"context_lines":[{"line_number":520,"context_line":"        self.lock_path \u003d os.path.join(CONF.instances_path, \u0027locks\u0027)"},{"line_number":521,"context_line":"        self.base_dir \u003d os.path.join(CONF.instances_path,"},{"line_number":522,"context_line":"                                     CONF.image_cache_subdirectory_name)"},{"line_number":523,"context_line":"        if not os.path.exists(self.base_dir):"},{"line_number":524,"context_line":"            fileutils.ensure_tree(self.base_dir)"},{"line_number":525,"context_line":""},{"line_number":526,"context_line":"    def is_path_in_image_cache(self, path):"}],"source_content_type":"text/x-python","patch_set":20,"id":"7aa08908_28b22d06","line":523,"range":{"start_line":523,"start_character":8,"end_line":523,"end_character":45},"updated":"2016-06-14 21:13:08.000000000","message":"nit: you don\u0027t really need to check the existence first to use fileutils.ensure_tree, it won\u0027t fail if the path already exists and is a directory.","commit_id":"a395dddf30ee8414042f5333a4bf0eeb77f6a8a9"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"a4007c103f0a471e825bb36915bd2c21421f8f7d","unresolved":false,"context_lines":[{"line_number":520,"context_line":"        self.lock_path \u003d os.path.join(CONF.instances_path, \u0027locks\u0027)"},{"line_number":521,"context_line":"        self.base_dir \u003d os.path.join(CONF.instances_path,"},{"line_number":522,"context_line":"                                     CONF.image_cache_subdirectory_name)"},{"line_number":523,"context_line":"        if not os.path.exists(self.base_dir):"},{"line_number":524,"context_line":"            fileutils.ensure_tree(self.base_dir)"},{"line_number":525,"context_line":""},{"line_number":526,"context_line":"    def is_path_in_image_cache(self, path):"}],"source_content_type":"text/x-python","patch_set":20,"id":"7aa08908_63e6b245","line":523,"range":{"start_line":523,"start_character":8,"end_line":523,"end_character":45},"in_reply_to":"7aa08908_28b22d06","updated":"2016-06-16 15:07:49.000000000","message":"Done","commit_id":"a395dddf30ee8414042f5333a4bf0eeb77f6a8a9"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b3c3e6178e400b692770683bae2ef275942dd52a","unresolved":false,"context_lines":[{"line_number":523,"context_line":"        if not os.path.exists(self.base_dir):"},{"line_number":524,"context_line":"            fileutils.ensure_tree(self.base_dir)"},{"line_number":525,"context_line":""},{"line_number":526,"context_line":"    def is_path_in_image_cache(self, path):"},{"line_number":527,"context_line":"        return os.path.dirname(path) \u003d\u003d self.base_dir"},{"line_number":528,"context_line":""},{"line_number":529,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":20,"id":"7aa08908_a8745d35","line":526,"updated":"2016-06-14 21:13:08.000000000","message":"This reads a little odd to me, might want to docstring it. It\u0027s the \u0027in\u0027 versus \u0027equal\u0027 part that confuses me.","commit_id":"a395dddf30ee8414042f5333a4bf0eeb77f6a8a9"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"a4007c103f0a471e825bb36915bd2c21421f8f7d","unresolved":false,"context_lines":[{"line_number":523,"context_line":"        if not os.path.exists(self.base_dir):"},{"line_number":524,"context_line":"            fileutils.ensure_tree(self.base_dir)"},{"line_number":525,"context_line":""},{"line_number":526,"context_line":"    def is_path_in_image_cache(self, path):"},{"line_number":527,"context_line":"        return os.path.dirname(path) \u003d\u003d self.base_dir"},{"line_number":528,"context_line":""},{"line_number":529,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":20,"id":"7aa08908_ae2401b2","line":526,"in_reply_to":"7aa08908_a8745d35","updated":"2016-06-16 15:07:49.000000000","message":"Done","commit_id":"a395dddf30ee8414042f5333a4bf0eeb77f6a8a9"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b3c3e6178e400b692770683bae2ef275942dd52a","unresolved":false,"context_lines":[{"line_number":540,"context_line":"        \"\"\"Fetch an image from the cache, side-loading it from"},{"line_number":541,"context_line":"        `fallback_from_host` if it isn\u0027t in glance."},{"line_number":542,"context_line":""},{"line_number":543,"context_line":"        Args:"},{"line_number":544,"context_line":"            context: The current RequestContext."},{"line_number":545,"context_line":"            image_id: The image_id of the image to be fetched."},{"line_number":546,"context_line":"            fallback: A compute host to side-load the image from"}],"source_content_type":"text/x-python","patch_set":20,"id":"7aa08908_68cc356a","line":543,"range":{"start_line":543,"start_character":8,"end_line":543,"end_character":12},"updated":"2016-06-14 21:13:08.000000000","message":"nit: would be better to use actual docstrings, i.e.:\n\n@param context: ...\n@param image_id: ...\n...\n@returns: ...","commit_id":"a395dddf30ee8414042f5333a4bf0eeb77f6a8a9"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"a4007c103f0a471e825bb36915bd2c21421f8f7d","unresolved":false,"context_lines":[{"line_number":540,"context_line":"        \"\"\"Fetch an image from the cache, side-loading it from"},{"line_number":541,"context_line":"        `fallback_from_host` if it isn\u0027t in glance."},{"line_number":542,"context_line":""},{"line_number":543,"context_line":"        Args:"},{"line_number":544,"context_line":"            context: The current RequestContext."},{"line_number":545,"context_line":"            image_id: The image_id of the image to be fetched."},{"line_number":546,"context_line":"            fallback: A compute host to side-load the image from"}],"source_content_type":"text/x-python","patch_set":20,"id":"7aa08908_231f8a0a","line":543,"range":{"start_line":543,"start_character":8,"end_line":543,"end_character":12},"in_reply_to":"7aa08908_68cc356a","updated":"2016-06-16 15:07:49.000000000","message":"Ah ha! Apparently I had pyCharm configure to generate \u0027Google\u0027 standard docstrings. Updated to reStructuredText.","commit_id":"a395dddf30ee8414042f5333a4bf0eeb77f6a8a9"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b3c3e6178e400b692770683bae2ef275942dd52a","unresolved":false,"context_lines":[{"line_number":560,"context_line":"            try:"},{"line_number":561,"context_line":"                libvirt_utils.fetch_image(context, path, image_id)"},{"line_number":562,"context_line":"            except exception.ImageNotFound:"},{"line_number":563,"context_line":"                if fallback is None:"},{"line_number":564,"context_line":"                    raise"},{"line_number":565,"context_line":""},{"line_number":566,"context_line":"                LOG.debug(\"Image %(image_id)s no longer exists in the image \""},{"line_number":567,"context_line":"                          \"service. Attempting to copy it from %(host)s\","}],"source_content_type":"text/x-python","patch_set":20,"id":"7aa08908_28586d89","line":564,"range":{"start_line":563,"start_character":16,"end_line":564,"end_character":25},"updated":"2016-06-14 21:13:08.000000000","message":"I have a feeling we should be using oslo_utils.excutils.save_and_reraise_exception in here somehow, where we would set ctxt.reraise \u003d False if we get to libvirt_utils.copy_image and that passes. But I might be over-thinking this.\n\nMaybe this?\n\nwith excutils.save_and_reraise_exception() as ctxt:\n    if fallback:\n        LOG.debug(...)\n        libvirt_utils.copy_image(...)\n        ctxt.reraise \u003d False","commit_id":"a395dddf30ee8414042f5333a4bf0eeb77f6a8a9"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"a4007c103f0a471e825bb36915bd2c21421f8f7d","unresolved":false,"context_lines":[{"line_number":560,"context_line":"            try:"},{"line_number":561,"context_line":"                libvirt_utils.fetch_image(context, path, image_id)"},{"line_number":562,"context_line":"            except exception.ImageNotFound:"},{"line_number":563,"context_line":"                if fallback is None:"},{"line_number":564,"context_line":"                    raise"},{"line_number":565,"context_line":""},{"line_number":566,"context_line":"                LOG.debug(\"Image %(image_id)s no longer exists in the image \""},{"line_number":567,"context_line":"                          \"service. Attempting to copy it from %(host)s\","}],"source_content_type":"text/x-python","patch_set":20,"id":"7aa08908_ee0a8978","line":564,"range":{"start_line":563,"start_character":16,"end_line":564,"end_character":25},"in_reply_to":"7aa08908_28586d89","updated":"2016-06-16 15:07:49.000000000","message":"I don\u0027t think save_and_reraise_exception is strictly necessary here, as there\u0027s no opportunity for an intermediate exception.\n\nIf fallback is given, we\u0027re genuinely no longer interested in the original exception. The explosion of copy_image() below accompanied by the log message is the only interesting context.","commit_id":"a395dddf30ee8414042f5333a4bf0eeb77f6a8a9"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b3c3e6178e400b692770683bae2ef275942dd52a","unresolved":false,"context_lines":[{"line_number":564,"context_line":"                    raise"},{"line_number":565,"context_line":""},{"line_number":566,"context_line":"                LOG.debug(\"Image %(image_id)s no longer exists in the image \""},{"line_number":567,"context_line":"                          \"service. Attempting to copy it from %(host)s\","},{"line_number":568,"context_line":"                          {\u0027image_id\u0027: image_id, \u0027host\u0027: fallback})"},{"line_number":569,"context_line":""},{"line_number":570,"context_line":"                libvirt_utils.copy_image(src\u003dpath, dest\u003dpath,"}],"source_content_type":"text/x-python","patch_set":20,"id":"7aa08908_483cd92e","line":567,"range":{"start_line":567,"start_character":58,"end_line":567,"end_character":62},"updated":"2016-06-14 21:13:08.000000000","message":"nit: \u0027from host %(host)s\u0027\n\nfor context","commit_id":"a395dddf30ee8414042f5333a4bf0eeb77f6a8a9"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"a4007c103f0a471e825bb36915bd2c21421f8f7d","unresolved":false,"context_lines":[{"line_number":564,"context_line":"                    raise"},{"line_number":565,"context_line":""},{"line_number":566,"context_line":"                LOG.debug(\"Image %(image_id)s no longer exists in the image \""},{"line_number":567,"context_line":"                          \"service. Attempting to copy it from %(host)s\","},{"line_number":568,"context_line":"                          {\u0027image_id\u0027: image_id, \u0027host\u0027: fallback})"},{"line_number":569,"context_line":""},{"line_number":570,"context_line":"                libvirt_utils.copy_image(src\u003dpath, dest\u003dpath,"}],"source_content_type":"text/x-python","patch_set":20,"id":"7aa08908_2e6c9135","line":567,"range":{"start_line":567,"start_character":58,"end_line":567,"end_character":62},"in_reply_to":"7aa08908_483cd92e","updated":"2016-06-16 15:07:49.000000000","message":"Done (and in a couple of other places)","commit_id":"a395dddf30ee8414042f5333a4bf0eeb77f6a8a9"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b3c3e6178e400b692770683bae2ef275942dd52a","unresolved":false,"context_lines":[{"line_number":586,"context_line":"        \"\"\"Get the cached output of a function from the cache. If it"},{"line_number":587,"context_line":"        don\u0027t exist, try to side-load it from another compute host."},{"line_number":588,"context_line":"        Failing that, generate it again locally."},{"line_number":589,"context_line":"        Args:"},{"line_number":590,"context_line":"            func: A function, taking a path as an argument, which will write"},{"line_number":591,"context_line":"                data to path"},{"line_number":592,"context_line":"            name: The name of the function output, used to reference cached"}],"source_content_type":"text/x-python","patch_set":20,"id":"7aa08908_888c01d8","line":589,"range":{"start_line":589,"start_character":8,"end_line":589,"end_character":12},"updated":"2016-06-14 21:13:08.000000000","message":"nit: same comment about docstring @param and @returns","commit_id":"a395dddf30ee8414042f5333a4bf0eeb77f6a8a9"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b3c3e6178e400b692770683bae2ef275942dd52a","unresolved":false,"context_lines":[{"line_number":614,"context_line":"                    # only) subtle data corruption."},{"line_number":615,"context_line":""},{"line_number":616,"context_line":"                    LOG.debug(\"Disk template %(name)s doesn\u0027t exist in the \""},{"line_number":617,"context_line":"                              \"image cache, attempting to copy it from \""},{"line_number":618,"context_line":"                              \"%(host)s\","},{"line_number":619,"context_line":"                              {\u0027name\u0027: name, \u0027host\u0027: fallback})"},{"line_number":620,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"7aa08908_887a41b8","line":617,"range":{"start_line":617,"start_character":66,"end_line":617,"end_character":70},"updated":"2016-06-14 21:13:08.000000000","message":"nit: from host","commit_id":"a395dddf30ee8414042f5333a4bf0eeb77f6a8a9"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b3c3e6178e400b692770683bae2ef275942dd52a","unresolved":false,"context_lines":[{"line_number":623,"context_line":"                                             receive\u003dTrue)"},{"line_number":624,"context_line":"                    created \u003d True"},{"line_number":625,"context_line":"                except processutils.ProcessExecutionError:"},{"line_number":626,"context_line":"                    LOG.exception(_LW(\u0027Failed to side-load %(path)s from \u0027"},{"line_number":627,"context_line":"                                      \u0027%(host)s\u0027),"},{"line_number":628,"context_line":"                                  {\u0027path\u0027: path, \u0027host\u0027: fallback})"},{"line_number":629,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"7aa08908_6879d5c7","line":626,"range":{"start_line":626,"start_character":20,"end_line":626,"end_character":37},"updated":"2016-06-14 21:13:08.000000000","message":"I think Dan already commented on this being wrong.","commit_id":"a395dddf30ee8414042f5333a4bf0eeb77f6a8a9"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"b6d23b0331e1b954790b68305e5ad802590bb0bd","unresolved":false,"context_lines":[{"line_number":625,"context_line":"        # differences in the underlying tools may produce wildly different"},{"line_number":626,"context_line":"        # outputs."},{"line_number":627,"context_line":"        #"},{"line_number":628,"context_line":"        # If this worked as intended it would be a bug, because filesystem"},{"line_number":629,"context_line":"        # uuids are intended to be \u0027universally unique\u0027, which they are not"},{"line_number":630,"context_line":"        # if you cache them."},{"line_number":631,"context_line":"        #"},{"line_number":632,"context_line":"        # It is a data integrity problem for migrations involving qcow2"},{"line_number":633,"context_line":"        # disks, because the invocation on the target host is guaranteed to"}],"source_content_type":"text/x-python","patch_set":21,"id":"7aa08908_3e9ace14","line":630,"range":{"start_line":628,"start_character":10,"end_line":630,"end_character":28},"updated":"2016-06-17 12:53:46.000000000","message":"Is this \"universally uniqueness\" honoured for root disks, which content comes from Glance images?","commit_id":"64a1555fbb73639e527ea5153111f19348e53148"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"71d74c99e1f8cc6c611cc517459ea55186dba636","unresolved":false,"context_lines":[{"line_number":625,"context_line":"        # differences in the underlying tools may produce wildly different"},{"line_number":626,"context_line":"        # outputs."},{"line_number":627,"context_line":"        #"},{"line_number":628,"context_line":"        # If this worked as intended it would be a bug, because filesystem"},{"line_number":629,"context_line":"        # uuids are intended to be \u0027universally unique\u0027, which they are not"},{"line_number":630,"context_line":"        # if you cache them."},{"line_number":631,"context_line":"        #"},{"line_number":632,"context_line":"        # It is a data integrity problem for migrations involving qcow2"},{"line_number":633,"context_line":"        # disks, because the invocation on the target host is guaranteed to"}],"source_content_type":"text/x-python","patch_set":21,"id":"3aaa91ec_71d539e3","line":630,"range":{"start_line":628,"start_character":10,"end_line":630,"end_character":28},"in_reply_to":"7aa08908_3e9ace14","updated":"2016-06-27 09:44:11.000000000","message":"Nope, but that\u0027s at least predictable. For ephemeral disks we\u0027re creating non-deterministic buggy filesystems, because they\u0027re different depending on which compute host created them.","commit_id":"64a1555fbb73639e527ea5153111f19348e53148"}],"tests-py3.txt":[{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"92e30a36642131bc9bf350aa9d11d93ae8684e07","unresolved":false,"context_lines":[{"line_number":157,"context_line":"nova.tests.unit.virt.libvirt.test_fakelibvirt.FakeLibvirtTests.test_numa_topology_generation"},{"line_number":158,"context_line":"nova.tests.unit.virt.libvirt.test_firewall.IptablesFirewallTestCase"},{"line_number":159,"context_line":"nova.tests.unit.virt.libvirt.test_imagebackend.EncryptedLvmTestCase"},{"line_number":160,"context_line":"nova.tests.unit.virt.libvirt.test_imagebackend.ImageCacheLocalPoolTestCase"},{"line_number":161,"context_line":"nova.tests.unit.virt.libvirt.test_imagebackend.LvmTestCase"},{"line_number":162,"context_line":"nova.tests.unit.virt.libvirt.test_imagebackend.FlatTestCase"},{"line_number":163,"context_line":"nova.tests.unit.virt.libvirt.test_imagebackend.RbdTestCase"}],"source_content_type":"text/plain","patch_set":13,"id":"bab6814e_4d82e395","line":160,"updated":"2016-05-18 19:14:10.000000000","message":"forgot to update this?","commit_id":"bfb929ee9dde14defee76cc9fa1d4d9fa7b82124"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"48e2f9e2277b97018b12ce0c7fdffd40b4bb8a01","unresolved":false,"context_lines":[{"line_number":157,"context_line":"nova.tests.unit.virt.libvirt.test_fakelibvirt.FakeLibvirtTests.test_numa_topology_generation"},{"line_number":158,"context_line":"nova.tests.unit.virt.libvirt.test_firewall.IptablesFirewallTestCase"},{"line_number":159,"context_line":"nova.tests.unit.virt.libvirt.test_imagebackend.EncryptedLvmTestCase"},{"line_number":160,"context_line":"nova.tests.unit.virt.libvirt.test_imagebackend.ImageCacheLocalPoolTestCase"},{"line_number":161,"context_line":"nova.tests.unit.virt.libvirt.test_imagebackend.LvmTestCase"},{"line_number":162,"context_line":"nova.tests.unit.virt.libvirt.test_imagebackend.FlatTestCase"},{"line_number":163,"context_line":"nova.tests.unit.virt.libvirt.test_imagebackend.RbdTestCase"}],"source_content_type":"text/plain","patch_set":16,"id":"9abb7d3a_563730e3","line":160,"updated":"2016-05-30 12:08:08.000000000","message":"Should it be ...test_imagecache.ImageCacheLocalDirTestCase?","commit_id":"93f09db2a8c3353b1a2cf53760d78e370c2e1667"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"321202a7b381b4e09b6f6e85497d23867e3a1cc2","unresolved":false,"context_lines":[{"line_number":157,"context_line":"nova.tests.unit.virt.libvirt.test_fakelibvirt.FakeLibvirtTests.test_numa_topology_generation"},{"line_number":158,"context_line":"nova.tests.unit.virt.libvirt.test_firewall.IptablesFirewallTestCase"},{"line_number":159,"context_line":"nova.tests.unit.virt.libvirt.test_imagebackend.EncryptedLvmTestCase"},{"line_number":160,"context_line":"nova.tests.unit.virt.libvirt.test_imagebackend.ImageCacheLocalPoolTestCase"},{"line_number":161,"context_line":"nova.tests.unit.virt.libvirt.test_imagebackend.LvmTestCase"},{"line_number":162,"context_line":"nova.tests.unit.virt.libvirt.test_imagebackend.FlatTestCase"},{"line_number":163,"context_line":"nova.tests.unit.virt.libvirt.test_imagebackend.RbdTestCase"}],"source_content_type":"text/plain","patch_set":16,"id":"9abb7d3a_4dec5a8f","line":160,"in_reply_to":"9abb7d3a_563730e3","updated":"2016-05-31 08:44:02.000000000","message":"Yes, which presumably means it now passes under python 3 and I can remove this. See prior patch which fixes a python 3 nit.","commit_id":"93f09db2a8c3353b1a2cf53760d78e370c2e1667"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"15f1cbef500d68a2de57e1d22d616e38605844c1","unresolved":false,"context_lines":[{"line_number":154,"context_line":"nova.tests.unit.virt.libvirt.test_fakelibvirt.FakeLibvirtTests.test_numa_topology_generation"},{"line_number":155,"context_line":"nova.tests.unit.virt.libvirt.test_firewall.IptablesFirewallTestCase"},{"line_number":156,"context_line":"nova.tests.unit.virt.libvirt.test_imagebackend.EncryptedLvmTestCase"},{"line_number":157,"context_line":"nova.tests.unit.virt.libvirt.test_imagebackend.ImageCacheLocalPoolTestCase"},{"line_number":158,"context_line":"nova.tests.unit.virt.libvirt.test_imagebackend.LvmTestCase"},{"line_number":159,"context_line":"nova.tests.unit.virt.libvirt.test_imagebackend.FlatTestCase"},{"line_number":160,"context_line":"nova.tests.unit.virt.libvirt.test_imagebackend.RbdTestCase"}],"source_content_type":"text/plain","patch_set":17,"id":"9abb7d3a_791ce013","line":157,"updated":"2016-06-02 15:33:11.000000000","message":"Did I miss where we justified this?","commit_id":"04d738f5ce064fac449e62dba67b9ccfdb35a04a"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"2163bc68363b8fb7f8b9792b969a08d0b56c8208","unresolved":false,"context_lines":[{"line_number":154,"context_line":"nova.tests.unit.virt.libvirt.test_fakelibvirt.FakeLibvirtTests.test_numa_topology_generation"},{"line_number":155,"context_line":"nova.tests.unit.virt.libvirt.test_firewall.IptablesFirewallTestCase"},{"line_number":156,"context_line":"nova.tests.unit.virt.libvirt.test_imagebackend.EncryptedLvmTestCase"},{"line_number":157,"context_line":"nova.tests.unit.virt.libvirt.test_imagebackend.ImageCacheLocalPoolTestCase"},{"line_number":158,"context_line":"nova.tests.unit.virt.libvirt.test_imagebackend.LvmTestCase"},{"line_number":159,"context_line":"nova.tests.unit.virt.libvirt.test_imagebackend.FlatTestCase"},{"line_number":160,"context_line":"nova.tests.unit.virt.libvirt.test_imagebackend.RbdTestCase"}],"source_content_type":"text/plain","patch_set":17,"id":"9abb7d3a_31354cf7","line":157,"in_reply_to":"9abb7d3a_791ce013","updated":"2016-06-02 16:51:47.000000000","message":"This is a leftover from a previous version of this patch, and I was convinced I\u0027d removed it :/ The prior py3 fix removes the need for this. Probably for a bunch of other stuff too, but somebody else can enjoy working that out.","commit_id":"04d738f5ce064fac449e62dba67b9ccfdb35a04a"}]}